Skip to content

fix(agent): deduplicate tool_call_id across pre-API sanitizers (#58327) - #58350

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage/dedup-tool-call-id
Jul 4, 2026
Merged

fix(agent): deduplicate tool_call_id across pre-API sanitizers (#58327)#58350
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage/dedup-tool-call-id

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Summary

Strict providers (DeepSeek) reject a payload where the same tool_call_id appears more than once with HTTP 400: Duplicate value for 'tool_call_id'. This deduplicates tool_call_ids at both pre-API sanitizer chokepoints so the duplicate never reaches the provider.

Note on the issue framing: #58327 was filed as an orphaned tool message (tool without a preceding assistant tool_calls). Orphans are already handled on main (both repair_message_sequence and sanitize_api_messages drop them). The reporter's actual pasted error is a duplicate tool_call_id — a different, unhandled gap. Reproduced live on main: both duplicate shapes leaked through.

Changes (agent/agent_runtime_helpers.py)

Validation

  • tests/run_agent/test_message_sequence_repair.py — 33 passed (existing + 4 new: duplicate-tool-result dedup at both functions, duplicate-assistant-tool_call-id collapse, and a negative control proving distinct ids are never dropped).
  • tests/run_agent/test_message_sequence_repair.py + full tests/agent/test_context_compressor.py — 178 passed.
  • Phase 2 dataflow/invariant trace: both dedup shapes work; message role alternation preserved (dropping a duplicate tool never creates two-assistant-in-a-row and never leaves an assistant tool_call result-less); negative control confirms no over-dedup; operates on the ephemeral per-call api_messages copy (not the cached prompt prefix), so prompt caching is unaffected.
  • ruff clean; ty lint-diff 🆕 New issues: none.
Before (main) After
Two tool msgs sharing one tool_call_id both sent → DeepSeek 400 later duplicate dropped
Assistant with two tool_calls sharing an id both sent → DeepSeek 400 duplicate collapsed
Distinct tool_call_ids preserved preserved (negative control)

Follow-up (not in this PR)

agent/context_compressor.py::_sanitize_tool_pairs is a parallel reimplementation of the orphan-repair logic that also lacks dedup (analogous to the whitespace divergence noted in #55845). It is not a live bypass — compressor output re-passes through sanitize_api_messages before any provider call, so duplicates it emits are still caught downstream. Left unpatched here; worth consolidating the two sanitizers for defense-in-depth/consistency in a separate change.

Closes #58327. Credit: @Robinlovelace (#55436) for the repair_message_sequence dedup kernel.

…ousResearch#58327)

Strict providers (DeepSeek) reject a payload where the same tool_call_id
appears more than once with HTTP 400 'Duplicate value for tool_call_id'.
The issue was filed as an 'orphaned tool message' compression bug, but the
pasted error is a DUPLICATE tool_call_id — orphans are already handled on
main; duplicates were not. Reproduced live on main: both shapes leaked
through repair_message_sequence and sanitize_api_messages.

Two chokepoints, two shapes:
- repair_message_sequence: consume the id from known_tool_ids on first
  match so a SECOND tool result reusing it falls into the drop branch
  (duplicate tool-result shape). This is @Robinlovelace's kernel from
  NousResearch#55436 (applied manually — that PR was ~800 commits stale and bundled
  an unrelated duplicate-DB-write change for NousResearch#860, which is dropped here).
- sanitize_api_messages (final pre-API pass): add a dedup pass covering
  BOTH (a) duplicate tool_calls sharing an id WITHIN one assistant message
  (the message[6] shape) and (b) later tool result messages reusing an
  already-seen id. NousResearch#55436 covered neither of these at this chokepoint.

Tests: duplicate-tool-result dedup at both functions, duplicate-assistant-
tool_call-id collapse, and a negative control proving distinct ids are
never dropped (no over-dedup).

Credit: @Robinlovelace (NousResearch#55436) for the repair_message_sequence dedup kernel.
Closes NousResearch#58327.
@Robinlovelace

Copy link
Copy Markdown

Can I be listed as coauthor please? Many thanks!

@kshitijk4poor
kshitijk4poor merged commit 7203898 into NousResearch:main Jul 4, 2026
29 checks passed
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 4, 2026
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…p-tool-call-id

fix(agent): deduplicate tool_call_id across pre-API sanitizers (NousResearch#58327)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…p-tool-call-id

fix(agent): deduplicate tool_call_id across pre-API sanitizers (NousResearch#58327)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…p-tool-call-id

fix(agent): deduplicate tool_call_id across pre-API sanitizers (NousResearch#58327)
@kshitijk4poor
kshitijk4poor deleted the salvage/dedup-tool-call-id branch August 5, 2026 07:10
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…p-tool-call-id

fix(agent): deduplicate tool_call_id across pre-API sanitizers (NousResearch#58327)
teknium1 pushed a commit that referenced this pull request Aug 28, 2026
…iminate drift

sanitize_api_messages (agent_runtime_helpers) and
_sanitize_tool_pairs (context_compressor) both collected
tool-call IDs and classified orphans with near-identical logic
that had already drifted: the canonical sanitizer added dedup
(#58350), but the compressor's copy did not.

Extract the shared orphan-detection logic into
_classify_tool_call_orphans(messages) in agent_runtime_helpers.
Both call sites now delegate to it, preserving their divergent
remediation strategies (insert-stubs vs strip-orphans) while
ensuring id-resolution rules and dedup stay in sync.

Closes #58357
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…p-tool-call-id

fix(agent): deduplicate tool_call_id across pre-API sanitizers (NousResearch#58327)
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…iminate drift

sanitize_api_messages (agent_runtime_helpers) and
_sanitize_tool_pairs (context_compressor) both collected
tool-call IDs and classified orphans with near-identical logic
that had already drifted: the canonical sanitizer added dedup
(NousResearch#58350), but the compressor's copy did not.

Extract the shared orphan-detection logic into
_classify_tool_call_orphans(messages) in agent_runtime_helpers.
Both call sites now delegate to it, preserving their divergent
remediation strategies (insert-stubs vs strip-orphans) while
ensuring id-resolution rules and dedup stay in sync.

Closes NousResearch#58357
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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

3 participants