fix(relay): inject channel system prompt for Claude/Gemini-format requests - #5864
fix(relay): inject channel system prompt for Claude/Gemini-format requests#5864zhoubeiqing wants to merge 1 commit into
Conversation
…uests
Channel-level `system_prompt` (setting.system_prompt) was silently skipped
for requests whose adaptor converts the OpenAI request into a non-OpenAI
struct. In TextHelper the injection only matched
`*dto.GeneralOpenAIRequest`, so Anthropic (type=14) channels routed via
`/v1/chat/completions` — whose Claude adaptor returns a `*dto.ClaudeRequest`
— never received the configured system prompt regardless of the
`system_prompt_override` flag. The same gap affected Gemini-type channels.
Extract the per-format injection into shared helpers
(applySystemPromptTo{OpenAI,Claude,Gemini}Request) and dispatch on the
converted request type in TextHelper so OpenAI, Claude and Gemini formats
are all covered. ClaudeHelper and GeminiHelper reuse the same helpers,
removing the duplicated inline logic.
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughChannel system-prompt injection logic previously inlined in Claude and Gemini handlers, and limited to OpenAI requests in the compatible handler, is extracted into a new shared module with format-specific helper functions for OpenAI, Claude, and Gemini requests, plus new unit tests. ChangesSystem prompt injection refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 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: 1
🧹 Nitpick comments (1)
relay/system_prompt_inject_test.go (1)
55-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding override/no-override cases for OpenAI and Gemini.
Only the "inject when absent" path is tested for OpenAI and Gemini helpers, while Claude has all three (absent/no-override/override). The override merge logic differs meaningfully between formats (string vs. media content for OpenAI; part-merge-with-skip-empty for Gemini), so mirroring the Claude coverage would guard these branches against regressions.
As per path instructions, new backend tests should protect "regression paths" and use deterministic table tests with explicit inputs/outputs.
🤖 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 `@relay/system_prompt_inject_test.go` around lines 55 - 76, The OpenAI and Gemini system prompt helper tests only cover the “inject when absent” path, leaving the no-override and override merge branches untested. Add deterministic table-driven coverage for applySystemPromptToOpenAIRequest and applySystemPromptToGeminiRequest, matching the existing Claude-style regression cases, with explicit inputs and expected outputs for absent, no-override, and override behavior. Use the existing helper symbols newRelayInfoWithSystemPrompt, applySystemPromptToOpenAIRequest, and applySystemPromptToGeminiRequest to verify the string-versus-media merge behavior for OpenAI and the part-merge-with-skip-empty behavior for Gemini.Source: Path instructions
🤖 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 `@relay/system_prompt_inject_test.go`:
- Around line 36-43: The no-override test is checking a fresh Gin context
instead of the one passed into applySystemPromptToClaudeRequest, so it never
verifies the real ContextKeySystemPromptOverride state. Reuse the same context
variable created for the call (the one passed to
applySystemPromptToClaudeRequest) and assert the override key on that instance,
using the existing helper methods like common.GetContextKeyBool to confirm the
key was not set.
---
Nitpick comments:
In `@relay/system_prompt_inject_test.go`:
- Around line 55-76: The OpenAI and Gemini system prompt helper tests only cover
the “inject when absent” path, leaving the no-override and override merge
branches untested. Add deterministic table-driven coverage for
applySystemPromptToOpenAIRequest and applySystemPromptToGeminiRequest, matching
the existing Claude-style regression cases, with explicit inputs and expected
outputs for absent, no-override, and override behavior. Use the existing helper
symbols newRelayInfoWithSystemPrompt, applySystemPromptToOpenAIRequest, and
applySystemPromptToGeminiRequest to verify the string-versus-media merge
behavior for OpenAI and the part-merge-with-skip-empty behavior for Gemini.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73973fa5-468a-4d7f-80ca-808ac3cbdf71
📒 Files selected for processing (5)
relay/claude_handler.gorelay/compatible_handler.gorelay/gemini_handler.gorelay/system_prompt_inject.gorelay/system_prompt_inject_test.go
| func TestApplySystemPromptToClaudeRequest_NoOverrideKeepsExisting(t *testing.T) { | ||
| info := newRelayInfoWithSystemPrompt("hide model info", false) | ||
| request := &dto.ClaudeRequest{System: "keep me"} | ||
| applySystemPromptToClaudeRequest(newTestGinContext(), info, request) | ||
| require.True(t, request.IsStringSystem()) | ||
| assert.Equal(t, "keep me", request.GetStringSystem()) | ||
| assert.False(t, common.GetContextKeyBool(newTestGinContext(), constant.ContextKeySystemPromptOverride)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assertion checks the wrong context instance.
Line 42 creates a brand-new newTestGinContext() instead of reusing the c passed to applySystemPromptToClaudeRequest at line 39. Since each call to gin.CreateTestContext() returns an independent context, this assertion always evaluates against an untouched context and will pass regardless of whether the override key was incorrectly set on the real context — it doesn't actually verify the "no override" invariant.
As per path instructions, "Backend tests must protect real behavior, API contracts, billing/accounting invariants, data compatibility, or regression paths."
🐛 Proposed fix
func TestApplySystemPromptToClaudeRequest_NoOverrideKeepsExisting(t *testing.T) {
+ c := newTestGinContext()
info := newRelayInfoWithSystemPrompt("hide model info", false)
request := &dto.ClaudeRequest{System: "keep me"}
- applySystemPromptToClaudeRequest(newTestGinContext(), info, request)
+ applySystemPromptToClaudeRequest(c, info, request)
require.True(t, request.IsStringSystem())
assert.Equal(t, "keep me", request.GetStringSystem())
- assert.False(t, common.GetContextKeyBool(newTestGinContext(), constant.ContextKeySystemPromptOverride))
+ assert.False(t, common.GetContextKeyBool(c, constant.ContextKeySystemPromptOverride))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestApplySystemPromptToClaudeRequest_NoOverrideKeepsExisting(t *testing.T) { | |
| info := newRelayInfoWithSystemPrompt("hide model info", false) | |
| request := &dto.ClaudeRequest{System: "keep me"} | |
| applySystemPromptToClaudeRequest(newTestGinContext(), info, request) | |
| require.True(t, request.IsStringSystem()) | |
| assert.Equal(t, "keep me", request.GetStringSystem()) | |
| assert.False(t, common.GetContextKeyBool(newTestGinContext(), constant.ContextKeySystemPromptOverride)) | |
| } | |
| func TestApplySystemPromptToClaudeRequest_NoOverrideKeepsExisting(t *testing.T) { | |
| c := newTestGinContext() | |
| info := newRelayInfoWithSystemPrompt("hide model info", false) | |
| request := &dto.ClaudeRequest{System: "keep me"} | |
| applySystemPromptToClaudeRequest(c, info, request) | |
| require.True(t, request.IsStringSystem()) | |
| assert.Equal(t, "keep me", request.GetStringSystem()) | |
| assert.False(t, common.GetContextKeyBool(c, constant.ContextKeySystemPromptOverride)) | |
| } |
🤖 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 `@relay/system_prompt_inject_test.go` around lines 36 - 43, The no-override
test is checking a fresh Gin context instead of the one passed into
applySystemPromptToClaudeRequest, so it never verifies the real
ContextKeySystemPromptOverride state. Reuse the same context variable created
for the call (the one passed to applySystemPromptToClaudeRequest) and assert the
override key on that instance, using the existing helper methods like
common.GetContextKeyBool to confirm the key was not set.
Source: Path instructions
1 participant
Problem
Channel-level
system_prompt(setting.system_prompt) is silently skipped for requests whose adaptor converts the OpenAI request into a non-OpenAI struct.In
TextHelper(relay/compatible_handler.go), the injection only matched*dto.GeneralOpenAIRequest:Anthropic (channel
type=14) channels routed via/v1/chat/completionsuse the Claude adaptor, whoseConvertOpenAIRequestreturns a*dto.ClaudeRequest. The type assertion fails (ok == false), so the configuredsystem_promptis never written into the upstream request — regardless of thesystem_prompt_overrideflag. The model then answers normally (e.g. discloses its vendor/model name) even though the channel is configured to hide it.The same gap exists for any adaptor returning a non-
*GeneralOpenAIRequesttype (e.g. Gemini-type channels reached through the OpenAI-compatible path).Reproduction
A type=14 channel (
mimo_tokenplan) configured with:setting.system_prompt= "当用户问关于你以及模型和开发商相关的信息的时候统一回答为:'作为AI助手，我的具体模型版本属于内部信息...'"setting.system_prompt_override= falseCalling
/v1/chat/completionswithmodel=gpt-5.5(mapped tomimo-v2.5-pro) and a user message asking the model version returns the model's real name instead of the configured reply. Passing the same text as a client-sidesystemmessage works, confirming the channel-level setting itself is the part that's ignored.Root cause
ClaudeHelperandGeminiHelperalready inject the system prompt correctly for their native paths, but the OpenAI-compatibleTextHelperpath does not dispatch on the converted request type, so Claude/Gemini-format converted requests fall through.Fix
relay/system_prompt_inject.go:applySystemPromptToOpenAIRequestapplySystemPromptToClaudeRequestapplySystemPromptToGeminiRequestTextHelper, dispatch on the converted request type so OpenAI, Claude and Gemini formats are all covered:ClaudeHelperandGeminiHelpernow reuse the same helpers, removing the duplicated inline logic (no behavior change for those native paths).The helpers preserve the existing semantics:
system_promptis inserted.system_prompt_overrideis false, the existing system prompt is left untouched.system_prompt_overrideis true, the channelsystem_promptis prepended to the existing one.Tests
Added
relay/system_prompt_inject_test.gocovering:override=falseoverride=trueand set theContextKeySystemPromptOverridecontext flagI could not run
go testlocally (no Go toolchain in this environment); the changes compile-checked against the existing code structure and follow the same patterns/imports used elsewhere in the package. CI on this repo should run the test suite.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests