Skip to content

refactor(cli, runner): replace Any with VisualQARunner/ClaimVerifier in runner-handling signatures - #290

Merged
dhruvbatra merged 1 commit into
mainfrom
claude/admiring-hawking-g5bnbs
Aug 23, 2026
Merged

refactor(cli, runner): replace Any with VisualQARunner/ClaimVerifier in runner-handling signatures#290
dhruvbatra merged 1 commit into
mainfrom
claude/admiring-hawking-g5bnbs

Conversation

@dhruvbatra

@dhruvbatra dhruvbatra commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

What

Continues the Any->concrete-type tightening vein from #277/#281/#283/#284/#285/#289:

  • cli.py's _new_runner() (-> Any) and _runner_scope() (AsyncIterator[Any]) — every real call site constructs/yields a VisualQARunner. Added a TYPE_CHECKING-guarded import of VisualQARunner, matching the pattern already used in runner.py/claim_verifier.py/actions.py to avoid a circular import (cli.py -> runner.py -> ... at module scope).
  • runner.py's _resolve_default_visualize(claim_verifier: Any, fallback: bool) — its only two call sites always pass self.claim_verifier, which is already typed ClaimVerifier | None on VisualQARunner.__init__ and is never None by the time _resolve_default_visualize is called. runner.py already had a TYPE_CHECKING-guarded import of ClaimVerifier, so this just reuses it.

Why it's safe (no behavior change)

Both files already use from __future__ import annotations, so these annotations are never evaluated at runtime — this is purely a static-typing improvement with zero effect on program behavior. getattr(claim_verifier, "_visualize", fallback) inside _resolve_default_visualize is unchanged and still defensively falls back for any non-ClaimVerifier duck-typed test double.

Verification

  • python3 -m py_compile on both touched files — clean.
  • uv run ruff check src/ tests/ — clean.
  • uv run ruff format --check on the touched files — runner.py clean; cli.py shows the same 2 pre-existing "would reformat" hunks (lines ~274, ~468) that exist on main today and are unrelated to this diff (confirmed via ruff format --diff) — consistent with AGENTS.md's note not to reformat existing files.
  • uv run pytest tests/ — 322 passed / 21 failed, identical to the main baseline (the 21 failures are the documented pre-existing environment-only failures from a missing Chromium binary in this sandbox).

🤖 Generated with Claude Code


Generated by Claude Code


Note

Low Risk
Annotation-only change; annotations are not evaluated at runtime and duck-typed test doubles still work via getattr.

Overview
Tightens static types on runner-handling helpers with no runtime behavior change (from __future__ import annotations).

_new_runner and _runner_scope in cli.py now return VisualQARunner instead of Any, using a TYPE_CHECKING import to avoid a circular import. _resolve_default_visualize in runner.py now takes ClaimVerifier instead of Any.

Reviewed by Cursor Bugbot for commit 1858560. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Refactor
    • Improved internal type annotations for visual quality assurance components.
    • No user-facing behavior or functionality has changed.

…in runner-handling signatures

cli.py's _new_runner()/_runner_scope() and runner.py's _resolve_default_visualize()
were typed Any even though every real call site passes/returns a concrete
VisualQARunner or ClaimVerifier instance, continuing the same Any->concrete-type
vein as #277/#281/#283/#284/#285/#289. Tightened using a TYPE_CHECKING-guarded
import in cli.py (matching runner.py's own existing pattern) to avoid circular
import risk. Annotation-only, zero behavior change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FAav5x8va7JZ8E1LmLYQKy
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cf4a50b-b8fb-4468-a074-ad534d3d1fd2

📥 Commits

Reviewing files that changed from the base of the PR and between 2094b33 and 1858560.

📒 Files selected for processing (2)
  • src/frontend_visualqa/cli.py
  • src/frontend_visualqa/runner.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The changes improve static typing for CLI runner helpers and the visualizer claim verifier. Runtime behavior remains unchanged.

Changes

Static type annotation refinement

Layer / File(s) Summary
Concrete helper annotations
src/frontend_visualqa/cli.py, src/frontend_visualqa/runner.py
The CLI imports VisualQARunner for type checking and uses it for runner helper annotations. _resolve_default_visualize uses ClaimVerifier instead of Any.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 18585

This change narrows runner-related type annotations without changing runtime behavior, so no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: juanpin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes replacing Any with concrete types in CLI and runner signatures.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/admiring-hawking-g5bnbs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dhruvbatra
dhruvbatra merged commit a7d1958 into main Aug 23, 2026
4 checks passed
dhruvbatra added a commit that referenced this pull request Aug 23, 2026
…n types (#291)

BrowserManager.__aexit__ typed exc_type as bare `type | None` and traceback
as `Any`, even though Python's own contextmanager protocol has concrete
stdlib types for both: `type[BaseException] | None` for the exception class
and `types.TracebackType | None` for the traceback. This is the same
Any->concrete-type vein this repo has mined repeatedly (most recently
#277-#290), just applied to a dunder signature using a stdlib type instead
of a project type.

Annotation-only, zero behavior change: the method body only calls
self.close() and never inspects any of its three parameters. Verified
BrowserManager is only ever driven through `async with` (13 call sites in
tests/test_browser.py) with no direct __aexit__ invocation anywhere in the
repo, so no caller depends on the old signature.

Co-authored-by: Claude <noreply@anthropic.com>
dhruvbatra added a commit that referenced this pull request Aug 25, 2026
…| type[ClaimVerifier] (#297)

_load_class returned Any even though _DEFERRED_IMPORTS only ever names
NavigatorClient or ClaimVerifier, both already imported under
TYPE_CHECKING in this file, and both call sites immediately instantiate
the result. Continuing the same Any/type[Any]-to-concrete-type vein as
PRs #290-#296.


Claude-Session: https://claude.ai/code/session_01UNp1qd4G67kz4mMxt5Z6u3

Co-authored-by: Claude <noreply@anthropic.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.

2 participants