Strip the billing header from the first part of the system msg in anthropic#323
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbe38ee7bc
ℹ️ 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 (@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 (@codex) address that feedback".
| UserContent::Array(parts) => { | ||
| let before = parts.len(); | ||
| parts.retain(|part| !is_billing_header_part(part)); |
There was a problem hiding this comment.
Strip only the marker line from text blocks
When Anthropic system is an array and the first text block contains both the billing line and real instructions (for example "x-anthropic-billing-header: ...\nYou are helpful"), this retain drops the entire block, so non-Anthropic transforms silently lose the actual system prompt. The string branch handles this shape by removing only the first line; the array branch should do the same for matching Text parts and only remove the part if it becomes empty.
Useful? React with 👍 / 👎.
|
Codex (@codex) review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2367608647
ℹ️ 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 (@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 (@codex) address that feedback".
| parts.retain_mut(|part| { | ||
| let UserContentPart::Text(text) = part else { | ||
| return true; | ||
| }; | ||
| if !text | ||
| .text | ||
| .trim_start() | ||
| .starts_with(CLAUDE_CODE_BILLING_PREFIX) |
There was a problem hiding this comment.
Limit stripping to the leading billing block
This scans every text block in every system message, so an Anthropic prompt like [{text: "real instructions"}, {text: "x-anthropic-billing-header: ..."}] loses the later block even though the Claude Code attribution described above is only the prepended leading block. For non-Anthropic transforms that include literal examples or instructions beginning with this header name after the real prompt, this silently changes user-visible system instructions; restrict the removal to the first system content block/line that represents the attribution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Erin McNulty (@erin2722) I don't actually know if this is correct. I think you can have multiple system messages with anthropic now so im not sure if it's correct to only check the first block
There was a problem hiding this comment.
Ahhhh interesting, yeah I believe you can, and the edge case it calls out is super niche lol. Lets leave this as is for now
| parts.retain_mut(|part| { | ||
| let UserContentPart::Text(text) = part else { | ||
| return true; | ||
| }; | ||
| if !text | ||
| .text | ||
| .trim_start() | ||
| .starts_with(CLAUDE_CODE_BILLING_PREFIX) |
There was a problem hiding this comment.
Erin McNulty (@erin2722) I don't actually know if this is correct. I think you can have multiple system messages with anthropic now so im not sure if it's correct to only check the first block
This was causing caching to be broken for claude when using the gateway with different models-- see musistudio/claude-code-router#1220 for a similar issue.
We can just strip the stray text in lingua so that our users do not need to worry about this.