Skip to content

fix(agent): uniquify duplicate tool_call ids and never send an empty assistant turn - #72766

Closed
jonnyallum wants to merge 1 commit into
NousResearch:mainfrom
jonnyallum:fix/duplicate-tool-call-ids-empty-assistant
Closed

fix(agent): uniquify duplicate tool_call ids and never send an empty assistant turn#72766
jonnyallum wants to merge 1 commit into
NousResearch:mainfrom
jonnyallum:fix/duplicate-tool-call-ids-empty-assistant

Conversation

@jonnyallum

Copy link
Copy Markdown

What does this PR do?

Some providers reuse tool_call ids across turns instead of emitting globally unique ones. Kimi K3 (Moonshot, via OpenRouter) emits ids shaped <tool_name>_<n>. In one observed session terminal_46 was emitted eight separate times, and after a compression pass three of them were live in the same context window at once.

Duplicate ids break result pairing. The results attach to the first assistant message carrying that id, the later assistant turns lose their tool_calls, and one ends up with neither content nor tool_calls. Moonshot rejects that shape with a non-retryable 400:

Invalid request: the message at position 30 with role 'assistant' must not be empty

The failed turn is then persisted, so every subsequent message rebuilds the same illegal transcript and fails in roughly a second with a single API call. The agent never recovers on its own — it presents as a hang rather than an error, which is how it was originally reported ("it dies and won't come back"). Restarting the gateway does not help, because the poison is in the session, not the process.

This adds sanitize_tool_call_pairing() to the pre-request path, next to the existing surrogate sanitizer. It operates on the outbound copy only, so stored history is never mutated. It:

  • renames a tool_call id already seen earlier in the same request, rewriting the matching tool results together so pairing is preserved;
  • replaces an assistant turn that has neither content nor tool_calls with a short placeholder.

An assistant turn with empty content but real tool_calls is deliberately left untouched — that shape is legal, and rewriting it would change what the model sees.

Why this approach

Fixing it on the outbound copy rather than at persist time means no migration, no mutation of existing sessions, and no behaviour change for providers that already emit unique ids (the sanitizer returns False and makes no edits on a clean transcript, which is pinned by a test).

Related Issue

Relates to #66429.

That issue tracks the same visible shape — empty assistant messages reaching the wire and never appearing in state.db — but a different producer: the builder appending {"role":"assistant","content":"","tool_calls":[]} in a runaway loop. What I hit is an id collision stripping tool_calls off a real turn. The id-collision cause does not appear to have been identified yet, so I believe this is complementary rather than a duplicate.

The second guard here (never emit an assistant turn with neither content nor tool_calls) also covers the shape described in #66429 and the phantom content:"" messages in #63200 at the wire, though it does not address the runaway append site itself — that is still worth fixing separately, since this only stops the bad shape leaving.

Searched open and merged issues and PRs first, per CONTRIBUTING. #31175, #31582 and #47280 sanitize adjacent shapes (empty content with tool_calls, partial-stream empty turns); none covers duplicate ids.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/message_sanitization.py — new sanitize_tool_call_pairing(), exported in __all__.
  • agent/conversation_loop.py — import it and call it immediately after _sanitize_messages_surrogates(api_messages).
  • tests/agent/test_duplicate_tool_call_ids.py — 7 tests.

Testing

pytest tests/agent/test_duplicate_tool_call_ids.py — 7 passed.

Coverage:

  • two turns reusing one id: ids come out unique and each result stays attached to the call it actually answered
  • the observed three-way collision in a single window
  • the exact rejected shape (no content, no tool_calls) gets a placeholder
  • empty content with tool_calls is left alone (legality guard)
  • content: None handled
  • a clean transcript is returned unchanged and reports False (no churn)
  • non-dict entries do not raise

Also validated against the real captured request that Moonshot rejected: replaying it through the sanitizer took illegal empty-assistant turns from 1 to 0 with no orphaned tool results introduced. Then ran live against Moonshot for several hours of agent traffic, including multi-tool turns, with no recurrence.

Reproduced on 0.18.2 with moonshotai/kimi-k3 via OpenRouter, Windows 11, Python 3.11.

…assistant turn

Some providers reuse tool_call ids across turns rather than emitting globally
unique ones. Kimi K3 (Moonshot, via OpenRouter) emits ids shaped
`<tool_name>_<n>`; one observed session emitted `terminal_46` eight separate
times, and after a compression pass three of them were live in the same
context window at once.

Duplicate ids break result pairing. The results attach to the first assistant
carrying that id, the later assistant turns lose their `tool_calls`, and one is
left with neither content nor tool_calls. Moonshot rejects that shape with a
non-retryable 400:

    Invalid request: the message at position 30 with role 'assistant'
    must not be empty

Because the failed turn is persisted, every later request rebuilds the same
illegal transcript and fails in about a second with a single API call. The
agent never recovers on its own, so it presents as a hang rather than an
error.

`sanitize_tool_call_pairing()` runs on the outbound copy next to the existing
surrogate sanitizer, so stored history is never mutated. It:

  - renames a tool_call id that has already been seen in this request and
    rewrites the matching results together, preserving pairing;
  - replaces an assistant turn that has neither content nor tool_calls with a
    short placeholder.

An assistant turn with empty content but real tool_calls is deliberately left
alone, since that shape is legal and rewriting it would change what the model
sees.

This is a distinct producer of the empty-assistant shape in NousResearch#66429, which
tracks the builder appending empty turns in a runaway loop, and of the phantom
`content:""` messages in NousResearch#63200. The second guard here covers those shapes on
the wire as well, but the id-collision cause appears not to have been
identified yet.

Co-Authored-By: Claude <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/kimi Kimi / Moonshot sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 27, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Both halves of this PR are now resolved on main. The duplicate tool_call id uniquification landed via commit 474c84e (_uniquify_tool_call_ids, wired at the finalization chokepoint in conversation_loop) — your Kimi K3 terminal_46-emitted-eight-times analysis matches exactly the case it covers, including post-compression collisions. The empty-assistant-turn half is resolved via PR #73071: repair_empty_non_final_messages in the sanitize_api_messages chokepoint now owns empty-content wire repair for all user/assistant turns. Thanks for the detailed session forensics @jonnyallum — closing as resolved on both fronts.

@teknium1 teknium1 closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/kimi Kimi / Moonshot sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants