Normalize mixed Anthropic tool history in Chat requests - #40
Merged
Merged
Conversation
Reviewer's GuideThis PR adds pre-routing normalization for recognizable Anthropic tool, result, image, and thinking blocks embedded in Chat history, with fail-closed validation and preserved native behavior, then verifies the conversion across request policies, streaming, backend profiles, and documented client behavior. Sequence diagram for Chat history normalization before routingsequenceDiagram
participant Client
participant ChatEndpoint
participant normalize_chat_messages
participant Upstream
Client->>ChatEndpoint: POST /v1/chat/completions
ChatEndpoint->>normalize_chat_messages: normalize_chat_messages(messages)
normalize_chat_messages->>normalize_chat_messages: Convert tool_use to tool_calls
normalize_chat_messages->>normalize_chat_messages: Convert tool_result to role=tool
normalize_chat_messages->>normalize_chat_messages: Map thinking to reasoning_content
alt Valid history
normalize_chat_messages-->>ChatEndpoint: Normalized messages
ChatEndpoint->>Upstream: Route Chat request
Upstream-->>Client: Chat response or stream
else Invalid or unsupported history
normalize_chat_messages-->>ChatEndpoint: HTTPException 400
ChatEndpoint-->>Client: Local HTTP 400
end
Flow diagram for Anthropic block conversion in Chat messagesflowchart LR
A[Anthropic block array in Chat message] --> B{Block type}
B -->|tool_use| C[Chat tool_calls]
B -->|tool_result| D[role=tool with tool_call_id]
B -->|thinking| E[reasoning_content]
B -->|image or text| F[Chat content parts]
C --> G[Preserve ID and JSON arguments]
D --> H[Preserve result images and error marker]
E --> I[Do not expose thinking as visible text]
F --> J[Keep content order]
G --> K[Validated Chat history]
H --> K
I --> K
J --> K
B -->|redacted_thinking or unsupported block| L[Local HTTP 400]
K --> M[Upstream routing]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/adapters/chat_input.py" line_range="106-116" />
<code_context>
+ else:
+ parts.append(_content_part(block, location))
+ if results:
+ # Keep results adjacent to the assistant calls, before any follow-up user text/images.
+ if parts and result_ids != pending.keys():
+ raise _invalid(param, "All pending tool results must precede follow-up user content")
+ return [*results, *([{**message, "content": parts}] if parts else [])]
+ out = {**message, "content": parts if parts else None}
+ if calls:
</code_context>
<issue_to_address>
**issue (bug_risk):** When a user content array contains ordinary text or images before and after a `tool_result`, `_convert_message` emits every tool result first and then emits all ordinary parts in a separate user message. This moves content that preceded the tool result after it, so the normalized history does not preserve the original text/image order.
**Triggers:** When an Anthropic user message mixes `tool_result` blocks with ordinary text or image blocks.
**Suggested fix:** Preserve the original sequence when splitting the message, or reject layouts whose ordinary content cannot be represented without reordering.
```suggestion
elif kind == "tool_result":
if role != "user":
raise _invalid(location, "tool_result requires a user message")
if message.keys() - {"role", "content"}:
raise _invalid(param, "Message-level attributes cannot be assigned safely when splitting tool_result content")
if parts:
raise _invalid(param, "Ordinary content must follow tool results")
result = _tool_result(block, location)
identifier = result["tool_call_id"]
if pending.get(identifier) != 1 or identifier in result_ids:
raise _invalid(location + ".tool_use_id", "tool_result must match one preceding, unanswered tool call")
result_ids.add(identifier)
results.append(result)
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: app/adapters/chat_input.py:116
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfcb3d1b4b
ℹ️ 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".
| if not isinstance(messages, list) or not messages or any(not isinstance(message, dict) for message in messages): | ||
| raise HTTPException(status_code=400, detail={"error": { | ||
| "message": "messages must be a non-empty array of objects", "type": "invalid_request_error"}}) | ||
| messages = normalize_chat_messages(messages) |
There was a problem hiding this comment.
When a Chat request has no leading system message and places its sole system or developer message between an assistant tool_use and its matching user tool_result, normalization runs before the existing system-message hoisting. The normalizer clears its pending-call state on that intervening message and returns HTTP 400, even though the subsequent logic would have moved the system message to the front and produced valid adjacent assistant/tool messages. Hoist/normalize system messages first, or preserve pending calls across a message that will be hoisted.
Useful? React with 👍 / 👎.
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.
Changes
tool_use/tool_resulthistory embedded in Chat requests after system/developer placement and before upstream routing, fixing HTTP 400 / code11101for unsupported content blocks. Keep error parameter paths tied to the caller's original message indices.thinkingtoreasoning_contentwhile retaining native Chat fields and leaving the caller's input unchanged.redacted_thinkinglocally with a precise parameter path. Keep image limits before conversion and enforce the final wire-size limit afterward.Verification
hy3requests throughintl-cli: mixed tool history, native Chat history, late system/developer instructions and streaming thinking/tool history all returned HTTP 200 and the expected tool-result marker; the stream completed with[DONE]and no error events.Scope and rollback
redacted_thinkingrequires the Messages protocol. User messages split into tool results accept onlyroleandcontent, with all results before ordinary text/images rather than silently reordering them.Summary by Sourcery
Normalize mixed Anthropic conversation history before routing Chat requests while preserving supported content and rejecting unsupported input locally.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: