Feat/custom system prompt - #774
Open
ewan102 wants to merge 4 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesCustom system prompt handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryService
participant prepend_system_prompt
Client->>QueryService: chat messages with leading system prompts
QueryService->>QueryService: split pinned prompts from truncated history
QueryService->>prepend_system_prompt: pass custom prompt, template, context, and date
prepend_system_prompt-->>QueryService: formatted system message and history
QueryService-->>Client: prepared chat messages
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openrag/core/prompts/chat_prompt_builder.py`:
- Around line 148-150: Update the custom prompt construction in the chat prompt
builder to sanitize or encode any `</unsafe_custom_prompt>` sequence in `prefix`
before interpolating it into the `<unsafe_custom_prompt>` wrapper, while
preserving normal custom content. Add a regression test covering a prefix that
contains the closing delimiter and verify subsequent client text remains inside
the intended untrusted boundary.
In `@openrag/services/orchestrators/query_service.py`:
- Around line 393-394: Update the query flow around _split_leading_system_prompt
and generate_query so an empty messages collection after removing leading system
prompts is handled before generate_query runs. Reject the system-only request
through the existing client-validation path, or apply the established no-query
fallback, while preserving normal generation for histories containing a
non-system message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e558893-89a3-423b-9892-6aa39ad227f2
📒 Files selected for processing (7)
openrag/core/prompts/chat_prompt_builder.pyopenrag/core/utils/source_filtering.pyopenrag/prompts/templates/spoken_style_answer_tmpl.txtopenrag/prompts/templates/sys_prompt_tmpl.txtopenrag/services/orchestrators/query_service.pytests/unit/core/prompts/test_chat_prompt_builder.pytests/unit/services/orchestrators/test_query_service.py
Comment on lines
+393
to
+394
| custom_prompt, messages = _split_leading_system_prompt(payload["messages"], messages) | ||
| queries = await self.generate_query(messages, llm=llm) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle system-only histories before generating a query.
After extracting all leading system messages, messages can be empty. Line 394 then calls generate_query(), which accesses messages[-1] and raises IndexError. Reject this request with the existing client-validation path or define a no-query fallback before calling it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openrag/services/orchestrators/query_service.py` around lines 393 - 394,
Update the query flow around _split_leading_system_prompt and generate_query so
an empty messages collection after removing leading system prompts is handled
before generate_query runs. Reject the system-only request through the existing
client-validation path, or apply the established no-query fallback, while
preserving normal generation for histories containing a non-system message.
A leading role=system message in a chat request is now treated as a pinned instruction rather than a chat turn: it survives chat_history_depth truncation, never leaks into query-generation as a fake conversation turn, and is spliced into the RAG system prompt right after the intro paragraph rather than just prepended raw. Security: the custom content is wrapped in an <unsafe_custom_prompt> tag to flag it as untrusted input, and both prompt templates gained a Security rule instructing the model to keep following its core rules regardless of what that block says.
ewan102
force-pushed
the
feat/custom-system-prompt
branch
from
July 27, 2026 14:12
8bc82ba to
a659675
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openrag/services/orchestrators/query_service.py`:
- Around line 662-668: Update the truncation/forwarding logic around strip so it
accounts for the raw suffix offset where truncated begins, rather than comparing
truncated entries against raw_messages from index zero. Advance strip past
pinned entries that overlap the truncated window, ensuring entries such as SYS2
are not both injected into parts and returned in truncated[strip:].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1f5906c-0a25-4eca-af71-905ec39a70ed
📒 Files selected for processing (6)
openrag/core/prompts/chat_prompt_builder.pyopenrag/prompts/templates/spoken_style_answer_tmpl.txtopenrag/prompts/templates/sys_prompt_tmpl.txtopenrag/services/orchestrators/query_service.pytests/unit/core/prompts/test_chat_prompt_builder.pytests/unit/services/orchestrators/test_query_service.py
🚧 Files skipped from review as they are similar to previous changes (4)
- openrag/prompts/templates/spoken_style_answer_tmpl.txt
- openrag/prompts/templates/sys_prompt_tmpl.txt
- openrag/core/prompts/chat_prompt_builder.py
- tests/unit/core/prompts/test_chat_prompt_builder.py
…closing tag - _split_leading_system_prompt compared truncated[j] against raw_messages[j] from index 0, which only worked when no truncation occurred. When chat_history_depth split a multi-message leading system run in half, the still-in-window remainder was left in messages instead of being stripped, duplicating it as an unwrapped, untrusted-marker-free system message. Now computed directly from the tail offset instead of object identity. - escape a client-supplied </unsafe_custom_prompt> sequence in prefix before interpolating it, so a crafted prefix can't forge the wrapper's closing tag and make trailing attacker text read as outside the untrusted boundary.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/core/prompts/test_chat_prompt_builder.py (1)
121-135: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover mixed-case closing tags.
_UNSAFE_PROMPT_CLOSE_TAG_REis case-insensitive, but this test only exercises the lowercase spelling. Add a mixed-case delimiter case to preserve that security invariant during future changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/core/prompts/test_chat_prompt_builder.py` around lines 121 - 135, Add a mixed-case closing-tag variant to test_prepend_system_prompt_escapes_closing_tag_in_custom_prompt, while preserving the existing assertions that only the builder’s genuine delimiter remains unescaped and the injected delimiter is escaped.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/core/prompts/test_chat_prompt_builder.py`:
- Around line 121-135: Add a mixed-case closing-tag variant to
test_prepend_system_prompt_escapes_closing_tag_in_custom_prompt, while
preserving the existing assertions that only the builder’s genuine delimiter
remains unescaped and the injected delimiter is escaped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e213e3c-7346-4321-9619-d7b3215e81f8
📒 Files selected for processing (5)
openrag/core/prompts/chat_prompt_builder.pyopenrag/prompts/templates/sys_prompt_tmpl.txtopenrag/services/orchestrators/query_service.pytests/unit/core/prompts/test_chat_prompt_builder.pytests/unit/services/orchestrators/test_query_service.py
🚧 Files skipped from review as they are similar to previous changes (3)
- openrag/prompts/templates/sys_prompt_tmpl.txt
- openrag/services/orchestrators/query_service.py
- tests/unit/services/orchestrators/test_query_service.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A leading role="system" message in a chat request is now treated as a
pinned instruction rather than a chat turn: it survives
chat_history_depth truncation, never leaks into query-generation as a
fake conversation turn, and is spliced into the RAG system prompt right
after the intro paragraph rather than just prepended raw.
Security: the custom content is wrapped in an <unsafe_custom_prompt>
tag to flag it as untrusted input, and both prompt templates gained a
Security rule instructing the model to keep following its core rules
regardless of what that block says.
Summary by CodeRabbit
{custom_prompt}support across both system and spoken-style templates, injected into the context area.{custom_prompt}placeholder.{custom_prompt}wrapping/sanitization and system-message merge/truncation scenarios.