fix(run): keep input item order when collapsing duplicates - #4140
Conversation
|
heads up, I think this swapped the failure rather than removing it, and it is on main now.
which is the ordering the Responses API rejects, just mirrored. ran that against bdc294f to be sure rather than reading it off the diff. it looks reachable rather than theoretical. in session_persistence.py:282-287 dedupe runs last in the chain, and neither pass before it repairs order: anchoring one smaller thing worth knowing since the new test asserts it as correct: for |
|
Thanks for catching this, and thanks for offering to send a follow-up. I agree that the first-occurrence placement introduced by #4140 is too broad: the inverse call/output case remains possible, and I will take the follow-up on our side so we can revisit the complete ordering contract rather than add another local condition. The intended direction is to preserve latest-occurrence placement for outputs and other items, matching the released behavior, while keeping duplicate tool-call items at their earliest call position so they cannot move behind the matching output. We will derive the call types from the existing Thanks again for the detailed report. |
Summary
deduplicate_input_items_preferring_latestcollapses items that share a stable identifier (id,call_id,approval_request_id) and keeps the latest value. It is implemented by reversing the list, running the first-winsdeduplicate_input_items, and reversing back:That keeps the latest value, but it also relocates the surviving item to the position of its last duplicate rather than its first. Deduplication therefore reorders the conversation.
The concrete failure: when a
function_callis repeated after itsfunction_call_output, the call is moved behind the output. The Responses API rejects that ordering, and the same helper also decides the order items are written to a session, so an invalid ordering can be persisted and replayed on every later turn.Affected component:
src/agents/run_internal/items.py. The helper feeds three call sites:run_internal/run_loop.py— the final model input for streamed and non-streamed turns (applied to whateverRunConfig.call_model_input_filterreturned).run_internal/session_persistence.py:287—prepare_input_with_session.run_internal/session_persistence.py:423—save_result_to_session, i.e. the persisted session history.Minimal reproduction
Helper level:
Session level —
save_result_to_sessionwith an input list carrying a tool call plus its output, and a run item that replays the same call, persists["function_call_output", "function_call"].Run level — a
call_model_input_filterwhose returned list repeats an earlierfunction_callsends the output before the call to the model, in bothRunner.runandRunner.run_streamed.No API key, network access, or paid model call is involved; the repro uses
FakeModeland the existing in-repo session test doubles.Current behavior
The surviving item is emitted at the index of its final duplicate, so the item list is reordered and a
function_callcan end up after itsfunction_call_output.Corrected behavior
Duplicates collapse onto the first occurrence of their dedupe key, so caller order is preserved, while the last occurrence still supplies the value. Latest-value preference is unchanged.
Root cause
Reverse → first-wins dedupe → reverse is only equivalent to "prefer the latest value" when position does not matter. It conflates which value survives with where it survives.
Implementation
deduplicate_input_items_preferring_latestnow records the latest item per dedupe key, delegates ordering to the existingdeduplicate_input_items(first-wins, order-preserving), and substitutes the recorded latest value for each surviving key.Why this is minimal
deduplicate_input_itemsstays the single source of truth for dedupe keys and ordering; no parallel dedupe path is introduced.Regression tests
tests/test_run_internal_items.pytest_deduplicate_input_items_preferring_latest_keeps_original_order— a repeatedfunction_callstays ahead of its output.test_deduplicate_input_items_preferring_latest_uses_latest_value_at_first_position— latest value, earliest position.test_deduplicate_input_items_preferring_latest_leaves_unique_items_untouched— no-duplicate lists are returned identically.tests/test_call_model_input_filter.pytest_call_model_input_filter_keeps_duplicate_item_order_non_streamedtest_call_model_input_filter_keeps_duplicate_item_order_streamedtests/test_agent_runner.pytest_save_result_to_session_keeps_tool_call_before_its_output— persisted session order.All five order assertions fail on
mainat9f4292e5and pass with this change. The existing latest-value tests added in #2411 (test_call_model_input_filter_prefers_latest_duplicate_outputs_non_streamed/_streamed,test_save_result_to_session_prefers_latest_duplicate_function_outputs,test_prepare_input_with_session_prefers_latest_function_call_output) still pass unchanged.Execution modes covered
Runner.runandRunner.run_streamedfor the model-input path, plus a directsave_result_to_sessiontest for the persistence path.Runner.run_syncshares theRunner.runcode path and needs no separate case.Cleanup and lifecycle considerations
Pure function change: no tasks, streams, sessions, traces, or transports are created or closed, no global or per-run state is introduced, and caller-owned lists are not mutated (a new list is returned, as before).
Validation
Run from the repository root on macOS (Darwin 24.6.0), Python 3.12.13,
uv0.11.24, branch based onupstream/mainat9f4292e5:make format847 files left unchanged,All checks passed!make lintAll checks passed!make typecheckSuccess: no issues found in 835 source files; pyright:0 errors, 0 warnings, 0 informationsmake tests6227 passed, 3 skipped; serial:45 passed, 4 skippedbash .agents/skills/code-change-verification/scripts/run.shcode-change-verification: all commands passed.git diff --checkuv run pytest tests/test_run_internal_items.py tests/test_call_model_input_filter.py tests/test_agent_runner.py -q240 passed(repeated 3x, stable)Pre-fix confirmation: reverting only
src/agents/run_internal/items.pyand rerunning that focused command gives5 failed, 235 passed, failing exactly on the five new order assertions.Not run: the Python 3.10 matrix (
UV_PROJECT_ENVIRONMENT=.venv_310),make coverage,make build-docs, and the integration-test profiles. The change adds no new syntax or dependency and touches no docs, examples, or generated files.Compatibility considerations
No public API, signature, field order, import path, or
__all__change. The latest-value contract from #2411 is preserved; only the position of a collapsed duplicate changes, and only when the input actually contains duplicates. Lists without duplicate identifiers are returned exactly as before.Non-goals
deduplicate_input_items(first-wins) or to the dedupe-key rules in_dedupe_key.drop_orphan_function_calls,normalize_input_items_for_api, or any session backend.Test plan
See Validation above.
make format,make lint,make typecheck, andmake testsall pass; the five new tests are demonstrated red onmainand green with the fix.Issue number
N/A — filed directly as a small fix, consistent with recent maintenance PRs in this repository.
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR