From 6d7905a712899aa7d0a192c2983ec21eb2742570 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Sun, 26 Jul 2026 23:14:58 -0700 Subject: [PATCH 1/7] fix(session): give the completion verifier room to answer (ENG-1081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verdict call is a forced tool call with max_tokens=256. Models that narrate before acting — the open-weight aliases MindsHub serves through Fireworks (mindshub_air/kimi, deepseek) — spend the whole budget on plain prose and never reach the tool call, so `generate_object_code` raises and the fail-safe converts it into a silent "task complete": the turn ends with no message and the agent looks like it died mid-task. Measured in prod Langfuse over 2026-07-14..07-27: 171 of 687 verdict calls returned no tool call (24.9%; 37.1% for external users), 145/147 of them on mindshub_air, every single one with completion_tokens == max_tokens exactly. Not new with ENG-716 — the old free-text verifier also passed max_tokens=256 and 21% of its verdicts were unparseable for the same reason; ENG-716 only changed the consequence from "keep working" to "stop silently". - Verifier budget 256 -> 2048, with one retry at 4096 when truncated. 2048 is measured: mindshub_air spent 1,654 output tokens even with the no-preamble instruction, so 1024 was not enough (1/3 at 1024, 3/3 at 2048). - Verifier prompt now asks for the tool call first, no preamble. Needed alongside the budget, not instead of it (0/3 at 256 even with it). - StructuredOutputError carries `truncated` so a blown budget is retryable while a genuine failure isn't. Detected by output-token count, because the gateway reports finish_reason "stop" at the cap for most aliases (ENG-1082); "length"/"max_tokens" are honoured when reported. - Truncation is logged distinctly from other verifier failures, which all logged identically as "verifier unavailable" before — the reason a 25% failure rate went unseen for 10 days. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/llm/client.py | 39 ++++- anton/core/llm/provider.py | 40 +++++ anton/core/session.py | 79 ++++++++- tests/test_verifier_truncation.py | 270 ++++++++++++++++++++++++++++++ 4 files changed, 413 insertions(+), 15 deletions(-) create mode 100644 tests/test_verifier_truncation.py diff --git a/anton/core/llm/client.py b/anton/core/llm/client.py index 88c96629..0f3e98e6 100644 --- a/anton/core/llm/client.py +++ b/anton/core/llm/client.py @@ -3,7 +3,7 @@ from collections.abc import AsyncIterator from typing import TYPE_CHECKING -from .provider import LLMProvider, LLMResponse, StreamEvent +from .provider import LLMProvider, LLMResponse, StreamEvent, StructuredOutputError if TYPE_CHECKING: from anton.config.settings import AntonSettings @@ -204,18 +204,42 @@ async def _generate_object_with( tool, validator_class, is_list = build_structured_tool(schema_class) + budget = max_tokens or self._max_tokens + response = await provider.complete( model=model, system=system, messages=messages, tools=[tool], tool_choice={"type": "tool", "name": tool["name"]}, - max_tokens=max_tokens or self._max_tokens, + max_tokens=budget, ) if not response.tool_calls: - raise ValueError( - f"LLM did not return a tool call for forced schema {tool['name']}." + # Report *why* there is no tool call, so callers can retry a + # truncated call with more room instead of treating a budget + # problem as a hard failure (ENG-1081). Token count is the + # reliable signal here — the MindsHub gateway reports + # `finish_reason: "stop"` at the cap for most aliases (ENG-1082), + # so `stop_reason` alone would miss it. + # Both provider dialects: OpenAI/gateway say "length", Anthropic says + # "max_tokens" (passed through raw by AnthropicProvider). + output_tokens = response.usage.output_tokens + truncated = response.stop_reason in ("length", "max_tokens") or ( + budget > 0 and output_tokens >= budget + ) + raise StructuredOutputError( + f"LLM did not return a tool call for forced schema {tool['name']}" + + ( + f" (truncated: {output_tokens}/{budget} output tokens spent " + "on text before the call)." + if truncated + else "." + ), + truncated=truncated, + output_tokens=output_tokens, + max_tokens=budget, + stop_reason=response.stop_reason, ) return unwrap_structured_response( @@ -263,8 +287,11 @@ async def generate_object( input was a list annotation. Raises: - ValueError: If the LLM fails to produce a tool call (rare — - forced tool_choice usually prevents this). + StructuredOutputError: If the LLM fails to produce a tool call. + A ``ValueError`` subclass, so callers that catch + ``ValueError`` are unaffected. Check ``.truncated`` to tell a + blown ``max_tokens`` budget (retry with more room) from a + genuine failure (ENG-1081). pydantic.ValidationError: If the tool's input doesn't match the schema. diff --git a/anton/core/llm/provider.py b/anton/core/llm/provider.py index 7831cd2e..52f5f496 100644 --- a/anton/core/llm/provider.py +++ b/anton/core/llm/provider.py @@ -312,6 +312,46 @@ class TokenLimitExceeded(Exception): """Raised when the LLM returns 429 due to billing/token limits.""" +class StructuredOutputError(ValueError): + """Raised when a forced-tool-call structured-output call returns no tool call. + + ``truncated`` distinguishes the two very different reasons this happens: + + - **Truncated** (``True``) — the model narrated in plain ``content`` and ran + out of the ``max_tokens`` budget before reaching the tool call. This is a + budget problem and a retry with more room usually succeeds. Models served + through MindsHub's Fireworks aliases (``mindshub_air``/``kimi``, + ``deepseek``) always narrate first, so a tight budget fails them + deterministically (ENG-1081). + - **Anything else** (``False``) — the provider errored, refused, or returned + an empty response. A retry at the same size is not expected to help. + + Truncation is detected by output-token count, not by ``stop_reason``: the + MindsHub gateway reports ``finish_reason: "stop"`` while returning exactly + ``max_tokens`` tokens for most aliases (ENG-1082), so the standard + ``"length"`` check cannot be relied on. ``stop_reason`` is still honoured + when the provider does report it correctly. + + Subclasses ``ValueError`` so existing call sites that catch the documented + ``ValueError`` from ``generate_object``/``generate_object_code`` keep working. + """ + + def __init__( + self, + message: str, + *, + truncated: bool = False, + output_tokens: int = 0, + max_tokens: int = 0, + stop_reason: str | None = None, + ): + super().__init__(message) + self.truncated = truncated + self.output_tokens = output_tokens + self.max_tokens = max_tokens + self.stop_reason = stop_reason + + class TransientProviderError(ConnectionError): """Raised when the provider fails in a way that a retry might fix. diff --git a/anton/core/session.py b/anton/core/session.py index 77eece9a..0b9f94ad 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -39,6 +39,7 @@ StreamTaskProgress, StreamTextDelta, StreamToolResult, + StructuredOutputError, TokenLimitExceeded, ToolCall, TransientProviderError, @@ -158,6 +159,37 @@ class _VerifierVerdict(BaseModel): reason: str = Field(description="One brief sentence explaining the verdict.") +# Output budgets for the completion-verifier verdict call: first attempt, then +# the retry used when the first one is truncated (ENG-1081). +# +# The verdict itself is tiny — first-party models answer in 43–115 tokens with no +# preamble. But the open-weight aliases MindsHub serves through Fireworks +# (`mindshub_air`/`kimi`, `deepseek`) narrate ~1,300 characters of reasoning as +# plain text *before* the forced tool call, because `tool_choice` is advisory on +# that endpoint. The original 256 truncated them mid-sentence, every single time: +# 98.6% of `mindshub_air` verdict calls in prod returned no tool call, which the +# fail-safe below then turned into a silent "task complete" and a turn that ended +# without a message. +# +# 2048 was measured, not guessed — `mindshub_air` spent 1,654 output tokens even +# *with* the no-preamble instruction, so 1024 was not enough. Nothing pays for +# the headroom it doesn't use. +_VERIFIER_TOKEN_BUDGETS = (2048, 4096) + +# Appended to the verifier system prompt. Halves the preamble on narrating +# models; on its own it is not sufficient (0/3 at 256 even with it), which is why +# it is paired with the budgets above rather than used instead of them. +# Names the tool off the schema class so the instruction can't go stale if the +# class is renamed (the forced tool name is derived from it in +# `build_structured_tool`). This is the exact wording measured at 3/3. +_VERIFIER_NO_PREAMBLE = ( + f"Call the {_VerifierVerdict.__name__} tool immediately as your first " + "action. Do not think out loud, restate the conversation, or explain your " + "reasoning before calling it — put your one-sentence justification in the " + "tool's `reason` field." +) + + def _render_tool_result_content(content, cap: int) -> str: """Render a tool_result's content as bounded plain text. @@ -2656,18 +2688,47 @@ async def _stream_and_handle_tools( "You are a task-completion verifier. Decide whether the user's " "request is complete, the assistant is waiting on the user, the work " "is unfinished, or the assistant is blocked. Follow the status " - "definitions exactly." + "definitions exactly.\n\n" + # Models that narrate before acting spend the whole budget on + # prose and never reach the tool call (ENG-1081). Asking for the + # call first shortens the preamble; it does not eliminate it, + # which is why the budget below is generous as well. + f"{_VERIFIER_NO_PREAMBLE}" ) - try: - verdict = await self._llm.generate_object_code( - _VerifierVerdict, - system=verifier_system, - messages=verify_messages, - max_tokens=256, - ) + verdict = None + for attempt, budget in enumerate(_VERIFIER_TOKEN_BUDGETS): + try: + verdict = await self._llm.generate_object_code( + _VerifierVerdict, + system=verifier_system, + messages=verify_messages, + max_tokens=budget, + ) + break + except StructuredOutputError as exc: + # A truncated verdict is a budget problem, not a verdict: the + # model narrated past `budget` before it reached the tool call + # (ENG-1081). Retry with more room. Any other structured-output + # failure won't be fixed by a bigger budget, so don't pay for it. + retrying = exc.truncated and attempt + 1 < len(_VERIFIER_TOKEN_BUDGETS) + _verifier_log.info( + "completion-verifier verdict=%s budget=%d output_tokens=%d " + "stop_reason=%s retrying=%s", + "TRUNCATED" if exc.truncated else "NO_TOOL_CALL", + budget, exc.output_tokens, exc.stop_reason, retrying, + ) + if not retrying: + break + except Exception: + _verifier_log.info( + "completion-verifier verdict=ERROR budget=%d", budget + ) + break + + if verdict is not None: status = verdict.status reason = verdict.reason.strip() - except Exception: + else: # Verifier failed — fail safe by treating the turn as done rather # than forcing a continuation the user never asked for. status, reason = "COMPLETE", "verifier unavailable" diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py new file mode 100644 index 00000000..2043c6bd --- /dev/null +++ b/tests/test_verifier_truncation.py @@ -0,0 +1,270 @@ +"""Completion-verifier truncation handling (ENG-1081). + +The verdict call is a forced tool call. Models that narrate before acting +(MindsHub's Fireworks aliases — `mindshub_air`/`kimi`, `deepseek`) spend the +output budget on plain prose and never reach the call, so a tight `max_tokens` +fails them deterministically: 98.6% of `mindshub_air` verdict calls in prod +returned no tool call, and the fail-safe turned each one into a silent +"task complete" with no message to the user. + +Two behaviours are covered here: + +1. `_generate_object_with` reports *why* there was no tool call, distinguishing a + blown budget (retryable) from a genuine failure — by token count, because the + MindsHub gateway reports `finish_reason: "stop"` at the cap (ENG-1082). +2. The verifier retries a truncated verdict once with a bigger budget, and does + NOT spend a retry on a failure a bigger budget can't fix. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from tests.conftest import make_mock_llm + +from anton.core.llm.client import LLMClient +from anton.core.llm.provider import ( + LLMResponse, + StreamComplete, + StructuredOutputError, + ToolCall, + Usage, +) +from anton.core.session import ( + _VERIFIER_TOKEN_BUDGETS, + ChatSession, + ChatSessionConfig, + _VerifierVerdict, +) + + +@pytest.fixture() +def workspace(): + # Keep scratchpad venvs inside the repo workspace (pytest runs sandboxed and + # can't write to the real home directory). + base = Path(__file__).resolve().parents[1] / ".pytest-workspace" + base.mkdir(parents=True, exist_ok=True) + return MagicMock(base=base) + + +def _text_response(text: str, output_tokens: int = 20, stop_reason: str = "end_turn") -> LLMResponse: + return LLMResponse( + content=text, + tool_calls=[], + usage=Usage(input_tokens=10, output_tokens=output_tokens), + stop_reason=stop_reason, + ) + + +def _scratchpad_response(text: str, code: str = "print(1)") -> LLMResponse: + return LLMResponse( + content=text, + tool_calls=[ToolCall( + id="tc_1", name="scratchpad", + input={"action": "exec", "name": "main", "code": code}, + )], + usage=Usage(input_tokens=10, output_tokens=20), + stop_reason="tool_use", + ) + + +class _FakeAsyncIter: + def __init__(self, items): + self._items = items + + def __aiter__(self): + return self + + async def __anext__(self): + if not self._items: + raise StopAsyncIteration + return self._items.pop(0) + + +# -------------------------------------------------------------------------- +# 1. The client reports *why* the tool call is missing. +# -------------------------------------------------------------------------- + + +def _client_with_response(response: LLMResponse) -> LLMClient: + provider = MagicMock() + provider.complete = AsyncMock(return_value=response) + return LLMClient( + planning_provider=provider, + planning_model="planner", + coding_provider=provider, + coding_model="coder", + ) + + +async def test_no_tool_call_at_the_cap_is_reported_as_truncated(): + """Prose that spends the whole budget == truncation, even though the + gateway calls it `finish_reason: "stop"` (ENG-1082).""" + llm = _client_with_response( + _text_response("Let me analyze this conversation carefully. The user...", + output_tokens=256, stop_reason="stop") + ) + + with pytest.raises(StructuredOutputError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=256, + ) + + exc = exc_info.value + assert exc.truncated is True + assert exc.output_tokens == 256 + assert exc.max_tokens == 256 + # Callers that only know the documented ValueError still catch it. + assert isinstance(exc, ValueError) + + +@pytest.mark.parametrize("stop_reason", ["length", "max_tokens"]) +async def test_both_provider_dialects_for_truncation(stop_reason): + """The gateway/OpenAI dialect says "length"; AnthropicProvider passes + Anthropic's own "max_tokens" through raw. Both mean truncated.""" + llm = _client_with_response(_text_response("narrating…", output_tokens=100, + stop_reason=stop_reason)) + + with pytest.raises(StructuredOutputError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=2048, + ) + + assert exc_info.value.truncated is True + + +async def test_stop_reason_length_is_honoured_below_the_cap(): + """Gemini reports truncation honestly and can return almost nothing — + trust `stop_reason` too, not only the token count.""" + llm = _client_with_response(_text_response("", output_tokens=9, stop_reason="length")) + + with pytest.raises(StructuredOutputError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=256, + ) + + assert exc_info.value.truncated is True + + +async def test_short_empty_response_is_not_truncated(): + """A provider that returns nothing well inside the budget is a genuine + failure — a bigger budget won't fix it, so it must not be retried.""" + llm = _client_with_response(_text_response("", output_tokens=5, stop_reason="stop")) + + with pytest.raises(StructuredOutputError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=256, + ) + + assert exc_info.value.truncated is False + + +# -------------------------------------------------------------------------- +# 2. The verifier retries a truncated verdict, once, with more room. +# -------------------------------------------------------------------------- + + +def _session_that_uses_a_tool(mock_llm, workspace) -> ChatSession: + """Drive one turn that uses a tool, so the completion verifier runs.""" + call_count = 0 + + def fake_plan_stream(**kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + return _FakeAsyncIter([StreamComplete(response=_scratchpad_response("Running."))]) + return _FakeAsyncIter([StreamComplete(response=_text_response("Done."))]) + + mock_llm.plan_stream = fake_plan_stream + return ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace)) + + +async def test_truncated_verdict_is_retried_with_a_bigger_budget(workspace): + """The narrating-model case: first budget truncates, the retry succeeds, and + the retried verdict is the one that counts — no silent 'COMPLETE'.""" + budgets: list[int] = [] + + async def fake_verdict(_schema, *, system, messages, max_tokens): + budgets.append(max_tokens) + if len(budgets) == 1: + raise StructuredOutputError( + "no tool call", truncated=True, output_tokens=max_tokens, + max_tokens=max_tokens, stop_reason="stop", + ) + return _VerifierVerdict(status="WAITING", reason="asked the user a question") + + mock_llm = make_mock_llm() + mock_llm.generate_object_code = AsyncMock(side_effect=fake_verdict) + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + async for _ in session.turn_stream("build me a dashboard"): + pass + finally: + await session.close() + + assert budgets == list(_VERIFIER_TOKEN_BUDGETS[:2]), ( + "a truncated verdict must be retried once, with the larger budget" + ) + # The verdict came from the retry (WAITING → a valid stop), so no + # "Continue working" continuation was injected. + assert not any( + "SYSTEM: Task verification determined this task is not yet complete" + in str(m.get("content", "")) + for m in session.history + ) + + +async def test_non_truncated_failure_is_not_retried(workspace): + """A failure a bigger budget can't fix costs exactly one call, then falls + through to the fail-safe.""" + calls: list[int] = [] + + async def fake_verdict(_schema, *, system, messages, max_tokens): + calls.append(max_tokens) + raise StructuredOutputError( + "no tool call", truncated=False, output_tokens=3, + max_tokens=max_tokens, stop_reason="stop", + ) + + mock_llm = make_mock_llm() + mock_llm.generate_object_code = AsyncMock(side_effect=fake_verdict) + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + async for _ in session.turn_stream("build me a dashboard"): + pass + finally: + await session.close() + + assert calls == [_VERIFIER_TOKEN_BUDGETS[0]], "must not pay for a hopeless retry" + + +async def test_verifier_prompt_forbids_preamble(workspace): + """The no-preamble instruction reaches the model. It is not sufficient on its + own (0/3 at 256 with it), but it shortens the preamble enough to matter.""" + seen: dict = {} + + async def fake_verdict(_schema, *, system, messages, max_tokens): + seen["system"] = system + return _VerifierVerdict(status="COMPLETE", reason="done") + + mock_llm = make_mock_llm() + mock_llm.generate_object_code = AsyncMock(side_effect=fake_verdict) + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + async for _ in session.turn_stream("build me a dashboard"): + pass + finally: + await session.close() + + assert "immediately as your first action" in seen["system"] + assert "Do not think out loud" in seen["system"] From 84fa1eafb2b6bd8c7fbe4916365f5401bef85881 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Sun, 26 Jul 2026 23:18:02 -0700 Subject: [PATCH 2/7] fix(session): bound the verifier retry to once per session (ENG-1081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review follow-up. If the truncation retry is itself truncated, the model won't fit a verdict at any budget we're willing to pay for — so a chronically-narrating model would otherwise cost 2048+4096 wasted output tokens on every turn instead of the old 256. Latch it after the first proof and stop buying the retry for the rest of the session; the first attempt still runs, so a transient truncation still recovers. Also drops a pointless f-string in the verifier prompt concatenation. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/session.py | 18 +++++++++++--- tests/test_verifier_truncation.py | 41 +++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index 0b9f94ad..7a680e38 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -351,6 +351,10 @@ def __init__(self, config: ChatSessionConfig) -> None: self._max_tool_rounds = s.max_tool_rounds self._max_continuations = s.max_continuations self._verify_min_tool_rounds = s.verify_min_tool_rounds + # Latched once the verifier's truncation retry has itself been truncated: + # this model narrates past even the larger budget, so the retry is pure + # waste on every later turn of this session (ENG-1081). + self._verifier_retry_exhausted = False self._context_pressure_threshold = s.context_pressure_threshold self._max_consecutive_errors = s.max_consecutive_errors self._resilience_nudge_at = s.resilience_nudge_at @@ -2693,10 +2697,13 @@ async def _stream_and_handle_tools( # prose and never reach the tool call (ENG-1081). Asking for the # call first shortens the preamble; it does not eliminate it, # which is why the budget below is generous as well. - f"{_VERIFIER_NO_PREAMBLE}" + + _VERIFIER_NO_PREAMBLE ) verdict = None - for attempt, budget in enumerate(_VERIFIER_TOKEN_BUDGETS): + budgets = _VERIFIER_TOKEN_BUDGETS + if self._verifier_retry_exhausted: + budgets = budgets[:1] + for attempt, budget in enumerate(budgets): try: verdict = await self._llm.generate_object_code( _VerifierVerdict, @@ -2710,7 +2717,12 @@ async def _stream_and_handle_tools( # model narrated past `budget` before it reached the tool call # (ENG-1081). Retry with more room. Any other structured-output # failure won't be fixed by a bigger budget, so don't pay for it. - retrying = exc.truncated and attempt + 1 < len(_VERIFIER_TOKEN_BUDGETS) + retrying = exc.truncated and attempt + 1 < len(budgets) + if exc.truncated and not retrying and len(budgets) > 1: + # The retry ran and was truncated too: this model won't + # fit the verdict at any budget we're willing to pay for. + # Stop buying the retry for the rest of the session. + self._verifier_retry_exhausted = True _verifier_log.info( "completion-verifier verdict=%s budget=%d output_tokens=%d " "stop_reason=%s retrying=%s", diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index 2043c6bd..d451c4af 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -172,13 +172,17 @@ async def test_short_empty_response_is_not_truncated(): def _session_that_uses_a_tool(mock_llm, workspace) -> ChatSession: - """Drive one turn that uses a tool, so the completion verifier runs.""" + """Session whose every turn uses one tool, so the completion verifier runs. + + Each turn makes exactly two `plan_stream` calls — the tool round, then the + final text — so alternating keeps the pattern correct across several turns. + """ call_count = 0 def fake_plan_stream(**kwargs): nonlocal call_count call_count += 1 - if call_count == 1: + if call_count % 2 == 1: return _FakeAsyncIter([StreamComplete(response=_scratchpad_response("Running."))]) return _FakeAsyncIter([StreamComplete(response=_text_response("Done."))]) @@ -247,6 +251,39 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): assert calls == [_VERIFIER_TOKEN_BUDGETS[0]], "must not pay for a hopeless retry" +async def test_retry_is_not_bought_twice_in_one_session(workspace): + """If the retry is truncated too, the model can't fit a verdict at any budget + we'll pay for — later turns in the session must stop paying for the retry.""" + budgets: list[int] = [] + + async def fake_verdict(_schema, *, system, messages, max_tokens): + budgets.append(max_tokens) + raise StructuredOutputError( + "no tool call", truncated=True, output_tokens=max_tokens, + max_tokens=max_tokens, stop_reason="stop", + ) + + mock_llm = make_mock_llm() + mock_llm.generate_object_code = AsyncMock(side_effect=fake_verdict) + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + async for _ in session.turn_stream("first turn"): + pass + assert budgets == list(_VERIFIER_TOKEN_BUDGETS), "turn 1 tries both budgets" + + # Same session, second turn: only the first budget should be attempted. + budgets.clear() + async for _ in session.turn_stream("second turn"): + pass + finally: + await session.close() + + assert budgets == [_VERIFIER_TOKEN_BUDGETS[0]], ( + "the retry must not be re-bought on every later turn" + ) + + async def test_verifier_prompt_forbids_preamble(workspace): """The no-preamble instruction reaches the model. It is not sufficient on its own (0/3 at 256 with it), but it shortens the preamble enough to matter.""" From c5a31ea171d41b8f9e28f4748293678fc3cf70c1 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Sun, 26 Jul 2026 23:39:59 -0700 Subject: [PATCH 3/7] fix(llm): share the truncation classifier with the scratchpad path (ENG-1081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review follow-up. Four fixes: 1. The detection lived in `client.py`, so the sync twin in the scratchpad subprocess (`_ScratchpadLLM.generate_object`) still raised a blind `ValueError("LLM did not return structured output.")` — the same silent, causeless failure this branch set out to remove, on the path that is handed to model-written scratchpad code with a caller-chosen max_tokens. `structured.py` exists to keep those two paths in lockstep, so the classifier moves there as `raise_missing_tool_call` and both call it. 2. Drop the per-session retry latch. It assumed a double truncation proves the model can never fit a verdict. Measured: 16 identical verdict calls to mindshub_air spent 245..1654 output tokens — a 6.7x spread for the same request — so one truncation is a tail sample, not proof about the next turn, and latching the retry off on that evidence would reintroduce silent stops. The retry is cheap because it only fires when the first attempt truncates (0 of 12 at 2048). 3. The session tests hand-built StructuredOutputError, so they would have passed even with the detection broken. Added an end-to-end test wiring a fake provider through the real LLMClient into the session; verified by mutation (breaking the classifier fails it, and 4 others). 4. The plan_stream fake alternated on `call_count % 2`, coupling it to the tool loop's internal call count; a third call in a turn would desync it and the test would assert the wrong thing. Now keyed off turn state. Also records the measured distribution next to the budget constant, so the 2048 number carries its evidence (median ~290, max 1654, ~1.24x headroom) instead of a single sample. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/backends/scratchpad_boot.py | 10 +- anton/core/llm/client.py | 31 +----- anton/core/llm/structured.py | 57 ++++++++++ anton/core/session.py | 38 ++++--- tests/test_verifier_truncation.py | 148 ++++++++++++++++++++++--- 5 files changed, 223 insertions(+), 61 deletions(-) diff --git a/anton/core/backends/scratchpad_boot.py b/anton/core/backends/scratchpad_boot.py index a949752b..1da8397c 100644 --- a/anton/core/backends/scratchpad_boot.py +++ b/anton/core/backends/scratchpad_boot.py @@ -217,6 +217,7 @@ def generate_object( """ from anton.core.llm.structured import ( build_structured_tool, + raise_missing_tool_call, unwrap_structured_response, ) @@ -233,7 +234,14 @@ def generate_object( ) if not response.tool_calls: - raise ValueError("LLM did not return structured output.") + # Same classification as the async path: a model that + # narrates before acting (mindshub_air/kimi, deepseek) can + # blow `max_tokens` on prose and never reach the call, and + # scratchpad code picks its own budget. Says so instead of + # raising a blind ValueError (ENG-1081). + raise_missing_tool_call( + response, tool_name=tool["name"], budget=max_tokens + ) return unwrap_structured_response( response.tool_calls[0].input, validator_class, is_list diff --git a/anton/core/llm/client.py b/anton/core/llm/client.py index 0f3e98e6..b8bf14a3 100644 --- a/anton/core/llm/client.py +++ b/anton/core/llm/client.py @@ -3,7 +3,7 @@ from collections.abc import AsyncIterator from typing import TYPE_CHECKING -from .provider import LLMProvider, LLMResponse, StreamEvent, StructuredOutputError +from .provider import LLMProvider, LLMResponse, StreamEvent if TYPE_CHECKING: from anton.config.settings import AntonSettings @@ -199,6 +199,7 @@ async def _generate_object_with( """ from anton.core.llm.structured import ( build_structured_tool, + raise_missing_tool_call, unwrap_structured_response, ) @@ -216,31 +217,9 @@ async def _generate_object_with( ) if not response.tool_calls: - # Report *why* there is no tool call, so callers can retry a - # truncated call with more room instead of treating a budget - # problem as a hard failure (ENG-1081). Token count is the - # reliable signal here — the MindsHub gateway reports - # `finish_reason: "stop"` at the cap for most aliases (ENG-1082), - # so `stop_reason` alone would miss it. - # Both provider dialects: OpenAI/gateway say "length", Anthropic says - # "max_tokens" (passed through raw by AnthropicProvider). - output_tokens = response.usage.output_tokens - truncated = response.stop_reason in ("length", "max_tokens") or ( - budget > 0 and output_tokens >= budget - ) - raise StructuredOutputError( - f"LLM did not return a tool call for forced schema {tool['name']}" - + ( - f" (truncated: {output_tokens}/{budget} output tokens spent " - "on text before the call)." - if truncated - else "." - ), - truncated=truncated, - output_tokens=output_tokens, - max_tokens=budget, - stop_reason=response.stop_reason, - ) + # Shared with the scratchpad's sync twin so both paths classify the + # failure identically — see `raise_missing_tool_call` (ENG-1081). + raise_missing_tool_call(response, tool_name=tool["name"], budget=budget) return unwrap_structured_response( response.tool_calls[0].input, validator_class, is_list diff --git a/anton/core/llm/structured.py b/anton/core/llm/structured.py index 02372c8e..62550658 100644 --- a/anton/core/llm/structured.py +++ b/anton/core/llm/structured.py @@ -29,6 +29,63 @@ from typing import Any +from .provider import StructuredOutputError + + +def raise_missing_tool_call(response, *, tool_name: str, budget: int) -> None: + """Raise `StructuredOutputError` explaining *why* there is no tool call. + + Lives here, next to `build_structured_tool`/`unwrap_structured_response`, + because both structured-output paths must classify the failure the same + way — the async `LLMClient._generate_object_with` and the sync + `_ScratchpadLLM.generate_object` in the scratchpad subprocess. Keeping it + in one of the two callers is how they drift (ENG-1081). + + A forced tool call can come back empty for two very different reasons: + + - **Truncated** — the model narrated in plain ``content`` and ran out of + ``budget`` before reaching the call. Retrying with more room usually + works. Models served through MindsHub's Fireworks aliases + (``mindshub_air``/``kimi``, ``deepseek``) narrate before acting, so a + tight budget fails them. + - **Anything else** — the provider errored, refused, or returned nothing. + A bigger budget won't help. + + Truncation is detected by output-token count *first*, because the MindsHub + gateway reports ``finish_reason: "stop"`` while returning exactly + ``max_tokens`` for most aliases (ENG-1082) — the standard ``"length"`` + check cannot be relied on there. Both provider dialects are still honoured + when they do report it: OpenAI/gateway say ``"length"``, Anthropic says + ``"max_tokens"``. + + Args: + response: The provider's ``LLMResponse`` (no tool calls on it). + tool_name: Name of the forced tool, for the message. + budget: The ``max_tokens`` the call was given. + + Raises: + StructuredOutputError: Always. ``.truncated`` carries the verdict. + """ + usage = getattr(response, "usage", None) + output_tokens = getattr(usage, "output_tokens", 0) or 0 + stop_reason = getattr(response, "stop_reason", None) + truncated = stop_reason in ("length", "max_tokens") or ( + budget > 0 and output_tokens >= budget + ) + detail = ( + f" (truncated: {output_tokens}/{budget} output tokens spent on text " + "before the call)." + if truncated + else "." + ) + raise StructuredOutputError( + f"LLM did not return a tool call for forced schema {tool_name}{detail}", + truncated=truncated, + output_tokens=output_tokens, + max_tokens=budget, + stop_reason=stop_reason, + ) + def build_structured_tool(schema_class) -> tuple[dict, type, bool]: """Build a forced tool-call definition from a Pydantic schema. diff --git a/anton/core/session.py b/anton/core/session.py index 7a680e38..b317e59a 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -171,9 +171,23 @@ class _VerifierVerdict(BaseModel): # fail-safe below then turned into a silent "task complete" and a turn that ended # without a message. # -# 2048 was measured, not guessed — `mindshub_air` spent 1,654 output tokens even -# *with* the no-preamble instruction, so 1024 was not enough. Nothing pays for -# the headroom it doesn't use. +# Sized from a measured distribution, not one sample. 16 identical verdict calls +# to `mindshub_air` (2048 + the no-preamble clause below) spent: +# +# 245 246 247 253 253 254 267 279 287 295 392 407 571 573 610 865 1654 +# median ≈ 290, max 1654 — a 6.7x spread for the *same* request. +# +# So output length is stochastic per call, not a fixed property of the model: +# 2048 clears the median comfortably but is only ~1.24x the observed worst case, +# which is why the 4096 retry exists rather than a single larger budget. 1024 +# would have been wrong (1/3 in the earlier run). Nothing pays for headroom it +# doesn't use — first-party models answer in 43–115 tokens either way. +# +# Deliberately no "this model can't do it" latch: with a 6.7x per-call spread, +# one truncation is a tail sample, not proof about the next turn, and a latch +# that skips the retry on that evidence would reintroduce silent stops. The +# retry is cheap because it only fires when the first attempt truncates — 0 of +# 12 at 2048 in the sample above. _VERIFIER_TOKEN_BUDGETS = (2048, 4096) # Appended to the verifier system prompt. Halves the preamble on narrating @@ -351,10 +365,6 @@ def __init__(self, config: ChatSessionConfig) -> None: self._max_tool_rounds = s.max_tool_rounds self._max_continuations = s.max_continuations self._verify_min_tool_rounds = s.verify_min_tool_rounds - # Latched once the verifier's truncation retry has itself been truncated: - # this model narrates past even the larger budget, so the retry is pure - # waste on every later turn of this session (ENG-1081). - self._verifier_retry_exhausted = False self._context_pressure_threshold = s.context_pressure_threshold self._max_consecutive_errors = s.max_consecutive_errors self._resilience_nudge_at = s.resilience_nudge_at @@ -2700,10 +2710,7 @@ async def _stream_and_handle_tools( + _VERIFIER_NO_PREAMBLE ) verdict = None - budgets = _VERIFIER_TOKEN_BUDGETS - if self._verifier_retry_exhausted: - budgets = budgets[:1] - for attempt, budget in enumerate(budgets): + for attempt, budget in enumerate(_VERIFIER_TOKEN_BUDGETS): try: verdict = await self._llm.generate_object_code( _VerifierVerdict, @@ -2717,12 +2724,9 @@ async def _stream_and_handle_tools( # model narrated past `budget` before it reached the tool call # (ENG-1081). Retry with more room. Any other structured-output # failure won't be fixed by a bigger budget, so don't pay for it. - retrying = exc.truncated and attempt + 1 < len(budgets) - if exc.truncated and not retrying and len(budgets) > 1: - # The retry ran and was truncated too: this model won't - # fit the verdict at any budget we're willing to pay for. - # Stop buying the retry for the rest of the session. - self._verifier_retry_exhausted = True + retrying = exc.truncated and attempt + 1 < len( + _VERIFIER_TOKEN_BUDGETS + ) _verifier_log.info( "completion-verifier verdict=%s budget=%d output_tokens=%d " "stop_reason=%s retrying=%s", diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index d451c4af..9f2c0c8d 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -166,27 +166,79 @@ async def test_short_empty_response_is_not_truncated(): assert exc_info.value.truncated is False +def test_shared_classifier_is_used_by_both_structured_paths(): + """The async client and the scratchpad's sync twin must classify identically. + + `raise_missing_tool_call` is the single implementation both call — the whole + point of `structured.py`. Guarded here because the previous version of this + fix put the logic in `client.py`, which silently left the sync path + (exposed to model-written scratchpad code, with a caller-chosen budget) + raising a blind ValueError. + """ + from anton.core.llm import structured + + assert hasattr(structured, "raise_missing_tool_call") + # Read the source rather than importing it: `scratchpad_boot` is a + # subprocess bootstrap and reads stdin at import time. + boot_src = ( + Path(__file__).resolve().parents[1] + / "anton" / "core" / "backends" / "scratchpad_boot.py" + ).read_text() + assert "raise_missing_tool_call" in boot_src, ( + "the sync scratchpad path must use the shared classifier, not its own raise" + ) + assert "LLM did not return structured output." not in boot_src, ( + "the old blind ValueError should be gone from the sync path" + ) + + +def test_classifier_tolerates_a_response_without_usage(): + """Defensive: a provider response missing `usage` must not blow up the + classifier — it just can't prove truncation, which is the safe direction + (no retry bought without evidence).""" + from anton.core.llm.structured import raise_missing_tool_call + + class _Bare: + content = "some prose" + + with pytest.raises(StructuredOutputError) as exc_info: + raise_missing_tool_call(_Bare(), tool_name="_VerifierVerdict", budget=2048) + + assert exc_info.value.truncated is False + assert exc_info.value.output_tokens == 0 + + # -------------------------------------------------------------------------- # 2. The verifier retries a truncated verdict, once, with more room. # -------------------------------------------------------------------------- -def _session_that_uses_a_tool(mock_llm, workspace) -> ChatSession: - """Session whose every turn uses one tool, so the completion verifier runs. +class _ToolThenText: + """`plan_stream` fake: one tool round per turn, then plain text. - Each turn makes exactly two `plan_stream` calls — the tool round, then the - final text — so alternating keeps the pattern correct across several turns. + Keyed off "have I already used the tool this turn?" rather than a call + counter — a counter with `% 2` would silently desync if a turn ever made a + third `plan_stream` call (an extra tool round, an internal recovery retry), + and the test would then be asserting something other than it claims. + Call `next_turn()` between turns. """ - call_count = 0 - def fake_plan_stream(**kwargs): - nonlocal call_count - call_count += 1 - if call_count % 2 == 1: + def __init__(self): + self.tool_used = False + + def next_turn(self) -> None: + self.tool_used = False + + def __call__(self, **kwargs): + if not self.tool_used: + self.tool_used = True return _FakeAsyncIter([StreamComplete(response=_scratchpad_response("Running."))]) return _FakeAsyncIter([StreamComplete(response=_text_response("Done."))]) - mock_llm.plan_stream = fake_plan_stream + +def _session_that_uses_a_tool(mock_llm, workspace) -> ChatSession: + """Session whose turn uses one tool, so the completion verifier runs.""" + mock_llm.plan_stream = _ToolThenText() return ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace)) @@ -251,9 +303,14 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): assert calls == [_VERIFIER_TOKEN_BUDGETS[0]], "must not pay for a hopeless retry" -async def test_retry_is_not_bought_twice_in_one_session(workspace): - """If the retry is truncated too, the model can't fit a verdict at any budget - we'll pay for — later turns in the session must stop paying for the retry.""" +async def test_attempts_are_bounded_and_repeat_each_turn(workspace): + """Truncation retries are bounded by the budget list — and are re-tried on a + later turn rather than latched off. + + Output length varies ~6.7x per call for an identical request, so one + truncation is a tail sample, not proof the model can never fit a verdict. + Latching the retry off on that evidence would bring silent stops back. + """ budgets: list[int] = [] async def fake_verdict(_schema, *, system, messages, max_tokens): @@ -270,17 +327,74 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): try: async for _ in session.turn_stream("first turn"): pass - assert budgets == list(_VERIFIER_TOKEN_BUDGETS), "turn 1 tries both budgets" + assert budgets == list(_VERIFIER_TOKEN_BUDGETS), "bounded by the budget list" - # Same session, second turn: only the first budget should be attempted. budgets.clear() + mock_llm.plan_stream.next_turn() async for _ in session.turn_stream("second turn"): pass finally: await session.close() - assert budgets == [_VERIFIER_TOKEN_BUDGETS[0]], ( - "the retry must not be re-bought on every later turn" + assert budgets == list(_VERIFIER_TOKEN_BUDGETS), ( + "a later turn gets a fresh chance — the retry is not latched off" + ) + + +async def test_truncation_retry_through_the_real_client(workspace): + """End-to-end: a provider response with prose-at-the-cap must flow through + the real `LLMClient` detection into a session-level retry. + + The other session tests raise `StructuredOutputError` by hand, so they would + pass even if the detection in `raise_missing_tool_call` were wrong. This one + wires a fake *provider* through the real client, so provider → client → + session is the actual code path (ENG-747: don't hand-build error fixtures). + """ + calls: list[int] = [] + + async def fake_complete(*, model, system, messages, tools, tool_choice, max_tokens): + calls.append(max_tokens) + if len(calls) == 1: + # Narrated right up to the ceiling, never reached the tool call — + # and the gateway calls that `finish_reason: "stop"` (ENG-1082). + return _text_response("Let me analyze this conversation carefully...", + output_tokens=max_tokens, stop_reason="stop") + return LLMResponse( + content="", + tool_calls=[ToolCall(id="tc_v", name="_VerifierVerdict", + input={"status": "WAITING", "reason": "asked the user"})], + usage=Usage(input_tokens=100, output_tokens=60), + stop_reason="tool_calls", + ) + + provider = MagicMock() + provider.complete = AsyncMock(side_effect=fake_complete) + real_client = LLMClient( + planning_provider=provider, planning_model="planner", + coding_provider=provider, coding_model="coder", + ) + + # Only the verdict call goes through the real client; the turn itself still + # uses the mock so the test stays a unit test. + mock_llm = make_mock_llm() + mock_llm.generate_object_code = real_client.generate_object_code + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + async for _ in session.turn_stream("build me a dashboard"): + pass + finally: + await session.close() + + assert calls == list(_VERIFIER_TOKEN_BUDGETS), ( + "the real client must classify prose-at-the-cap as truncated and the " + "session must retry it with the larger budget" + ) + # The retried verdict (WAITING) stands: no forced continuation. + assert not any( + "SYSTEM: Task verification determined this task is not yet complete" + in str(m.get("content", "")) + for m in session.history ) From 19caf530eb2d8d92d48f4d3b917d66306728f799 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Sun, 26 Jul 2026 23:47:39 -0700 Subject: [PATCH 4/7] fix(llm): treat a tool call truncated mid-arguments as truncation too (ENG-1081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review pass found the same bug behind a second door. The budget can run out *inside* the forced tool call's JSON, not only in the prose before it. That arrives as a non-empty but incomplete tool call — safe_parse_tool_input repairs what it can and sets parse_error — so the missing-call check never sees it, unwrap_structured_response raises a bare ValidationError, the verifier's `except Exception` treats it as a hard failure, skips the retry, and lands on exactly the silent-stop fail-safe this branch exists to remove. Given output length varies ~6.7x per call, a model that narrates ~2000 tokens and then starts emitting the call is precisely the case that lands here. - `looks_truncated` extracted so the same evidence rule covers both shapes. - `raise_missing_tool_call` -> `raise_unusable_tool_call`, message adapts to "did not return" vs "returned an unusable", and now typed NoReturn. - Both the async client and the scratchpad's sync twin classify a failed unwrap as truncation *only* when the response shows it; a malformed call with budget to spare still propagates as a validation error, so genuine schema bugs stay visible instead of buying a hopeless retry. - Parity test upgraded from a substring grep (which a comment could satisfy) to an AST check that the call really is inside the sync `generate_object`. Tests: 15 in the file, mutation-verified — breaking `looks_truncated` fails 7 of them, including both end-to-end shapes. Full unit suite 1329 passed with the same 2 pre-existing scratchpad-exec failures; all 36 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/backends/scratchpad_boot.py | 23 ++++-- anton/core/llm/client.py | 27 +++++-- anton/core/llm/structured.py | 58 +++++++++++---- tests/test_verifier_truncation.py | 97 ++++++++++++++++++++++---- 4 files changed, 165 insertions(+), 40 deletions(-) diff --git a/anton/core/backends/scratchpad_boot.py b/anton/core/backends/scratchpad_boot.py index 1da8397c..6b419262 100644 --- a/anton/core/backends/scratchpad_boot.py +++ b/anton/core/backends/scratchpad_boot.py @@ -217,7 +217,8 @@ def generate_object( """ from anton.core.llm.structured import ( build_structured_tool, - raise_missing_tool_call, + looks_truncated, + raise_unusable_tool_call, unwrap_structured_response, ) @@ -238,14 +239,24 @@ def generate_object( # narrates before acting (mindshub_air/kimi, deepseek) can # blow `max_tokens` on prose and never reach the call, and # scratchpad code picks its own budget. Says so instead of - # raising a blind ValueError (ENG-1081). - raise_missing_tool_call( + # raising a blind ValueError (ENG-1081). Nothing retries + # here, but the message reaches the model as a traceback, + # so "you ran out of budget" is actionable where "did not + # return structured output" was not. + raise_unusable_tool_call( response, tool_name=tool["name"], budget=max_tokens ) - return unwrap_structured_response( - response.tool_calls[0].input, validator_class, is_list - ) + try: + return unwrap_structured_response( + response.tool_calls[0].input, validator_class, is_list + ) + except Exception: + if looks_truncated(response, max_tokens): + raise_unusable_tool_call( + response, tool_name=tool["name"], budget=max_tokens + ) + raise _scratchpad_llm_instance = _ScratchpadLLM() diff --git a/anton/core/llm/client.py b/anton/core/llm/client.py index b8bf14a3..3e9375e6 100644 --- a/anton/core/llm/client.py +++ b/anton/core/llm/client.py @@ -199,7 +199,8 @@ async def _generate_object_with( """ from anton.core.llm.structured import ( build_structured_tool, - raise_missing_tool_call, + looks_truncated, + raise_unusable_tool_call, unwrap_structured_response, ) @@ -218,12 +219,24 @@ async def _generate_object_with( if not response.tool_calls: # Shared with the scratchpad's sync twin so both paths classify the - # failure identically — see `raise_missing_tool_call` (ENG-1081). - raise_missing_tool_call(response, tool_name=tool["name"], budget=budget) - - return unwrap_structured_response( - response.tool_calls[0].input, validator_class, is_list - ) + # failure identically — see `raise_unusable_tool_call` (ENG-1081). + raise_unusable_tool_call(response, tool_name=tool["name"], budget=budget) + + try: + return unwrap_structured_response( + response.tool_calls[0].input, validator_class, is_list + ) + except Exception: + # The budget can also run out *inside* the tool call's JSON, in + # which case `safe_parse_tool_input` salvages a partial dict (with + # `parse_error` set) and validation fails here instead. Same cause, + # so it deserves the same retry — otherwise it reads like a schema + # bug and silently falls through to the caller's fail-safe. + if looks_truncated(response, budget): + raise_unusable_tool_call( + response, tool_name=tool["name"], budget=budget + ) + raise async def generate_object( self, diff --git a/anton/core/llm/structured.py b/anton/core/llm/structured.py index 62550658..6b99301b 100644 --- a/anton/core/llm/structured.py +++ b/anton/core/llm/structured.py @@ -27,13 +27,43 @@ from __future__ import annotations -from typing import Any +from typing import Any, NoReturn from .provider import StructuredOutputError -def raise_missing_tool_call(response, *, tool_name: str, budget: int) -> None: - """Raise `StructuredOutputError` explaining *why* there is no tool call. +def looks_truncated(response, budget: int) -> bool: + """True if `response` was cut off by the `max_tokens` budget. + + Token count is checked *first* because the MindsHub gateway reports + ``finish_reason: "stop"`` while returning exactly ``max_tokens`` for most + aliases (ENG-1082) — the standard ``"length"`` check cannot be relied on + there. Both provider dialects are still honoured when they do report it: + OpenAI/the gateway say ``"length"``, Anthropic says ``"max_tokens"``. + + A response with no usage information yields ``False``: without evidence we + don't claim truncation, which is the safe direction (no retry is bought). + """ + usage = getattr(response, "usage", None) + output_tokens = getattr(usage, "output_tokens", 0) or 0 + stop_reason = getattr(response, "stop_reason", None) + return stop_reason in ("length", "max_tokens") or ( + budget > 0 and output_tokens >= budget + ) + + +def raise_unusable_tool_call(response, *, tool_name: str, budget: int) -> NoReturn: + """Raise `StructuredOutputError` explaining *why* the tool call is unusable. + + Covers both shapes of the same underlying problem: + + - **No tool call at all** — the model narrated in plain ``content`` until + the budget ran out. + - **A damaged tool call** — the budget ran out *inside* the call's JSON + arguments, so ``safe_parse_tool_input`` salvaged a partial dict and set + ``parse_error``, and validation then fails. Without this, that case + surfaces as a bare ``ValidationError``, which reads like a schema bug and + never gets the retry a truncation deserves (ENG-1081). Lives here, next to `build_structured_tool`/`unwrap_structured_response`, because both structured-output paths must classify the failure the same @@ -51,15 +81,10 @@ def raise_missing_tool_call(response, *, tool_name: str, budget: int) -> None: - **Anything else** — the provider errored, refused, or returned nothing. A bigger budget won't help. - Truncation is detected by output-token count *first*, because the MindsHub - gateway reports ``finish_reason: "stop"`` while returning exactly - ``max_tokens`` for most aliases (ENG-1082) — the standard ``"length"`` - check cannot be relied on there. Both provider dialects are still honoured - when they do report it: OpenAI/gateway say ``"length"``, Anthropic says - ``"max_tokens"``. + See `looks_truncated` for how truncation is detected. Args: - response: The provider's ``LLMResponse`` (no tool calls on it). + response: The provider's ``LLMResponse``. tool_name: Name of the forced tool, for the message. budget: The ``max_tokens`` the call was given. @@ -69,17 +94,20 @@ def raise_missing_tool_call(response, *, tool_name: str, budget: int) -> None: usage = getattr(response, "usage", None) output_tokens = getattr(usage, "output_tokens", 0) or 0 stop_reason = getattr(response, "stop_reason", None) - truncated = stop_reason in ("length", "max_tokens") or ( - budget > 0 and output_tokens >= budget + truncated = looks_truncated(response, budget) + what = ( + "returned an unusable tool call for" + if getattr(response, "tool_calls", None) + else "did not return a tool call for" ) detail = ( - f" (truncated: {output_tokens}/{budget} output tokens spent on text " - "before the call)." + f" (truncated: {output_tokens}/{budget} output tokens spent before the " + "call was complete)." if truncated else "." ) raise StructuredOutputError( - f"LLM did not return a tool call for forced schema {tool_name}{detail}", + f"LLM {what} forced schema {tool_name}{detail}", truncated=truncated, output_tokens=output_tokens, max_tokens=budget, diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index 9f2c0c8d..e20562c7 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -166,26 +166,94 @@ async def test_short_empty_response_is_not_truncated(): assert exc_info.value.truncated is False +def _damaged_tool_call_response(output_tokens: int) -> LLMResponse: + """A forced tool call that ran out of budget *inside* its JSON arguments. + + `safe_parse_tool_input` repairs what it can and sets `parse_error`, so the + call arrives non-empty but incomplete — here missing the required `reason`. + """ + return LLMResponse( + content="", + tool_calls=[ToolCall(id="tc_v", name="_VerifierVerdict", + input={"status": "WAITING"}, + parse_error="Unterminated string starting at: line 1")], + usage=Usage(input_tokens=100, output_tokens=output_tokens), + stop_reason="stop", + ) + + +async def test_tool_call_truncated_mid_arguments_is_retryable(): + """The budget can run out *inside* the tool call, not only before it. + + That arrives as a non-empty but incomplete tool call, so the missing-call + check doesn't see it and validation fails instead — which without this + would read as a schema bug, skip the retry, and land right back on the + silent-stop fail-safe this whole change exists to remove. + """ + llm = _client_with_response(_damaged_tool_call_response(output_tokens=2048)) + + with pytest.raises(StructuredOutputError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=2048, + ) + + assert exc_info.value.truncated is True, "must be retryable, not a schema error" + assert "unusable tool call" in str(exc_info.value) + + +async def test_schema_mismatch_under_budget_is_not_disguised_as_truncation(): + """The mirror case: a malformed tool call with budget to spare is a real + schema failure. It must keep propagating as a validation error rather than + being relabelled truncation and buying a retry that cannot help.""" + llm = _client_with_response(_damaged_tool_call_response(output_tokens=60)) + + with pytest.raises(ValueError) as exc_info: + await llm.generate_object_code( + _VerifierVerdict, system="s", messages=[{"role": "user", "content": "m"}], + max_tokens=2048, + ) + + assert not isinstance(exc_info.value, StructuredOutputError), ( + "a genuine schema mismatch must stay a validation error" + ) + + def test_shared_classifier_is_used_by_both_structured_paths(): """The async client and the scratchpad's sync twin must classify identically. - `raise_missing_tool_call` is the single implementation both call — the whole + `raise_unusable_tool_call` is the single implementation both call — the whole point of `structured.py`. Guarded here because the previous version of this fix put the logic in `client.py`, which silently left the sync path (exposed to model-written scratchpad code, with a caller-chosen budget) raising a blind ValueError. """ + import ast + from anton.core.llm import structured - assert hasattr(structured, "raise_missing_tool_call") - # Read the source rather than importing it: `scratchpad_boot` is a - # subprocess bootstrap and reads stdin at import time. + assert hasattr(structured, "raise_unusable_tool_call") + # Parse the source rather than importing it — `scratchpad_boot` is a + # subprocess bootstrap and reads stdin at import time. AST rather than a + # substring search, so a passing mention in a comment or a stale import + # can't satisfy it: the call must be inside `generate_object` itself. boot_src = ( Path(__file__).resolve().parents[1] / "anton" / "core" / "backends" / "scratchpad_boot.py" ).read_text() - assert "raise_missing_tool_call" in boot_src, ( - "the sync scratchpad path must use the shared classifier, not its own raise" + tree = ast.parse(boot_src) + generate_object = next( + (n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name == "generate_object"), + None, + ) + assert generate_object is not None, "sync generate_object not found" + called = { + n.func.id for n in ast.walk(generate_object) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Name) + } + assert "raise_unusable_tool_call" in called, ( + "the sync scratchpad path must call the shared classifier, not raise its own" ) assert "LLM did not return structured output." not in boot_src, ( "the old blind ValueError should be gone from the sync path" @@ -196,13 +264,13 @@ def test_classifier_tolerates_a_response_without_usage(): """Defensive: a provider response missing `usage` must not blow up the classifier — it just can't prove truncation, which is the safe direction (no retry bought without evidence).""" - from anton.core.llm.structured import raise_missing_tool_call + from anton.core.llm.structured import raise_unusable_tool_call class _Bare: content = "some prose" with pytest.raises(StructuredOutputError) as exc_info: - raise_missing_tool_call(_Bare(), tool_name="_VerifierVerdict", budget=2048) + raise_unusable_tool_call(_Bare(), tool_name="_VerifierVerdict", budget=2048) assert exc_info.value.truncated is False assert exc_info.value.output_tokens == 0 @@ -341,12 +409,14 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): ) -async def test_truncation_retry_through_the_real_client(workspace): - """End-to-end: a provider response with prose-at-the-cap must flow through - the real `LLMClient` detection into a session-level retry. +@pytest.mark.parametrize("first_failure", ["no_tool_call", "damaged_tool_call"]) +async def test_truncation_retry_through_the_real_client(workspace, first_failure): + """End-to-end: both shapes of a truncated verdict must flow through the real + `LLMClient` detection into a session-level retry — prose-at-the-cap with no + call at all, and a call cut off inside its own arguments. The other session tests raise `StructuredOutputError` by hand, so they would - pass even if the detection in `raise_missing_tool_call` were wrong. This one + pass even if the detection in `raise_unusable_tool_call` were wrong. This one wires a fake *provider* through the real client, so provider → client → session is the actual code path (ENG-747: don't hand-build error fixtures). """ @@ -355,6 +425,9 @@ async def test_truncation_retry_through_the_real_client(workspace): async def fake_complete(*, model, system, messages, tools, tool_choice, max_tokens): calls.append(max_tokens) if len(calls) == 1: + if first_failure == "damaged_tool_call": + # Budget ran out *inside* the call's JSON arguments. + return _damaged_tool_call_response(output_tokens=max_tokens) # Narrated right up to the ceiling, never reached the tool call — # and the gateway calls that `finish_reason: "stop"` (ENG-1082). return _text_response("Let me analyze this conversation carefully...", From 866e8c8b97b4dd5f0ca3dc0ac6d92754d67d3811 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Sun, 26 Jul 2026 23:54:39 -0700 Subject: [PATCH 5/7] test(e2e): cover the truncation retry end-to-end; log the verifier's error cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tool-assisted review pass. - The e2e stub could not express a truncated response (usage.completion_tokens was hardcoded to 10), so the retry had no coverage on the real subprocess path. `_Response.output_tokens` + `queue_verification_truncated()` emulate what the gateway actually returns — prose, no tool call, completion_tokens at the cap, and `finish_reason: "stop"` anyway (ENG-1082). New scenario asserts the session makes a second verdict call with a larger budget and honours the retried verdict instead of stopping silently. - The verifier's generic `except Exception` logged a bare `verdict=ERROR` with no cause — the same undiagnosable swallow this branch exists to fix, and the same thing I flagged on #276. Now logs the exception type and message. Full unit suite 1329 passed; all 37 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/session.py | 10 +++++-- tests/e2e/scenarios/test_loop_safety.py | 39 +++++++++++++++++++++++++ tests/e2e/stub_server.py | 27 ++++++++++++++++- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index b317e59a..48bc5b25 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -2735,9 +2735,15 @@ async def _stream_and_handle_tools( ) if not retrying: break - except Exception: + except Exception as exc: + # Log the cause. Four different failures used to collapse + # into one indistinguishable "verifier unavailable" line, + # which is why a 25%-of-calls truncation went unnoticed for + # ten days (ENG-1081); a bare `verdict=ERROR` would repeat + # that mistake for whatever comes next. _verifier_log.info( - "completion-verifier verdict=ERROR budget=%d", budget + "completion-verifier verdict=ERROR budget=%d error=%s: %s", + budget, type(exc).__name__, exc, ) break diff --git a/tests/e2e/scenarios/test_loop_safety.py b/tests/e2e/scenarios/test_loop_safety.py index 3f83dcfe..642b5738 100644 --- a/tests/e2e/scenarios/test_loop_safety.py +++ b/tests/e2e/scenarios/test_loop_safety.py @@ -50,6 +50,45 @@ def test_continuation_limit_respected(cfg, stub, tmp_path): ), f"Budget-exhausted message not found. Request count: {stub.request_count}" +@pytest.mark.stub_only +def test_truncated_verdict_is_retried_not_silently_dropped(cfg, stub, tmp_path): + # ENG-1081: the verdict call is a forced tool call, and models that narrate + # before acting (mindshub_air/kimi, deepseek) spend the whole budget on prose + # and never reach it. That used to raise, get swallowed as a fake COMPLETE, + # and end the turn with no message at all. The session must retry with a + # bigger budget and then honour the real verdict. + stub.queue_tool_call("scratchpad", {"action": "exec", "name": "c", "code": "print(1)"}) + stub.queue_text("Ran the script. TURN_TEXT") + stub.queue_verification_truncated() # first budget: narrated to the cap + stub.queue_verification_incomplete("the summary table is still missing") + stub.queue_text("Finished the summary table. RETRY_VERDICT_HONOURED") + stub.queue_verification_ok() + result = run_anton(["--folder", str(tmp_path)], ["build me a summary", "exit"], + env=base_env(stub), timeout=cfg.timeout(60)) + + assert_exit_ok(result) + assert_not_output(result, "Traceback (most recent call last)") + # The retried verdict (INCOMPLETE) drove a continuation, so the turn kept + # working instead of stopping silently on a fabricated COMPLETE. + assert_output(result, "RETRY_VERDICT_HONOURED") + verdict_calls = [ + r for r in stub.requests + if "task-completion verifier" in json.dumps(r.get("messages", [])) + or "task-completion verifier" in str(r.get("system", "")) + ] + assert len(verdict_calls) >= 2, ( + f"expected a retried verdict call, saw {len(verdict_calls)}. " + f"Request count: {stub.request_count}" + ) + # The OpenAI provider sends the budget as `max_completion_tokens`. + budgets = [ + r.get("max_completion_tokens") or r.get("max_tokens") for r in verdict_calls[:2] + ] + assert budgets[0] < budgets[1], ( + f"the retry must ask for more room than the first attempt, got {budgets}" + ) + + @pytest.mark.stub_only def test_waiting_verdict_stops_without_continuation(cfg, stub, tmp_path): # ENG-716: a tool-using turn that ends by asking the user a question must diff --git a/tests/e2e/stub_server.py b/tests/e2e/stub_server.py index c8c20125..b97bfeff 100644 --- a/tests/e2e/stub_server.py +++ b/tests/e2e/stub_server.py @@ -29,6 +29,11 @@ class _Response: tool_calls: list[dict] = field(default_factory=list) # None = honour request's `stream` flag; True/False = override force_streaming: bool | None = None + # Reported as `usage.completion_tokens`. Set it equal to the request's + # `max_tokens` to emulate a truncated response — which is how the real + # MindsHub gateway presents one, since it still says + # `finish_reason: "stop"` at the cap (ENG-1082). + output_tokens: int = 10 class StubServer: @@ -84,6 +89,22 @@ def _queue_verdict(self, status: str, reason: str) -> "StubServer": )) return self + def queue_verification_truncated(self, output_tokens: int = 2048) -> "StubServer": + """Queue a verdict call that narrated instead of calling the tool and ran + out of budget doing it (ENG-1081). + + Text, no tool call, and `completion_tokens` equal to the verifier's first + budget — exactly what `mindshub_air`/`kimi`/`deepseek` return. The + session should retry with the larger budget rather than treating this as + a verdict. + """ + self._queue.put(_Response( + content="Let me analyze this conversation carefully. The user asked for", + force_streaming=False, + output_tokens=output_tokens, + )) + return self + def queue_verification_ok(self) -> "StubServer": """Queue a COMPLETE verifier verdict.""" return self._queue_verdict("COMPLETE", "task is done.") @@ -243,7 +264,11 @@ def _send_json(handler: BaseHTTPRequestHandler, resp: _Response) -> None: "message": message, "finish_reason": finish_reason, }], - "usage": {"prompt_tokens": 10, "completion_tokens": 10, "total_tokens": 20}, + "usage": { + "prompt_tokens": 10, + "completion_tokens": resp.output_tokens, + "total_tokens": 10 + resp.output_tokens, + }, } body = json.dumps(data).encode() handler.send_response(200) From 9122e65c0b8dc36c126d14f727ae101a101445da Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Mon, 27 Jul 2026 08:59:39 -0700 Subject: [PATCH 6/7] fix(session): keep conversation content out of verifier logs (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @pnewsam, Major on #277: logging `str(exc)` can copy conversation content into ordinary application logs. Correct — a pydantic ValidationError embeds the rejected `input_value`, which for the verifier is model-generated text derived from the user's conversation, and provider exceptions can carry response bodies. It also contradicted this PR's own security claim. - `_safe_error_detail(exc)` emits the exception type plus, for validation errors, the failing field locations and error codes only — via `errors(include_input=False, include_url=False, include_context=False)`, and reading only `loc`/`type` even on the v1 fallback, so `msg`/`input`/`ctx` can never be quoted. Provider errors reduce to type + status_code. - Same leak class found in a pre-existing line: the summary log wrote `reason=%s`, the model's free-text justification, on EVERY verdict — a broader exposure than the one reported. Dropped; status and counters stay, and the reason remains in the Langfuse trace where conversation content belongs. - 3 regression tests, including one asserting a planted value never reaches the log detail. Also shortened the long comment blocks flagged in review (structured.py, session.py budget/prompt constants, client.py, scratchpad_boot.py) — same facts, roughly half the lines. 1332 unit tests pass; all 37 e2e scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/backends/scratchpad_boot.py | 12 +-- anton/core/llm/client.py | 7 +- anton/core/llm/provider.py | 32 +++----- anton/core/llm/structured.py | 13 ++- anton/core/session.py | 105 +++++++++++++++---------- tests/test_verifier_truncation.py | 49 ++++++++++++ 6 files changed, 134 insertions(+), 84 deletions(-) diff --git a/anton/core/backends/scratchpad_boot.py b/anton/core/backends/scratchpad_boot.py index 6b419262..d7eaf108 100644 --- a/anton/core/backends/scratchpad_boot.py +++ b/anton/core/backends/scratchpad_boot.py @@ -235,14 +235,10 @@ def generate_object( ) if not response.tool_calls: - # Same classification as the async path: a model that - # narrates before acting (mindshub_air/kimi, deepseek) can - # blow `max_tokens` on prose and never reach the call, and - # scratchpad code picks its own budget. Says so instead of - # raising a blind ValueError (ENG-1081). Nothing retries - # here, but the message reaches the model as a traceback, - # so "you ran out of budget" is actionable where "did not - # return structured output" was not. + # Same classification as the async path (ENG-1081). + # Nothing retries here, but the message reaches the model as + # a traceback, so "you ran out of budget" is actionable + # where "did not return structured output" was not. raise_unusable_tool_call( response, tool_name=tool["name"], budget=max_tokens ) diff --git a/anton/core/llm/client.py b/anton/core/llm/client.py index 3e9375e6..30b79bf0 100644 --- a/anton/core/llm/client.py +++ b/anton/core/llm/client.py @@ -227,11 +227,8 @@ async def _generate_object_with( response.tool_calls[0].input, validator_class, is_list ) except Exception: - # The budget can also run out *inside* the tool call's JSON, in - # which case `safe_parse_tool_input` salvages a partial dict (with - # `parse_error` set) and validation fails here instead. Same cause, - # so it deserves the same retry — otherwise it reads like a schema - # bug and silently falls through to the caller's fail-safe. + # The budget can also run out *inside* the tool call's JSON, so + # validation fails here instead. Same cause, same retry (ENG-1081). if looks_truncated(response, budget): raise_unusable_tool_call( response, tool_name=tool["name"], budget=budget diff --git a/anton/core/llm/provider.py b/anton/core/llm/provider.py index 52f5f496..719ba67b 100644 --- a/anton/core/llm/provider.py +++ b/anton/core/llm/provider.py @@ -313,27 +313,17 @@ class TokenLimitExceeded(Exception): class StructuredOutputError(ValueError): - """Raised when a forced-tool-call structured-output call returns no tool call. - - ``truncated`` distinguishes the two very different reasons this happens: - - - **Truncated** (``True``) — the model narrated in plain ``content`` and ran - out of the ``max_tokens`` budget before reaching the tool call. This is a - budget problem and a retry with more room usually succeeds. Models served - through MindsHub's Fireworks aliases (``mindshub_air``/``kimi``, - ``deepseek``) always narrate first, so a tight budget fails them - deterministically (ENG-1081). - - **Anything else** (``False``) — the provider errored, refused, or returned - an empty response. A retry at the same size is not expected to help. - - Truncation is detected by output-token count, not by ``stop_reason``: the - MindsHub gateway reports ``finish_reason: "stop"`` while returning exactly - ``max_tokens`` tokens for most aliases (ENG-1082), so the standard - ``"length"`` check cannot be relied on. ``stop_reason`` is still honoured - when the provider does report it correctly. - - Subclasses ``ValueError`` so existing call sites that catch the documented - ``ValueError`` from ``generate_object``/``generate_object_code`` keep working. + """Raised when a forced-tool-call structured-output call yields no usable call. + + ``truncated`` says whether a retry can help: ``True`` means the model spent + the ``max_tokens`` budget narrating before it reached the tool call (the + narrating aliases — ``mindshub_air``/``kimi``, ``deepseek``, ``qwen`` — do + this deterministically under a tight budget, ENG-1081); ``False`` means the + provider errored, refused, or returned nothing, and a bigger budget won't + fix it. See `structured.looks_truncated` for how that's decided. + + Subclasses ``ValueError`` so call sites catching the documented ``ValueError`` + from ``generate_object``/``generate_object_code`` keep working. """ def __init__( diff --git a/anton/core/llm/structured.py b/anton/core/llm/structured.py index 6b99301b..4f497eb1 100644 --- a/anton/core/llm/structured.py +++ b/anton/core/llm/structured.py @@ -35,14 +35,11 @@ def looks_truncated(response, budget: int) -> bool: """True if `response` was cut off by the `max_tokens` budget. - Token count is checked *first* because the MindsHub gateway reports - ``finish_reason: "stop"`` while returning exactly ``max_tokens`` for most - aliases (ENG-1082) — the standard ``"length"`` check cannot be relied on - there. Both provider dialects are still honoured when they do report it: - OpenAI/the gateway say ``"length"``, Anthropic says ``"max_tokens"``. - - A response with no usage information yields ``False``: without evidence we - don't claim truncation, which is the safe direction (no retry is bought). + Token count first, because the MindsHub gateway reports + ``finish_reason: "stop"`` at the cap for most aliases (ENG-1082) — the + standard ``"length"`` check can't be relied on there. Both dialects are + honoured when reported: OpenAI says ``"length"``, Anthropic ``"max_tokens"``. + No usage information → ``False``; without evidence we don't buy a retry. """ usage = getattr(response, "usage", None) output_tokens = getattr(usage, "output_tokens", 0) or 0 diff --git a/anton/core/session.py b/anton/core/session.py index 48bc5b25..1378c056 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -159,43 +159,26 @@ class _VerifierVerdict(BaseModel): reason: str = Field(description="One brief sentence explaining the verdict.") -# Output budgets for the completion-verifier verdict call: first attempt, then -# the retry used when the first one is truncated (ENG-1081). +# Output budgets for the verdict call: first attempt, then the retry used when +# it comes back truncated (ENG-1081). # -# The verdict itself is tiny — first-party models answer in 43–115 tokens with no -# preamble. But the open-weight aliases MindsHub serves through Fireworks -# (`mindshub_air`/`kimi`, `deepseek`) narrate ~1,300 characters of reasoning as -# plain text *before* the forced tool call, because `tool_choice` is advisory on -# that endpoint. The original 256 truncated them mid-sentence, every single time: -# 98.6% of `mindshub_air` verdict calls in prod returned no tool call, which the -# fail-safe below then turned into a silent "task complete" and a turn that ended -# without a message. +# Models that narrate before acting (`mindshub_air`/`kimi`, `deepseek`, `qwen`) +# spend the budget on prose and never reach the forced tool call. The original +# 256 truncated them on essentially every call — 98.6% of `mindshub_air` verdicts +# in prod returned no tool call, which the fail-safe below turned into a silent +# "task complete". # -# Sized from a measured distribution, not one sample. 16 identical verdict calls -# to `mindshub_air` (2048 + the no-preamble clause below) spent: -# -# 245 246 247 253 253 254 267 279 287 295 392 407 571 573 610 865 1654 -# median ≈ 290, max 1654 — a 6.7x spread for the *same* request. -# -# So output length is stochastic per call, not a fixed property of the model: -# 2048 clears the median comfortably but is only ~1.24x the observed worst case, -# which is why the 4096 retry exists rather than a single larger budget. 1024 -# would have been wrong (1/3 in the earlier run). Nothing pays for headroom it -# doesn't use — first-party models answer in 43–115 tokens either way. -# -# Deliberately no "this model can't do it" latch: with a 6.7x per-call spread, -# one truncation is a tail sample, not proof about the next turn, and a latch -# that skips the retry on that evidence would reintroduce silent stops. The -# retry is cheap because it only fires when the first attempt truncates — 0 of -# 12 at 2048 in the sample above. +# 2048 is sized from a measured distribution, not one sample: 16 identical calls +# spanned 245–1654 output tokens (median ~290). That 6.7x per-call spread is also +# why there is no "this model can't do it" latch — one truncation is a tail +# sample, not proof about the next turn — and why the 4096 retry exists rather +# than a single bigger budget. 1024 was measurably too small. Nothing pays for +# headroom it doesn't use; first-party models answer in 43–115 tokens either way. _VERIFIER_TOKEN_BUDGETS = (2048, 4096) -# Appended to the verifier system prompt. Halves the preamble on narrating -# models; on its own it is not sufficient (0/3 at 256 even with it), which is why -# it is paired with the budgets above rather than used instead of them. -# Names the tool off the schema class so the instruction can't go stale if the -# class is renamed (the forced tool name is derived from it in -# `build_structured_tool`). This is the exact wording measured at 3/3. +# Appended to the verifier system prompt. Shortens the preamble on narrating +# models but is not sufficient alone (0/3 at 256 with it), so it pairs with the +# budgets above. Tool name comes off the schema class so it can't go stale. _VERIFIER_NO_PREAMBLE = ( f"Call the {_VerifierVerdict.__name__} tool immediately as your first " "action. Do not think out loud, restate the conversation, or explain your " @@ -204,6 +187,42 @@ class _VerifierVerdict(BaseModel): ) +def _safe_error_detail(exc: BaseException) -> str: + """Describe an exception for logs without copying model or user content. + + Neither ``str(exc)`` nor ``repr(exc)`` is safe here: a Pydantic + ``ValidationError`` embeds the rejected ``input_value`` — for the verifier + that's model-generated text derived from the user's conversation — and + provider exceptions can carry response bodies. Emits the exception type, + plus for validation errors the field locations and error codes only, which + is what actually identifies the failure. + """ + name = type(exc).__name__ + status = getattr(exc, "status_code", None) + if status is not None: + return f"{name}(status={status})" + errors = getattr(exc, "errors", None) + if callable(errors): + try: + try: + details = errors( + include_input=False, include_url=False, include_context=False + ) + except TypeError: # pydantic v1 has no include_* kwargs + details = errors() + # Only `loc` and `type` are ever read — `msg`/`input`/`ctx` can + # quote the rejected value. + fields = ",".join( + f"{'.'.join(str(p) for p in e.get('loc') or ())}:{e.get('type', '?')}" + for e in details + ) + if fields: + return f"{name}({fields})" + except Exception: + pass + return name + + def _render_tool_result_content(content, cap: int) -> str: """Render a tool_result's content as bounded plain text. @@ -2736,14 +2755,13 @@ async def _stream_and_handle_tools( if not retrying: break except Exception as exc: - # Log the cause. Four different failures used to collapse - # into one indistinguishable "verifier unavailable" line, - # which is why a 25%-of-calls truncation went unnoticed for - # ten days (ENG-1081); a bare `verdict=ERROR` would repeat - # that mistake for whatever comes next. + # Enough to tell the failure modes apart — four used to + # collapse into one "verifier unavailable" line — but never + # the exception message, which can carry conversation + # content (ENG-1081). _verifier_log.info( - "completion-verifier verdict=ERROR budget=%d error=%s: %s", - budget, type(exc).__name__, exc, + "completion-verifier verdict=ERROR budget=%d error=%s", + budget, _safe_error_detail(exc), ) break @@ -2756,8 +2774,11 @@ async def _stream_and_handle_tools( status, reason = "COMPLETE", "verifier unavailable" _verifier_log.info( - "completion-verifier verdict=%s continuation=%d/%d tool_rounds=%d reason=%s", - status, continuation, self._max_continuations, tool_round, reason, + # No `reason` — it's the model's free-text justification, derived + # from the user's conversation, and this is an ordinary app log. + # It stays in the Langfuse trace, where that content belongs. + "completion-verifier verdict=%s continuation=%d/%d tool_rounds=%d", + status, continuation, self._max_continuations, tool_round, ) if status in ("COMPLETE", "WAITING"): diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index e20562c7..1e24b365 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -492,3 +492,52 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): assert "immediately as your first action" in seen["system"] assert "Do not think out loud" in seen["system"] + + +# -------------------------------------------------------------------------- +# 3. Verifier logs must not carry conversation content (review finding). +# -------------------------------------------------------------------------- + + +def test_error_detail_never_leaks_the_rejected_value(): + """`_safe_error_detail` must identify the failure without quoting content. + + A pydantic ValidationError's message embeds the rejected `input_value` — for + the verifier that's model-generated text derived from the user's + conversation — so `str(exc)` cannot go into ordinary application logs. + """ + import pydantic + + from anton.core.session import _safe_error_detail + + secret = "the user's bank balance is 12345" + try: + _VerifierVerdict.model_validate({"status": secret, "reason": secret}) + except pydantic.ValidationError as exc: + detail = _safe_error_detail(exc) + else: + raise AssertionError("expected a ValidationError") + + assert secret not in detail, f"rejected value leaked into the log detail: {detail!r}" + assert "12345" not in detail + # Still useful: names the exception type and which field failed. + assert "ValidationError" in detail + assert "status" in detail + + +def test_error_detail_of_a_provider_error_is_type_and_status_only(): + from anton.core.session import _safe_error_detail + + class _FakeAPIError(Exception): + status_code = 503 + + detail = _safe_error_detail(_FakeAPIError("upstream said: ")) + assert detail == "_FakeAPIError(status=503)" + assert "response body" not in detail + + +def test_error_detail_falls_back_to_the_type_name(): + from anton.core.session import _safe_error_detail + + detail = _safe_error_detail(RuntimeError("provider hiccup with conversation text")) + assert detail == "RuntimeError" From 9b7906b12dfcd6a64940193cc386d04a9672eded Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Mon, 27 Jul 2026 09:34:39 -0700 Subject: [PATCH 7/7] fix(session): _safe_error_detail must never raise (hotfix hardening) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It runs inside the verifier's `except` handler, so an exception escaping it would convert a gracefully-handled verifier failure into a dead turn — the exact failure mode this branch exists to remove. A custom exception can expose `status_code`/`errors` as a property that raises, so the attribute reads are wrapped too. Degrades to the exception type name, never a crash and never the message. Found while assessing hotfix risk for #278. Co-Authored-By: Claude Opus 5 (1M context) --- anton/core/session.py | 29 ++++++++++++++++++++--------- tests/test_verifier_truncation.py | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index 1378c056..c6920bba 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -197,13 +197,21 @@ def _safe_error_detail(exc: BaseException) -> str: plus for validation errors the field locations and error codes only, which is what actually identifies the failure. """ - name = type(exc).__name__ - status = getattr(exc, "status_code", None) - if status is not None: - return f"{name}(status={status})" - errors = getattr(exc, "errors", None) - if callable(errors): - try: + # This runs *inside* an `except` handler, so it must not raise: an exception + # escaping here would turn a gracefully-handled verifier failure into a dead + # turn. Everything below is therefore wrapped, including the attribute reads + # (a custom exception can expose `status_code`/`errors` as a property that + # raises). + try: + name = type(exc).__name__ + except Exception: # pragma: no cover — defensive + return "unavailable" + try: + status = getattr(exc, "status_code", None) + if status is not None: + return f"{name}(status={status})" + errors = getattr(exc, "errors", None) + if callable(errors): try: details = errors( include_input=False, include_url=False, include_context=False @@ -218,8 +226,11 @@ def _safe_error_detail(exc: BaseException) -> str: ) if fields: return f"{name}({fields})" - except Exception: - pass + except Exception: + # Anything odd about this exception object — a property that raises, an + # `errors()` that misbehaves — degrades to the type name, never a crash + # and never the message. + pass return name diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index 1e24b365..748e6c52 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -541,3 +541,22 @@ def test_error_detail_falls_back_to_the_type_name(): detail = _safe_error_detail(RuntimeError("provider hiccup with conversation text")) assert detail == "RuntimeError" + + +def test_error_detail_never_raises_from_inside_an_except_handler(): + """It runs inside `except`, so raising would turn a handled verifier failure + into a dead turn. Hostile exceptions must still produce a string.""" + from anton.core.session import _safe_error_detail + + class _Hostile(Exception): + @property + def status_code(self): + raise RuntimeError("boom") + + class _HostileErrors(Exception): + def errors(self, **kwargs): + raise RuntimeError("boom") + + # Degrades to the exception type — never raises, never the message. + assert _safe_error_detail(_Hostile()) == "_Hostile" + assert _safe_error_detail(_HostileErrors()) == "_HostileErrors"