fix(anthropic): preserve interleaved thinking/tool_use block order on replay - #35586
fix(anthropic): preserve interleaved thinking/tool_use block order on replay#35586Spaceman-Spiffy wants to merge 3 commits into
Conversation
… replay
Interleaved-thinking turns (adaptive thinking, Claude 4.6+/Opus 4.8) emit
content blocks like:
thinking_1(signed) tool_use_1 thinking_2(signed) tool_use_2
Anthropic signs each thinking block against the turn content preceding it
at its position. normalize_response split the turn into two parallel lists
(reasoning_details + tool_calls), discarding cross-type order, and
_convert_assistant_message rebuilt it as [all thinking][text][all tool_use].
That moved thinking_2 ahead of tool_use_1, invalidating its signature, so
Anthropic rejected the latest assistant message with HTTP 400:
messages.N.content.M: `thinking` or `redacted_thinking` blocks in the
latest assistant message cannot be modified.
Observed repeatedly in agent.conversation_loop against api.anthropic.com /
claude-opus-4-8, recurring across sessions on multi-thinking-block turns.
Fix: carry a verbatim, order-preserving copy of the turn's content blocks
(anthropic_content_blocks) end-to-end - capture in normalize_response,
persist/restore through state.db, and replay unchanged for the latest
assistant message. Gated to turns that actually interleave signed thinking
with tool_use, so normal turns are unaffected.
Adds 3 regression tests including a SQLite round-trip covering the
crash-recovery reload path.
alt-glitch
added
type/bug
|
Related: #24107 (preserve thinking blocks on prior tool_use turns — don't strip) and #17861 (multi-turn history loses thinking blocks). This PR addresses a different aspect: preserving the interleaved order of thinking + tool_use blocks within a single turn, which matters for Claude 4.6+ signed thinking blocks. The signature verification fails when blocks are reordered even if all blocks are present. |
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ✅
Changes
- Multiple files across the Anthropic adapter, transport, state layer, and run_agent: preserve verbatim interleaved thinking/tool_use block order through replay
tests/agent/test_anthropic_thinking_block_order.py: 3 new regression tests
Review
✅ Correctness
- Root cause analysis is thorough:
normalize_responsesplit interleaved thinking+tool_use into parallel lists, losing cross-type ordering;_convert_assistant_messagereconstructed them as[all thinking][text][all tool_use], front-loading thinking blocks and invalidating Anthropic's signatures - Fix is elegant and targeted: carry a verbatim, order-preserving copy of content blocks that bypasses the reconstruction path entirely
- The gating logic (
_has_signed_thinking and _has_tool_use) means pure-text and single-leading-thinking turns are completely unaffected — zero overhead for common cases - Fallback path is preserved: old sessions without the column degrade gracefully to the existing reconstruction
_ensure_columnsinhermes_state.pyauto-migrates — no manual schema changes needed
✅ Testing
- 3 regression tests: lossy-split confirmation, replay-order preservation, SQLite round-trip (crash-recovery mirror)
- All fail on
main, pass with this change — correct RED-GREEN - Broader test suite: 496 passed in related modules
✅ Code Quality
- Well-documented — each change includes inline comments explaining the why
- Clean separation of concerns: transport captures blocks, adapter replays them, state persists them
_sanitize_tool_idis preserved in the replay path- New SQLite column follows existing patterns exactly (consistent with
codex_reasoning_items,reasoning_details, etc.)
✅ Schema Evolution
- Column is NULLABLE and only populated for interleaved-thinking turns — no migration burden
- Old sessions without the column work fine (existing reconstruction path)
Summary
Excellent bug fix. Thorough root cause analysis, minimal change, well-tested, backward-compatible. This was a tricky intermittent bug (only occurs on multi-thinking-block turns) and the fix is surgical.
Reviewed by Hermes Agent (cron job)
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ✅
Review
A well-engineered fix for a tricky P1 bug where Anthropic's interleaved thinking blocks would get reordered on message replay, triggering HTTP 400 on the API.
✅ Looks Good
- Root cause analysis is thorough: Identified that
normalize_responsesplits content into parallel lists (reasoning_details + tool_calls), losing cross-type ordering, and Anthropic signs each thinking block against its position. - Elegant fix: Carries a verbatim
anthropic_content_blockschannel that bypasses the reconstruction path. Gated to only interleaved signed-thinking + tool_use turns — zero overhead for normal turns. - Full end-to-end coverage: The channel is preserved through
normalize_response→ stored message → SQLite → reload →convert_messages_to_anthropic. DB migration via auto-column-add. - Comprehensive tests: 3 regression tests covering lossy-split confirmation, replay-order preservation, and SQLite round-trip (crash-recovery path).
- Clean architecture: Changes touch 7 files but each change is minimal and focused. The
NormalizedResponseproperty pattern matches existingcodex_reasoning_items.
Checklist Summary
| Category | Status |
|---|---|
| Correctness | ✅ Interleaved thinking order preserved; gated to only affected cases |
| Security | ✅ No security concerns |
| Code Quality | ✅ Verbatim replay avoids reconstruction fragility; gated channel |
| Testing | ✅ 3 regression tests covering all paths, 496 existing tests pass |
| Performance | ✅ Near-zero for non-interleaved turns |
Reviewed by Hermes Agent (cron job)
…ocks HTTP 400 "messages.N.content.M.text.parsed_output: Extra inputs are not permitted" on the native Anthropic transport. Anthropic SDK 0.87.0 response blocks carry output-only attributes the Messages *input* schema forbids: text blocks get `parsed_output` and `citations=None`, tool_use blocks get `caller`. normalize_response captured blocks verbatim via _to_plain_data and replayed them as request input on the next turn, so the forbidden fields leaked back -> 400. Like the earlier thinking-block bug, one poisoned turn wedges every subsequent request in the session (even the diagnostic turn), recoverable only by switching models or deleting the session. This is a defect in the anthropic_content_blocks channel added for the interleaved-thinking fix: it preserved block ORDER correctly but copied every SDK attribute, including output-only ones. Fix — whitelist input-permitted fields per block type at all three leak points: - agent/transports/anthropic.py normalize_response: sanitize at CAPTURE so the poison never persists to state.db (defence-in-depth). - agent/anthropic_adapter.py _sanitize_replay_block (new): whitelist used on the ordered-blocks replay path; also recovers already-poisoned stored sessions. - agent/anthropic_adapter.py _convert_content_part_to_anthropic: a stored `text` part is rebuilt from whitelisted fields instead of dict(part) verbatim (this was the exact content.N.text.parsed_output failure locus). Whitelist not blacklist, so future SDK output-only fields can't reintroduce it. Block order and thinking-block signatures are preserved (the reason the channel exists). Adds tests/agent/test_anthropic_output_field_leak.py; full adapter suite green (163 tests). Existing poisoned state.db rows scrubbed out-of-band.
|
Update: pushed a follow-up commit ( While dogfooding this change on Opus 4.x with interleaved thinking + tools, I hit a session-wedging HTTP 400: Cause: the Fix (in Apologies for the re-review, but I thought it better to fold the fix in here than merge a version with a known wedging defect. |
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved ✅ — Preserve interleaved thinking/tool_use block order on replay for Anthropic API. Important correctness fix for streaming.
Reviewed by Hermes Agent
…ay 400 recovery Two additive hardening changes on the interleaved-thinking replay path introduced by this PR's anthropic_content_blocks channel. Both are scoped to that channel's blast radius; neither changes correct behavior. 1. Replay-time tool-input re-sourcing (credential safety). The ordered-block channel captures each tool_use `input` from the RAW API response in normalize_response, which is NOT credential-redacted. The parallel tool_calls[].function.arguments IS redacted at storage time (build_assistant_message, NousResearch#19798). The verbatim-replay fast path in _convert_assistant_message replayed the raw block input, so a secret a model inlined into a tool call (e.g. an Authorization header value passed inside a terminal command) would ride back onto the wire even though it is redacted everywhere else in history. Re-source tool_use input from the redacted tool_calls map by sanitized id; interleave order (the reason this channel exists) is unaffected. Adapted from NousResearch#36071, which re-sources tool inputs the same way on its replay path. 2. Broaden the thinking-replay 400 classifier (defense-in-depth). error_classifier only matched "signature" + "thinking", so the frozen-block variant — "thinking ... blocks in the latest assistant message cannot be modified. These blocks must remain as they were in the original response." — carried no "signature" token and fell through to a non-retryable abort. The anthropic_content_blocks channel prevents the reorder that triggers this 400 at the source, but if any future mutator reintroduces it, the turn now self-heals via the existing strip-reasoning-and-retry recovery instead of crash-looping. A negative case ensures an unrelated "cannot be modified" 400 (no "thinking") is not swept in. Mirrors the classifier broadening in NousResearch#36087 and NousResearch#36071. Tests - tests/agent/test_anthropic_thinking_block_order.py: a replay test asserting an inlined secret is redacted on the wire while interleave order is preserved. - tests/agent/test_error_classifier.py: three cases — frozen-block 400 native and via OpenRouter route to thinking_signature/retryable; an unrelated "cannot be modified" 400 does not. Both grafts verified RED (tests fail with the change reverted) then GREEN. Full adapter, transport, classifier and output-field-leak suites pass. Co-authored-by: AlexanderBFoley <92330381+AlexanderBFoley@users.noreply.github.com>
|
Merged via PR #43943 — your commits were cherry-picked onto current main with your authorship preserved in git log (aaccaad, 529bb1c, 7a1eed8). One adjustment during salvage: the state.db persistence was dropped in favor of an in-memory-only channel (crash-resume falls back to reconstruction, absorbed by the #43667 recovery), and the error_classifier hunk was already on main via #43667. Excellent work — the RED/GREEN-verified tests and the production block coordinates made this an easy salvage. |
4 participants
What does this PR do?
Fixes an intermittent HTTP 400 from Anthropic on multi-step agentic turns:
Root cause. With adaptive/interleaved thinking (Claude 4.6+, e.g. Opus 4.8), a single assistant turn interleaves signed
thinkingblocks withtool_useblocks. Anthropic signs each thinking block against the turn content preceding it at its position.AnthropicTransport.normalize_responsesplit the turn into two parallel lists —reasoning_details(thinking) andtool_calls(tool_use) — discarding cross-type ordering, and_convert_assistant_messagerebuilt the turn as[all thinking][text][all tool_use]. This front-loads thinking, movingthinking_2(signed withtool_use_1before it) ahead oftool_use_1. The signature no longer matches its position, and the API rejects the latest assistant message.It recurs only on turns with multiple thinking blocks interleaved with tool calls (high/xhigh-effort agentic work), which is why it is intermittent. Confirmed in local logs as
agent.conversation_loopfailures againstapi.anthropic.com/claude-opus-4-8, at block indicescontent.6/.7/.8/.10/.13.Fix (preserve original order). Carry a verbatim, order-preserving copy of the turn's content blocks (
anthropic_content_blocks) end-to-end and replay it unchanged for the latest assistant message, instead of reconstructing from the parallel lists. The channel is gated — populated only when a turn actually interleaves signed thinking withtool_use, so pure-text and single-leading-thinking turns are untouched (near-zero overhead).Related Issue
No existing issue — root cause analysis and reproduction are documented inline.
Type of Change
Changes Made
agent/transports/anthropic.py—normalize_responsecaptures ordered blocks; gated to interleaved signed-thinking + tool_use turns.agent/transports/types.py—anthropic_content_blocksproperty onNormalizedResponse.agent/chat_completion_helpers.py—build_assistant_messagelifts the channel onto the stored message.agent/anthropic_adapter.py—_convert_assistant_messagereplays verbatim blocks when present.hermes_state.py— newanthropic_content_blockscolumn (auto-migrates via_ensure_columns), wired through both insert paths and the conversation-restore deserialize.run_agent.py— passes the field through toappend_message.tests/agent/test_anthropic_thinking_block_order.py— 3 regression tests.How to Test
pytest tests/agent/test_anthropic_thinking_block_order.py -v— 3 tests: lossy-split confirmation, replay-order preservation, and a SQLite round-trip mirroring crash-recovery reload. All fail onmain, pass with this change.pytest tests/agent/test_anthropic_adapter.py tests/agent/transports/ -q→ 496 passed.Checklist
fix(anthropic):)pytest tests/and tests pass