fix(claude): keep mid-conversation system messages in the timeline - #4161
Conversation
Claude Code sends role:"system" entries inside `messages`, and every one of
them was folded into `instructions`. The parser pushes `data.instructions`
onto the system prompt before anything else, so each injected reminder rewrote
the head of the prompt and invalidated the upstream KV cache prefix. Where no
metadata.user_id is present, the Desktop prompt_cache_key fallback hashes the
same system parts, so the cache key rotated along with it.
Emit each non-empty in-messages system message as a chronological
{ type: "message", role: "developer", content: [{ type: "input_text", text }] }
input item instead. Top-level Anthropic `system` keeps flowing through
systemToInstructions into body.instructions unchanged, so `instructions` now
has exactly one source and stops moving mid-conversation.
Not role:"system" in `input`. The schema allows it, but ChatGPT Codex rejects
it, parseRequest re-hoists it onto the system prompt, and the canonical
Responses forward folds text-only system items back into instructions - so it
would land back where it started. `developer` is first-class in the schema and
keeps timeline order.
The fold was deliberate in origin (cee918c) so native ChatGPT would not 400
on a system input item. That constraint is real and still respected; the
2026-07-11 note that folding is "the only shape that works on every route" is
what turned out to be stale.
systemParts keeps its array shape in the cache-key hash on purpose. Joining it
to a string would rotate every existing cohort key for a reason unrelated to
this bug.
Closes #4148.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughClaude inbound translation now keeps top-level system content in ChangesClaude system translation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Message-level Claude system messages now remain chronological developer inputs while top-level instructions stay stable, preserving prompt-cache reuse without leaving a concrete current-head merge risk. Sequence Diagram(s)sequenceDiagram
participant ClaudeMessages
participant InboundTranslator
participant ResponsesRequest
participant PromptCache
ClaudeMessages->>InboundTranslator: Send top-level and message-level system content
InboundTranslator->>ResponsesRequest: Set top-level content as instructions
InboundTranslator->>ResponsesRequest: Append message-level content as developer input items
ResponsesRequest->>PromptCache: Preserve the existing input prefix
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
리뷰 · 우선순위 74 / 80이 PR은 이슈 #4148을 닫는 Lane A 다음 단위다. 바로 직전 tip 고치는 방법은 단순하다. 비어 있지 않은 in-messages system은 시간 순서대로 Responses 범위 결정도 문서에 분명히 적혀 있다. 맨 앞 system만 접고 중간만 timeline에 남기는 좁은 안은 계약 테스트를 덜 건드렸겠지만, 매 턴 맨 앞에 새 system을 넣는 클라이언트는 여전히 회귀는 src/claude/inbound.ts (system 분기) - 옛 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 799330bcfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // schema and survives parseRequest as a chronological message, where `system` | ||
| // would be re-hoisted back onto the system prompt and defeat the point. | ||
| if (text.length > 0) { | ||
| input.push({ type: "message", role: "developer", content: [{ type: "input_text", text }] }); |
There was a problem hiding this comment.
Avoid splitting tool calls from their results
When a role:"system" entry occurs between an assistant tool_use and its matching user tool_result, inserting this developer item breaks their adjacency. On the Ollama-native route, buildNativeMessages treats the developer item as a hard boundary and calls flushPending(), which throws because the following result has not been processed yet; the Anthropic and Google adapters similarly synthesize a missing result and later downgrade the real result to an orphan. This request shape worked before because the system entry was folded out of the message timeline, so defer such reminders until after the result batch or make each affected adapter preserve the pending pair across this barrier.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| // schema and survives parseRequest as a chronological message, where `system` | ||
| // would be re-hoisted back onto the system prompt and defeat the point. | ||
| if (text.length > 0) { | ||
| input.push({ type: "message", role: "developer", content: [{ type: "input_text", text }] }); |
There was a problem hiding this comment.
Retain cache affinity without a top-level system field
For an accepted request that has in-message system entries but neither top-level system nor metadata.user_id, moving the text here leaves systemParts empty, so the fallback at the later else if (systemParts.length > 0) no longer emits any prompt_cache_key. The same request previously received a stable system-derived key, meaning clients that express all system context through messages now lose cache routing entirely. Track that this request contained system reminders and derive a stable model/tool cohort key that excludes their growing timeline text.
Useful? React with 👍 / 👎.
Summary
Claude Code sends
role: "system"entries insidemessages, and every one of them was folded intoinstructions.parseRequestpushesdata.instructionsonto the system prompt before anything else, so each injected reminder rewrote the head of the prompt — which is exactly the span an upstream KV cache matches on. Every mid-conversation reminder therefore invalidated the cache prefix. When the client sends nometadata.user_id, the Desktopprompt_cache_keyfallback hashes the same system parts, so the cache key rotated along with the prefix and the request could not even land on the same routing cohort.Each non-empty in-messages system message now becomes a chronological input item:
{ "type": "message", "role": "developer", "content": [{ "type": "input_text", "text": "..." }] }Top-level Anthropic
systemkeeps flowing throughsystemToInstructionsintobody.instructionsunchanged.instructionsnow has exactly one source and stops moving mid-conversation.Why
developerand notsystem. The schema admits asystemrole ininput, but ChatGPT Codex rejects it live,parseRequestre-hoists it onto the system prompt, and the canonical Responses forward folds text-only system items back intoinstructions— so it would land back where it started.developeris first-class in the schema, survivesparseRequestas an ordinary chronological message, and preserves timeline order.The original fold was deliberate, not an oversight:
cee918ce3introduced it so the native ChatGPT backend would not 400 on a system input item. That constraint is real and is still respected here — it rules outsystem, notdeveloper. What turned out to be stale is the accompanying 2026-07-11 note that folding is "the only shape that works on every route".A scope decision a reviewer may want to object to
All in-messages system messages become developer items, not only the ones after the first user turn.
The narrower alternative — hoist a leading system message, keep only mid-conversation ones in the timeline — would have left the old contract test untouched, but it does not close the issue. A client that injects a fresh leading system message on every turn still mutates
instructionsevery turn, and that is the reported failure. Taking the narrow option would have meant shipping something that looks like a fix and still rotates the prefix for the reporter.The cost is that privileged system text now arrives as developer text. On Anthropic and Google outbound, developer items are presented as chronological
usermessages, so there is real semantic drift from "this is a system instruction" to "this is part of the conversation". That is prefix-stable, which is the property this change is buying.systemPartsdeliberately keeps its array shape inside the cache-key hash. Joining it to a string would rotate every existing cohort key for a reason unrelated to this bug.Deliberately out of scope
src/adapters/openai-chat.tsre-hoists all text developer messages into a leadingsystemchat message for non-api.openai.comChat Completions, locked bytests/adapters/openai/openai-chat-system-order.test.ts. That is what keeps the reporter's DeepSeek/SenseNova path broken, and reversing it is a separate compatibility tradeoff with its own regression surface. OpenCode Go's Muse isopenai-responses, so this inbound fix does reach it.src/chat/inbound.tshas the same anti-pattern for Chat inbound; also not this issue.Verification
Remote CI at this PR's exact head SHA is the gate for this change.
Local checks: NOT RUN.
bun test,bun run test:changed,bun run typecheck,bun install,bun run build:gui,bun run lint:gui, andbun run privacy:scanwere all skipped by explicit maintainer instruction for this delivery round, which overrides the PR-ready gate inAGENTS.md. Nothing here claims a local check passed.Independent review that was done: a read-only reviewer confirmed that
developeris admitted bysrc/responses/schema.tsand kept chronological bysrc/responses/parser.tswhile asystemitem would be re-hoisted; that theprompt_cache_keyfallback hashes an array whose shape is unchanged; that the canonical Responses forward folds onlyrole: "system", so developer items survive it; and — the part that mattered most — searchedtests/exhaustively for any other assertion encoding the old fold and found none.Regression coverage in
tests/claude-integration/claude-inbound.test.ts:instructions === "top-level"and input roles["developer", "developer", "user"], and it still asserts that no input item carriesrole: "system".system: "S"with a reminder injected on each turn.instructionsis"S"on both turns, turn 2's roles are["user", "developer", "assistant", "user", "developer"]with textsu1, r1, a1, u2, r2, turn 1's items are a byte-equal prefix of turn 2's, and with nometadata.user_idthe two turns produce the sameprompt_cache_key. Both bodies passresponsesRequestSchema.parseandparseRequest.The cases using top-level
systemand the prompt-cache-key provenance suite are untouched and must stay green.Checklist
Closes #4148.
Summary by CodeRabbit