fix(anthropic): keep mid-conversation system out of the chat completions system prompt - #38053
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR keeps only the leading Anthropic system-message run in the top-level system prompt and places or converts later system messages according to model capability, preserving prompt-cache prefixes.
Confidence Score: 4/5The PR appears safe to merge after the non-blocking test-helper typing issue is addressed The production translation changes preserve leading-system behavior and add model-aware handling with substantial regression coverage; the only accepted issue is incomplete typing in new test helpers Files Needing Attention: tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py and sibling provider test files
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/mid_conversation_system.py | Adds the shared pure placement, conversion, content, and cache-control policy for later Anthropic system messages |
| litellm/llms/anthropic/chat/transformation.py | Splits the leading system run and applies cost-map-driven mid-conversation handling before Anthropic wire translation |
| litellm/litellm_core_utils/prompt_templates/factory.py | Extends Anthropic message formatting to preserve placement-approved system-role messages |
| tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py | Adds broad transformation and cache-prefix regression coverage, but new helpers violate the repository's full-typing requirement |
| tests/e2e/llm_translation/test_chat_mid_conversation_system_e2e.py | Adds live Anthropic and Bedrock Invoke checks for successful completions and retained prompt-cache reads |
Reviews (1): Last reviewed commit: "test(e2e): prove the prompt cache surviv..." | Re-trigger Greptile
| CACHED_SYSTEM_BLOCK = {"type": "text", "text": "You are terse.", "cache_control": {"type": "ephemeral"}} | ||
|
|
||
|
|
||
| def _chat_request(config, model, messages): |
There was a problem hiding this comment.
The new _chat_request helper and related helpers omit parameter or return annotations, increasing untyped code and regressing the repository's type-discipline checks
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Bedrock Invoke proof, same runner as the description ( Before (f005afa)After (2999f96) |
2999f96 to
324213a
Compare
| note, | ||
| *(_openai_text_block(part) for part in _text_parts(message)), | ||
| ] | ||
| turn: Final[ChatCompletionUserMessage] = {"role": "user", "content": content} |
There was a problem hiding this comment.
Low: System instructions are downgraded to user content
This sends a trusted mid-conversation instruction in the same role—and often the same merged turn—as attacker-controlled user text. On unflagged or unknown models across Anthropic, Bedrock Invoke, Vertex AI, and Azure AI, a user can now override a later safety or data-handling system reminder; the textual operator note does not preserve the provider-enforced role hierarchy. Preserve the top-level system role by default, or reject these requests unless the caller explicitly opts into the cache-preserving role downgrade.
There was a problem hiding this comment.
Deliberate, and the same policy #36968 shipped for /v1/messages after the discussion on #36559 (maintainer and reporter both preferred convert-in-place as the default). Models flagged supports_mid_conversation_system keep the message as role=system, so system authority is preserved wherever the provider can honor it. Models without the flag reject the role inside messages outright, so the only alternatives are hoisting (which rewrites the cached prefix and re-bills the whole history every turn, the bug this PR fixes) or dropping the instruction. The converted turn is prefixed with an explicit operator note so the model can tell it apart from user text, and the leading system prompt, where standing instructions belong, is untouched.
There was a problem hiding this comment.
Thanks for the detail. I can't automatically confirm this is safe to dismiss, so I'm leaving the thread open for a maintainer to make the call.
PR overviewThis pull request changes Anthropic message handling so mid-conversation system messages are not placed in the Chat Completions system prompt. The updated logic applies across the supported Anthropic-related provider paths. One security issue remains open: mid-conversation system instructions may be converted into user-role content and merged with user-controlled text. On affected model configurations, this can allow a user to override later behavioral or data-handling instructions, though exploitation depends on callers using such instructions and an unflagged or unknown model. Open issues (1)
Fixed/addressed: 0 · PR risk: 4/10 |
e285564 to
154bb53
Compare
Pure functions over the OpenAI-format message list: split off the leading system run, keep later system messages as role=system at a placement Anthropic accepts on models flagged supports_mid_conversation_system (after a user turn, before an assistant turn or the end, never adjacent), and convert them to user turns in place elsewhere, keeping tool_result first in a merged user turn.
…ons system prompt translate_system_message hoisted every role=system message, at any index, into the top-level system block. On a conversation carrying a mid-session reminder that rewrites the cached prefix, so the provider re-bills the whole history at cache-write pricing on every turn (BerriAI#36559). BerriAI#36968 fixed this on /v1/messages; the chat completions path, shared by first-party Anthropic, Vertex, Azure AI and Bedrock Invoke, still hoisted. Only the leading system run becomes the system prompt now. Later system messages go through the placement policy, and anthropic_messages_pt emits a system message instead of rejecting the role. The caller's message list is no longer mutated. Tests pin the two-turn prefix invariant across all four chat configs and both flag states.
The /v1/messages pass-through and the chat completions path must prefix a converted system turn with the same operator note.
…reminder on chat completions Same priming and assertions as the /v1/messages cases, through /v1/chat/completions with OpenAI-format messages, for first-party Anthropic and Bedrock Invoke on a flagged (Opus 4.8) and an unflagged (Haiku 4.5) model. The reminder sits between the assistant turn and the next user turn, the shape OpenAI-style agent frameworks send, which is the placement the chat path has to translate.
…test helpers Codecov flagged the 5m ttl branch and the empty-system path of the wire builder; both now have a test. Greptile asked for full typing on the new test helpers.
…supports_ helper supports_mid_conversation_system joins the other supports_* helpers in litellm.utils, so the chat transformation stops importing the private _supports_factory.
The Final sweep tightened LIT010, which exempts TypeAlias declarations but counts a bare alias assignment as an unannotated binding.
…sage union The translator now emits role=system inside messages for models that accept it, so anthropic_messages_pt returns the pass-through union. add_code_execution_tool still declared the narrower user/assistant union while only ever reading content, so upstream's strip_advisor_blocks_from_messages call in between made the mismatch visible to the type checker.
154bb53 to
c13c9c8
Compare
TLDR
Problem this solves:
systemmessage into the top-level system prompt/v1/messagesonly;/v1/chat/completionsstill hoistsHow it solves it:
role: systemat a placement Anthropic acceptsUser Flow
Before: an agent framework that talks OpenAI-style to the proxy and injects a system reminder mid-session pays cache-write pricing for the whole history on every reminder
claude-opus-4-7with a cached system prompt and a short first turn, and getcache_creation_input_tokensfor the prefixcache_read_input_tokensequal to that prefix{"role": "system", "content": "<system-reminder>...</system-reminder>"}before the new user messagecache_read_input_tokens: 0and the whole prefix back undercache_creation_input_tokens: the reminder was folded into the system prompt, so the prefix no longer matchesAfter: the same conversation keeps reading its cached prefix across reminders
claude-opus-4-7with a cached system prompt and a short first turn, and getcache_creation_input_tokensfor the prefixcache_read_input_tokensequal to that prefix{"role": "system", "content": "<system-reminder>...</system-reminder>"}before the new user messagecache_read_input_tokensat least the primed prefix and only the new turn undercache_creation_input_tokens; the reply still follows the reminderclaude-opus-4-8the same request also keeps reading the prefix, and the reminder reaches the model with system authorityRelevant issues
Follow-up to #36968, which fixed the same class on
/v1/messages(see the closing discussion on #36559, where making the policy universal and cost-map driven was agreed as the next step).Pre-Submission checklist
Screenshots / Proof of Fix
Live proxy on localhost:4000 (
python litellm/proxy/proxy_cli.py --config proof_config.yaml --port 4000, two deploymentsanthropic/claude-opus-4-7andanthropic/claude-opus-4-8on a real Anthropic key), same three curls per case: turn one writes the cache, turn one again reads it, turn two adds the mid-conversation system reminder. The cached prefix spans a short system prompt plus a ~4.7k-token user turn carrying the breakpoint, the shape a hoisted reminder invalidates. Runner script and config:pr1-proof.shandproof_config.yamlin the description below.Before (f005afa)
anthropic-opus-4-7 (unflagged: reminder converted to a user turn)
anthropic-opus-4-8 (flagged: reminder stays role=system)
After (05180a5)
anthropic-opus-4-7 (unflagged: reminder converted to a user turn)
anthropic-opus-4-8 (flagged: reminder stays role=system)
pr1-proof.sh
proof_config.yaml (Anthropic deployments used above; the Bedrock ones are for the follow-up comment once the new AWS account clears verification)