Skip to content

ref(typing)!: enforce strict mypy across Sentry's Python codebase - #129447

Draft
joshuarli wants to merge 104 commits into
getsentry:masterfrom
joshuarli:josh/typing-strict-combined
Draft

joshuarli wants to merge 104 commits into
getsentry:masterfrom
joshuarli:josh/typing-strict-combined

Conversation

@joshuarli

Copy link
Copy Markdown
Member

Every first-party Python module now passes mypy --strict, so [tool.mypy] sets strict = true and the weaklist, module allowlist, and strict-error ratchet that tracked the gap are gone.

This combines #129431 (weaklist removal) and #129444 (strict mode, stacked on the first). The branch head is #129444's head, so it contains both change sets: 103 commits, 2,661 files. #129431 and #129444 can be closed in favor of this PR.

Result

Measurement Before (master) After
disallow_untyped_defs weaklist entries 1,776 (1,480 exact, 296 wildcard) 0
disable_error_code module allowlist 2 0
mypy --strict errors 1,411 in 712 files, measured after the weaklist was removed 0

Per-code strict errors removed: no-any-return 1,065, no-untyped-call 271, typeddict-item 29, untyped-decorator 26, extra_checks / misc (subclassing Any) 20.

Deleted: check_strict_typing.py, strict_baseline.json, their pre-commit hook, and the gate unit tests. check_stronglist accepts a pyproject with no weaklist or allowlist. check_typing_campaign checks a whole-branch diff against the base revision's weaklist, while --status / --goal use the working tree.

No error was suppressed and no module was opted out. No net-new # type: ignore; the ones that remain are pre-existing and carry an error code.

How the errors were removed

Each fix reads the value where it enters first-party code:

  • HTTP clients: self.client.* responses are declared as Response / HttpResponseBase; shared integration clients return BaseApiResponse instead of object.
  • Decoded JSON: json.loads / orjson.loads results are checked for object or array shape before use, or cast to a named TypedDict at the boundary.
  • Caches: values read from Redis / Django cache are validated or cast to the type the writer stores.
  • Framework seams: DRF validators, Django model metadata, and serializer spreads are read as the type their callers receive.
  • Provider SDKs: local stubs for sentry_relay, selenium, slack_sdk, google.cloud, google.api_core, emmett55, and django_zero_downtime_migrations, plus typed adapters for datadog, google, and drf-spectacular.
  • Typed helpers: get_cached returns BaseApiResponse; callers that need structured payloads use get_mapping / get_sequence / get_cached_mapping / get_cached_sequence.

snuba_sdk.* keeps follow_imports = "skip"; following it surfaces 963 errors, so it stays a tracked exception.

Contracts worth reviewing

  • Event.group remains Group | None; callers and tests narrow with assertions.
  • BoundedIntegerField, CharField, UUIDField, and WrappingU32IntegerField are generic over set/get types. Nullable declarations must name an optional get type, e.g. BoundedBigIntegerField[int | None, int | None](null=True). Nullable file and blob sizes declare optional get types; download responses keep their existing header behavior.
  • snuba.query overloads distinguish the totals=True tuple from the scalar result. Result keys are str | int because grouped columns keep their runtime type; tagstore narrows tag keys to strings at its boundary.
  • RuleCallback is a named-parameter protocol, so rule futures accept keyword futures.
  • Check-in helpers take partial TypedDict overrides; monitor audit writes use explicit calls.
  • Group-snooze rate paths assert their configured thresholds before use.
  • IntegrationIssueConfigField keeps Any only for arbitrary JSON form defaults and choices so the OpenAPI schema stays resolvable; client key choices are emitted as arrays.
  • The apidocs failure fixture builds an intentionally untyped serializer dynamically instead of a weaklist entry or ignore.

Integrated behavior fixes

Runtime changes that typing surfaced:

OpenAPI

Schema generation needed fixes for arbitrary-JSON integration form fields, client key choices (now arrays), nullable environment facet names, and nullable release file sizes. The optional prod-vs-dev schema diff still reports annotation-driven changes and should be reviewed separately.

Verification

Carried over from the two source PRs; this branch's head is #129444's head (cdb948aa6c9) and was not re-run for this PR.

Known red checks

  • pre-commit lint fails with Argument list too long because a 2,661-file diff is fed to prek run --files; it passes locally in batches.
  • migration drift reports raw-SQL indexes from 1234_grouprelease_new_release_id_indexes missing from state; pre-existing on master, and this branch has no migration changes.
  • The prod-vs-dev OpenAPI schema diff differs by design (see above).
  • CodeQL re-reports pre-existing alerts in touched files because of the diff size.
  • getsentry dispatch needs a same-repo token and Vercel needs deploy authorization; neither runs for fork PRs.

getsentry

getsentry/getsentry#22868 parameterizes the remaining nullable bounded fields and aligns the affected billing, audit, export, and Vercel contracts. Without it the downstream getsentry backend typing job reports 68 errors in 31 files against the sentry side (master baseline: 4); with it, only the four baseline errors remain. It needs this PR's merge SHA to typecheck.

There are no migrations, so no migration-lock ordering applies.

Rebuild the accepted typing campaign on current master as an
annotation-only change. Excludes the campaign evaluator, its policy
modules, the campaign log, and owner-owned baseline edits. Behavior
changes are carved into separate PRs; this commit preserves runtime
behavior, including ReleaseFile.save positional compatibility and the
release serializer's existing stats and newGroups output.

Fix two refactor regressions: starred group search views keep their
createdBy/starred/stars fields, and Seer commit lookups stay keyed by
primary key instead of sha.

Allow the weaklist guard to accept narrowing a wildcard into named
modules, which is how the flags endpoints wildcard graduates.
OpenAPI schema generation raised UnableToProceedError for object-valued
mappings and abstract Mapping/Sequence annotations introduced by the
typing pass. Resolve object as a generic object and treat Mapping and
Sequence like dict and list.
BasePaginator.build_queryset assigned the reversed ordering to queryset.query.order_b, a typo for order_by, when the queryset was already ordered by the descending key and a previous-page cursor needed ascending order. The assignment was silently dropped, so the previous page was filtered as if ascending but still sorted descending, returning the wrong rows. The ordering is now written to order_by, matching the sibling branch.
Exercise the branch that reverses an explicitly descending queryset for
a previous-page cursor. Without the order_by fix, the previous page
returns the current row instead of the preceding page.
Postponed annotations hide NotRequired from TypedDict runtime
introspection, so ActorSerializerResponse.email was emitted as required
and rejected the team-owner examples. Remove the postponed annotations
from that module.

Resolve object-valued mappings as unconstrained values instead of
objects; the examples contain arrays and strings in those maps.
The release archive manifest and artifact index are user-uploaded JSON.
Validate that the decoded value is an object before treating it as one,
so malformed input fails at the boundary instead of raising
AttributeError deeper in the archive reader.

Review split: getsentry#129432.
Bring the reviewed behavior changes into the canonical campaign branch so
the typing diff reflects the final contracts: release serializer defaults,
Teams missing-id failure, blank origin preservation, Jira query
preconditions, and the paginator previous-page fix.
The manifest root is validated, but `files` still needs an object shape
before it is counted and indexed. Reject non-object values at the archive
boundary and cover both malformed shapes.

Review split: getsentry#129432.
Reject non-object thresholds, unknown or non-integer maximum keys, and
non-string units at write time so the serializer casts are backed by a
validated invariant.

Review split: getsentry#129434.
Manifest headers come from uploaded archives. Treat any non-string
debug-id value as absent instead of passing it into symbolic, and drop
the casts that assumed the header value was always a string.

Review split: getsentry#129435.
Gate new Any usage in graduated modules and require a
`# justification:` comment for casts that read JSON, external
responses, metadata, or config. The hook checks staged changes locally;
`--base` and `--status` support branch-wide checks and campaign metrics.

