From e245ae9dfa894b1188633fe9bee1cb99ed1c8fe9 Mon Sep 17 00:00:00 2001 From: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:14:43 -0700 Subject: [PATCH] fix(repair): match answered tool calls by call_id Pass 1.5 now consumes the answered-result budget against both id and call_id, preserving Codex-shaped turns whose tool result references call_id. Remove the two temporary xfail markers. RED: both call_id regressions fail with the production hunk reverted. GREEN: scripts/run_tests.sh tests/run_agent/test_message_sequence_repair.py -q (25 passed). Ruff passes on both changed files. --- agent/agent_runtime_helpers.py | 14 +++++-- .../run_agent/test_message_sequence_repair.py | 38 ------------------- 2 files changed, 11 insertions(+), 41 deletions(-) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index ad22c0db31f2..5db79abd336a 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -819,12 +819,20 @@ def _is_verification_candidate(m: Dict) -> bool: answered_counts[tc_id] += 1 j += 1 # Consume the answered budget left-to-right: a call is "answered" - # only while its id still has an unconsumed result. Calls with no id - # are unanswerable (can't be matched to a result) → unanswered. + # only while one of its identifiers has an unconsumed result. Calls + # with no id/call_id are unanswerable (can't be matched to a result) + # → unanswered. _remaining = dict(answered_counts) answered_flags = [] # parallel to ``calls``: True = this entry answered for tc in calls: - _cid = tc.get("id") if isinstance(tc, dict) else None + _cid = None + if isinstance(tc, dict): + for _key in ("id", "call_id"): + if tc.get(_key) and _remaining.get(tc[_key], 0) > 0: + _cid = tc[_key] + break + else: + _cid = tc.get("id") or tc.get("call_id") if _cid and _remaining.get(_cid, 0) > 0: _remaining[_cid] -= 1 answered_flags.append(True) diff --git a/tests/run_agent/test_message_sequence_repair.py b/tests/run_agent/test_message_sequence_repair.py index 8aa7baf15c41..c5805cf380de 100644 --- a/tests/run_agent/test_message_sequence_repair.py +++ b/tests/run_agent/test_message_sequence_repair.py @@ -108,25 +108,6 @@ def test_repair_drops_stray_tool_with_unknown_tool_call_id(): assert all(m.get("role") != "tool" for m in messages) -@pytest.mark.xfail( - reason=( - "INHERITED fork/main defect, NOT parity-merge damage -- see card t_08cca32f. " - "Pass 1.5 of repair_message_sequence (agent/agent_runtime_helpers.py, " - "fork-only code from PR #196 / aaa34311e0) resolves a tool_call's answered " - "budget against tc.get('id') ONLY, while Pass 1 correctly matches the " - "id||call_id superset per PR #58168. A tool_call carrying only call_id (or a " - "Codex-Responses call whose id and call_id differ) therefore reads as " - "unanswered, and the 'none answered' branch deletes an assistant turn that " - "WAS answered. Evidence: reproduced identically on a clean detached worktree " - "at fork/main ee2fce2876 (repairs=1, expected 0); the merge's diff over the " - "Pass 1.5 region is empty; 'Pass 1.5' appears 6x at fork/main HEAD and 0x at " - "both merge-base a7a696ba and upstream target 1e5b5074. These tests exist at " - "the merge base and upstream but were dropped from fork/main -- the merge " - "restored them, which is how the bug surfaced. strict=False so this xpasses " - "and flags itself for deletion once t_08cca32f lands." - ), - strict=False, -) def test_repair_keeps_tool_matching_codex_call_id(): """A valid tool result must survive when the assistant tool_call carries a Codex-format ``call_id`` distinct from ``id`` and the result matches on @@ -157,25 +138,6 @@ def test_repair_keeps_tool_matching_codex_call_id(): assert messages[2]["tool_call_id"] == "call_ABC" -@pytest.mark.xfail( - reason=( - "INHERITED fork/main defect, NOT parity-merge damage -- see card t_08cca32f. " - "Pass 1.5 of repair_message_sequence (agent/agent_runtime_helpers.py, " - "fork-only code from PR #196 / aaa34311e0) resolves a tool_call's answered " - "budget against tc.get('id') ONLY, while Pass 1 correctly matches the " - "id||call_id superset per PR #58168. A tool_call carrying only call_id (or a " - "Codex-Responses call whose id and call_id differ) therefore reads as " - "unanswered, and the 'none answered' branch deletes an assistant turn that " - "WAS answered. Evidence: reproduced identically on a clean detached worktree " - "at fork/main ee2fce2876 (repairs=1, expected 0); the merge's diff over the " - "Pass 1.5 region is empty; 'Pass 1.5' appears 6x at fork/main HEAD and 0x at " - "both merge-base a7a696ba and upstream target 1e5b5074. These tests exist at " - "the merge base and upstream but were dropped from fork/main -- the merge " - "restored them, which is how the bug surfaced. strict=False so this xpasses " - "and flags itself for deletion once t_08cca32f lands." - ), - strict=False, -) def test_repair_keeps_tool_matching_only_call_id(): """Same as above but the assistant tool_call carries ONLY ``call_id`` (no ``id``). The result keyed on ``call_id`` must still be recognized (#58168).