diff --git a/src/llm/base.py b/src/llm/base.py index 6df322ab5..75f768379 100644 --- a/src/llm/base.py +++ b/src/llm/base.py @@ -6,6 +6,41 @@ from dataclasses import dataclass +class EmptyCompletionError(RuntimeError): + """The backend returned a completion with no visible content. + + Raised where it happens rather than letting ``None`` travel. A reasoning + model can spend its whole token budget on ``reasoning_content`` and return + ``content=None`` with ``finish_reason='length'``; passed on, that surfaces + several frames later as ``TypeError: expected string or bytes-like object, + got 'NoneType'`` inside a regex, which blames the parser for the backend's + result. + """ + + def __init__( + self, + model: str, + finish_reason: str | None = None, + completion_tokens: int = 0, + max_tokens: int | None = None, + ) -> None: + self.model = model + self.finish_reason = finish_reason + self.completion_tokens = completion_tokens + self.max_tokens = max_tokens + + detail = f"{model} returned no content (finish_reason={finish_reason!r})" + if finish_reason == "length": + detail += ( + f"; the completion hit the token cap" + f"{f' of {max_tokens}' if max_tokens else ''}" + f" after {completion_tokens} tokens, which reasoning models can" + f" consume entirely on reasoning_content. Raise the cap with" + f" AOB_LLM_MAX_TOKENS." + ) + super().__init__(detail) + + @dataclass(frozen=True) class LLMResult: """Return type for :meth:`LLMBackend.generate_with_usage`. diff --git a/src/llm/litellm.py b/src/llm/litellm.py index 3a1edd1b7..383262c0c 100644 --- a/src/llm/litellm.py +++ b/src/llm/litellm.py @@ -16,7 +16,7 @@ import os -from .base import LLMBackend, LLMResult +from .base import EmptyCompletionError, LLMBackend, LLMResult _WATSONX_PREFIX = "watsonx/" @@ -43,7 +43,10 @@ def generate_with_usage(self, prompt: str, temperature: float = 0.0) -> LLMResul "model": self._model_id, "messages": [{"role": "user", "content": prompt}], "temperature": temperature, - "max_tokens": 2048, + # Overridable: 2048 is comfortable for a non-reasoning model and + # too tight for one that emits reasoning_content, where the visible + # answer is what is left after the thinking is paid for. + "max_tokens": int(os.environ.get("AOB_LLM_MAX_TOKENS", "2048")), } if self._model_id.startswith(_WATSONX_PREFIX): @@ -57,8 +60,25 @@ def generate_with_usage(self, prompt: str, temperature: float = 0.0) -> LLMResul response = litellm.completion(**kwargs) usage = getattr(response, "usage", None) + choice = response.choices[0] + text = choice.message.content + completion_tokens = int(getattr(usage, "completion_tokens", 0) or 0) + + # Fail where the fact is, not four frames later. `content` is None + # whenever the model produced no visible text -- most often a reasoning + # model that spent the whole cap on reasoning_content and stopped with + # finish_reason='length'. Returning it lets a TypeError surface inside + # parse_plan's regex, blaming the parser for the backend's result. + if text is None: + raise EmptyCompletionError( + model=self._model_id, + finish_reason=getattr(choice, "finish_reason", None), + completion_tokens=completion_tokens, + max_tokens=kwargs.get("max_tokens"), + ) + return LLMResult( - text=response.choices[0].message.content, + text=text, input_tokens=int(getattr(usage, "prompt_tokens", 0) or 0), - output_tokens=int(getattr(usage, "completion_tokens", 0) or 0), + output_tokens=completion_tokens, ) diff --git a/src/llm/tests/test_empty_completion.py b/src/llm/tests/test_empty_completion.py new file mode 100644 index 000000000..13e4b6695 --- /dev/null +++ b/src/llm/tests/test_empty_completion.py @@ -0,0 +1,101 @@ +"""An empty completion must fail where it happens, not four frames later. + +`content` is None whenever the model produced no visible text -- most often a +reasoning model that spent its whole token budget on `reasoning_content` and +stopped with finish_reason='length'. Returning that None let it travel into +`plan_execute.planner.parse_plan`, where a regex raised + + TypeError: expected string or bytes-like object, got 'NoneType' + +blaming the parser for the backend's result. +""" + +import sys +import types + +import pytest + +from llm.base import EmptyCompletionError +from llm.litellm import LiteLLMBackend + + +def _response(content, finish_reason="stop", completion_tokens=2048): + """A litellm-shaped response object.""" + message = types.SimpleNamespace(content=content) + choice = types.SimpleNamespace(message=message, finish_reason=finish_reason) + usage = types.SimpleNamespace( + prompt_tokens=100, completion_tokens=completion_tokens + ) + return types.SimpleNamespace(choices=[choice], usage=usage) + + +@pytest.fixture +def fake_litellm(monkeypatch): + """Stand in for the litellm module the backend imports at call time.""" + module = types.ModuleType("litellm") + module.captured = {} + + def completion(**kwargs): + module.captured.update(kwargs) + return module.next_response + + module.completion = completion + monkeypatch.setitem(sys.modules, "litellm", module) + monkeypatch.setenv("LITELLM_API_KEY", "test-key") + monkeypatch.setenv("LITELLM_BASE_URL", "http://localhost") + return module + + +def test_empty_content_raises_where_it_happens(fake_litellm): + fake_litellm.next_response = _response(None, finish_reason="length") + backend = LiteLLMBackend("litellm_proxy/some-reasoning-model") + + with pytest.raises(EmptyCompletionError) as excinfo: + backend.generate_with_usage("plan this") + + error = excinfo.value + assert error.finish_reason == "length" + assert error.completion_tokens == 2048 + # The message must name the cause, not merely the symptom. + assert "token cap" in str(error) + assert "AOB_LLM_MAX_TOKENS" in str(error) + + +def test_empty_content_for_other_reasons_still_raises(fake_litellm): + fake_litellm.next_response = _response(None, finish_reason="content_filter") + backend = LiteLLMBackend("litellm_proxy/model") + + with pytest.raises(EmptyCompletionError) as excinfo: + backend.generate_with_usage("prompt") + assert excinfo.value.finish_reason == "content_filter" + + +def test_ordinary_completions_are_unaffected(fake_litellm): + fake_litellm.next_response = _response("1. do the thing") + backend = LiteLLMBackend("litellm_proxy/model") + + result = backend.generate_with_usage("prompt") + assert result.text == "1. do the thing" + assert result.input_tokens == 100 + assert result.output_tokens == 2048 + + +def test_empty_string_is_not_an_empty_completion(fake_litellm): + """Only None means "no content". An empty string is a real answer shape.""" + fake_litellm.next_response = _response("") + backend = LiteLLMBackend("litellm_proxy/model") + assert backend.generate_with_usage("prompt").text == "" + + +def test_token_cap_is_overridable(fake_litellm, monkeypatch): + monkeypatch.setenv("AOB_LLM_MAX_TOKENS", "8192") + fake_litellm.next_response = _response("ok") + LiteLLMBackend("litellm_proxy/model").generate_with_usage("prompt") + assert fake_litellm.captured["max_tokens"] == 8192 + + +def test_token_cap_defaults_to_the_previous_value(fake_litellm, monkeypatch): + monkeypatch.delenv("AOB_LLM_MAX_TOKENS", raising=False) + fake_litellm.next_response = _response("ok") + LiteLLMBackend("litellm_proxy/model").generate_with_usage("prompt") + assert fake_litellm.captured["max_tokens"] == 2048