diff --git a/nisse-dev/src/nisse_dev/llm.py b/nisse-dev/src/nisse_dev/llm.py index e035a98..adc34f9 100644 --- a/nisse-dev/src/nisse_dev/llm.py +++ b/nisse-dev/src/nisse_dev/llm.py @@ -41,6 +41,28 @@ class LLMError(RuntimeError): RATE_LIMIT_STATUS = 429 RATE_LIMIT_RETRY_S = 20.0 +# Generous by default. Output is billed by what is produced, not by the +# ceiling, and a ceiling too low does not fail loudly — it truncates a tool +# call mid-argument and hands back something that parses. Measured: Opus +# writing an architecture plan routinely spends 3,000-4,000 output tokens and +# was observed at 4,064 against a 4,096 limit. +DEFAULT_MAX_OUTPUT_TOKENS = 16384 + + +def max_output_tokens(env: Mapping[str, str] = os.environ) -> int: + raw = env.get("NISSE_MAX_OUTPUT_TOKENS") + return int(raw) if raw else DEFAULT_MAX_OUTPUT_TOKENS + + +class TruncatedResponse(LLMError): + """The model was cut off mid-answer. + + Raised rather than returned, because the damage is silent: a truncated + tool call arrives with required arguments simply absent, and a truncated + write_file would write half a file and report success. A cut-off turn is + a failed turn, and every caller already retries or gates on LLMError. + """ + class LLMClient(Protocol): def complete( @@ -131,9 +153,15 @@ class LiteLLMClient: self, data: dict[str, Any], requested_model: str ) -> list[CanonicalEvent]: try: - message = data["choices"][0]["message"] + choice = data["choices"][0] + message = choice["message"] except (KeyError, IndexError) as exc: raise LLMError(f"malformed completion response: {exc}") from exc + if choice.get("finish_reason") == "length": + raise TruncatedResponse( + f"{requested_model} hit its output limit mid-answer; " + "raise NISSE_MAX_OUTPUT_TOKENS" + ) produced_by = data.get("model") or requested_model out: list[CanonicalEvent] = [] if message.get("content"): @@ -184,7 +212,7 @@ class AnthropicPassthroughClient: token: str, *, timeout_s: float = 300.0, - max_tokens: int = 4096, + max_tokens: int | None = None, transport: httpx.BaseTransport | None = None, sleep: Any = time.sleep, ) -> None: @@ -199,7 +227,7 @@ class AnthropicPassthroughClient: transport=transport, ) self._renderer = AnthropicMessagesRenderer() - self._max_tokens = max_tokens + self._max_tokens = max_tokens or max_output_tokens() self._sleep = sleep @classmethod @@ -234,6 +262,11 @@ class AnthropicPassthroughClient: def _parse( self, data: dict[str, Any], requested_model: str ) -> list[CanonicalEvent]: + if data.get("stop_reason") == "max_tokens": + raise TruncatedResponse( + f"{requested_model} hit the output limit of {self._max_tokens} " + f"tokens mid-answer; raise NISSE_MAX_OUTPUT_TOKENS" + ) content = data.get("content") if not isinstance(content, list): raise LLMError(f"malformed passthrough response: {str(data)[:300]}") diff --git a/nisse-dev/tests/test_llm.py b/nisse-dev/tests/test_llm.py index abcec92..c68c701 100644 --- a/nisse-dev/tests/test_llm.py +++ b/nisse-dev/tests/test_llm.py @@ -12,11 +12,15 @@ from nisse_dev.canonical import ( ) from nisse_dev.envfile import load_env_file from nisse_dev.llm import ( + DEFAULT_MAX_OUTPUT_TOKENS, RATE_LIMIT_RETRY_S, + AnthropicPassthroughClient, LiteLLMClient, LLMError, RoleMap, + TruncatedResponse, has_native_adapter, + max_output_tokens, select_client, ) @@ -235,3 +239,107 @@ def test_native_adapter_detection_is_explicit() -> None: assert not has_native_adapter("gpt-6-astra") # A model merely mentioning the vendor is not the vendor. assert not has_native_adapter("my-claude-clone") + + +# --- truncation (silent until measured against Opus) --------------------- + + +def test_openai_route_raises_on_truncation() -> None: + # finish_reason "length" means the answer was cut off; parsing it as + # complete yields a tool call with required arguments simply absent. + truncated = httpx.Response( + 200, + json={ + "model": "gpt-test-1", + "choices": [ + { + "finish_reason": "length", + "message": { + "content": None, + "tool_calls": [ + { + "id": "c", + "type": "function", + "function": { + "name": "submit_plan", + "arguments": '{"type": "arch"}', + }, + } + ], + }, + } + ], + }, + ) + client = make_client(lambda request: truncated) + with pytest.raises(TruncatedResponse, match="output limit"): + client.complete(EVENTS, tools=None, model="gpt-6-astra") + + +def test_anthropic_route_raises_on_truncation() -> None: + # Observed for real: Opus spent 4,064 output tokens against a 4,096 + # ceiling, and a longer plan came back as submit_plan with no content. + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "model": "claude-opus-5", + "stop_reason": "max_tokens", + "content": [ + { + "type": "tool_use", + "id": "t", + "name": "submit_plan", + "input": {"type": "arch"}, + } + ], + }, + ) + + client = AnthropicPassthroughClient( + "https://litellm.test", + "tok", + transport=httpx.MockTransport(handler), + sleep=lambda _s: None, + ) + with pytest.raises(TruncatedResponse, match="output limit"): + client.complete(EVENTS, tools=None, model="claude-opus-5") + + +def test_a_complete_answer_is_not_mistaken_for_truncation() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "model": "claude-opus-5", + "stop_reason": "tool_use", + "content": [ + { + "type": "tool_use", + "id": "t", + "name": "submit_plan", + "input": {"type": "arch", "content": "a real plan"}, + } + ], + }, + ) + + client = AnthropicPassthroughClient( + "https://litellm.test", "tok", transport=httpx.MockTransport(handler) + ) + [call] = client.complete(EVENTS, tools=None, model="claude-opus-5") + assert call.arguments["content"] == "a real plan" + + +def test_output_ceiling_is_configurable_and_generous_by_default() -> None: + # Output is billed by what is produced, so a low ceiling buys nothing and + # costs silent truncation. + assert DEFAULT_MAX_OUTPUT_TOKENS >= 8192 + assert max_output_tokens({}) == DEFAULT_MAX_OUTPUT_TOKENS + assert max_output_tokens({"NISSE_MAX_OUTPUT_TOKENS": "2048"}) == 2048 + + +def test_truncation_is_an_llm_error_so_callers_already_handle_it() -> None: + # The planner retries, the executor and delegation gate. None of them + # need to learn a new exception type. + assert issubclass(TruncatedResponse, LLMError)