diff --git a/strix/config/tool_call_ids.py b/strix/config/tool_call_ids.py index c418643db..0d9251bf0 100644 --- a/strix/config/tool_call_ids.py +++ b/strix/config/tool_call_ids.py @@ -100,56 +100,37 @@ class TurnCallIdRewriter: """Rewrite a single turn's tool-call ids that are blank or collide with the history. A turn's items surface several times (streamed item events, then the - completed response), so the same original id must always map to the same - replacement within the turn. + completed response), so each distinct item must keep its assigned id + across sightings, even when other items carry the same original id. """ def __init__(self, model_input: str | list[Any]) -> None: self._used = set() if isinstance(model_input, str) else collect_call_ids(model_input) - self._remap: dict[str, str] = {} - self._blank_remap: dict[str, str] = {} - self._settled: set[str] = set() - - def _rewrite_blank( - self, item: ResponseFunctionToolCall, position: int | None - ) -> ResponseFunctionToolCall: - """Give a call with no id one that stays the same on every sighting. - - A real item id tells parallel calls apart. Chat Completions routes give - every item the same placeholder id, so there the call's position in - the turn's output tells them apart instead. - """ - if item.id and item.id != FAKE_RESPONSES_ID: - key = item.id - else: - key = f"{FAKE_RESPONSES_ID}#{position}" - replacement = self._blank_remap.get(key) - if replacement is None: - replacement = new_call_id() - self._blank_remap[key] = replacement - self._used.add(replacement) - self._settled.add(replacement) - return item.model_copy(update={"call_id": replacement}) + self._remap: dict[tuple[str, str | int | None], str] = {} def rewrite_item(self, item: Any, position: int | None = None) -> Any: """Rewrite one item; ``position`` is its index in the turn's output.""" if not isinstance(item, ResponseFunctionToolCall): return item original = item.call_id - if not original: - return self._rewrite_blank(item, position) - if original in self._settled: - return item - replacement = self._remap.get(original) + # Responses supplies a real item id; Chat Completions uses one shared + # placeholder, so its output position distinguishes parallel calls. + key: tuple[str, str | int | None] + if item.id and item.id != FAKE_RESPONSES_ID: + key = ("item", item.id) + elif position is not None: + key = ("position", position) + else: + key = ("call", original) + replacement = self._remap.get(key) if replacement is None: - if original not in self._used: - self._used.add(original) - self._settled.add(original) - return item - replacement = new_call_id() - self._remap[original] = replacement + replacement = new_call_id() if not original or original in self._used else original + self._remap[key] = replacement self._used.add(replacement) - self._settled.add(replacement) + if key[0] == "call": + self._remap[("call", replacement)] = replacement + if replacement == original: + return item return item.model_copy(update={"call_id": replacement}) def rewrite_items(self, items: list[Any]) -> list[Any]: diff --git a/tests/test_tool_call_ids.py b/tests/test_tool_call_ids.py index 2647315f8..8cf6c1c32 100644 --- a/tests/test_tool_call_ids.py +++ b/tests/test_tool_call_ids.py @@ -18,14 +18,22 @@ import pytest from agents import Agent, Runner, function_tool +from agents.models.fake_id import FAKE_RESPONSES_ID from agents.models.interface import Model, ModelProvider from agents.models.openai_chatcompletions import OpenAIChatCompletionsModel from agents.run import RunConfig from openai import AsyncOpenAI -from openai.types.responses import ResponseFunctionToolCall - -from strix.config.models import _NonStreamingModel, _TurnGuardModel +from openai.types.responses import ( + Response, + ResponseCompletedEvent, + ResponseFunctionToolCall, + ResponseOutputItemAddedEvent, + ResponseOutputItemDoneEvent, +) + +from strix.config.models import _guard_event, _NonStreamingModel, _TurnGuardModel from strix.config.tool_call_ids import TurnCallIdRewriter, dedupe_history_call_ids +from strix.config.tool_call_limits import TurnToolCallLimiter if TYPE_CHECKING: @@ -380,3 +388,101 @@ def test_turn_rewriter_gives_parallel_blank_calls_distinct_ids() -> None: ) assert rewriter.rewrite_item(a).call_id != rewriter.rewrite_item(b).call_id + + +@pytest.mark.parametrize("item_id", ["real", FAKE_RESPONSES_ID, None]) +@pytest.mark.parametrize("historical_id", ["dup", "unrelated"]) +def test_duplicate_calls_keep_distinct_ids_across_stream_sightings( + item_id: str | None, historical_id: str +) -> None: + rewriter = TurnCallIdRewriter([{"type": "function_call", "call_id": historical_id}]) + calls = [ + ResponseFunctionToolCall( + id=f"fc_{position}" if item_id == "real" else item_id, + call_id="dup", + name="do_thing", + arguments=json.dumps({"n": position}), + type="function_call", + ) + for position in range(2) + ] + # A preceding non-tool item is part of the output index as well. + prefix = {"type": "reasoning"} + added = [rewriter.rewrite_item(call, i + 1) for i, call in enumerate(calls)] + done = [rewriter.rewrite_item(call, i + 1) for i, call in enumerate(calls)] + completed = rewriter.rewrite_items([prefix, *calls]) + + ids = [call.call_id for call in added] + assert len(set(ids)) == 2 + assert historical_id not in ids + assert [call.call_id for call in done] == ids + assert completed[0] is prefix + assert [call.call_id for call in completed[1:]] == ids + assert [rewriter.rewrite_item(call, i + 1).call_id for i, call in enumerate(added)] == ids + + +@pytest.mark.parametrize("item_id", ["real", FAKE_RESPONSES_ID, None]) +def test_stream_guard_counts_duplicate_calls_separately_and_only_once( + item_id: str | None, +) -> None: + rewriter = TurnCallIdRewriter([]) + limiter = TurnToolCallLimiter(2) + calls = [ + ResponseFunctionToolCall( + id=f"fc_{position}" if item_id == "real" else item_id, + call_id="dup", + name="do_thing", + arguments=json.dumps({"n": position}), + type="function_call", + ) + for position in range(3) + ] + accepted: list[str] = [] + for position, call in enumerate(calls): + added = _guard_event( + ResponseOutputItemAddedEvent( + item=call, + output_index=position, + sequence_number=position * 2, + type="response.output_item.added", + ), + rewriter, + limiter, + ) + done = _guard_event( + ResponseOutputItemDoneEvent( + item=call, + output_index=position, + sequence_number=position * 2 + 1, + type="response.output_item.done", + ), + rewriter, + limiter, + ) + if position == 2: + assert added is None + assert done is None + continue + assert isinstance(added, ResponseOutputItemAddedEvent) + assert isinstance(done, ResponseOutputItemDoneEvent) + assert isinstance(added.item, ResponseFunctionToolCall) + assert isinstance(done.item, ResponseFunctionToolCall) + assert added.item.call_id == done.item.call_id + accepted.append(added.item.call_id) + + completed = _guard_event( + ResponseCompletedEvent( + response=Response.model_construct(output=calls), + sequence_number=6, + type="response.completed", + ), + rewriter, + limiter, + ) + assert isinstance(completed, ResponseCompletedEvent) + output = completed.response.output + assert [ + call.call_id for call in output if isinstance(call, ResponseFunctionToolCall) + ] == accepted + assert len(set(accepted)) == 2 + assert limiter.dropped == 1 diff --git a/tests/test_tool_call_ids_providers.py b/tests/test_tool_call_ids_providers.py index e6dfda29a..e5e109898 100644 --- a/tests/test_tool_call_ids_providers.py +++ b/tests/test_tool_call_ids_providers.py @@ -61,6 +61,8 @@ def has_null_id(self) -> bool: "null-id": _Scenario(turns=((None,),)), "missing-id": _Scenario(turns=((_MISSING,),)), "parallel-empty-ids": _Scenario(turns=(("", ""),)), + "parallel-duplicate-ids": _Scenario(turns=(("dup", "dup"),)), + "parallel-duplicates-after-recycle": _Scenario(turns=(("dup",), ("dup", "dup"))), "empty-id-every-turn": _Scenario(turns=(("",), ("",), ("",))), "recycled-counter": _Scenario(turns=(("exec_command:0",), ("exec_command:0",))), "parallel-recycled-counter": _Scenario(