Run the hook against the stage and add tests for graduated/weaklisted
modules, trust-boundary casts, and local casts.
Compare Any tokens against the file's previous revision instead of
flagging every modified line that still contains an existing Any. Limit
the whole-branch mode to Python files under src/.
Use a validating sourcemap-header helper instead of casting arbitrary
header values, and document the remaining provider-response casts with
explicit justifications so the campaign gate enforces the boundary.
Annotate the remaining functions in these modules and remove their
weaklist entries. Full mypy passes with the stricter config, so these
modules are now enforced rather than merely annotated.
getsentry deployments extend the admin broadcast response with plans,
roles, trial status, region, platform, product, early-adopter, and
organization filters. Model them as optional fields on the shared
response so getsentry code stays type-compatible without a companion
change.
Audit the full weaklist against a no-weaklist mypy run. These 399
modules and 12 wildcard families already pass the stricter config, so
remove their exemptions and keep the enforcement they already satisfy.
`--goal` reports the remaining weaklist count and exits nonzero until
it reaches zero, making the campaign target explicit and measurable.
Annotate the transfer payload and artifact download return types and
remove the two weaklist exemptions. Full mypy and the endpoint tests
pass.
Track the two burn-down shapes separately so wildcard narrowing and
exact-module graduation are both visible.
Type the broadcast serializer/validator selection and API key serializer
and endpoint parameters, then remove both weaklist exemptions. Full mypy
and the endpoint tests pass.
Type the alert payload, notification responses, and metric-alert status
lookup, then remove the weaklist exemption. Full mypy and the opsgenie
tests pass.
Add precise annotations so each module passes strict mypy without its weaklist override. The uptime consumer tuple annotation follows the now-typed redis client.
Annotate 25 lane 1 modules so they type-check without the
disallow_untyped_defs override. Leave EmptyListField weaklisted: its
empty-string passthrough widens the return beyond ListField's contract,
which would need Any or an override ignore to express.
Integrate the three lane batches and remove their weaklist entries.
Fix the dashboard permission user-id widening and the Seer detected-issue
mapping variance exposed by the stricter signatures.
Return an empty list for blank input so the field satisfies the DRF
list contract; allowed-domain validation already treats empty input as
invalid, so endpoint behavior is unchanged.
Annotate revision, version, and docker helpers and remove the package
weaklist exemption. Full mypy passes.
Annotate the legacy culprit generators and remove the weaklist
exemption. Full mypy and the culprit tests pass.
Replace the new Any usages introduced by the lane batches with precise
contracts, and allow justified Any at dynamic boundaries in the gate.
Annotate the lane 6 modules so their disallow_untyped_defs weaklist entries can be removed. Widen two helper parameter annotations the new typed call sites exercise.
Drop the implicit untyped-def allowance these test modules relied on so
they type-check under the strict config. Optional model fields are
narrowed where the test fixtures guarantee a value, and untyped mock or
serializer boundaries are narrowed at the point they are read.
Remove the remaining untyped-def and lax-config gaps from the integrations
and plugins test modules so they hold under the strict config.

Event callbacks now receive a real GroupEvent built from the stored event,
installations are narrowed to their concrete provider type instead of the
base IntegrationInstallation, and organization arguments are serialized at
the RPC boundary the provider methods declare.
Type the seer, workflow engine, model, utility, and monitor test
modules so they pass with the disallow_untyped_defs weaklist removed.
Nullability is narrowed with asserts on the relation rather than on the
`*_id` accessor, which django-stubs types as non-null for nullable
foreign keys.
Remove the disallow_untyped_defs weaklist entries for the assigned test files by
annotating their functions and fixtures.

Optional Django fields that mypy types as non-optional (FlexibleForeignKey,
HybridCloudForeignKey, bitfields, nullable TextField) are narrowed with
truthiness checks instead of `is None`, preserving runtime behavior. Test
harness, RPC, and decoded-JSON boundaries use casts with justifications, and
colliding local helpers were renamed: create_auth_provider to
create_fly_auth_provider and event to make_event.

Two helper contracts tightened: the project index token helper now returns the
str token, and the artifact bundle upload helper requires a release string.
Remove the remaining `disallow_untyped_defs` weaklist entries owned by
this lane, narrowing decoded JSON and framework values at their
boundaries instead of leaving unchecked `Any`.

Also repair test harness regressions from earlier typing lanes that
prevented collection and teardown: lazy model imports in pytest
plugins/conftest, self forwarding in the commit-hook override, and the
runtime-safe generic base for the GitLab search endpoint. Widen
`shortId` to reflect that a group may not have a qualified short id.
All source and test modules now pass strict mypy, so the campaign's
disallow_untyped_defs weaklist and the remaining disable_error_code
allowlist are removed. The stronglist check accepts absent lists, and the
campaign gate compares a whole-branch diff against the base revision's
weaklist instead of treating every changed module as newly graduated.

Test helpers now type the elevated-mode request, the stored performance
event, and the nullable metric and uptime inputs without changing runtime
behavior.
Integration form defaults and choices are arbitrary JSON, so they keep
the Any contract the schema generator can render. Client key choices are
emitted as lists, which matches the documented array schema and avoids
the tuple type drf-spectacular cannot resolve.
The backend typing job regenerates the module ignore block and fails on
any resulting diff. With no ignored modules the generator now removes the
block instead of appending an empty override.
Several annotated contracts were narrower than the values production
actually carries, and a few checks changed semantics while becoming
type-checkable. Restore the runtime behavior:

- nullable file and blob sizes keep their documented None state, and
  Content-Length still reflects it; group.times_seen again treats only
  None as unset so reprocessing can seed a zero count
- Snuba filter values and result keys keep their real types, with tag
  keys narrowed at the tagstore boundary
- monitor environment routing consumes the URL environment parameter
  again; GitLab allows a missing installation id; GitHub Enterprise
  check_file returns the API response it always did
- missing samples return None again, dropped orderbys stay None, and
  runs without a state id are serialized instead of asserted away
Full strict mode still reports 1,348 errors in 694 files, mostly Any
values returned across dynamic boundaries. Record those per file and
error code, and fail changed files that add a strict error or leave a
stale baseline entry. This makes the remaining gap explicit and
monotonic instead of invisible.

Also enable disallow_incomplete_defs, remove three stale third-party
import suppressions, and type the cache-key callables and role
decorator that were losing their signatures at call sites.
Mypy strict can only check values that cross a typed boundary. Relay,
selenium, slack_sdk, google.cloud, drf-spectacular and datadog all return
Any, so callers either leaked Any or suppressed the diagnostic.

Add local stubs for the surface we use and typed adapters where the library
cannot be annotated, so the strict pass can see these values.
The client returned Any from every request, so integration call sites could
not be checked and silently accepted any JSON shape. Give the request
methods their real response type and add mapping/sequence accessors that
validate the payload shape at the API boundary.

Clients that parse a documented response narrow it locally instead of
indexing Any.
Settings, caches, decoded JSON, and dashboard payloads all hand back Any.
Read the expected type at the boundary instead of passing Any through, so
the strict pass can follow these values into their callers.

Where the runtime value is genuinely dynamic, cast at the boundary with the
reason recorded next to it.
Settings, options, JSON fields, and untyped SDKs all hand back Any. Read
the expected type where the value crosses into first-party code instead of
letting Any flow into typed returns.

Where the runtime value is genuinely dynamic, the cast says which contract
it satisfies and why.
JSON fields, Redis replies, third-party SDKs, and DRF validate hooks all
return Any. Check the shape where the value enters first-party code and
keep the narrowed type, so callers no longer have to trust an implicit
contract.
Provider SDKs, caches, and profile samples hand back decoded values with no
usable annotation. Check their shape where first-party code consumes them,
and keep the cast reason next to the few genuinely dynamic reads.
DRF validators, Django model metadata, request-response clients, and
provider SDKs return Any at the framework seam. Check the value where it
enters first-party code, so the declared return type is the type callers
actually receive.
Tests, fixtures, and the last production modules returned values that
their declared types could not prove: DRF and Django client responses,
cached payloads, decoded JSON, mock call arguments, and SDK objects.
Read each of those at the boundary that produced it so the declaration
reflects what callers actually receive.

Add stubs for the google.api_core surface the code uses and keep the
Bigtable table stub precise, with the node-store fake casting only its
own duck-typed rows.
Every first-party module now type-checks under strict mode, so the
project config can require it directly instead of relying on a
per-file error baseline. The ratchet, its baseline, and its gate have
nothing left to track.

BREAKING CHANGE: mypy runs with strict = true for all first-party code.
Reading values at their trust boundaries must keep what the boundary
actually returned. Several validations were stricter than the data:
session counts arrive as JSON numbers, the legacy indexer cache stores
raw integers, unenrolled SMS interfaces have a null phone number, and
workflow configs are arbitrary JSON. Accept those shapes again, keep
Jira's issue type metadata a mapping, and let the GitHub Enterprise
installation id stay an integer formatted into URLs.

Revert the seer chat suggestion helper to its nullable result and
register the GitLab note response as the empty JSON object the client
parses.
@github-actions github-actions Bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

The changed-file list was passed to the prek step through an environment
variable. Past 128 KiB a single env string makes every exec fail with
E2BIG, which xargs batching cannot avoid. A 2,661-file PR produces a
137 KB list. Read the list from the pull request files API (or git for
pushes) and pipe it straight into xargs.

try:
# grab the filter out of the brackets of the string that looks
# like so: members[value eq "123124"]
regex_search = re.search(r"\[(.*?)\]", operation["path"])
regex_search = re.search(r"\[(.*?)\]", cast(str, operation["path"]))

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants