test(util): regression coverage for parallel tool_calls - #479
Open
Rubyglask wants to merge 1 commit into
Open
Conversation
Guards sanitize_message_list / fix_message_list against the parallel tool_calls reorder hang (see aliasrobotics#469). Test-only, no runtime change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds regression tests for
cai.util.terminal.sanitize_message_list(exported asfix_message_list) covering assistant turns that issue multiple paralleltool_callswhose responses arrive out of order. This is test-only — noruntime change.
Why
sanitize_message_listruns on every model request. Its second-pass sequencecheck compares each tool message only against its immediate predecessor, so
when an assistant emits 2+ tool calls a sibling tool response can be judged
out-of-order and reordered without the loop index ever advancing — an unbounded
loop that presents as an indefinite hang (issues #469, #401, #410; earlier fix
attempts #411/#424/#449).
mainalready fixes this: the reorder loop now tracksprocessed positions and always makes forward progress. This PR adds coverage so
that guard can't be dropped again in a future refactor (roughly how the loop
slipped in originally).
What's already covered vs. what this adds
tests/cli/test_tool_visualization.py::test_fix_message_list_no_duplicate_tool_responsesalready exercises an in-order 3-way parallel turn and asserts no
dropped/duplicated responses. What it doesn't cover — and what this adds:
triggered the loop; in-order never did),
@pytest.mark.timeout), so a regressed buildfails fast instead of hanging the whole suite,
sanitize_message_listsymbol and itsfix_message_listalias.Tests —
tests/util/test_sanitize_message_list.pytest_parallel_tool_calls_terminate_and_stay_valid— 6 shapes: in-order,reversed, 3-way scrambled, two interleaved assistants, the CAI hangs indefinitely when a single turn issues multiple tool_calls (not covered by #370 idle-timeout) #469 two-calls-in-
one-turn shape, and a repeated sibling response. Asserts termination, a valid
tool/assistant ordering, and that no tool response is dropped. It deliberately
does not assert order-preservation —
mainlegitimately reorders evenalready-valid input, and each tool response only has to follow its owning
assistant.
test_long_history_of_parallel_pairs_terminates— 200 reversed parallel pairs.test_tool_message_without_owning_assistant_is_repaired— the index-0 repairbranch.
test_fix_message_list_alias_is_preserved— the public alias contract.All pass on
main. The parallel cases time out (fail) if the reorder loop'sposition guard is removed, so the suite pins the fix in place.
Notes
Pure tests, no new dependencies. Happy to adjust placement or naming.