fix(core): keep every tool result paired when a tool-call id collides [K-03]

[haydarkadioglu]

Summary

Close the two ways a tool-call id collision loses work in one round:

1. Duplicate ids are now renamed deterministically (<id>_d<n>) before dispatch, so every tool result keeps its own pairing id.
2. The 400 "dangling tool_calls" recovery now only drops history and re-requests when the history actually has a dangling call — it no longer hijacks unrelated errors whose text merely contains 400.

Why

A. Collisions drop results. core.py:1104 (pre-change) coalesced the streaming id with call_id = stc["id"] or stc["name"]. Nothing made the ids distinct, but the id is the key of every downstream result map: completed_results[call["id"]] = (res, t_elapsed) (core.py:1268), blocked_results[call["id"]] (core.py:1204), and the tool_call_id of each written tool message (core.py:1289). Two calls in one batch sharing an id therefore overwrite each other — one tool's result is lost, the other's is replayed for both — and the model gets two role:"tool" rows with the same tool_call_id, which strict providers answer with a 400.

Measured on the parallel dispatch path with two read_file calls sharing id call_dup (scratch probe, real _run_conversation_loop):

OLD (main): tool results = [('call_dup', 'read b.py'), ('call_dup', 'read b.py')]
NEW (fix):  tool results = [('call_dup', 'read a.py'), ('call_dup_d2', 'read b.py')]

a.py's result never reached the model at all.

B. The 400 recovery fires on the wrong error. core.py:975 (pre-change) matched "tool_calls" in msg or "400" in msg, set the one-shot _stream_retried, ran the destructive _drop_dangling_tool_calls() and retried — regardless of whether anything was dangling, and before the key-rotation/backoff branches below it. An unrelated error whose text contains 400 (a token count like "maximum context length is 4000 tokens", or "… 400 requests/min") was therefore stolen from the rate-limit recovery path and retried with no backoff, while a legitimate round was removed from history. Same scenario, real loop, on main: 5 stream attempts before the turn gave up; with the fix, 1 and the error is surfaced.

What changed

• utils/tool_ids.py (new): uniquify_tool_call_ids(calls) -> int. Renames later duplicates to <id>_d<n> (n from 2, first occurrence wins), mutating in place, returning the number renamed. Deterministic on purpose — these ids ride the prompt-cache prefix, so a random suffix is not acceptable. Blank / non-string ids are skipped.
• core.py: the round assembly builds calls, uniquifies their ids, then keys invalid_args off the final ids (so a vetoed call and the tool_call_id written for it can no longer disagree). Logs a warning when a rename happens.
• core.py: the 400 branch calls _drop_dangling_tool_calls() first and only consumes _stream_retried + retries when it returned > 0 messages; otherwise it logs and falls through to the existing rate-limit / overloaded recovery.

Provenance

hermes:agent/message_sanitization.py:531 uniquify_tool_call_ids — same _d<n> scheme and the same determinism constraint ("never uuid4 — these ids feed prompt-cache prefixes"); hermes:run_agent.py:1220-1241 for the id policy. Re-implemented for koza's dict-shaped calls with koza names/logging; no upstream code or identifier was copied.

Verification

python -m pytest tests -q -p no:cacheprovider   ->  38 passed, 3 warnings in 19.86s
                                                    (main baseline before this change: 29 passed)
ruff check .                                    ->  Found 1991 errors   (identical to main: 1991)
ruff check core.py                              ->  Found 69 errors     (identical to main: 69)
ruff check utils/tool_ids.py tests/test_tool_call_ids.py  ->  All checks passed!

tests/test_tool_call_ids.py (new, force-added — tests/ is gitignored): 9 tests, 3 of them driving the real _run_conversation_loop with a stubbed streaming provider. Proven to catch the bugs — with core.py rolled back to main and the new tests in place:

FAILED tests/test_tool_call_ids.py::test_colliding_ids_keep_every_tool_result
FAILED tests/test_tool_call_ids.py::test_unrelated_400_does_not_retry_a_history_with_nothing_dangling
2 failed, 7 passed

and the restored tree is 9 passed. Scratch probe (~/.hermes/cache/scratch/k03_probe.py, not committed) produced the before/after output quoted above and confirmed the genuine dangling-400 path still repairs and retries (rounds_started == 2).

Risk / rollback

Low. No new dependency, no config key, no signature change outside core.py's loop. The id rename only triggers when the provider actually reused an id, so the common case is byte-identical on the wire (no prompt-cache change). Reverting the single core.py hunk plus dropping utils/tool_ids.py restores prior behaviour.

Roadmap: K-03