From e2049cef87081ff8e76812ea1668f01234661c8e Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Tue, 11 Aug 2026 23:22:17 -0700 Subject: [PATCH] fix(query): recover from OpenAI-compat length truncations; forward non-interactive to headless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two model-agnostic fixes ported from the DeepSeek-flash harness work (feat/deepseek-flash-harness), benefiting every provider: 1. finish_reason="length" -> stop_reason "max_tokens" normalization. Every stop_reason consumer in the query loop keys on the internal (Anthropic) "max_tokens" vocabulary — the withheld-content escalation lane, the resume-recovery nudge, _is_withheld_max_output_tokens — and none matched the OpenAI wire's "length". So a truncated response from ANY OpenAI-compatible provider (openai, openrouter, zai, deepseek, …) fell through as a normal end-of-turn: no escalation to 64K, no resume nudge, and a headless run ended with a silently-clipped answer. Anthropic already emits "max_tokens"; this maps the other wire onto the same word. The tag is now applied ONLY to tool-free truncations — a truncation that still carries a surviving tool call must be yielded (headless persistence is the only route into the saved session) rather than withheld while its tools execute; this also closes the same latent leak on the Anthropic wire, where max_tokens + tool_use has always been possible. 2. build_effective_system_prompt now forwards is_non_interactive_session into build_full_system_prompt_blocks, so the headless (-p) path actually receives the "# Non-Interactive Mode" section (it was silently omitted). Interactive callers leave the flag unset and are unchanged. Tests: test_openai_length_stop_reason.py (escalation fires on "length"; tool-carrying truncation is yielded not withheld) and test_headless_non_interactive_section.py. Co-Authored-By: Claude Opus 5 --- src/query/agent_loop_compat.py | 14 ++ src/query/query.py | 28 ++- .../test_headless_non_interactive_section.py | 46 +++++ tests/test_openai_length_stop_reason.py | 171 ++++++++++++++++++ 4 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 tests/test_headless_non_interactive_section.py create mode 100644 tests/test_openai_length_stop_reason.py diff --git a/src/query/agent_loop_compat.py b/src/query/agent_loop_compat.py index 8c25f6ca0..410fbffe0 100644 --- a/src/query/agent_loop_compat.py +++ b/src/query/agent_loop_compat.py @@ -286,6 +286,20 @@ def build_effective_system_prompt( provider=provider, mcp_servers=mcp_servers, skills=skills, + # Headless sets ``options.is_non_interactive_session`` before the + # prompt is built; without forwarding it, the headless model never + # received the "# Non-Interactive Mode" section and behaved as if a + # human would reply (observed on terminal-bench: agents narrating + # "let me save this for future sessions" mid-trial). Interactive + # callers (TUI bridge, agent-server) leave the flag unset and are + # unchanged. Model-agnostic. + non_interactive=bool( + getattr( + getattr(tool_context, "options", None), + "is_non_interactive_session", + False, + ) + ), ) # Preserve the workspace + git + CLAWCODEX.md context (CLAWCODEX.md is NOT diff --git a/src/query/query.py b/src/query/query.py index a26c118ad..122e15cbc 100644 --- a/src/query/query.py +++ b/src/query/query.py @@ -1525,7 +1525,20 @@ def _do_provider_call(): for raw in response.raw_content_blocks: assistant_blocks.append(dict(raw)) - stop_reason = response.finish_reason or "end_turn" + # Normalize the OpenAI-compat truncation vocabulary onto the internal + # (Anthropic) one. Every consumer of stop_reason in this codebase keys on + # "max_tokens" — the withheld-content escalation lane, the recovery-nudge + # lane, `_is_withheld_max_output_tokens` — and none of them ever matched + # the OpenAI wire's "length". So a truncated response from any + # OpenAI-compatible provider (openai, openrouter, zai, deepseek, …) fell + # through as a normal end-of-turn: no escalation, no "resume" nudge, and a + # headless run would end with a silently-clipped answer. Anthropic already + # emits "max_tokens", so this maps the other wire onto the same word. + stop_reason = ( + "max_tokens" + if response.finish_reason == "length" + else (response.finish_reason or "end_turn") + ) if _diag: _elapsed = time.monotonic() - _t0 @@ -1573,7 +1586,18 @@ def _do_provider_call(): # Preserve provider thinking metadata for follow-up turns. assistant_msg.reasoning_content = response.reasoning_content # type: ignore[attr-defined] - if stop_reason == "max_tokens": + # Tag ONLY tool-free truncations. The tag routes the message into the + # withholding gate + escalation/recovery lanes — but those lanes live + # under ``not needs_follow_up``, so for a truncated response that still + # carries surviving tool calls the tag would buy nothing and cost the + # turn: the assistant message is withheld from the yield stream while its + # tools execute anyway, so ``on_message`` consumers (headless persistence + # — the ONLY route into the saved session) never see the turn, and the + # persisted tool_result is orphaned on resume. With tool calls present, a + # truncation is recoverable the ordinary way: run the tools, let the model + # continue. (Latent on the Anthropic wire too, where max_tokens + tool_use + # has always been possible; now guarded there as well.) + if stop_reason == "max_tokens" and not tool_use_blocks: assistant_msg._api_error = "max_output_tokens" # type: ignore[attr-defined] assistant_msg.isApiErrorMessage = False diff --git a/tests/test_headless_non_interactive_section.py b/tests/test_headless_non_interactive_section.py new file mode 100644 index 000000000..07219716a --- /dev/null +++ b/tests/test_headless_non_interactive_section.py @@ -0,0 +1,46 @@ +"""The cutover system-prompt builder must forward non-interactive mode. + +Headless (``claude -p``) sets ``options.is_non_interactive_session`` before +the prompt is built, but ``build_effective_system_prompt`` (the headless/TUI +cutover builder) did not forward it into ``build_full_system_prompt_blocks`` +— so the headless model never received the "# Non-Interactive Mode" section +and behaved as if a human would reply. Model-agnostic: any provider run +headlessly is affected; interactive callers leave the flag unset and are +unchanged. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from src.query.agent_loop_compat import build_effective_system_prompt +from src.tool_system.context import ToolContext + + +def _flatten(blocks) -> str: + return "\n".join(b.get("text", "") for b in blocks if isinstance(b, dict)) + + +def _ctx(non_interactive: bool) -> ToolContext: + ctx = ToolContext(workspace_root="/tmp") + # ``options`` is a duck-typed holder; headless sets this flag on it. + ctx.options = SimpleNamespace(is_non_interactive_session=non_interactive) + return ctx + + +def test_non_interactive_section_present_when_flag_set(): + text = _flatten(build_effective_system_prompt("", _ctx(True))) + assert "# Non-Interactive Mode" in text + + +def test_non_interactive_section_absent_when_flag_unset(): + text = _flatten(build_effective_system_prompt("", _ctx(False))) + assert "# Non-Interactive Mode" not in text + + +def test_missing_options_defaults_to_interactive(): + """A context with no ``options`` (or no flag) must not crash and must + default to interactive (section absent).""" + ctx = ToolContext(workspace_root="/tmp") + text = _flatten(build_effective_system_prompt("", ctx)) + assert "# Non-Interactive Mode" not in text diff --git a/tests/test_openai_length_stop_reason.py b/tests/test_openai_length_stop_reason.py new file mode 100644 index 000000000..a820564e7 --- /dev/null +++ b/tests/test_openai_length_stop_reason.py @@ -0,0 +1,171 @@ +"""OpenAI-compat ``finish_reason="length"`` must reach the recovery lanes. + +Every stop_reason consumer in the query loop keys on the internal +(Anthropic) vocabulary — ``"max_tokens"`` — and none matched the OpenAI +wire's ``"length"``, so a truncated response from any OpenAI-compatible +provider (openai, openrouter, zai, deepseek, …) fell through as a normal +end-of-turn: no escalation, no "resume" recovery, and a headless run ended +with a silently-clipped answer. The second test also pins the tool-free +guard: a truncation that still carries a surviving tool call must be +yielded (so headless persistence sees the turn) rather than withheld. +""" + +from __future__ import annotations + +import asyncio +import tempfile +import unittest +from pathlib import Path +from unittest.mock import MagicMock + +from src.providers.base import ChatResponse +from src.query.query import ESCALATED_MAX_TOKENS, QueryParams, query +from src.tool_system.context import ToolContext +from src.tool_system.defaults import build_default_registry +from src.types.messages import UserMessage +from src.utils.abort_controller import AbortController + + +class TestLengthFinishReasonNormalization(unittest.TestCase): + def setUp(self): + self.temp_dir = tempfile.TemporaryDirectory() + self.registry = build_default_registry() + self.context = ToolContext(workspace_root=Path(self.temp_dir.name)) + self.abort = AbortController() + + def tearDown(self): + self.temp_dir.cleanup() + + def _params(self, provider): + return QueryParams( + messages=[UserMessage(content="Write a long story")], + system_prompt="You are helpful.", + tools=self.registry.list_tools(), + tool_registry=self.registry, + tool_use_context=self.context, + provider=provider, + abort_controller=self.abort, + max_turns=10, + ) + + def test_length_truncation_escalates_like_max_tokens(self): + """OpenAI-wire truncation engages the same 64K escalation retry the + Anthropic vocabulary always got.""" + provider = MagicMock() + provider.chat_stream_response.side_effect = NotImplementedError() + truncated = ChatResponse( + content="Partial output...", + model="test", + usage={"input_tokens": 10, "output_tokens": 8000}, + finish_reason="length", # OpenAI vocabulary, not "max_tokens" + tool_uses=None, + ) + full = ChatResponse( + content="Complete output.", + model="test", + usage={"input_tokens": 10, "output_tokens": 500}, + finish_reason="stop", + tool_uses=None, + ) + provider.chat.side_effect = [truncated, full] + + async def run(): + async for _ in query(self._params(provider)): + pass + + asyncio.run(run()) + + self.assertEqual(provider.chat.call_count, 2) + second_call = provider.chat.call_args_list[1] + self.assertEqual(second_call[1].get("max_tokens"), ESCALATED_MAX_TOKENS) + + def test_openai_stop_maps_to_end_turn_unchanged(self): + """The normalization touches ONLY "length" — a normal "stop" turn + must not grow a retry.""" + provider = MagicMock() + provider.chat_stream_response.side_effect = NotImplementedError() + provider.chat.side_effect = [ + ChatResponse( + content="Done.", + model="test", + usage={"input_tokens": 10, "output_tokens": 5}, + finish_reason="stop", + tool_uses=None, + ) + ] + + async def run(): + async for _ in query(self._params(provider)): + pass + + asyncio.run(run()) + self.assertEqual(provider.chat.call_count, 1) + + def test_truncated_turn_with_surviving_tool_call_is_yielded(self): + """A length-truncated response that still carries a complete tool + call must reach the yield stream (and thus on_message persistence). + + The max_output_tokens tag routes a message into the withholding + gate, whose re-surfacing lanes live under ``not needs_follow_up`` — + with tool calls present nothing re-surfaces, so tagging a + tool-carrying truncation withheld the assistant turn while its + tool_result still flowed: the saved session lost the turn and the + orphaned result was dropped on resume. The tag must therefore be + tool-free-only. + """ + provider = MagicMock() + provider.chat_stream_response.side_effect = NotImplementedError() + truncated_with_tool = ChatResponse( + content="", + model="test", + usage={"input_tokens": 10, "output_tokens": 8000}, + finish_reason="length", + tool_uses=[{ + "id": "t1", + "name": "Bash", + "input": {"command": "echo hi"}, + }], + ) + done = ChatResponse( + content="Finished.", + model="test", + usage={"input_tokens": 10, "output_tokens": 5}, + finish_reason="stop", + tool_uses=None, + ) + provider.chat.side_effect = [truncated_with_tool, done] + + yielded = [] + + async def run(): + async for msg in query(self._params(provider)): + yielded.append(msg) + + asyncio.run(run()) + + def _has_block(msg, btype): + content = getattr(msg, "content", None) + if not isinstance(content, list): + return False + return any(getattr(b, "type", None) == btype for b in content) + + tool_use_yielded = any(_has_block(m, "tool_use") for m in yielded) + tool_result_yielded = any( + isinstance(getattr(m, "content", None), list) + and any( + (isinstance(b, dict) and b.get("type") == "tool_result") + or getattr(b, "type", None) == "tool_result" + for b in m.content + ) + for m in yielded + ) + self.assertTrue( + tool_use_yielded, + "assistant turn carrying the tool_use must be yielded " + f"(yielded types: {[type(m).__name__ for m in yielded]})", + ) + self.assertTrue(tool_result_yielded) + + +if __name__ == "__main__": + unittest.main()