Skip to content

fix(agent): drop tool_calls key when dedup removes every call - #67933

Open
LiangYang666 wants to merge 1 commit into
NousResearch:mainfrom
LiangYang666:fix/sanitize-dedup-empty-tool-calls
Open

fix(agent): drop tool_calls key when dedup removes every call#67933
LiangYang666 wants to merge 1 commit into
NousResearch:mainfrom
LiangYang666:fix/sanitize-dedup-empty-tool-calls

Conversation

@LiangYang666

Copy link
Copy Markdown

Summary

sanitize_api_messages() Step 3 (tool_call_id deduplication, added in #58327) can leave an assistant message with tool_calls: [] when every tool_call in that turn is a duplicate of an already-seen id. Strict OpenAI-compatible providers such as Alibaba Cloud Qwen reject the empty array with:

Empty tool_calls is not supported in message.

This PR drops the tool_calls key entirely in that case.

Root cause

Commit dba585c17 / PR #58327 introduced cross-message deduplication of tool_call_id to satisfy DeepSeek's strict duplicate-id validation. The dedup loop builds kept_tcs and, when its length differs from the original list, unconditionally assigns:

msg = {**msg, "tool_calls": kept_tcs}

If all tool_calls in the assistant turn were duplicates, kept_tcs == [], so the message ends up with tool_calls: []. Step 1 of the same sanitizer already drops pre-existing empty arrays, but it runs before Step 3, so it cannot catch arrays produced by deduplication.

Reproduction

A session where a later assistant turn re-uses a previously-seen tool_call_id (e.g. retry, crash-resume, or context compression re-emitting a call) hits the bug:

messages = [
    {"role": "user", "content": "first"},
    {"role": "assistant", "content": None, "tool_calls": [
        {"id": "call_1", "type": "function",
         "function": {"name": "foo", "arguments": "{}"}},
    ]},
    {"role": "tool", "tool_call_id": "call_1", "content": "r1"},
    # retry re-uses call_1
    {"role": "assistant", "content": "retry", "tool_calls": [
        {"id": "call_1", "type": "function",
         "function": {"name": "foo", "arguments": "{}"}},
    ]},
]

Before this PR the second assistant message becomes {..., "tool_calls": []}; after this PR the tool_calls key is removed.

Fix

In agent/agent_runtime_helpers.py, when deduplication changed the tool_calls list but none survived, delete the key instead of assigning an empty list:

if len(kept_tcs) != len(msg.get("tool_calls") or []):
    if kept_tcs:
        msg = {**msg, "tool_calls": kept_tcs}
    else:
        msg = {k: v for k, v in msg.items() if k != "tool_calls"}

Test

Added test_sanitize_dedup_across_assistants_drops_empty_tool_calls in tests/run_agent/test_message_sequence_repair.py to cover the cross-assistant duplicate-id case.

uv run pytest tests/run_agent/test_message_sequence_repair.py -q38 passed.

Affected versions

Bug introduced in v2026.7.7 by #58327 and present in v2026.7.7.2 (v0.18.2). Earlier releases such as v2026.7.1 are not affected.

Related

sanitize_api_messages Step 3 deduplicates tool_call_ids across the
message sequence. When every tool_call in an assistant turn had already
been seen, the code assigned tool_calls: [] to the assistant message.
Strict OpenAI-compatible providers such as Alibaba Qwen reject empty
tool_calls arrays with:

    Empty tool_calls is not supported in message.

Drop the key entirely instead of leaving an empty array. Add a regression
test covering cross-assistant duplicate tool_call_id deduplication.
@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 duplicate This issue or pull request already exists labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #64345: the live patch makes the same sanitize_api_messages() dedup-output change (drop tool_calls when every call was removed). #64843 is related broader work in the same strict-provider repair family.

@LiangYang666

Copy link
Copy Markdown
Author

Thanks for triaging @alt-glitch — noted that #64345 targets the same bug.

A few points that may help with prioritization:

  1. This is a regression from a recent change. The bug was introduced by [Bug]: Context compression breaks tool message chain, causing "role 'tool' must be a response to a preceding message with 'tool_calls'" on strict providers #58327 / commit dba585c, which landed in v2026.7.7 and is present in v2026.7.7.2 (v0.18.2). Earlier releases such as v2026.7.1 are not affected.

  2. It breaks Qwen users on Alibaba Cloud. Strict OpenAI-compatible providers (notably Alibaba Cloud qwen3.7-max) reject an empty tool_calls array with:

    Empty tool_calls is not supported in message.
    This is a hard HTTP 400 that makes every subsequent turn in the affected session fail deterministically until the session is reset.

  3. This PR is a minimal, focused fix. Unlike fix(sanitize): drop tool_calls key when dedup removes all calls #64345, which contains the same core fix but also carries unrelated diffs, this patch only touches the dedup branch and adds one regression test. That should make it faster and safer to review/merge.

Happy to rebase or adjust if the maintainers prefer a different approach, but given that v0.18.2 is affected and Qwen is a commonly used model family, getting this regression fixed quickly would help a lot of users.

@LiangYang666

Copy link
Copy Markdown
Author

@alt-glitch @teknium1 — could you please reconsider the priority of this regression?

This is not a niche edge case. Since #58327 shipped in v2026.7.7 / v0.18.2, every user running Qwen models through Alibaba Cloud (and other strict OpenAI-compatible providers) can hit a deterministic HTTP 400 once the conversation contains a repeated tool_call_id:

Empty tool_calls is not supported in message.

Once the bad message is in the session, every subsequent turn fails until the session is reset. We are seeing this in production right now on qwen3.7-max.

Given the impact surface — Qwen is one of the most commonly used model families, especially in Asia-Pacific deployments — I would respectfully suggest raising this from P2 to P1. The fix is a one-line defensive change (drop the key instead of writing an empty array), and this PR is a minimal, clean patch ready to merge.

If the team prefers to land #64345 instead, that is fine, but please prioritize getting either fix into main and the next release. The current v0.18.2 release is effectively broken for Qwen tool-calling sessions.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The premise is confirmed on current main: agent/agent_runtime_helpers.py:3130-3132 runs after the pre-existing empty-array normalization at :2974-3007 and can reconstruct a later assistant message as tool_calls: [] when every call is removed by cross-message deduplication. The added regression test exercises that precise ordering.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 2026
@alt-glitch alt-glitch added provider/qwen Qwen / Alibaba Cloud (OAuth) and removed sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 2, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

Re-verified against current main (dd600d1, 2026-08-02) since the sweeper's line refs have drifted.

  • Premise still holds: the dedup branch that writes {**msg, 'tool_calls': kept_tcs} is now at agent/agent_runtime_helpers.py:3360-3361 on current main. The empty-array normalization that would catch a pre-existing [] runs earlier (:3204-3237) — before the dedup pass — so an empty array created by the dedup pass still escapes the sanitizer.
  • Merge check: applied this PR onto current main. agent/agent_runtime_helpers.py merges cleanly; only the test file needs a context rebase (surrounding tests drifted).
  • Tests: with the PR applied, tests/run_agent/test_message_sequence_repair.py passes 15/15 including test_sanitize_dedup_across_assistants_drops_empty_tool_calls. git diff --check is clean.

Green-light: minimal, correct, ready to merge as-is. #64345 covers the same dedup branch; consolidating on this one keeps the patch focused.

@andrexibiza

Copy link
Copy Markdown
Contributor

Rebase resolution for the GitHub-reported conflict (verified against current main 75901a2, 2026-08-03):

The conflict is test-file placement only — the source change merges cleanly. On current main, tests/run_agent/test_message_sequence_repair.py was restructured: the insertion anchor this PR used (test_sanitize_preserves_populated_tool_calls, previously at end-of-file ~L770) no longer exists, and the file is now 377 lines ending with a "Self-recovery: heal empty-content non-final messages" section header.

To rebase:

  1. agent/agent_runtime_helpers.py — applies as-is (the dedup block is at :3358-3362 on current main; same context).
  2. tests/run_agent/test_message_sequence_repair.py — drop the diff hunk that appends after test_sanitize_preserves_populated_tool_calls, and insert test_sanitize_dedup_across_assistants_drops_empty_tool_calls after test_sanitize_drops_empty_tool_calls_array (ends line 351), before the Self-recovery header (line 360).

Verification on the rebased form: scripts/run_tests.sh tests/run_agent/test_message_sequence_repair.py → 15/15 pass including the new test; RED proof confirms the test fails on unpatched code (tool_calls: [] recreated); git diff --check clean; scripts/check-windows-footguns.py clean. Once the test hunk is re-anchored, the PR merges cleanly.

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 provider/qwen Qwen / Alibaba Cloud (OAuth) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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.

4 participants