Treat empty extra-openai-models.yaml as no extra models (fixes #505)#1459
Open
jbbqqf wants to merge 1 commit into
Open
Treat empty extra-openai-models.yaml as no extra models (fixes #505)#1459jbbqqf wants to merge 1 commit into
jbbqqf wants to merge 1 commit into
Conversation
yaml.safe_load returns None for an empty / whitespace-only / comments-only YAML file, which then crashed register_models() with "TypeError: 'NoneType' object is not iterable" the next time llm did anything (e.g. `llm prompt`, `llm models`). Treat that case the same as "no extra models". Adds a regression test covering the three trip-wires from the issue: truly empty, whitespace only, and comments only. Refs simonw#505
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #505 — an empty
extra-openai-models.yamlfile (or one that contains only whitespace or comments) crashes everyllminvocation withTypeError: 'NoneType' object is not iterable.yaml.safe_loadreturnsNonefor those inputs. The previous code then ranfor extra_model in extra_models:againstNone. The fix adds an earlyreturnwhenextra_modelsis falsy.The original report describes the exact scenario this guards against: a user creates the file intending to come back to it later, leaves it empty, and is then locked out of every
llmcommand until they remember the file exists and delete it.Reproduce BEFORE/AFTER yourself (copy-paste)
What I ran locally
The new test fails on
mainwith the exactTypeErrorfrom the issue and passes on this branch. The other 754 tests are unchanged.ruff checkandblack --checkare clean on both touched files.Edge cases
extra_path.exists()is false): > file)TypeError: 'NoneType' object is not iterable" \n \n")TypeError"# placeholder\n")TypeErrortest_openai_localai_configuration+test_extra_openai_models_asyncstill passyaml.YAMLError(informative)PR drafted with assistance from Claude Code (Anthropic). The change was reviewed manually against
llm/default_plugins/openai_models.pyandtests/test_llm.py. The BEFORE/AFTER reproducer block above is the one I used during development; reviewers can paste it verbatim.