Skip to content

fix(agent): dedup duplicate tool_call_id after compaction (#58327) - #58362

Closed
PRATHAMESH75 wants to merge 3 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/dedup-tool-call-id-58327
Closed

fix(agent): dedup duplicate tool_call_id after compaction (#58327)#58362
PRATHAMESH75 wants to merge 3 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/dedup-tool-call-id-58327

Conversation

@PRATHAMESH75

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the strict-provider HTTP 400 Duplicate value for 'tool_call_id' of ... in message[N] (DeepSeek) that wedges a session immediately after context compaction (#58327).

When compaction cuts through a multi-turn tool sequence it can leave the same assistant(tool_calls) group duplicated across two messages. The pre-call stub-injection step then doubles a dropped result into two identical tool results. Strict providers reject any repeated tool_call_id; lenient providers (OpenAI/Anthropic) silently tolerate it, masking the bug. The reporter's debug log shows the session going permanently unrecoverable right after a 1047 → 643 compaction, with both an orphaned-tool 400 and the duplicate-tool_call_id 400.

The fix closes both routes to the duplicate, at the two distinct message-repair layers:

  1. Live send pathsanitize_api_messages is the unconditional last guard before every provider call (agent/conversation_loop.py, agent/chat_completion_helpers.py). It already repaired orphaned pairs but never duplicates. Added a pass that enforces unique tool_call_id: a repeated assistant tool_call is dropped (first wins; a resulting empty turn keeps a (tool call removed) placeholder) and a repeated tool result is dropped. Separate seen-sets for assistant calls vs. results, so a legitimate call+result pair is never misread as a duplicate.
  2. Reload / persistence pathrepair_message_sequence now drops a duplicate tool result for the same live tool_call_id while preserving distinct parallel results (folded from @enzo-adami's [codex] Drop duplicate tool results during message repair #54594 — see Credits).

Related Issue

Fixes #58327

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • agent/agent_runtime_helpers.py
  • tests/run_agent/test_agent_guardrails.py: 4 cases — duplicate result deduped, duplicate assistant call deduped, emptied-turn placeholder, distinct-ids-not-treated-as-duplicate.
  • tests/run_agent/test_message_sequence_repair.py: regression for the reload-path dedup (@enzo-adami).
  • scripts/release.py: AUTHOR_MAP entry for the folded author.

How to Test

scripts/run_tests.sh tests/run_agent/test_agent_guardrails.py tests/run_agent/test_message_sequence_repair.py -q

Result: 44 passed. Ruff clean; check-windows-footguns.py clean.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run the affected tests and all pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A (no config keys)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A (pure Python, no OS-specific calls)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Credits

  • Builds on @enzo-adami's #54594 — the repair_message_sequence duplicate-tool result dedup (reload/persistence path) is folded in verbatim, preserving their commit authorship. This PR adds the complementary send-path guard in sanitize_api_messages so the live request is deduped too.
  • Related sibling PRs that tackle the same duplicate-tool_call_id 400 on the base repair layer: @enzo-adami's #58024 and @Robinlovelace's #55436 (the latter also bundles a separate bug: SQLite session transcript accumulates duplicate messages (3-4x token inflation) #860 DB-write dedup). Maintainers may prefer to close those in favour of this consolidated PR, trim this one back, or land a different combination — entirely their call.

enzo-adami and others added 3 commits July 4, 2026 21:46
…Research#58327)

Context compaction can leave the SAME assistant(tool_calls) group duplicated
across two messages; the stub-injection step then doubles a dropped result
into two identical tool results. Strict providers (DeepSeek) reject any
repeated tool_call_id with HTTP 400 "Duplicate value for 'tool_call_id' of
... in message[N]", wedging the session right after compaction.

sanitize_api_messages is the unconditional last guard before every provider
call (conversation_loop, chat_completion_helpers) and already repairs orphaned
pairs, but never deduplicated. Add a pass enforcing unique tool_call_id on the
live send path: drop a repeated assistant tool_call (first wins; empty turns
get a placeholder) and drop a repeated tool result. Separate seen-sets for
assistant calls vs results so a valid call+result pair is not misread as a
duplicate.

Complements the repair_message_sequence dedup (reload/persistence path) with
the live send path, closing both routes to the 400.
@PRATHAMESH75

Copy link
Copy Markdown
Contributor Author

Closing as superseded: maintainer commit dba585c17 ("fix(agent): deduplicate tool_call_id across the pre-API sanitizers (#58327)", @kshitijk4poor) landed on main and closes this issue. It is functionally identical to this PR — the same sanitize_api_messages dedup pass (duplicate assistant tool_calls sharing an id + duplicate tool results, separate seen-sets) plus the repair_message_sequence consume-id kernel, independently crediting @Robinlovelace (#55436).

Proof of redundancy: my own send-path regression tests pass against pristine upstream/main with none of my implementation (12/13; the 1 failure is only a stricter assertion I wrote about how an emptied assistant turn is represented — (tool call removed) placeholder vs. an empty tool_calls: [] list — not a functional gap in the 400 fix). No point carrying redundant code. Thanks @kshitijk4poor / @Robinlovelace.

@PRATHAMESH75
PRATHAMESH75 deleted the fix/dedup-tool-call-id-58327 branch July 4, 2026 16:20
@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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state duplicate This issue or pull request already exists labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #58350, which merged 2026-07-04 and fixes the same duplicate tool_call_id HTTP 400 after compaction (#58327) via the same dedup passes at repair_message_sequence and sanitize_api_messages in agent/agent_runtime_helpers.py. This PR is already closed; recording the linkage for search/DB consistency.

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 duplicate This issue or pull request already exists 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