refactor(cli): type _progress_complete's result param as ClaimResult - #294
Conversation
runner.py's run_request already declares on_claim_complete as Callable[[int, str, ClaimResult], None], and the callback only reads result.status/result.finding, so the Any here was strictly looser than every real caller. Continues the Any->concrete-type vein already applied to actions.py/overlay.py/runner.py/mcp_server.py/claim_verifier.py. Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe CLI now imports ChangesCLI typing
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This is a localized type-hint-only change with no behavioral impact; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
cli.py::_run_verify's local_progress_complete(index, claim, result)callback typedresult: Any, even though it's passed directly asrunner.py::VisualQARunner.run_request'son_claim_completeparameter, which already declares the exact signatureCallable[[int, str, ClaimResult], None].tests/test_runner.py's own_build_progress_recorders()helper types the equivalent callback's third parameter asClaimResulttoo. Tightened the CLI's local callback to match.Why it's an improvement
This is the same
Any-> concrete-type tightening already applied across this repo toactions.py,overlay.py,runner.py,mcp_server.py, andclaim_verifier.py(#281, #283, #284, #285, #289, #290): a real caller-declared type existed one call away, and the looseAnyhere just meant readers/tooling couldn't see thatresult.status/result.finding(both accessed inside the function) are guaranteed attributes.Why it's safe
ClaimResultis imported underTYPE_CHECKING(the file already does this forVisualQARunner) and the module hasfrom __future__ import annotations, so there's no runtime import-cycle risk.resultis only ever passed in byVisualQARunner.run_request'son_claim_completecall site, which is typed and populated with realClaimResultinstances.uv run pytest tests/-> 322 passed / 21 pre-existing environment-only Chromium-sandbox failures (missing browser binary in sandbox), identical before and after.tests/test_cli.py(23/23) passes directly.uv run ruff check src/ tests/clean.uv run ruff format --checkflagscli.py, but that's pre-existing onmain(unrelated lines ~L275/L469) — confirmed viaruff format --diffthat no reformat touches this diff's lines.1 file changed, +2/-1.
Generated by Claude Code
Note
Low Risk
Type-hint-only change in a local CLI callback; Python does not enforce it at runtime.
Overview
Types
_progress_complete's third argument asClaimResultinstead ofAny, matchingVisualQARunner.run_request'son_claim_completecallback.ClaimResultis imported underTYPE_CHECKINGonly. Annotation-only; no runtime behavior change.Reviewed by Cursor Bugbot for commit d12b832. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit