Skip to content

Conversation

webjunkie
Copy link
Contributor

@webjunkie webjunkie commented Sep 29, 2025

Summary

Introduce ty (Astral's Rust-based type checker) to CI as non-blocking informational check. Runs before mypy for fast feedback.

Approach

  • CI only: Runs in ci-python.yml before mypy (not in lint-staged)
  • Non-blocking: continue-on-error: true → yellow ⚠️ warning, doesn't fail PR
  • Problem matcher: GitHub annotations for inline error display
  • Baseline: ty-baseline.txt filters pre-existing errors (310 errors, 257 warnings)

Django Limitations

ty doesn't support Django metaprogramming (no plugins like mypy). Disabled incompatible rules:

  • unresolved-import, unresolved-attribute, invalid-argument-type, invalid-base

See astral-sh/ty#291

Feedback Needed

Trial run to evaluate ty vs mypy. Report to #team-devex:

  • Usefulness of output
  • False positive rate
  • Real issues caught

Key Files

  • .github/workflows/ci-python.yml - ty check before mypy
  • .github/ty-problem-matcher.json - annotations
  • bin/ty.py - wrapper with baseline filtering
  • ty-baseline.txt - baseline (567 diagnostics)
  • pyproject.toml - ty config (Python 3.12, Django rule ignores)

@webjunkie webjunkie force-pushed the feature/ty-stage-lint branch from 34be4da to 75424e6 Compare September 29, 2025 13:21
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 6 comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

github-actions bot commented Sep 29, 2025

Size Change: 0 B

Total Size: 3.05 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 3.05 MB

compressed-size-action

@webjunkie webjunkie self-assigned this Sep 29, 2025
webjunkie and others added 10 commits October 14, 2025 12:08
Add ty (fast Python type checker) integration with mypy-baseline workflow:

- Add bin/ty.py wrapper script for ty + mypy-baseline integration
- Configure ty in pyproject.toml with appropriate excludes
- Add ty to lint-staged pipeline for automatic checking on staged files
- Add comprehensive documentation in docs/ty-baseline.md
- Block ty sync during alpha phase - only mypy updates baseline

ty serves as fast preflight check (~10-100x faster than mypy) while
mypy remains authoritative type checker for CI and baseline updates.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Changes:
- Add opt-in check via git config posthog.enableTy
- Only runs when called from lint-staged hook if opted in
- Manual runs (./bin/ty.py check) work without opt-in
- Create separate ty-baseline.txt (2136 errors) vs mypy-baseline.txt (1287 errors)
- Enable ty sync for baseline updates
- Fix file-level skip bug - now checks all files, filters at line level
- Update documentation explaining opt-in and baseline management

Why opt-in:
- ty finds errors in ~333 files not in mypy baseline
- Prevents blocking all developers unexpectedly
- Allows gradual adoption and validation
- Opted-in users fix errors they introduce, not pre-existing ones

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Replace uvx with 'uv run' for both ty and mypy-baseline
- uvx doesn't respect uv.lock, causing inconsistent tool versions
- Add type annotations and casts to fix ty type checking
- Clean up test artifacts from baseline
- Ignore unresolved-attribute, invalid-argument-type, possibly-unbound-attribute
- ty doesn't support Django's metaprogramming yet (see astral-sh/ty#291)
- Reduces baseline from 2136 to 468 errors (78% reduction)
- Remaining errors are more likely to be real issues
Removed opt-in requirement - ty now runs automatically on every commit
for all developers via lint-staged. The baseline system protects against
pre-existing errors, so developers only fix new issues they introduce.

Changes:
- Remove opt-in check from bin/ty.py (posthog.enableTy config)
- Update docs to reflect mandatory ty runs
- Fix ty rule configuration to use actual supported rules
  - Replace invalid `invalid-base-class` with `invalid-base`
  - Remove unsupported rules that don't exist in ty 0.0.1a21
- Clarify rule organization with better comments
Add ty type checking to CI (before mypy for faster feedback) as a
non-blocking step to collect feedback on its usefulness vs mypy.
The check will not fail CI but will show output for developers to evaluate.

Also sync ty-baseline.txt to reflect current state after latest rule
adjustments (disabled Django-incompatible rules).

Feedback requested in #team-devex channel.
Remove ty from lint-staged to allow proper CI trial evaluation - developers
were being blocked locally which defeats the purpose of an informational CI run.

Add ty problem matcher for GitHub Actions to display type errors as annotations
in the CI interface, making it easier to review ty's output.

ty now runs:
- In CI only (before mypy for faster feedback, non-blocking)
- With problem matcher annotations for better visibility

Feedback collection in #team-devex channel.
@webjunkie webjunkie force-pushed the feature/ty-stage-lint branch from def310b to 031f853 Compare October 14, 2025 10:21
@webjunkie
Copy link
Contributor Author

ty Baseline Analysis

Quick breakdown of the 567 diagnostics in the baseline:

Easy Fixes (~143, 25%)

  • 91 redundant-cast - Remove unnecessary type casts (e.g., cast(LicenseManager, x) when x is already that type)
  • 52 deprecated - Replace utcfromtimestamp with timezone-aware alternatives

Simple Safety Checks (~130, 23%)

  • 96 possibly-unbound-attribute - Add null checks (e.g., if backup: backup.shard)
  • 34 unknown-argument - Kwargs/argument issues

Real Type Issues (~294, 52%)

  • 119 missing-argument - Calling functions without required parameters
  • 28 invalid-assignment - Assigning wrong types (e.g., None to DateRange)
  • 32 too-many-positional-arguments - Function call arity mismatches
  • 22 invalid-return-type - Return type mismatches
  • Plus others (unresolved-reference, invalid-type-form, etc.)

Signal/noise ratio looks good for an alpha checker - about half are real bugs worth fixing, the rest are code quality improvements or trivial cleanups.

Keep test type error in posthog/api/query.py and remove from baseline
to demonstrate ty catches real type issues in CI. This will validate:
- ty problem matcher displays errors correctly
- Baseline filtering works as expected
- CI shows visible warnings for new type errors
- Change problem matcher severity to 'warning' to avoid check failures
- Fix string matching to use normalized output in both check and sync functions
- Add emoji indicators to CI output for better visibility

Problem matcher already correctly extracts file, line, column, severity, and code.
Addresses Greptile comments about string matching.
Update ty version from 0.0.1a21 to 0.0.1a22.

Revise ty-baseline.md to accurately describe current integration:
- ty runs in CI only (not lint-staged) as informational check
- Non-blocking warnings to evaluate usefulness vs mypy
- Uses GitHub problem matcher for inline PR annotations
- Clarifies 567 baseline errors are filtered out
- Adds baseline content breakdown (50% bugs, 25% safety, 25% cleanup)
- Removes mypy-baseline instructions (wrong doc for that)
Baseline reduced from 567 to 502 diagnostics with ty v0.0.1a22.
Excluded posthog/api/query.py test error to validate ty catches it in CI.

Updated uv.lock for ty version bump.
@webjunkie webjunkie changed the title feat: Add ty type checker as fast preflight check in lint-staged feat: Add ty type checker Oct 14, 2025
@PostHog PostHog deleted a comment from posthog-bot Oct 14, 2025
Copy link
Contributor

@joshsny joshsny left a comment

Choose a reason for hiding this comment

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

Definitely worth trying - if we end up replacing mypy and we're able to move this to a pre-commit check that'd be amazing for devex 🚀

@posthog-bot posthog-bot removed the stale label Oct 15, 2025
Clean up intentional type error used for testing ty integration.
@webjunkie webjunkie enabled auto-merge (squash) October 15, 2025 08:57
@webjunkie webjunkie merged commit 411abe0 into master Oct 15, 2025
230 of 234 checks passed
@webjunkie webjunkie deleted the feature/ty-stage-lint branch October 15, 2025 09:43
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.

3 participants