Skip to content

Add pyright and pyrefly type checkers - #180

Merged
thibaudcolas merged 2 commits into
mainfrom
python-type-checkers
Sep 15, 2026
Merged

thibaudcolas merged 2 commits into
mainfrom
python-type-checkers

Conversation

@thibaudcolas

Copy link
Copy Markdown
Member

Description

All Python code is now type-checked by four checkers — mypy, ty, pyright, and pyrefly — and all four must pass in just lint and CI. They implement the Python typing spec independently, so they disagree in useful ways: each caught real issues the others missed (see below), and staying compatible with all of them keeps the codebase's typing idiomatic rather than checker-specific.

Checker setups

Each checker got the smallest setup that adds distinct value, with a consistent policy: annotations required on production code, tests exempt from annotation requirements but still checked.

  • mypy 2.3.1 (unchanged version, tightened config in [tool.mypy]):
    • Scope widened from draftjs_exporter tests to . (exclude_gitignore = true keeps generated site/ out), matching the other checkers.
    • New: extra_checks, strict_equality_for_none, warn_unused_configs, and the error codes ignore-without-code, deprecated, possibly-undefined, truthy-bool, redundant-expr, redundant-self, mutable-override, unimported-reveal.
    • Deliberately not enabled: explicit-override and ty's missing-override-decorator — both require typing_extensions as a new runtime dependency for Python 3.10 support (TODO in config); warn_redundant_casts stays off because ty requires casts mypy considers redundant; --native-parser is ~17% faster but too new for a CI gate.
  • ty 0.0.80 (lockfile upgraded from 0.0.78): note that ty warnings do not fail CI by default. Enabled four ignore-by-default soundness rules as errors in [tool.ty.rules]: unsound-assignment, unsound-return-statement, unsound-yield, disjoint-cast.
  • pyright 1.1.414 (new, [tool.pyright]): standard mode (its strict mode is unusable here — 14k+ errors from the intentional Element = Any DOM abstraction), with reportTypedDictNotRequiredAccess relaxed for tests/ only via an execution environment.
  • pyrefly 1.3.0 (new, [tool.pyrefly]): configured to mirror mypy's annotation requirements (implicit-any, unannotated-return), plus not-required-key-access (matching pyright), treat-all-caps-as-final, unused-ignore, and direct-abstract-base-instantiation; a tests/** sub-config mirrors mypy's tests exemption.

Add-on QA tooling

  • just verify-types runs pyright --verifytypes to report the type completeness of the public API — currently 80.9%. The ceiling is intentional: Element is a TypeAlias of Any so all DOM engines can be used interchangeably, so this is informational (CI runs it with continue-on-error).
  • Considered and skipped: typeguard runtime type checking in tests (slow, false positives with Any-typed engines), pyrefly open-unpacking / redundant-cast (both conflict with casts that ty requires).

Issues found and fixed

Surfaced by pyright:

  • EntityState.render_entities accessed entity_details["type"] / ["data"] on a total=False TypedDict. The Entity TypedDict now requires type, data, and mutability, matching the Draft.js format. Runtime leniency is preserved: entities without mutability still render with a mutability of None (draftjs_exporter/entity_state.py).
  • BeautifulSoup was "possibly unbound" in the html5lib engine — a vestigial 2017-era try/except ImportError from when engines were imported eagerly. Engines load lazily by dotted path these days, so the module now imports bs4 plainly like the lxml engine, and selecting the html5lib engine without BeautifulSoup installed fails with a clear import error rather than a NameError at render time (draftjs_exporter/engines/html5lib.py).
  • Optional-key accesses on MARKDOWN_CONFIG in example.py switched to .get().
  • A test resolver passed untyped functions where list[EntityResolver] was expected (tests/markdown_parser/test_resolvers.py).

Surfaced by ty's new soundness rules:

  • apply_decorators was declared Generator[str, None, None] but also yields DOM elements — now Generator[str | Element, None, None] (draftjs_exporter/composite_decorators.py).
  • An unsound list[Any] → list[str] assignment in the promptfoo grader, now an explicit cast (single capture group, so findall returns strings — docs/prompts/graders/run_snippet.py).
  • A disjoint cast in benchmark.py (markov_draftjs has its own ContentState type declarations), suppressed with a targeted # ty: ignore[disjoint-cast].
  • A wrong Callable[[None], None] annotation on generated tests, now Callable[..., None] (tests/test_exports.py).

Surfaced by pyrefly's annotation checks:

  • Missing -> None return annotations on __init__ in the string and Markdown engines.
  • Inferred-Any attributes (last_child, type, props) in Wrapper, and an untyped empty container in WrapperState (draftjs_exporter/wrapper_state.py), plus an untyped empty container in resolvers.py.

Documentation

  • docs/CONTRIBUTING.md: new "Static typing" section with an explainer of why the project runs four checkers and a table of each checker's role and config location; commands and CI lists updated.
  • AGENTS.md: tools list updated, stale stubs/ directory reference removed.
  • User-facing docs advertise the package as fully typed rather than enumerating dev tooling.
  • CHANGELOG updated, including the Entity TypedDict typing change.

Test evidence

  • just lint — all four checkers, ruff, and import-linter pass.
  • just test — 734 passed.
  • just test-compatibility — 734 passed (Python 3.10, lowest-supported dependency versions).
  • just docs-build — strict build passes.
  • just verify-types — type completeness score 80.9%.

@github-actions

github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 8 issue(s) in this PR.

  • ✅ Successfully posted inline: 4 comment(s)
  • ❌ Failed to post inline: 4 comment(s)

other · medium

📄 .github/workflows/ci.yml (L15-L17)

⚠️ GitHub could not post this as an inline comment: Lines 15-17 could not be resolved (outside PR diff hunks)

The test job lacks a timeout-minutes specification. With expanded type checking scope (mypy .), this job may take longer and risk running indefinitely on errors. Consider adding a reasonable timeout (e.g., 15-20 minutes).

💡 Suggested Change

Before:

  test:
    runs-on: ubuntu-slim
    env:

After:

  test:
    runs-on: ubuntu-slim
    timeout-minutes: 20
    env:

other · low

📄 draftjs_exporter/entity_state.py (L131-L137)

⚠️ GitHub could not post this as an inline comment: Lines 131-137 could not be resolved (outside PR diff hunks)

The Entity type in get_entity_details now always includes the mutability key (explicitly set to None when missing), whereas previously it was absent. This changes the shape of returned entity data for entities without mutability, which could break consumers checking 'mutability' in entity.

💡 Suggested Change

Before:

            props["entity"] = {
                "type": entity_details["type"],
                "mutability": entity_details.get("mutability"),
                "block": block,
                "blocks": blocks,
                "entity_range": {"key": self.completed_entity},
            }

After:

            props["entity"] = {
                "type": entity_details["type"],
                "mutability": entity_details.get("mutability"),  # None if absent
                "block": block,
                "blocks": blocks,
                "entity_range": {"key": self.completed_entity},
            }

maintainability · medium

📄 draftjs_exporter/types.py (L95-L100)

⚠️ GitHub could not post this as an inline comment: Lines 95-100 could not be resolved (outside PR diff hunks)

The Entity TypedDict now requires mutability as a mandatory field (total=True is implicit), but entity_state.py line 133 uses .get("mutability") to gracefully handle missing mutability. The test test_render_entities_data_no_mutability (lines 152-171) also exercises runtime leniency with a cast. This creates a mismatch between the type definition and runtime behavior. Either the Entity TypedDict should use total=False to make all fields optional, or mutability should be the only optional field. The comment on line 130 ("Entities without mutability are tolerated at runtime") supports making it optional.

💡 Suggested Change

Before:

class Entity(TypedDict):
    """Draft.js entity data referenced by entity ranges."""

    type: str
    data: dict[str, Any]
    mutability: Mutability

After:

class Entity(TypedDict, total=False):
    """Draft.js entity data referenced by entity ranges."""

    type: str
    data: dict[str, Any]
    mutability: Mutability

other · medium

📄 draftjs_exporter/types.py (L95-L100)

⚠️ GitHub could not post this as an inline comment: Lines 95-100 could not be resolved (outside PR diff hunks)

The Entity TypedDict defines mutability: Mutability (required, non-None), but the code can produce mutability: None when entities lack the mutability key. This runtime leniency is tested with a cast and disjoint-cast ignore, indicating intentional relaxation. However, the TypedDict should reflect actual usage by declaring mutability: Mutability | None to maintain type safety and eliminate the need for casts.

💡 Suggested Change

Before:

class Entity(TypedDict):
    """Draft.js entity data referenced by entity ranges."""

    type: str
    data: dict[str, Any]
    mutability: Mutability

After:

class Entity(TypedDict):
    """Draft.js entity data referenced by entity ranges."""

    type: str
    data: dict[str, Any]
    mutability: Mutability | None

Comment on lines +240 to 244
block: dict[str, Any] = {
"type": options.type,
"depth": depth,
"data": {},
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maintainability · low
Using Any for the block variable type annotation bypasses the project's own Block TypedDict type. While dict[str, Any] may be technically correct for this synthesized object, using the project's Block type would provide consistency and better type safety.

Suggestion:

Suggested change
block: dict[str, Any] = {
"type": options.type,
"depth": depth,
"data": {},
}
block: Block = {
"type": options.type,
"depth": depth,
"data": {},
}

Comment on lines +156 to +158
entity_map_no_mutability = cast( # ty: ignore[disjoint-cast]
EntityMap, {"2": {"type": "LINK", "data": {"url": "http://test.com"}}}
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

other · low
The # ty: ignore[disjoint-cast] suppression silences pyright error "Type "dict[str, Any]" cannot be assigned to type "EntityMap"" — a false positive since the runtime value is structurally correct. The comment accurately documents why the cast exists.

Comment thread tests/test_exports.py
"""

pr: cProfile.Profile = None # type: ignore
pr: cProfile.Profile = None # type: ignore[assignment] # ty: ignore[invalid-assignment]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style · low
The # ty: prefix is a typo — should be # type:. This appears to be a leftover from combining two type checker ignores. Keep only the correct # type: ignore[assignment, invalid-assignment] form.

Suggestion:

Suggested change
pr: cProfile.Profile = None # type: ignore[assignment] # ty: ignore[invalid-assignment]
pr: cProfile.Profile = None # type: ignore[assignment, invalid-assignment]

Comment thread tests/test_exports.py
"""

pr: cProfile.Profile = None # type: ignore
pr: cProfile.Profile = None # type: ignore[assignment] # ty: ignore[invalid-assignment]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maintainability · medium
The # ty: prefix is a typo — should be # type:. This appears to be a leftover from combining two type checker ignores. Consolidate to a single correct directive: # type: ignore[assignment, invalid-assignment]

Suggestion:

Suggested change
pr: cProfile.Profile = None # type: ignore[assignment] # ty: ignore[invalid-assignment]
pr: cProfile.Profile = None # type: ignore[assignment, invalid-assignment]

@thibaudcolas
thibaudcolas merged commit 0898ecc into main Sep 15, 2026
12 checks passed
@thibaudcolas
thibaudcolas deleted the python-type-checkers branch September 15, 2026 21:04
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