From f8b81b5b0fffc8ca0120e8182425d9dd56e38b7d Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Mon, 27 Jul 2026 10:34:15 +0300 Subject: [PATCH] fix: stop submitting truncated reasoning as the browsecomp-plus answer The headline is a scoring bug. When a completion came back with `finish_reason == "length"`, the agent submitted `message.content` verbatim as its final answer. This target writes its chain-of-thought into ordinary `content` (it has no separate reasoning_content channel) and routinely runs past the token cap, so what got submitted was 46-52k characters of truncated mid-sentence reasoning. The judge scores that 0 every time, and the trial looks like a genuine wrong answer rather than a harness failure. Truncated turns are now treated as recoverable: the model is asked for just the three required labeled lines instead of having its thinking submitted for it. Four supporting changes, all aimed at the same failure mode of losing a trial that had real retrieved evidence behind it: - Wall clock, not turn count, is this agent's binding constraint. Most of a turn is sequential `search.py` calls, each booting a fresh pyserini/Lucene JVM against the BM25 index (~13s apiece) against ~3-9s of inference, so trials ran out of clock long before MAX_TURNS. The turn-budget fallback therefore never fired and harbor killed the trial mid-search with no answer recorded. There is now a deadline (BROWSECOMP_AGENT_BUDGET_SEC) with a final-answer reserve (BROWSECOMP_FINAL_RESERVE_SEC) so the best-effort answer is written before harbor's AgentTimeoutError. Tool calls past the deadline are stubbed rather than skipped, because every tool_call still needs a tool message or the forced-final request is rejected. - Completion shape is validated in one place. A gateway can answer 200 with a body the SDK does not turn into a completion, observed as `'str' object has no attribute 'usage'`, which crashed the trial on an attribute access two lines later. Anything without usable `.choices` is now rejected as a retryable error. - An empty or malformed assistant turn nudges instead of raising `RuntimeError("model returned neither a response nor a tool call")`, giving up only after MAX_EMPTY_RETRIES consecutive nudges. An assistant message carrying neither content nor tool calls is dropped rather than appended, since strict OpenAI-compatible servers reject it. - The forced-final call is bounded (max_retries=1, ANSWER_MAX_TOKENS) and degrades to a stub answer on failure rather than propagating, and the trace/metadata now record finish_reason, per-turn latency, remaining budget, and a `forced_final_reason` so a post-hoc conservative score can treat exactly the trials the pre-fix agent would have lost as zeros. Co-Authored-By: Claude Opus 5 (1M context) --- .../target/src/browsecomp_plus_agent/agent.py | 272 ++++++++++++++---- .../baseline/target/tests/test_agent.py | 174 ++++++++++- 2 files changed, 393 insertions(+), 53 deletions(-) diff --git a/harness-engineering-bench/browsecomp-plus/baseline/target/src/browsecomp_plus_agent/agent.py b/harness-engineering-bench/browsecomp-plus/baseline/target/src/browsecomp_plus_agent/agent.py index 3a332eb2..4ea230b3 100644 --- a/harness-engineering-bench/browsecomp-plus/baseline/target/src/browsecomp_plus_agent/agent.py +++ b/harness-engineering-bench/browsecomp-plus/baseline/target/src/browsecomp_plus_agent/agent.py @@ -5,6 +5,7 @@ import json import os import shlex +import time from typing import Any, override from harbor.agents.base import BaseAgent @@ -25,6 +26,28 @@ def _is_reasoning_model(model: str) -> bool: MAX_TURNS = 32 MAX_TOOL_OUTPUT_CHARS = 40_000 +# Wall clock, not turn count, is this agent's binding constraint. Most of a turn +# is the sequential `search.py` invocations, each of which boots a fresh +# pyserini/Lucene JVM against the BM25 index (~13s apiece), against ~3-9s of +# inference. Trials therefore run out of clock long before MAX_TURNS, so the +# turn-budget fallback below never fires and harbor instead kills the trial +# mid-search with no answer recorded. This gives that same best-effort safety +# net a deadline to watch. Harbor does not tell an agent its own timeout, so the +# budget is read from the environment; keep it in step with whatever +# --agent-timeout-multiplier the run uses. +AGENT_BUDGET_SEC = float(os.environ.get("BROWSECOMP_AGENT_BUDGET_SEC", "900")) +# Enough to prefill the accumulated history and write a short answer (~8s for a +# 120k-token prompt measured against the target endpoint), plus the answer +# upload and margin for one retry. +FINAL_RESERVE_SEC = float(os.environ.get("BROWSECOMP_FINAL_RESERVE_SEC", "120")) +# A malformed, empty, or length-truncated assistant turn is recoverable; nudge +# rather than abort, and only give up after this many consecutive nudges. +MAX_EMPTY_RETRIES = 3 +# The required answer is three short labeled lines, so the forced-final call is +# capped well below the per-turn budget to stop the model from spending its last +# seconds on chain-of-thought and getting truncated a second time. +ANSWER_MAX_TOKENS = 3_000 + INSTRUCTIONS = """You are a persistent deep-research agent working only against the fixed BrowseComp-Plus corpus. Find the precise answer by issuing focused searches, opening promising documents, reformulating queries, and cross-checking evidence. @@ -147,13 +170,17 @@ def _trace(self, event: dict[str, Any]) -> None: file.write(json.dumps(event, ensure_ascii=False, default=str) + "\n") async def _index_command( - self, environment: BaseEnvironment, command: str, value: str + self, + environment: BaseEnvironment, + command: str, + value: str, + timeout_sec: float = 120, ) -> dict[str, Any]: flag = "--query" if command == "search" else "--docid" result = await environment.exec( f"python3 /opt/browsecomp/search.py {command} {flag} {shlex.quote(value)}", cwd="/app", - timeout_sec=120, + timeout_sec=max(15, int(timeout_sec)), ) if result.return_code != 0: return { @@ -180,7 +207,11 @@ def _usage_value(value: Any, name: str) -> int: return int(result or 0) def _completion_kwargs( - self, messages: list[dict[str, Any]], *, tools: bool = True + self, + messages: list[dict[str, Any]], + *, + tools: bool = True, + max_tokens: int = 12_000, ) -> dict[str, Any]: # Reasoning models replaced max_tokens with max_completion_tokens and # reject the old name outright ("Unsupported parameter: 'max_tokens' @@ -195,7 +226,7 @@ def _completion_kwargs( "model": self._api_model, "messages": messages, } - kwargs[_token_limit_key] = 12_000 + kwargs[_token_limit_key] = max_tokens if tools: kwargs["tools"] = TOOLS # Capability, not provider: see _is_reasoning_model. @@ -203,6 +234,35 @@ def _completion_kwargs( kwargs["reasoning_effort"] = "medium" return kwargs + async def _complete( + self, + messages: list[dict[str, Any]], + *, + tools: bool = True, + timeout: float, + max_retries: int | None = None, + max_tokens: int = 12_000, + ) -> Any: + """One chat completion, validated for shape. + + A gateway can answer 200 with a body the SDK does not turn into a + completion (observed once as `'str' object has no attribute 'usage'`), + so anything without usable `.choices` is rejected here rather than + crashing the trial on an attribute access two lines later. + """ + options: dict[str, Any] = {"timeout": max(30.0, timeout)} + if max_retries is not None: + options["max_retries"] = max_retries + response = await self._client.with_options(**options).chat.completions.create( + **self._completion_kwargs(messages, tools=tools, max_tokens=max_tokens) + ) + choices = getattr(response, "choices", None) + if not choices or getattr(choices[0], "message", None) is None: + raise RuntimeError( + f"malformed completion from provider: {type(response).__name__}" + ) + return response + def _account(self, usage: Any, totals: dict[str, int]) -> None: totals["input"] += self._usage_value(usage, "prompt_tokens") totals["output"] += self._usage_value(usage, "completion_tokens") @@ -225,18 +285,41 @@ async def run( {"role": "user", "content": instruction}, ] totals = {"input": 0, "output": 0, "cached": 0} + deadline = time.monotonic() + AGENT_BUDGET_SEC + + def remaining() -> float: + return deadline - time.monotonic() + + completed = False + out_of_time = False + out_of_answers = False + empty_turns = 0 + last_turn = 0 for turn in range(1, MAX_TURNS + 1): - response = await self._client.chat.completions.create( - **self._completion_kwargs(messages) + last_turn = turn + # Stop starting new work once only the final-answer reserve is left, + # so the answer is written before harbor's AgentTimeoutError fires. + if remaining() <= FINAL_RESERVE_SEC: + out_of_time = True + self._trace({"turn": turn, "stopping": "deadline", "left": remaining()}) + break + call_started = time.monotonic() + response = await self._complete( + messages, timeout=remaining() - FINAL_RESERVE_SEC / 2 ) - self._account(response.usage, totals) + llm_sec = time.monotonic() - call_started + self._account(getattr(response, "usage", None), totals) message = response.choices[0].message + finish_reason = getattr(response.choices[0], "finish_reason", None) calls = message.tool_calls or [] self._trace( { "turn": turn, "content": message.content, + "finish_reason": finish_reason, + "llm_sec": round(llm_sec, 2), + "left_sec": round(remaining(), 1), "tool_calls": [ {"name": c.function.name, "arguments": c.function.arguments} for c in calls @@ -244,6 +327,8 @@ async def run( } ) # Record the assistant turn (with any tool calls) in the history. + # An assistant message carrying neither is rejected by strict + # OpenAI-compatible servers, so it is dropped rather than appended. assistant: dict[str, Any] = {"role": "assistant"} if message.content: assistant["content"] = message.content @@ -259,44 +344,89 @@ async def run( } for c in calls ] - messages.append(assistant) + if len(assistant) > 1: + messages.append(assistant) if not calls: - if (message.content or "").strip(): + # `length` means the model was cut off mid-thought, so its + # content is truncated reasoning, not an answer. This target + # writes its chain-of-thought into ordinary `content` (it has no + # separate reasoning_content channel) and can run past the token + # cap, and the truncated ramble was previously submitted verbatim + # as the final answer, which the judge always scores 0. Ask for + # the answer instead of submitting the thinking. + truncated = finish_reason == "length" + if (message.content or "").strip() and not truncated: await self._submit(environment, message.content) + completed = True context.metadata = { "turns": turn, "trace": "browsecomp-plus-trace.jsonl", } break - raise RuntimeError("model returned neither a response nor a tool call") + empty_turns += 1 + self._trace( + {"turn": turn, "recover": empty_turns, "truncated": truncated} + ) + if empty_turns >= MAX_EMPTY_RETRIES: + out_of_answers = True + break + messages.append( + { + "role": "user", + "content": ( + "You were cut off mid-thought. Stop reasoning and " + "reply with only the three required labeled lines " + "(Explanation, Exact Answer, Confidence), keeping the " + "explanation to a few sentences." + if truncated + else "Your last message was empty. Either call a tool " + "or give your final response in the required format." + ), + } + ) + continue + empty_turns = 0 submitted = False for call in calls: - try: - arguments = json.loads(call.function.arguments) - if call.function.name == "search": - result = await self._index_command( - environment, "search", arguments["query"] - ) - elif call.function.name == "get_document": - result = await self._index_command( - environment, "get-document", arguments["docid"] - ) - elif call.function.name == "submit_response": - await self._submit(environment, arguments["response"]) - result = {"submitted": True} - submitted = True - else: - result = {"error": f"unknown tool: {call.function.name}"} - except ( - json.JSONDecodeError, - KeyError, - OSError, - TypeError, - ValueError, - ) as error: - result = {"error": str(error)} + # Every tool_call must still get a tool message even once the + # deadline hits, or the forced-final request below is rejected + # for an unanswered tool call. So stub, never skip. + if out_of_time or remaining() <= FINAL_RESERVE_SEC: + if not out_of_time: + out_of_time = True + self._trace({"turn": turn, "stopping": "deadline-midturn"}) + result = {"error": "search budget exhausted"} + else: + try: + arguments = json.loads(call.function.arguments) + budget = remaining() - FINAL_RESERVE_SEC + if call.function.name == "search": + result = await self._index_command( + environment, "search", arguments["query"], budget + ) + elif call.function.name == "get_document": + result = await self._index_command( + environment, "get-document", arguments["docid"], budget + ) + elif call.function.name == "submit_response": + await self._submit(environment, arguments["response"]) + result = {"submitted": True} + submitted = True + else: + result = {"error": f"unknown tool: {call.function.name}"} + except ( + json.JSONDecodeError, + KeyError, + OSError, + TypeError, + ValueError, + ) as error: + result = {"error": str(error)} + # Traced on both paths. A stub is precisely what a post-hoc audit + # needs to see (which searches were abandoned to the clock), and + # tracing only the executed branch left it invisible. self._trace( {"turn": turn, "tool": call.function.name, "result": result} ) @@ -312,39 +442,81 @@ async def run( if submitted: break if submitted: + completed = True context.metadata = { "turns": turn, "trace": "browsecomp-plus-trace.jsonl", } break - else: - # Turn budget exhausted: force one final tool-free response from what - # was retrieved rather than crashing, so the case scores best-effort + if out_of_time: + break + + if not completed: + # Budget exhausted (wall clock, turns, or a model that kept coming + # back empty): force one final tool-free response from what was + # retrieved rather than crashing, so the case scores best-effort # instead of being lost with no answer recorded. + stub = ( + "Explanation: no answer could be determined within the search " + "budget.\nExact Answer: unknown\nConfidence: 0%" + ) + # Three distinct exits land here and a post-hoc scorer has to tell + # them apart: the wall clock, repeated truncated/empty turns, and + # plain MAX_TURNS exhaustion. + reason = ( + "deadline" + if out_of_time + else "empty_retries" + if out_of_answers + else "turns" + ) messages.append( { "role": "user", "content": ( - "You have used your full search budget. Give your single " - "best final response now, in the required format, based on " - "the documents you have retrieved." + "You have used your full search budget. Do not reason " + "further. Reply with only the three required labeled " + "lines (Explanation, Exact Answer, Confidence), keeping " + "the explanation to a few sentences, giving your single " + "best answer from the documents you have retrieved." ), } ) - final = await self._client.chat.completions.create( - **self._completion_kwargs(messages, tools=False) - ) - self._account(final.usage, totals) - answer = (final.choices[0].message.content or "").strip() or ( - "Explanation: no answer could be determined within the search " - "budget.\nExact Answer: unknown\nConfidence: 0%" - ) + # This call is the last thing standing between a completed trial and + # a lost one, so a failure here degrades to the stub answer instead + # of propagating. + answer = stub + try: + # Bounded retries: the SDK default of 8 could outlast the + # reserve on its own and hand the trial back to the killer. + # A tight token cap keeps the model from spending the whole + # reserve on chain-of-thought and getting truncated again. + final = await self._complete( + messages, + tools=False, + timeout=max(30.0, remaining() - 20), + max_retries=1, + max_tokens=ANSWER_MAX_TOKENS, + ) + self._account(getattr(final, "usage", None), totals) + answer = (final.choices[0].message.content or "").strip() or stub + except Exception as error: # noqa: BLE001 - last-resort safety net + self._trace({"forced_final_error": f"{type(error).__name__}: {error}"}) await self._submit(environment, answer) - self._trace({"turn": MAX_TURNS, "forced_final_answer": answer}) + self._trace( + { + "turn": last_turn, + "forced_final_answer": answer, + "reason": reason, + } + ) context.metadata = { - "turns": MAX_TURNS, + "turns": last_turn, "trace": "browsecomp-plus-trace.jsonl", "forced_final": True, + # Lets a post-hoc conservative score treat exactly the trials + # that the pre-fix agent would have lost as zeros. + "forced_final_reason": reason, } context.n_input_tokens = totals["input"] diff --git a/harness-engineering-bench/browsecomp-plus/baseline/target/tests/test_agent.py b/harness-engineering-bench/browsecomp-plus/baseline/target/tests/test_agent.py index be507575..8f13114b 100644 --- a/harness-engineering-bench/browsecomp-plus/baseline/target/tests/test_agent.py +++ b/harness-engineering-bench/browsecomp-plus/baseline/target/tests/test_agent.py @@ -48,6 +48,21 @@ async def create(self, **kwargs): ) +class FakeClient: + """Stands in for `AsyncOpenAI`, including the per-call options wrapper. + + `_complete` sets its timeout (and sometimes `max_retries`) through + `with_options`, so a fake without it fails on attribute access before any + request is made. + """ + + def __init__(self, completions): + self.chat = SimpleNamespace(completions=completions) + + def with_options(self, **_kwargs): + return self + + def json_response(value: str) -> str: import json @@ -61,9 +76,7 @@ async def test_agent_submits_response_and_populates_context(tmp_path, monkeypatc logs_dir=tmp_path / "logs", model_name="openai/gpt-5.4-mini-2026-03-17", ) - agent._client = SimpleNamespace( - chat=SimpleNamespace(completions=FakeCompletions()) - ) + agent._client = FakeClient(FakeCompletions()) environment = FakeEnvironment() context = SimpleNamespace( metadata=None, @@ -87,6 +100,161 @@ async def test_agent_submits_response_and_populates_context(tmp_path, monkeypatc } +TRUNCATED_REASONING = "So the candidate could be either one, unless " * 900 + + +class TruncatedThenAnswer: + """First turn is cut off mid-thought; the second gives the real answer.""" + + def __init__(self): + self.requests: list[dict] = [] + + async def create(self, **kwargs): + # Snapshot: the agent keeps mutating the same `messages` list, so a + # stored reference would only ever show the final state. + self.requests.append({**kwargs, "messages": list(kwargs["messages"])}) + first = len(self.requests) == 1 + message = SimpleNamespace( + content=TRUNCATED_REASONING if first else None, + tool_calls=None + if first + else [ + SimpleNamespace( + id="call-1", + type="function", + function=SimpleNamespace( + name="submit_response", + arguments=json_response( + "Explanation: Evidence [12].\n" + "Exact Answer: 42\n" + "Confidence: 100%" + ), + ), + ) + ], + ) + return SimpleNamespace( + choices=[ + SimpleNamespace( + message=message, + finish_reason="length" if first else "tool_calls", + ) + ], + usage=SimpleNamespace( + prompt_tokens=10, + completion_tokens=1, + prompt_tokens_details=SimpleNamespace(cached_tokens=0), + ), + ) + + +@pytest.mark.asyncio +async def test_truncated_reasoning_is_never_submitted_as_the_answer( + tmp_path, monkeypatch +): + """A `length` finish means truncated thinking, not an answer. + + Regression test for the scoring bug: the agent used to submit `content` + verbatim whenever a turn produced no tool calls, so 42k-52k characters of + mid-sentence reasoning went to the judge and scored 0, indistinguishable + from a genuinely wrong answer. + """ + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + agent = BrowseCompPlusAgent( + logs_dir=tmp_path / "logs", + model_name="openai/gpt-5.4-mini-2026-03-17", + ) + completions = TruncatedThenAnswer() + agent._client = FakeClient(completions) + environment = FakeEnvironment() + context = SimpleNamespace( + metadata=None, + n_input_tokens=None, + n_output_tokens=None, + n_cache_tokens=None, + ) + + await agent.run("Find the answer.", environment, context) + + assert len(environment.uploads) == 1 + submitted = environment.uploads[0][0].read_text(encoding="utf-8") + assert "Exact Answer: 42" in submitted + assert "unless" not in submitted, "truncated reasoning must not be submitted" + + # The truncated turn is recovered, not fatal: the model is asked for just + # the labeled lines, and the unfinished thinking is what it replaces. + assert len(completions.requests) == 2 + nudge = completions.requests[1]["messages"][-1] + assert nudge["role"] == "user" + assert "cut off mid-thought" in nudge["content"] + + +class AlwaysTruncated: + """Truncated every turn, then a clean answer on the forced-final call.""" + + def __init__(self): + self.calls = 0 + self.tool_free_calls = 0 + + async def create(self, **kwargs): + self.calls += 1 + if "tools" not in kwargs: + self.tool_free_calls += 1 + content = ( + "Explanation: best effort.\nExact Answer: 42\nConfidence: 10%" + ) + finish = "stop" + else: + content = TRUNCATED_REASONING + finish = "length" + return SimpleNamespace( + choices=[ + SimpleNamespace( + message=SimpleNamespace(content=content, tool_calls=None), + finish_reason=finish, + ) + ], + usage=SimpleNamespace( + prompt_tokens=10, + completion_tokens=1, + prompt_tokens_details=SimpleNamespace(cached_tokens=0), + ), + ) + + +@pytest.mark.asyncio +async def test_repeated_truncation_is_reported_as_empty_retries(tmp_path, monkeypatch): + """`forced_final_reason` must separate this from MAX_TURNS exhaustion. + + A post-hoc conservative score uses that field to pick out the trials the + pre-fix agent would have lost, so a trial that gave up on turn 4 after three + truncated turns cannot be labelled the same as one that used all 32 turns. + """ + monkeypatch.setenv("OPENAI_API_KEY", "test-key") + agent = BrowseCompPlusAgent( + logs_dir=tmp_path / "logs", + model_name="openai/gpt-5.4-mini-2026-03-17", + ) + completions = AlwaysTruncated() + agent._client = FakeClient(completions) + environment = FakeEnvironment() + context = SimpleNamespace( + metadata=None, + n_input_tokens=None, + n_output_tokens=None, + n_cache_tokens=None, + ) + + await agent.run("Find the answer.", environment, context) + + assert context.metadata["forced_final"] is True + assert context.metadata["forced_final_reason"] == "empty_retries" + # Bounded: three nudged turns, then exactly one tool-free forced final. + assert completions.tool_free_calls == 1 + assert completions.calls == 4 + assert "Exact Answer: 42" in environment.uploads[0][0].read_text(encoding="utf-8") + + @pytest.mark.asyncio async def test_search_uses_fixed_index_cli(tmp_path, monkeypatch): monkeypatch.setenv("OPENAI_API_KEY", "test-key")