From 6644f69a5ae0624aa6dd8e59fafd3282a1e46e0f Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Thu, 30 Jul 2026 07:34:56 +0300 Subject: [PATCH] fix: move the swe-bench-pro seed agent to chat completions ## Why Varun asked on #64 whether this is a gateway-sidecar bug we should fix there instead, and whether chat completions would be simpler. It is not a sidecar bug, and yes. The gateway is a deliberate provider-agnostic passthrough (`inference.py` "forward any inference endpoint ... to the upstream litellm proxy, which decides what it supports"). `previous_response_id` appears exactly once in vero, in `_RequestAttributor`, purely to stamp a `thread_id` on log records; that class is documented as unable to affect proxying. So the field is forwarded untouched to a proxy fronting `fireworks_ai/deepseek-v4-flash`, and Fireworks has no Responses API response store. We could give the sidecar one, but it would not help: to honour the pointer we would reconstruct the transcript and send it upstream in full anyway, because the provider cannot resolve it. Identical bytes on the wire, identical token cost. The only thing that changes is that the gateway becomes stateful, so a store bug corrupts conversations across every trial at once instead of in one agent. Chat completions is the better answer, and for a stronger reason than style: the only thing the Responses API gives us over it for these models is reasoning passthrough across turns, and we already discard it. The agent asks for `reasoning: {"effort": "high"}` every turn and then rebuilds a conversation of assistant text plus `function_call`/`function_call_output` pairs, with no reasoning items. We were paying for statefulness and getting a stateless conversation regardless. Chat completions has no field a proxy can accept and ignore, so the failure mode stops existing rather than being worked around. ## What changed - `_TOOL_SCHEMAS` holds the six schemas verbatim; `TOOLS` wraps them in the nested `{"type": "function", "function": {...}}` shape chat completions wants. - `_responses_create` -> `_completion_create` on `chat.completions`. Retry and backoff are untouched. - `instructions=` becomes a system message, `input` becomes `messages`, `max_output_tokens` becomes `max_tokens`, `reasoning: {effort}` becomes `reasoning_effort`. Usage reads prompt/completion tokens. - A turn is now ONE assistant message carrying both its text and its `tool_calls`, followed by `role: "tool"` results. Chat completions rejects a tool result whose `tool_call_id` was not declared by the assistant message before it, so unlike Responses-API items these cannot be split. The existing whole-turn trimming invariant already guaranteed this and now protects both directions. The gateway keeps forwarding the field, because rejecting it would be wrong: a gateway fronting real OpenAI honours it, and `_RequestAttributor` threads those chains. It now warns once per process instead, so a delegated conversation is one grep away rather than invisible. Non-null only, since SDKs serialise the key as null when unused. ## Tests Seed agent 5 -> 7 tests: the resend and whole-turn-trim tests are rewritten for the chat shape and assert the call/result pairing that chat completions enforces, plus new coverage for the nested tool schema and for `reasoning_effort` reaching only reasoning models (`deepseek-v4-flash` matches none, so target behaviour is unchanged). One new gateway test covers warn-once and that proxying is untouched. Seed agent 7 passed, `test_v05_harbor_inference` 18 passed, full vero suite 456 passed. The 2 failures there are `test_v05_docker_sandbox` and `test_v05_harbor_isolation_container`, both from a corrupted local Docker content store ("blob ... input/output error" on every `docker run`), not from this change. Not yet measured end to end: unit tests pin the wire shape, but only a real run confirms `deepseek-v4-flash` behaves the same through chat completions. Blocked on the same Docker corruption. Worth an A/B on the 66-case sample before this is trusted for headline numbers. Co-Authored-By: Claude Opus 5 (1M context) --- .../target/src/swebench_pro_agent/agent.py | 142 +++++----- .../baseline/target/tests/test_agent.py | 250 +++++++++++------- vero/src/vero/gateway/inference.py | 26 ++ vero/tests/test_v05_harbor_inference.py | 52 ++++ 4 files changed, 310 insertions(+), 160 deletions(-) diff --git a/harness-engineering-bench/swe-bench-pro/baseline/target/src/swebench_pro_agent/agent.py b/harness-engineering-bench/swe-bench-pro/baseline/target/src/swebench_pro_agent/agent.py index e9eb30cd..562c76b0 100644 --- a/harness-engineering-bench/swe-bench-pro/baseline/target/src/swebench_pro_agent/agent.py +++ b/harness-engineering-bench/swe-bench-pro/baseline/target/src/swebench_pro_agent/agent.py @@ -60,7 +60,7 @@ def _is_reasoning_model(model: str) -> bool: REPO_DIR = "/app" # Retry policy for the Responses API. The GAIA baseline scored 0.0 in the first -# VeRO run because a single transient error on ``responses.create`` crashed the +# VeRO run because a single transient error on the create call crashed the # whole rollout; the optimizer's winning fix was exactly this retry-with-backoff. # It ships here from the start so the baseline is robust before optimization. MAX_API_RETRIES = 6 @@ -93,7 +93,7 @@ def _is_reasoning_model(model: str) -> bool: When you are confident the change is correct and the tests you can see pass, call submit. Do not merely describe what you would do.""" -TOOLS: list[dict[str, Any]] = [ +_TOOL_SCHEMAS: list[dict[str, Any]] = [ { "type": "function", "name": "run_shell", @@ -215,6 +215,16 @@ def _is_reasoning_model(model: str) -> bool: }, ] +# Chat Completions nests the schema under a "function" key; the Responses API puts +# it at the top level. Same schemas either way, so wrap rather than restate them. +TOOLS: list[dict[str, Any]] = [ + { + "type": "function", + "function": {key: value for key, value in schema.items() if key != "type"}, + } + for schema in _TOOL_SCHEMAS +] + class SweBenchProAgent(BaseAgent): """Code-editing agent whose source is the editable optimization target.""" @@ -250,8 +260,8 @@ async def setup(self, environment: BaseEnvironment) -> None: result.stderr or f"task repository is missing at {REPO_DIR}" ) - async def _responses_create(self, **request: Any) -> Any: - """Call the Responses API with retry-and-backoff on transient errors. + async def _completion_create(self, **request: Any) -> Any: + """Call Chat Completions with retry-and-backoff on transient errors. This is the load-bearing robustness fix: an unguarded ``create`` call is what made the first GAIA baseline score 0.0. Any exception is retried with @@ -261,7 +271,7 @@ async def _responses_create(self, **request: Any) -> Any: last_error: Exception | None = None for attempt in range(1, MAX_API_RETRIES + 1): try: - return await self._client.responses.create(**request) + return await self._client.chat.completions.create(**request) except Exception as error: # noqa: BLE001 - transient classes vary by SDK last_error = error if attempt == MAX_API_RETRIES: @@ -273,7 +283,7 @@ async def _responses_create(self, **request: Any) -> Any: delay += random.uniform(0, delay / 2) self._trace( { - "event": "responses_retry", + "event": "completion_retry", "attempt": attempt, "delay": round(delay, 2), "error": repr(error), @@ -302,8 +312,9 @@ def _history_size(blocks: list[list[dict[str, Any]]]) -> int: def _trim_history(self, blocks: list[list[dict[str, Any]]]) -> int: """Drop whole oldest turns until the transcript fits the budget. - Whole turns, never individual items: a ``function_call`` sent without its - matching ``function_call_output`` is a hard 400 from the Responses API. + Whole turns, never individual items: a ``tool`` message whose + ``tool_call_id`` is not declared by the assistant message before it is a + hard 400, and so is an assistant ``tool_calls`` left without its results. """ dropped = 0 while len(blocks) > 1 and self._history_size(blocks) > MAX_HISTORY_CHARS: @@ -476,51 +487,57 @@ async def run( input_tokens = 0 output_tokens = 0 cached_tokens = 0 - # The conversation is held HERE, not on the provider. An earlier version - # sent only the newest tool result and passed ``previous_response_id`` to - # let the server reconstruct the rest. That silently loses everything on - # any OpenAI-compatible gateway that accepts the field without backing it - # with a response store: from turn 2 the model saw a bare tool result with - # no task attached, so it re-explored the repository until the turn budget - # ran out. Measured on the 66-case sample with deepseek-v4-flash: 66/66 - # exhausted all 50 turns, 3163 of 3300 tool calls were ls/find/git/cat, and - # write_file, apply_patch and submit were called zero times, for a reward - # of 0.0000 on every case. + # The conversation is held HERE, not on the provider, and Chat Completions + # is stateless by construction so there is no way to delegate it even by + # accident. An earlier version used the Responses API and passed + # ``previous_response_id`` to let the server reconstruct the history. Any + # OpenAI-compatible gateway that accepts that field without backing it with + # a response store silently loses everything: from turn 2 the model saw a + # bare tool result with no task attached, so it re-explored the repository + # until the turn budget ran out. Measured on the 66-case sample with + # deepseek-v4-flash: 66/66 exhausted all 50 turns, 3163 of 3300 tool calls + # were ls/find/git/cat, write_file/apply_patch/submit were called zero + # times, and every case scored 0.0000. Chat Completions has no such field. + system_item: dict[str, Any] = {"role": "system", "content": INSTRUCTIONS} task_item: dict[str, Any] = {"role": "user", "content": instruction} blocks: list[list[dict[str, Any]]] = [] for turn in range(1, MAX_TURNS + 1): - conversation: list[Any] = [task_item] + conversation: list[Any] = [system_item, task_item] for block in blocks: conversation.extend(block) request: dict[str, Any] = { "model": self._api_model, - "instructions": INSTRUCTIONS, - "input": conversation, + "messages": conversation, "tools": TOOLS, - "max_output_tokens": 12_000, + "max_tokens": 12_000, "parallel_tool_calls": False, } - # Only reasoning models accept `reasoning`; sending it to gpt-4o or - # gpt-4.1 is a hard 400 on the very first turn. + # Only reasoning models accept `reasoning_effort`; sending it to gpt-4o + # or gpt-4.1 is a hard 400 on the very first turn. if _is_reasoning_model(self._api_model): - request["reasoning"] = {"effort": "high"} - response = await self._responses_create(**request) + request["reasoning_effort"] = "high" + response = await self._completion_create(**request) usage = response.usage - input_tokens += self._usage_value(usage, "input_tokens") - output_tokens += self._usage_value(usage, "output_tokens") + input_tokens += self._usage_value(usage, "prompt_tokens") + output_tokens += self._usage_value(usage, "completion_tokens") cached_tokens += self._usage_value( - getattr(usage, "input_tokens_details", None), "cached_tokens" + getattr(usage, "prompt_tokens_details", None), "cached_tokens" ) - calls = [item for item in response.output if item.type == "function_call"] + message = response.choices[0].message + calls = list(message.tool_calls or []) + content = message.content or "" self._trace( { "turn": turn, "response_id": response.id, - "output_text": response.output_text, + "output_text": content, "function_calls": [ - {"name": call.name, "arguments": call.arguments} + { + "name": call.function.name, + "arguments": call.function.arguments, + } for call in calls ], } @@ -531,62 +548,69 @@ async def run( context.metadata = {"turns": turn, "trace": "swe-bench-pro-trace.jsonl"} break - turn_items: list[dict[str, Any]] = [] - if response.output_text: - turn_items.append( - {"role": "assistant", "content": response.output_text} - ) + # One assistant message carries the turn's text AND its tool calls. + # Chat Completions rejects a `tool` message whose tool_call_id is not + # declared by the immediately preceding assistant message, so the two + # cannot be split the way Responses-API items could be. + turn_items: list[dict[str, Any]] = [ + { + "role": "assistant", + "content": content or None, + "tool_calls": [ + { + "id": call.id, + "type": "function", + "function": { + "name": call.function.name, + "arguments": call.function.arguments, + }, + } + for call in calls + ], + } + ] submitted = False for call in calls: - # Echo the call itself before its output: the API matches the two - # by call_id, and an orphaned output is rejected. - turn_items.append( - { - "type": "function_call", - "call_id": call.call_id, - "name": call.name, - "arguments": call.arguments, - } - ) + name = call.function.name try: - arguments = json.loads(call.arguments or "{}") + arguments = json.loads(call.function.arguments or "{}") except json.JSONDecodeError as error: result: dict[str, Any] = {"error": f"invalid arguments: {error}"} else: - if call.name == "run_shell": + if name == "run_shell": result = await self._run_shell( environment, arguments["command"] ) - elif call.name == "read_file": + elif name == "read_file": result = await self._read_file( environment, arguments["path"], arguments.get("start_line"), arguments.get("end_line"), ) - elif call.name == "write_file": + elif name == "write_file": result = await self._write_file( environment, arguments["path"], arguments["content"] ) - elif call.name == "apply_patch": + elif name == "apply_patch": result = await self._apply_patch( environment, arguments["patch"] ) - elif call.name == "run_tests": + elif name == "run_tests": result = await self._run_tests( environment, arguments.get("command") ) - elif call.name == "submit": + elif name == "submit": result = {"submitted": True} submitted = True else: - result = {"error": f"unknown tool: {call.name}"} - self._trace({"turn": turn, "tool": call.name, "result": result}) + result = {"error": f"unknown tool: {name}"} + self._trace({"turn": turn, "tool": name, "result": result}) turn_items.append( { - "type": "function_call_output", - "call_id": call.call_id, - "output": json.dumps(result, ensure_ascii=False), + "role": "tool", + "tool_call_id": call.id, + "content": json.dumps(result, ensure_ascii=False), } ) if submitted: diff --git a/harness-engineering-bench/swe-bench-pro/baseline/target/tests/test_agent.py b/harness-engineering-bench/swe-bench-pro/baseline/target/tests/test_agent.py index 05a92782..c46bfc19 100644 --- a/harness-engineering-bench/swe-bench-pro/baseline/target/tests/test_agent.py +++ b/harness-engineering-bench/swe-bench-pro/baseline/target/tests/test_agent.py @@ -17,7 +17,37 @@ async def upload_file(self, source_path, target_path): # pragma: no cover pass -class FakeResponses: +def _tool_call(call_id, name, arguments): + return SimpleNamespace( + id=call_id, + type="function", + function=SimpleNamespace(name=name, arguments=arguments), + ) + + +def _completion(response_id, calls, *, content="", usage=(200, 12, 30)): + prompt, completion, cached = usage + return SimpleNamespace( + id=response_id, + choices=[ + SimpleNamespace( + message=SimpleNamespace(content=content, tool_calls=calls or None) + ) + ], + usage=SimpleNamespace( + prompt_tokens=prompt, + completion_tokens=completion, + prompt_tokens_details=SimpleNamespace(cached_tokens=cached), + ), + ) + + +def _client(completions): + """The agent calls ``client.chat.completions.create``.""" + return SimpleNamespace(chat=SimpleNamespace(completions=completions)) + + +class FakeCompletions: """Returns a single submit call, then would loop forever if asked again.""" def __init__(self): @@ -25,26 +55,12 @@ def __init__(self): async def create(self, **kwargs): self.calls += 1 - return SimpleNamespace( - id=f"response-{self.calls}", - output=[ - SimpleNamespace( - type="function_call", - name="submit", - arguments="{}", - call_id="call-1", - ) - ], - output_text="", - usage=SimpleNamespace( - input_tokens=200, - output_tokens=12, - input_tokens_details=SimpleNamespace(cached_tokens=30), - ), + return _completion( + f"response-{self.calls}", [_tool_call("call-1", "submit", "{}")] ) -class FlakyResponses: +class FlakyCompletions: """Fails once, then returns a submit call: exercises the retry helper.""" def __init__(self): @@ -54,25 +70,22 @@ async def create(self, **kwargs): self.calls += 1 if self.calls == 1: raise RuntimeError("transient upstream error") - return SimpleNamespace( - id="response-ok", - output=[ - SimpleNamespace( - type="function_call", - name="submit", - arguments="{}", - call_id="call-1", - ) - ], - output_text="", - usage=SimpleNamespace( - input_tokens=10, - output_tokens=1, - input_tokens_details=SimpleNamespace(cached_tokens=0), - ), + return _completion( + "response-ok", + [_tool_call("call-1", "submit", "{}")], + usage=(10, 1, 0), ) +def _context(): + return SimpleNamespace( + metadata=None, + n_input_tokens=None, + n_output_tokens=None, + n_cache_tokens=None, + ) + + @pytest.mark.asyncio async def test_agent_submits_and_populates_context(tmp_path, monkeypatch): monkeypatch.setenv("OPENAI_API_KEY", "test-key") @@ -80,17 +93,13 @@ async def test_agent_submits_and_populates_context(tmp_path, monkeypatch): logs_dir=tmp_path / "logs", model_name="openai/gpt-4o", ) - agent._client = SimpleNamespace(responses=FakeResponses()) + agent._client = _client(FakeCompletions()) environment = FakeEnvironment() - context = SimpleNamespace( - metadata=None, - n_input_tokens=None, - n_output_tokens=None, - n_cache_tokens=None, - ) + context = _context() await agent.run("Fix the failing test in the repository.", environment, context) + # Chat Completions reports usage as prompt/completion, not input/output. assert context.n_input_tokens == 200 assert context.n_output_tokens == 12 assert context.n_cache_tokens == 30 @@ -101,7 +110,7 @@ async def test_agent_submits_and_populates_context(tmp_path, monkeypatch): @pytest.mark.asyncio -async def test_responses_create_retries_transient_errors(tmp_path, monkeypatch): +async def test_completion_create_retries_transient_errors(tmp_path, monkeypatch): monkeypatch.setenv("OPENAI_API_KEY", "test-key") # Avoid real backoff sleeps in the test. monkeypatch.setattr("swebench_pro_agent.agent.API_RETRY_BASE_DELAY", 0.0) @@ -110,17 +119,11 @@ async def test_responses_create_retries_transient_errors(tmp_path, monkeypatch): logs_dir=tmp_path / "logs", model_name="openai/gpt-4o", ) - flaky = FlakyResponses() - agent._client = SimpleNamespace(responses=flaky) - environment = FakeEnvironment() - context = SimpleNamespace( - metadata=None, - n_input_tokens=None, - n_output_tokens=None, - n_cache_tokens=None, - ) + flaky = FlakyCompletions() + agent._client = _client(flaky) + context = _context() - await agent.run("Fix the failing test in the repository.", environment, context) + await agent.run("Fix the failing test in the repository.", FakeEnvironment(), context) assert flaky.calls == 2 # one failure, then one success assert context.metadata == { @@ -129,7 +132,7 @@ async def test_responses_create_retries_transient_errors(tmp_path, monkeypatch): } -class TwoTurnResponses: +class TwoTurnCompletions: """Runs one shell command, then submits. Records every request it received.""" def __init__(self): @@ -139,24 +142,17 @@ def __init__(self): async def create(self, **kwargs): self.calls += 1 self.requests.append(kwargs) - name = "run_shell" if self.calls == 1 else "submit" - arguments = '{"command": "git status --short"}' if self.calls == 1 else "{}" - return SimpleNamespace( - id=f"response-{self.calls}", - output=[ - SimpleNamespace( - type="function_call", - name=name, - arguments=arguments, - call_id=f"call-{self.calls}", + first = self.calls == 1 + return _completion( + f"response-{self.calls}", + [ + _tool_call( + f"call-{self.calls}", + "run_shell" if first else "submit", + '{"command": "git status --short"}' if first else "{}", ) ], - output_text="", - usage=SimpleNamespace( - input_tokens=10, - output_tokens=1, - input_tokens_details=SimpleNamespace(cached_tokens=0), - ), + usage=(10, 1, 0), ) @@ -166,57 +162,109 @@ async def test_conversation_is_resent_and_never_relies_on_the_provider( ): """The task and prior turns must be in the request, not on the server. - Regression test for the bug that scored 0.0000 on all 66 sampled cases: - the agent sent only the newest tool result plus ``previous_response_id``, - so a gateway without a response store dropped the task entirely. + Regression test for the bug that scored 0.0000 on all 66 sampled cases: the + agent sent only the newest tool result plus ``previous_response_id``, so a + gateway without a response store dropped the task entirely. Chat Completions + has no such field, and this asserts we never reintroduce one. """ monkeypatch.setenv("OPENAI_API_KEY", "test-key") agent = SweBenchProAgent(logs_dir=tmp_path / "logs", model_name="openai/gpt-4o") - responses = TwoTurnResponses() - agent._client = SimpleNamespace(responses=responses) - context = SimpleNamespace( - metadata=None, - n_input_tokens=None, - n_output_tokens=None, - n_cache_tokens=None, - ) + completions = TwoTurnCompletions() + agent._client = _client(completions) task = "Preserve this exact objective after every tool call." - await agent.run(task, FakeEnvironment(), context) - - assert len(responses.requests) == 2 - # Never delegate memory to the provider. - assert all("previous_response_id" not in r for r in responses.requests) - # The task survives into the second turn, carried by us. - second = responses.requests[1]["input"] - assert second[0] == {"role": "user", "content": task} - # ... and so does the first turn's call together with its output. - kinds = [item.get("type") for item in second[1:]] - assert "function_call" in kinds and "function_call_output" in kinds - call = next(i for i in second[1:] if i.get("type") == "function_call") - output = next(i for i in second[1:] if i.get("type") == "function_call_output") - assert call["call_id"] == output["call_id"], "orphaned output is a 400" - assert "git status --short" in call["arguments"] + await agent.run(task, FakeEnvironment(), _context()) + + assert len(completions.requests) == 2 + # Never delegate memory to the provider, by any spelling. + assert all("previous_response_id" not in r for r in completions.requests) + assert all("input" not in r for r in completions.requests) + + second = completions.requests[1]["messages"] + # The instructions and the task both survive into the second turn, carried by us. + assert second[0]["role"] == "system" + assert second[1] == {"role": "user", "content": task} + # ... and so does the first turn's call together with its result. + assistant = next(m for m in second[2:] if m["role"] == "assistant") + tool = next(m for m in second[2:] if m["role"] == "tool") + assert second.index(assistant) < second.index(tool), "result before its call is a 400" + declared = {c["id"] for c in assistant["tool_calls"]} + assert tool["tool_call_id"] in declared, "an undeclared tool_call_id is a 400" + assert "git status --short" in assistant["tool_calls"][0]["function"]["arguments"] + + +@pytest.mark.asyncio +async def test_tools_are_sent_in_chat_completions_shape(tmp_path, monkeypatch): + """Chat Completions nests the schema under "function"; a flat one is a 400.""" + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + agent = SweBenchProAgent(logs_dir=tmp_path / "logs", model_name="openai/gpt-4o") + completions = TwoTurnCompletions() + agent._client = _client(completions) + + await agent.run("Fix it.", FakeEnvironment(), _context()) + + tools = completions.requests[0]["tools"] + assert {tool["function"]["name"] for tool in tools} == { + "run_shell", + "read_file", + "write_file", + "apply_patch", + "run_tests", + "submit", + } + for tool in tools: + assert tool["type"] == "function" + assert set(tool) == {"type", "function"}, "the schema must be nested" + assert "parameters" in tool["function"] + + +@pytest.mark.asyncio +async def test_reasoning_effort_is_only_sent_to_reasoning_models(tmp_path, monkeypatch): + """`reasoning_effort` on gpt-4o is a hard 400 on the very first turn.""" + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + plain = SweBenchProAgent(logs_dir=tmp_path / "plain", model_name="openai/gpt-4o") + plain_completions = TwoTurnCompletions() + plain._client = _client(plain_completions) + await plain.run("Fix it.", FakeEnvironment(), _context()) + assert "reasoning_effort" not in plain_completions.requests[0] + + thinker = SweBenchProAgent(logs_dir=tmp_path / "o", model_name="openai/gpt-5.6-sol") + thinker_completions = TwoTurnCompletions() + thinker._client = _client(thinker_completions) + await thinker.run("Fix it.", FakeEnvironment(), _context()) + assert thinker_completions.requests[0]["reasoning_effort"] == "high" def test_trim_history_drops_whole_turns_and_keeps_pairs_matched(tmp_path, monkeypatch): monkeypatch.setenv("OPENAI_API_KEY", "test-key") agent = SweBenchProAgent(logs_dir=tmp_path / "logs", model_name="openai/gpt-4o") + from swebench_pro_agent.agent import MAX_HISTORY_CHARS + blocks = [ [ - {"type": "function_call", "call_id": f"c{i}", "name": "run_shell", - "arguments": "{}"}, - {"type": "function_call_output", "call_id": f"c{i}", "output": "x" * 80_000}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": f"c{i}", + "type": "function", + "function": {"name": "run_shell", "arguments": "{}"}, + } + ], + }, + {"role": "tool", "tool_call_id": f"c{i}", "content": "x" * 80_000}, ] for i in range(10) ] dropped = agent._trim_history(blocks) assert dropped > 0, "an oversized transcript must be trimmed" - assert agent._history_size(blocks) <= 300_000 + assert agent._history_size(blocks) <= MAX_HISTORY_CHARS for block in blocks: - ids = [i["call_id"] for i in block] - assert len(set(ids)) == 1, "a turn must keep its call and output together" + declared = {c["id"] for c in block[0]["tool_calls"]} + results = {m["tool_call_id"] for m in block[1:]} + assert results == declared, "a turn must keep its calls and results together" def test_agent_requires_model(tmp_path, monkeypatch): diff --git a/vero/src/vero/gateway/inference.py b/vero/src/vero/gateway/inference.py index 8057e7fc..561c3d46 100644 --- a/vero/src/vero/gateway/inference.py +++ b/vero/src/vero/gateway/inference.py @@ -690,6 +690,9 @@ async def lifespan(app: FastAPI): app.state.usage_store = store app.state.request_log = request_log app.state.request_attributor = attributor + # One-shot latch for the server-side-conversation warning in `proxy`. A set + # rather than a bool so the closure can mark it without `nonlocal`. + _stateful_warned: set[bool] = set() @app.get("/health") async def health(): @@ -802,6 +805,29 @@ async def log_request( return _provider_error( 403, "model is not allowed for this scope", "model_denied" ) + # `previous_response_id` asks the SERVER to hold the conversation. This + # gateway never can: it is a passthrough with no response store. Whether the + # request still works therefore depends entirely on the upstream, and a + # litellm proxy in front of a provider with no response store accepts the + # field and drops it. Nothing errors, so from turn two the model receives a + # bare tool result with no task attached -- that scored the swe-bench-pro + # seed agent 0.0000 on all 66 sampled cases with nothing in any log to show + # for it. Rejecting outright would be wrong (a gateway fronting real OpenAI + # honours the field, and _RequestAttributor threads those chains), so warn + # once per process instead and leave the proxying alone: a delegated + # conversation is now one grep away instead of invisible. + if ( + not _stateful_warned + and isinstance(value, dict) + and value.get("previous_response_id") is not None + ): + _stateful_warned.add(True) + logger.warning( + "request carried previous_response_id: this gateway has no response " + "store, so the conversation survives only if the upstream keeps one. " + "If it does not, the field is silently ignored and every turn after " + "the first loses its task. Send the full conversation instead." + ) if not attribution or any( not (character.isalnum() or character in "_.-") for character in attribution ): diff --git a/vero/tests/test_v05_harbor_inference.py b/vero/tests/test_v05_harbor_inference.py index 0ace32f6..efad3356 100644 --- a/vero/tests/test_v05_harbor_inference.py +++ b/vero/tests/test_v05_harbor_inference.py @@ -2,6 +2,7 @@ import asyncio import json +import logging import httpx from fastapi.testclient import TestClient @@ -773,3 +774,54 @@ def test_budget_exhaustion_status_is_not_retryable(): assert budget_exhausted_status not in EvaluationLimits().retry.retry_status_codes # The transient ones stay retryable. assert 429 in EvaluationLimits().retry.retry_status_codes + + +def test_gateway_warns_once_when_a_request_delegates_its_conversation(tmp_path, caplog): + """A conversation delegated to the server must be visible, not invisible. + + The swe-bench-pro seed agent scored 0.0000 on all 66 sampled cases because it + passed ``previous_response_id`` to an upstream with no response store: the + field was accepted, dropped, and from turn two the model saw a tool result + with no task attached. Nothing errored anywhere. We cannot reject the field + (a gateway fronting real OpenAI honours it) so proxying is untouched, but the + warning puts the diagnosis one grep away instead of costing a day. + """ + observed = [] + + def upstream(request: httpx.Request): + observed.append(request) + return httpx.Response(200, json={"id": "response", "usage": {}}) + + app = create_inference_gateway_app( + config=_config(tmp_path, max_requests=5, max_tokens=None), + upstream_api_key="upstream-secret", + upstream_base_url="https://provider.example/v1", + transport=httpx.MockTransport(upstream), + ) + with caplog.at_level(logging.WARNING, logger="vero.gateway.inference"): + with TestClient(app) as client: + headers = {"Authorization": "Bearer scoped-token"} + # SDKs serialise the key as null when unused: that must stay quiet. + quiet = client.post( + "/scopes/producer/optimizer/v1/responses", + headers=headers, + json={"model": "gpt-test", "input": "hi", "previous_response_id": None}, + ) + assert not [r for r in caplog.records if "previous_response_id" in r.message] + first = client.post( + "/scopes/producer/optimizer/v1/responses", + headers=headers, + json={"model": "gpt-test", "input": [], "previous_response_id": "resp_1"}, + ) + second = client.post( + "/scopes/producer/optimizer/v1/responses", + headers=headers, + json={"model": "gpt-test", "input": [], "previous_response_id": "resp_2"}, + ) + + # Proxying is untouched: every call still reached the provider. + assert [quiet.status_code, first.status_code, second.status_code] == [200, 200, 200] + assert len(observed) == 3 + warnings = [r for r in caplog.records if "previous_response_id" in r.message] + assert len(warnings) == 1, "warn once per process, not once per request" + assert "no response store" in warnings[0].message