Skip to content

fix(repair): Pass 1.5 must match tool results on call_id, not just id - #547

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/repair-sequence-call-id
Aug 10, 2026
Merged

fix(repair): Pass 1.5 must match tool results on call_id, not just id#547
Kyzcreig merged 1 commit into
mainfrom
fix/repair-sequence-call-id

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Symptom

AIAgent._repair_message_sequence deletes a valid assistant(tool_calls) turn whose tool_call carries only call_id (no id), even though the immediately-following tool message answers it correctly.

Reproduced on fork/main (0b2a95966f) before any change:

messages = [user,
            assistant(tool_calls=[{call_id: "call_XYZ", ...}]),
            tool(tool_call_id="call_XYZ"),
            user]

repairs      -> 1                          (expected 0)
roles after  -> ['user', 'tool', 'user']   # assistant turn destroyed

The result is a stray tool message with no preceding tool_calls — precisely the HTTP 400 shape this repair exists to prevent ("Messages with role 'tool' must be a response to a preceding message with 'tool_calls'" on DeepSeek/Kimi; "tool_use ids were found without tool_result blocks" on Anthropic).

Root cause

agent/agent_runtime_helpers.py:827 (pre-fix):

for tc in calls:
    _cid = tc.get("id") if isinstance(tc, dict) else None   # <-- id ONLY
    if _cid and _remaining.get(_cid, 0) > 0:

Pass 1 registers the answer budget against the superset of both keys (for key in ("id", "call_id"), per NousResearch#58168) — but Pass 1.5 consumes that budget against tc.get("id") alone. A tool_call carrying only call_id (or a Codex-Responses call whose id = fc_... and call_id = call_... differ) therefore matches nothing, answered_flags is all-False, and the "NONE answered — the whole tool-call turn is an orphan" branch drops the turn.

This is pre-existing on fork/main, not parity-merge damage. The 2026-08-07 parity merge only surfaced it by restoring the upstream tests that catch it.

Fix

Minimal and surgical — Pass 1.5 now consumes the budget on id OR call_id, mirroring Pass 1. No other pass touched:

for tc in calls:
    _matched = False
    if isinstance(tc, dict):
        for _key in ("id", "call_id"):
            _cid = tc.get(_key)
            if _cid and _remaining.get(_cid, 0) > 0:
                _remaining[_cid] -= 1
                _matched = True
                break
    answered_flags.append(_matched)

The multiset/count semantics are preserved exactly (each entry consumes at most one result slot, so a [X, X] turn answered once still keeps exactly one X), and the break keeps a single tool_call from consuming two slots when id and call_id are both present and both answered.

RED -> GREEN proof

RED first. New focused regression test test_repair_pass_1_5_keeps_call_id_only_assistant_turn written and run before any source change:

>       assert repairs == 0
E       assert 1 == 0
1 failed, 25 deselected

GREEN after the fixscripts/run_tests.sh (canonical parallel runner), exact observed counts:

Command Result
scripts/run_tests.sh tests/run_agent/test_message_sequence_repair.py -q 26 passed, 0 failed
scripts/run_tests.sh tests/run_agent -q 177 files, 1622 passed, 0 failed (66.9s)
scripts/run_tests.sh tests/agent -q 422 files, 4958 passed, 0 failed (110.7s)
ruff check (both changed files) All checks passed!

MUTATION PROOF

Explicitly performed. Reverting only the source change (git checkout HEAD~1 -- agent/agent_runtime_helpers.py, leaving the tests in place) and re-running the file:

=== Summary: 1 files, 23 tests passed, 3 failed ===
FAILED ...::test_repair_keeps_tool_matching_codex_call_id
FAILED ...::test_repair_keeps_tool_matching_only_call_id
FAILED ...::test_repair_pass_1_5_keeps_call_id_only_assistant_turn

Restoring the source (git checkout HEAD -- agent/agent_runtime_helpers.py) returns the file to 26 passed, 0 failed. The tests fail without the fix and pass with it — they are real gates on this exact behavior, not vacuous greens.

Removed xfail markers

Both @pytest.mark.xfail(strict=False) markers in tests/run_agent/test_message_sequence_repair.py — parked on the two upstream tests pending exactly this fix — are deleted, so those tests now gate for real:

  • test_repair_keeps_tool_matching_codex_call_id (distinct id + call_id)
  • test_repair_keeps_tool_matching_only_call_id (call_id only)

The now-unused import pytest was dropped with them (ruff-clean). Note both were strict=False and had begun xpassing for the codex_call_id case in some orderings — as markers they proved nothing; the mutation run above shows they genuinely fail against the unfixed source.

Files changed

  • agent/agent_runtime_helpers.py (+18 / -6) — Pass 1.5 id/call_id superset match + explanatory comment
  • tests/run_agent/test_message_sequence_repair.py (+30 / -39) — new regression test; two xfail markers removed

`_repair_message_sequence` Pass 1.5 resolved a tool_call's answered budget
against `tc.get("id")` ONLY, while Pass 1 correctly registers the
`id`/`call_id` SUPERSET (NousResearch#58168). An assistant turn whose tool_call carries
only `call_id` (or a Codex-Responses call whose `id` and `call_id` differ)
therefore read as entirely unanswered even when the following `tool` message
answered it -- and the "none answered" branch DELETED the valid assistant
turn.

Repro on fork/main:
    [user, assistant(tool_calls=[{call_id: "call_XYZ"}]),
     tool(tool_call_id="call_XYZ"), user]
    repairs -> 1 (expected 0); roles -> ['user','tool','user']

That leaves a stray tool result with no preceding tool_calls -- the exact
HTTP 400 shape this pass exists to prevent.

Fix: iterate ("id", "call_id") when consuming the answered budget, mirroring
Pass 1. Also removes the two `@pytest.mark.xfail(strict=False)` markers that
were parked on the upstream tests pending this fix, so they become real gates.
@Kyzcreig
Kyzcreig added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 1dc9ff1 Aug 10, 2026
44 checks passed
@Kyzcreig
Kyzcreig deleted the fix/repair-sequence-call-id branch August 10, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant