chore(deps): upgrade mypy to 2.1.0#58446
Merged
Merged
Conversation
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
✅ Hobby deploy smoke test: PASSEDHobby deployment smoke test passed successfully. |
…rade # Conflicts: # uv.lock
Contributor
|
Size Change: +6.81 kB (+0.01%) Total Size: 118 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
.github/workflows/ci-python.yml:158-159
The PR description states `MYPY_NUM_WORKERS=4` is used in CI and the performance table benchmarks against that value, but the actual setting here is `2`. If the intent is to limit to 2 workers to match the 2-core `depot-ubuntu-latest` runner, the description and performance table are misleading as written.
```suggestion
env:
MYPY_NUM_WORKERS: 4
```
### Issue 2 of 2
posthog/batch_exports/tests/debug/test_debugger.py:166-167
**`comparison-overlap` + `unreachable` silenced in a live assertion**
The same assertion pattern `assert bedbg.loaded_batch_exports == loaded` on lines 154 and 160 (destination-filtered calls) passes mypy without ignores, but here — on the name-filtered call — mypy flags a `comparison-overlap`, meaning it believes the two types can never be equal. That would make the `assert` always raise `AssertionError`, explaining why the following line is `unreachable`. Silencing both errors hides a potential real type mismatch: if `load_batch_exports(name=...)` returns a type genuinely incompatible with `loaded_batch_exports`, the test may be asserting something that mypy knows can never pass.
Reviews (1): Last reviewed commit: "Merge branch 'master' into worktree-mypy..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+166
to
+167
| assert bedbg.loaded_batch_exports == loaded # type: ignore[comparison-overlap] | ||
| assert all(batch_export.name == "test-batch-export-S3-False-False" for batch_export in loaded) # type: ignore[unreachable] |
Contributor
There was a problem hiding this comment.
comparison-overlap + unreachable silenced in a live assertion
The same assertion pattern assert bedbg.loaded_batch_exports == loaded on lines 154 and 160 (destination-filtered calls) passes mypy without ignores, but here — on the name-filtered call — mypy flags a comparison-overlap, meaning it believes the two types can never be equal. That would make the assert always raise AssertionError, explaining why the following line is unreachable. Silencing both errors hides a potential real type mismatch: if load_batch_exports(name=...) returns a type genuinely incompatible with loaded_batch_exports, the test may be asserting something that mypy knows can never pass.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/batch_exports/tests/debug/test_debugger.py
Line: 166-167
Comment:
**`comparison-overlap` + `unreachable` silenced in a live assertion**
The same assertion pattern `assert bedbg.loaded_batch_exports == loaded` on lines 154 and 160 (destination-filtered calls) passes mypy without ignores, but here — on the name-filtered call — mypy flags a `comparison-overlap`, meaning it believes the two types can never be equal. That would make the `assert` always raise `AssertionError`, explaining why the following line is `unreachable`. Silencing both errors hides a potential real type mismatch: if `load_batch_exports(name=...)` returns a type genuinely incompatible with `loaded_batch_exports`, the test may be asserting something that mypy knows can never pass.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Mypy 2.x adds parallel type checking via
--num-workers N(2.0 release notes); this PR moves us off 1.x so CI and local dev can use it.Performance
Local timings on the same machine (M-series, full 8169-file project):
MYPY_NUM_WORKERS=2)The win is on cold cache, which is what local devs hit on every branch switch and what CI hits whenever
pyproject.tomlchanges.Warm-cache is flat because mypy 1.x already had little left to do; the workers add small startup overhead that nets out near zero.
Reference master CI baseline for the same step over recent successful runs:
Check static typingaverages ~93 s warm-cache ondepot-ubuntu-latest.Branch CI required busting the
.mypy_cachecache key (added amypy2-segment) — mypy 2 reading a 1.x cache hangs the runner.Fresh branch CI timing will populate this row once a green run lands; the local cold-cache speedup is the closest proxy until then.
Changes
mypy~=2.1.0inpyproject.tomland enable parallel workers in the CI mypy step viaMYPY_NUM_WORKERS=2mypy2-segment) so the cache restore-key fallback doesn't hand mypy 2 a stale 1.x cachecast(...)where 1.x's decorator-aware narrowing was implicit (e.g.request.userafter@session_auth_required)bytes(...)wrappers around DjangoBinaryFieldreads under PEP 688 /--strict-bytes# type: ignore[unreachable]/[comparison-overlap]annotations on lines where 2.x's narrowing is more aggressive than 1.x'sRuntime impact: none
This PR is purely type-check level. Every call-site audited:
typing.cast(T, x)— identity function at runtime (def cast(typ, val): return val). Cannot raise.bytes(...)wraps — five sites, all on DjangoBinaryFieldvalues. PostHog runs psycopg3, which already returnsbytesforbytea, sobytes(bytes)is a defensive no-op. Theoretical psycopg2memoryviewcase is strictly safer (e.g.parse_contentswould actually decode where master silently returnedNone).bytes(...)sites are either guarded by aNone-check, wrapped in a pre-existingtry/except, or paired with an explicit early-return on falsy input.No business logic, control flow, or exception handling changes.
How did you test this code?
I'm an agent. Verification I actually ran:
mypy --cache-fine-grained .on the merged tree — clean (8169 source files, 0 errors)ruff checkandruff format --checkon the changed files — cleanuv lockre-resolves cleanly with the overridebytes()andcast()site against caller flow and field nullabilityNo manual product testing. Django and product test suites need to run in CI.
Publish to changelog?
no
Docs update
n/a
🤖 Agent context
Claude Code!