Skip to content

chore: pyright strict mode - #21

Merged
danieyal merged 1 commit into
masterfrom
chore/pyright-strict
Jul 24, 2026
Merged

chore: pyright strict mode#21
danieyal merged 1 commit into
masterfrom
chore/pyright-strict

Conversation

@danieyal

@danieyal danieyal commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Step 3, the last of the plan. typeCheckingMode: strict, and Pyright now runs strict in CI reporting 0.

Approach

Strict raw is 175 errors here, ~90% noise for this codebase's idioms. So: strict minus ten whole-rule categories, each a documented exception rather than an inline suppression repeated dozens of times. Turning a category off where every instance is noise keeps strict's genuine checks visible — the opposite of littering the code with # type: ignore.

Disabled categories, with why:

Rule(s) Why it's noise here
reportUnknown{Variable,Argument,Member,Parameter,Lambda}Type lxml-stubs + Pydantic internals surface ~90 Unknowns, nothing actionable
reportPrivateUsage UBL models share a deliberate cross-module internal API (_leaf, _UblModel, …)
reportUnnecessaryIsInstance Pydantic mode="before" validators really do get Any at runtime
reportMissingParameterType annotating every test lambda is noise; src is mypy --strict anyway

The 8 real findings strict surfaced — fixed, not suppressed

  • services/models.py — removed _as_items. Genuinely dead: defined, never called anywhere. A real find strict earned us.
  • ubl/_base.py — added __all__ = ["_UblModel", "_leaf", "_money"]. The unused-symbol flags were a private-name heuristic not seeing cross-module imports; __all__ marks them exported, which is more honest than a suppression.
  • scripts/extract_codes.pyTABLES typed as a TypedDict. Previously spec["out"] inferred as str | list[tuple], which the OUT / spec["out"] path-join rejected.
  • tests/unit/test_auth.py — annotated a local dict[str, object]; an inferred dict[str, str | int] isn't assignable there (dict value types are invariant).
  • ubl/invoice.py, ubl/reference.py — the two is not None serializer guards deferred from refactor: delete validators that could never fire #20. Statically dead but they protect model_construct()-built models from raising during serialization. Suppressed per-line with the rationale in a comment, and verified load-bearing: removing both ignores brings the 2 errors straight back.

Checks

All green: ruff, ruff format, mypy, pyright (strict), 405 tests, lock in sync.

Where this leaves things

reportUnnecessaryComparison and the rest are now enforced at strict in CI. That's the type-checking track done. Outstanding for 1.0 is unchanged and needs you: PyPI publish (go-ahead) and live submission (a CA certificate).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved invoice serialization to safely omit missing invoice type codes in malformed or partially constructed records.
  • Refactor

    • Strengthened static type checking across the project.
    • Added clearer type definitions for code extraction specifications.
    • Clarified and formalized supported UBL model helpers.
  • Tests

    • Improved type annotations in authentication test coverage.

Step 3 of the strict-mode plan. Flips typeCheckingMode to strict with ten
whole-rule exceptions, each disabling a category that is pure noise for this
codebase rather than a suppression of real findings:

- reportUnknown{Variable,Argument,Member,Parameter,Lambda}Type: lxml-stubs and
  Pydantic internals surface many values as Unknown (~90 hits), nothing
  actionable behind them.
- reportPrivateUsage: the UBL models share a deliberate cross-module internal
  API (_leaf, _money, _UblModel, _Base). By design.
- reportUnnecessaryIsInstance: Pydantic mode="before" validators genuinely
  receive Any at runtime, so their isinstance checks are load-bearing.
- reportMissingParameterType: annotating every test helper/lambda is noise;
  src is covered by mypy --strict anyway.

With those off, strict surfaced 8 real findings, fixed individually rather
than blanket-suppressed:

- services/models.py: removed _as_items, genuinely dead -- defined, never
  called anywhere. A real find strict earned.
- ubl/_base.py: added __all__ = ["_UblModel", "_leaf", "_money"]. These are
  used across sibling modules; the reportUnusedFunction/Class flags were a
  private-name heuristic not seeing cross-module imports. __all__ marks them
  exported and is more honest than a suppression.
- scripts/extract_codes.py: TABLES typed as dict[str, _TableSpec] (a
  TypedDict). Previously spec["out"] inferred as str | list[tuple], which the
  `OUT / spec["out"]` path join rejected. The TypedDict gives each key its own
  type.
- tests/unit/test_auth.py: annotated a local dict[str, object]; an inferred
  dict[str, str | int] is not assignable there because dict value types are
  invariant.
- ubl/invoice.py, ubl/reference.py: the two `is not None` serializer guards
  deferred from #20. Statically dead, but they protect model_construct()-built
  models (which bypass validation) from raising during serialization.
  Suppressed per-line with reportUnnecessaryComparison and the rationale in a
  comment. Verified load-bearing: removing both ignores brings the 2 errors
  straight back.

pyright now runs strict in CI (the config drives it) and reports 0. All gates
green: ruff, ruff format, mypy, pyright, 405 tests, lock in sync.

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

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cd69428e-728d-4f3e-a03f-2503c8afc498

📥 Commits

Reviewing files that changed from the base of the PR and between 101e653 and eb2773f.

📒 Files selected for processing (7)
  • pyrightconfig.json
  • scripts/extract_codes.py
  • src/myinvois/services/models.py
  • src/myinvois/ubl/_base.py
  • src/myinvois/ubl/invoice.py
  • src/myinvois/ubl/reference.py
  • tests/unit/test_auth.py
💤 Files with no reviewable changes (1)
  • src/myinvois/services/models.py

📝 Walkthrough

Walkthrough

Pyright is switched to strict mode with selected diagnostics disabled. Table extraction metadata is typed, UBL helpers are explicitly exported, serializer guards are annotated, an unused response helper is removed, and a test input receives an explicit type.

Changes

Typing and Pyright alignment

Layer / File(s) Summary
Strict checking and typed metadata
pyrightconfig.json, scripts/extract_codes.py, tests/unit/test_auth.py
Pyright uses strict mode with targeted diagnostic suppressions; extraction table specifications and an OAuth response dictionary are explicitly typed.
UBL helper exports and serializer guards
src/myinvois/ubl/_base.py, src/myinvois/ubl/invoice.py, src/myinvois/ubl/reference.py
UBL helpers are listed in __all__, and serializer field guards retain conditional emission with Pyright suppressions for constructed-model cases.
Response helper removal
src/myinvois/services/models.py
The private _as_items response conversion helper is deleted.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 matches the main change: switching the project to Pyright strict mode and related type-checking cleanup.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/pyright-strict

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 17: End of file expected; Line 17: End of file expected; Line 17: End of file expected; Line 17: End of file expected; Line 19: End of file expected; Line 19: End of file expected; Line 19: End of file expected;

... [truncated 222 characters] ...

25: End of file expected; Line 25: End of file expected; Line 26: End of file expected; Line 26: End of file expected; Line 26: End of file expected; Line 26: End of file expected; Line 27: End of file expected; Line 27: End of file expected; Line 27: End of file expected; Line 27: End of file expected; Line 28: End of file expected; Line 28: End of file expected; Line 28: End of file expected; Line 28: End of file expected; Line 32: End of file expected; Line 32: End of file expected; Line 32: End of file expected; Line 32: End of file expected; Line 36: End of file expected; Line 36: End of file expected; Line 36: End of file expected; Line 36: End of file expected; Line 40: End of file expected; Line 40: End of file expected; Line 40: End of file expected; Line 41: End of file expected


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.

@danieyal
danieyal merged commit 7878ce1 into master Jul 24, 2026
7 checks passed
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