Merge 79c82d5f1f into 851743adcf

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]:

tests/test_tool_call_ids.py

@@ -18,14 +18,22 @@ from typing import TYPE_CHECKING, Any
 
 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 openai.types.responses import (
+    Response,
+    ResponseCompletedEvent,
+    ResponseFunctionToolCall,
+    ResponseOutputItemAddedEvent,
+    ResponseOutputItemDoneEvent,
+)
 
-from strix.config.models import _NonStreamingModel, _TurnGuardModel
+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

tests/test_tool_call_ids_providers.py

@@ -61,6 +61,8 @@ SCENARIOS = {
     "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(