Repository navigation
Fix crash when a non-streaming Chat Completions response has no usage - #1698
Conversation
chrikrah
left a comment
There was a problem hiding this comment.
@tao-hpu the production change is right and should land, and the test as written breaks on current main. Your four parameters do reproduce for me on Linux and Python 3.12.3.
blocking: 0b51c61 ("Close SQLite connections after CLI commands and tests", 2026-09-22) dropped import sqlite_utils from tests/test_cli_openai_models.py and added a db_factory fixture at tests/conftest.py:40. Your branch appends below that import block, so git merges clean and line 779 then raises NameError. Rebase on main first, since ae8624e has no such fixture, then line 779 becomes db = db_factory(str(log_db)) with db_factory taken as a test argument.
Head ae8624e, merge base 508b4f8, main at 764dc38, pytest 9.1.1, exported trees under /var/tmp:
$ PYTHONPATH=$HEAD python -m pytest tests/test_cli_openai_models.py -q
51 passed in 29.76s
# same tree, openai_models.py replaced by its 508b4f8 copy, your tests kept
$ PYTHONPATH=$BASE python -m pytest tests/test_cli_openai_models.py -q -k without_usage
4 failed, 47 deselected in 11.22s
E AttributeError: 'NoneType' object has no attribute 'model_dump'
# git merge-tree --write-tree ae8624e origin/main -> 94fbcd0, rc=0, no conflict
$ PYTHONPATH=$MERGED python -m pytest tests/test_cli_openai_models.py -q
4 failed, 47 passed in 28.63s
E NameError: name 'sqlite_utils' is not defined # tests/test_cli_openai_models.py:779
# not run: the rest of tests/, and no live OpenAI-compatible server
@simonw the guard applies to 764dc38 unchanged, where openai_models.py:1490 and :1607 are both still a bare completion.usage.model_dump(), and only Read the Docs has run here. If tao-hpu stays quiet until 2026-10-08, shall I open the rebased one-file version?
Chat.execute() and AsyncChat.execute() called completion.usage.model_dump() without checking that usage is set. OpenAI-compatible servers can omit the "usage" key or send "usage": null, which raised AttributeError: 'NoneType' object has no attribute 'model_dump' for every --no-stream prompt. The streaming branches already guard with "if chunk.usage:" and set_usage() already accepts None. Apply the same guard to both non-streaming branches and add sync and async tests for a missing and a null usage field. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RL9G1DHE5yNVtkm3FFPPNX
ae8624e to
b9e8af4
Compare
|
Rebased onto |
chrikrah
left a comment
There was a problem hiding this comment.
@tao-hpu I would merge this at b9e8af4. The rebase onto 764dc38 clears the NameError, and the four parameters still fail with openai_models.py put back.
# Python 3.12.3, pytest 9.1.1, tree from git archive b9e8af4
$ PYTHONPATH=$HEAD python -m pytest tests/test_cli_openai_models.py -q -p no:randomly
51 passed in 96.32s
# llm/default_plugins/openai_models.py from 764dc38, tests kept
4 failed, 47 passed in 46.53s
E AttributeError: 'NoneType' object has no attribute 'model_dump'
# not run: the rest of tests/, and no live OpenAI-compatible server
@simonw tao-hpu did the rebase, so my 2026-10-08 offer is off. The Test and Run Cog workflows on b9e8af4 are waiting for your approval to run.
Chat.execute()andAsyncChat.execute()calledcompletion.usage.model_dump()without checking thatusageis set. OpenAI-compatible servers can omit theusagekey or send"usage": null, and every--no-streamprompt against them failed withAttributeError: 'NoneType' object has no attribute 'model_dump'.This applies the guard the streaming branches already use (
if chunk.usage:) to both non-streaming branches.usageis already initialised toNonein both methods andset_usage()already accepts that, so the response is logged with empty token counts.Changes:
llm/default_plugins/openai_models.py: 2 guarded lines, one inChat.execute()and one inAsyncChat.execute().tests/test_cli_openai_models.py:test_chat_completion_without_usage_sync_and_async, 4 cases (missing key andnull, sync and--async). All 4 fail on main with the AttributeError and pass with the change. The test also checks that the turn is logged withinput_tokensandoutput_tokensset toNULL.Checks run locally (macOS arm64, Python 3.13.5):
uv run pytest: 1175 passeduv run black . --check: 53 files would be left unchangeduv run cog --check ... README.md docs/*.md: no changesuv run mypy llm: no issues found in 19 source filesuv run ruff check .: all checks passedThe repro script from the issue raises the AttributeError on
llm==0.35from PyPI and printstext: helloon this branch.Closes #1697
AI assistance (Claude Code) was used for this change.