Skip to content

fix(llm): fail on an empty completion where it happens, and make the cap overridable - #512

Open
arthapraha wants to merge 1 commit into
IBM:mainfrom
arthapraha:fix/empty-completion-typeerror
Open

fix(llm): fail on an empty completion where it happens, and make the cap overridable#512
arthapraha wants to merge 1 commit into
IBM:mainfrom
arthapraha:fix/empty-completion-typeerror

Conversation

@arthapraha

Copy link
Copy Markdown

Description

A completion with no visible content currently travels as None from
LiteLLMBackend into parse_plan, where it reaches a regex and raises:

TypeError: expected string or bytes-like object, got 'NoneType'

The traceback names _TASK_RE.finditer — four frames from the cause — and
mentions neither max_tokens nor finish_reason. Full report in #511.

Fix Details

Two changes, both small.

1. Raise where the fact is. EmptyCompletionError is raised in
LiteLLMBackend.generate_with_usage, after the completion returns and before
LLMResult is constructed
— so a None never enters the system. The message
carries the model, finish_reason, tokens consumed and the cap:

<model> returned no content (finish_reason='length'); the completion hit the
token cap of 2048 after 2048 tokens, which reasoning models can consume
entirely on reasoning_content. Raise the cap with AOB_LLM_MAX_TOKENS.

2. Make the cap overridable. max_tokens reads AOB_LLM_MAX_TOKENS,
defaulting to the current 2048 — so behaviour is unchanged unless someone
changes it.

What this deliberately does not do: no retry, no fallback, no silent empty
string. Those are policy decisions for the project; this makes the failure
legible so the policy can be chosen with the facts visible.

Impact on Benchmarking

  • No change to baselines: this only converts a crash into a named error.

A run that previously died with a TypeError now dies with an
EmptyCompletionError naming the cause. No run that previously succeeded
behaves differently
— the raise fires only where content is None, which is
already fatal today.

Related Issues

Verification Steps

  1. tests/integration does not exist at e11d1c1. What was run instead, from
    src/:

    python -m pytest llm/tests/ -q
    

    main at e11d1c1: 19 passed
    this branch: 25 passed (+6)

    The six new tests, one per behaviour:

    test what it pins
    empty_content_raises_where_it_happens content=None, finish_reason='length' → raises, cap named
    empty_content_for_other_reasons_still_raises not only the length case
    ordinary_completions_are_unaffected the success path is untouched
    empty_string_is_not_an_empty_completion "" is a valid answer and must pass through — only None raises
    token_cap_is_overridable AOB_LLM_MAX_TOKENS takes effect
    token_cap_defaults_to_the_previous_value absent the env var, still 2048
  2. The 6 pre-existing failures in src/evaluation/tests/ are untouched — this
    branch does not import from evaluation.

  3. AOB_LLM_MAX_TOKENS is read in-process and never crosses a spawn.
    os.environ.get at litellm.py:49 and litellm.completion at :61 are in
    the same function in the same process, litellm is imported in-process, and
    there is no subprocess or Popen anywhere under src/llm/. Checked
    deliberately: a fix introducing a new environment variable should not depend
    on that variable surviving a process boundary.

Checklist

  • I have added tests that prove my fix is effective. (+6)
  • My code follows the project's Ruff formatting and linting rules.
    ruff check src/llm/all checks passed; ruff format --check src/llm/9 files already formatted, on this branch. Unlike the
    scorer files, this path is clean both before and after.
  • I have signed off my commits (DCO).

…cap overridable

litellm.py returned response.choices[0].message.content unchecked. That value
is None whenever the model produced no visible text -- most often a reasoning
model that spent its whole budget on reasoning_content and stopped with
finish_reason='length'. The None travelled into plan_execute.planner and
raised

    TypeError: expected string or bytes-like object, got 'NoneType'

inside parse_plan's regex, four frames from the cause, blaming the parser for
the backend's result.

EmptyCompletionError names the model, the finish_reason and the token count,
and says how to raise the cap. An empty string is deliberately still a valid
answer; only None is treated as no completion.

max_tokens was hard-coded at 2048, which is comfortable for a non-reasoning
model and too tight for one whose visible answer is what remains after the
thinking is paid for. It now reads AOB_LLM_MAX_TOKENS and defaults to 2048, so
existing behaviour is unchanged.

llm suite 25/25 passing.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Attila <29815676+arthapraha@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant