Skip to content

ci: run pyright, and fix the config noise it surfaced - #19

Merged
danieyal merged 1 commit into
masterfrom
chore/pyright-in-ci
Jul 22, 2026
Merged

ci: run pyright, and fix the config noise it surfaced#19
danieyal merged 1 commit into
masterfrom
chore/pyright-in-ci

Conversation

@danieyal

@danieyal danieyal commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Adds Pyright to the lint job, now that the error count is at zero.

Why it earns its place

Not a hypothetical. Reintroducing the exact bug #16 fixed — a module-level del of a name that's unbound when a constant tuple is empty, which raises NameError at import time and breaks every import of the package:

mypy    -> Success: no issues found in 47 source files
pyright -> error: "_extra_name" is possibly unbound   (exit 1)

I verified the gate fails, not merely that it passes. A gate that can't fail is worthless — a recurring theme this session.

How it's set up

  • Pinned via uv.lock, deliberately. Pyright ships new checks frequently; an unpinned upgrade would fail unrelated PRs with findings nobody introduced.
  • pyright[nodejs] so the node runtime comes from a wheel instead of being downloaded on first run. CI stays hermetic. Runs in ~3s.
  • Scope from pyrightconfig.json (src, tests, scripts) — so CI and the editor can't drift apart, which is why the config was checked in to begin with.

Config problems it surfaced immediately

Running Pyright as a gate exposed two things I got wrong in #17:

  1. The "//" comment keys were reported as unrecognized settings on every run. pyrightconfig.json accepts real comments, so they're now real comments.
  2. Specifying exclude replaces Pyright's defaults rather than extending them — so **/node_modules and **/.* were being analysed. Restored explicitly.

Neither was visible while Pyright was only run ad hoc, which is a small argument for the gate on its own.

Checks

  • Dev dependency does not reach the shipped wheel — no pyright or node entries
  • uv.lock in sync for UV_LOCKED=1
  • Full lint job green locally: ruff, ruff format, mypy, pyright
  • 405 tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added Pyright static type checking to the continuous integration workflow alongside mypy.
    • Improved type-checking configuration for more reliable virtual environment discovery and broader exclusion of generated or system files.

Adds pyright to the lint job. It catches things mypy does not, and the case
that motivated it is concrete: reintroducing the bug #16 fixed -- a
module-level `del` of a name that is unbound when a constant tuple is empty,
which would raise NameError at import time and break every import of the
package -- gives

    mypy    -> Success: no issues found in 47 source files
    pyright -> error: "_extra_name" is possibly unbound   (exit 1)

Verified the gate fails, not just that it passes.

Pinned via uv.lock, deliberately. Pyright ships new checks frequently, and an
unpinned upgrade would fail unrelated PRs with findings nobody introduced.
Installed as pyright[nodejs] so the node runtime comes from a wheel rather
than being downloaded on first run, keeping CI hermetic. Runs in ~3s.

Scope comes from pyrightconfig.json (src, tests, scripts), so CI and the
editor cannot drift apart -- the whole reason the config was checked in.

Also fixes two config problems that only became visible once pyright ran as a
gate:
- The "//" comment keys added in #17 were reported as unrecognized settings on
  every run. pyrightconfig.json accepts real comments, so they are now real
  comments.
- Specifying `exclude` replaces Pyright's defaults rather than extending them,
  so **/node_modules and **/.* were being analysed. Restored explicitly.

Verified the dev dependency does not reach the shipped wheel (no pyright or
node entries), uv.lock stays in sync for UV_LOCKED=1, and the full lint job
passes locally: ruff, ruff format, mypy, pyright all clean.

405 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Pyright is added to the development dependency group, configured to use the project virtual environment with explicit exclusions, and executed in the CI lint job after mypy.

Changes

Pyright lint integration

Layer / File(s) Summary
Configure and run Pyright
pyproject.toml, pyrightconfig.json, .github/workflows/ci.yml
Adds the Pyright dependency, updates virtual environment and exclusion settings, and runs uv run pyright in CI after mypy.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding Pyright to CI and addressing the config issues it exposed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/pyright-in-ci

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Biome (2.5.3)
pyrightconfig.json

File contains syntax errors that prevent linting: Line 2: Expected a property but instead found '// Pyright/Pylance config. Without venvPath/venv, a fresh clone reports ~78'.; Line 6: End of file expected; Line 6: End of file expected; Line 6: End of file expected; Line 6: End of file expected; Line 7: End of file expected; Line 7: End of file expected; Line 7: End of file expected; Line 7: End of file expected; Line 8: End of file expected; Line 8: End of file expected; Line 8: End of file expected; Line 8: End of file expected; Line 11: End of file expected; Line 11: End of file expected; Line 11: End of file expected; Line 11: End of file expected; Line 12: End of file expected; Line 12: End of file expected; Line 12: End of file expected; Line 12: End of file expected; Line 13: End of file expected; Line 13: End of file expected; Line 13: End of file expected; Line 13: End of file expected; Line 14: End of file expected; Line 14: End of file expected; Line 14: End of file expected; Line 15: End of file expected


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 61-62: Update the CI dependency installation or sync step
preceding the pyright step to run with UV_LOCKED set to "1", preferably at the
job level so all relevant uv commands inherit it. Keep the existing pyright
invocation unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 318237c7-5f25-4a06-87b6-a31af05bec09

📥 Commits

Reviewing files that changed from the base of the PR and between 11dfac7 and a9b63de.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • pyproject.toml
  • pyrightconfig.json

Comment thread .github/workflows/ci.yml
@danieyal
danieyal merged commit 48840ed into master Jul 22, 2026
7 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 24, 2026
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.

1 participant