ci: extend mypy to bench/ and scripts/, fix a real type error it finds - #130
Open
shrdgn wants to merge 1 commit into
Open
ci: extend mypy to bench/ and scripts/, fix a real type error it finds#130shrdgn wants to merge 1 commit into
shrdgn wants to merge 1 commit into
Conversation
CI's mypy step only ever checked openfusion/, so bench/ and scripts/ (both shipped, non-trivial Python) got zero static type checking. Extending the check immediately surfaces a real bug: bench/run.py's _chat() helper returned a usage value typed Any | dict[Any, Any] | None against a tuple[str, float, dict[str, Any]] return annotation, because the isinstance narrowing was applied to a throwaway payload.get() call rather than the value actually returned. Fixed by narrowing a bound variable instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013gp2rKKYP4qAFGgrX8cXLg
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.
What & why
CI's
mypystep (andpyproject.toml's[tool.mypy]) only ever scoped toopenfusion/. The repo also ships non-trivial, non-test Python inbench/(the benchmark harness) andscripts/(the OpenRouter smoke test) that got zero static type checking.Running
mypy bench/ scripts/ --ignore-missing-importssurfaces a real bug today:bench/run.py::_chat()is annotated to returntuple[str, float, dict[str, Any]], but theusagevalue it actually returns typed asAny | dict[Any, Any] | None— theisinstancenarrowing was applied to a throwawaypayload.get("usage")call rather than to the value actually returned, so the annotation wasn't backed by anything. Fixed by binding the raw value to a variable first, then narrowing that same variable.CI now runs
mypy openfusion/ bench/ scripts/ ...so this class of bug is caught going forward instead of only being visible to a human who happens to run mypy against those directories manually.Note: this touches the same
ci.ymlmypy line as the other currently-open PR "fix: resolve mypy config drift between pyproject.toml and CI" (different scope — that one reconciles CLI flags vs.pyproject.toml's[tool.mypy]foropenfusion/itself; this one widens which directories get checked and fixes a bug the widening finds). Whichever merges second will need a small, mechanical rebase of that one line.How it was tested
ruff check .passesmypy openfusion/ bench/ scripts/ --ignore-missing-imports --disable-error-code import-untyped --exclude openfusion/cli.pypasses (0 errors, was 1 error before the fix)pytest -qpasses (existingtests/test_bench*.pystill pass unchanged)_chat's behavior; no runtime behavior changedCHANGELOG.md"Fixed" entry addedNotes for reviewers
bench/run.pyandscripts/openrouter_smoke.pywere already clean underruff check .; this PR only adds mypy coverage and fixes the one issue it surfaces. No runtime behavior changes.Generated by Claude Code