Skip to content

ref(typing): Upgrade mypy to 1.20.1#113419

Draft
JoshFerge wants to merge 2 commits intomasterfrom
joshferge/ref/upgrade-mypy-to-1-20-1
Draft

ref(typing): Upgrade mypy to 1.20.1#113419
JoshFerge wants to merge 2 commits intomasterfrom
joshferge/ref/upgrade-mypy-to-1-20-1

Conversation

@JoshFerge
Copy link
Copy Markdown
Member

@JoshFerge JoshFerge commented Apr 20, 2026

Bumps the mypy pin to 1.20.1 and cleans up the remaining new type-check failures that don't belong in a standalone prep PR.

Prep PRs (merge these first, with the old mypy still active)

All six are draft / 1.19.1-safe / can be reviewed independently:

What's left for this PR after the prep merges

  • Bump `mypy>=1.19.1` → `mypy>=1.20.1` (+ `uv.lock`)
  • Drop the now-stale `kombu.*` entry from `[[tool.mypy.overrides]]`
  • Remove unused `# type: ignore` comments that 1.20 flags
    (`explore_saved_queries.py`, `test_logic.py`)
  • Add `# type: ignore[arg-type]` (with FIXME) in `organization_member_details.py`
    where 1.20 catches a preexisting bug: when `organizations:team-roles`
    is disabled, the code passes `list[tuple[Team, None]]` to a function
    expecting `list[tuple[Team, str]]`
  • Misc src cleanups 1.20 surfaces:
    • `digests/backends/base.py` — options map is `Mapping[str, Any]`
    • `auth/access.from_auth` — narrow `organization_id` through a local
    • `cache/backends/reconnectingmemcache` — `None` guard before `**self._options`
    • `discover/compare_timeseries` — heterogeneous mismatches dict typed as `dict[int, dict[str, Any]]`

Performance

version cold warm
1.19.1 49.22s 17.82s
1.20.1 39.04s 5.73s

CI expectation after prep PRs land

  • `mypy` job: clean (0 errors)
  • `uv` jobs: resolve against `pypi.devinfra.sentry.io` (1.20.1 has been published there)

Bump the mypy pin to 1.20.1 and fix the bulk of new type-check failures.

1.20.1 adds a stricter check for accessing instance-only attributes on a
class object. Several class-config patterns in the codebase (RegressionDetector,
GroupType, AttributeHandler, ReplayGroupTypeDefaults) declared configuration
fields as dataclass instance attributes but only ever accessed them via the
class. Convert those to ClassVar so access through the class is valid and drop
the no-op @DataClass(frozen=True) decorators.

The GroupType category validation that previously lived in __post_init__ (dead
code — no one instantiated GroupType) moves into __init_subclass__ so it still
runs at subclass-definition time.

Also addresses:
- Incorrect GroupStatus annotation where int is passed in practice
  (fetch_alert_threshold, fetch_resolve_threshold, from_workflow_engine_models,
  build_alert_context)
- Redundant casts in search/eap and search/events builders
- Unused kombu.* mypy override
- Unused type: ignore comments
- Mapping type fix in digests/backends/base.py
- None-narrowing fix in auth/access.from_auth
- None guard in reconnectingmemcache

Cuts the full-repo mypy failure count from 134 -> 15.

Agent transcript: https://claudescope.sentry.dev/share/mi26dmBkpSB5zU45vLb9vayuq61XpEbOEAaKoC-2AGQ
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 20, 2026
Fixes the 15 remaining mypy 1.20 test failures so the full-repo run is clean.

Patterns fixed:
- Replace `x.refresh_from_db()` with a fresh `Model.objects.get(id=x.id)`
  assignment in tests where mypy retained narrowing from an earlier assertion
  (e.g. `assert widget.discover_widget_split is None`). refresh_from_db mutates
  in place so mypy never re-widens; an assignment does.
- Guard or cast `Mapping | None` / `object` before `**kwargs` splats.
- Assert `is not None` before dereferencing an Optional parsed MRI.
- Type a heterogeneous debug_meta case list as `list[dict[str, Any]]`.
- Annotate exchange_token test result as `dict[str, Any]` — the declared
  return type is `dict[str, str]` but real OAuth responses include ints.
- Compare notification_context.integration_id against an int, not a str.

Agent transcript: https://claudescope.sentry.dev/share/_Bezr4OWYESAQM3P9KhWPgffIOzp6lSEgTG_mq5brNM
JoshFerge added a commit that referenced this pull request Apr 20, 2026
Both `RegressionDetector` and `AttributeHandler` declared their configuration
fields as dataclass instance attributes (via `@dataclass(frozen=True)`), but
every usage accesses them through the class (`cls.source`, `cls.minimum_path_length`).
The classes are never instantiated; subclasses override the fields as class-
level attributes.

Convert the fields to `ClassVar[T]` so access through the class is valid and
drop the no-op `@dataclass(frozen=True)` decorators.

Prep work for the mypy 1.20 upgrade (#113419), which adds a new `misc` error
for 'Cannot access instance-only attribute on class object'.

Agent transcript: https://claudescope.sentry.dev/share/JZ-p3R-cM7eIdQz7AaZNFl38wjaYVyV7YpBDM4yLTyU
JoshFerge added a commit that referenced this pull request Apr 20, 2026
`GroupType` is never instantiated in production: subclasses declare their
configuration as class-level attributes (`type_id = 1001`, `slug = "foo"`,
etc.) and the registry stores the classes themselves. Despite that, the base
declared fields as `@dataclass(frozen=True)` instance attributes, so every
access like `group_type.type_id` (where `group_type: type[GroupType]`) is
really class-level access.

Convert the fields to `ClassVar[T]`, preserving the same defaults. Also
convert the `ReplayGroupTypeDefaults` mixin's `notification_config` for the
same reason.

Behavior change: `GroupType.__post_init__` was previously dead code (nothing
instantiates `GroupType`), so category validation never actually ran. Move
the same validation to `__init_subclass__` so it fires at subclass-definition
time. Update the two tests that were constructing `GroupType(...)` (not a real
production usage pattern) to define subclasses and the category-validation
test to expect the failure at class-definition time.

Prep work for the mypy 1.20 upgrade (#113419) — safe under 1.19.1.

Agent transcript: https://claudescope.sentry.dev/share/8kuOPo_F7g0JelnKiHguToNZr0_y4AKNCOGNvydSZac
JoshFerge added a commit that referenced this pull request Apr 20, 2026
`sentry.models.group.GroupStatus` is a plain class whose members are ints
(`RESOLVED = 1`, `IGNORED = 2`, etc.), not an enum. Callers of these
functions all pass ints, and the in-function `==` comparisons against
`GroupStatus.RESOLVED` are int-vs-int. But the parameters were annotated as
`GroupStatus`, i.e. 'instance of GroupStatus'.

Under `strict_equality = true` the comparison becomes semantically
unreachable (instance-of-class never equals int), which is surfaced as
`[unreachable]` errors by stricter mypy. Change the annotations to `int`
to match how the values are actually used:

- `fetch_alert_threshold`
- `fetch_resolve_threshold`
- `AlertContext.from_workflow_engine_models`
- `MetricAlertNotificationMessageBuilder.build_alert_context`

No runtime behavior change. Prep for the mypy 1.20 upgrade (#113419).

Agent transcript: https://claudescope.sentry.dev/share/zuptd8j6fTczb1hxTiv3VRqXHCYcX_7vZGRag0vi_DY
JoshFerge added a commit that referenced this pull request Apr 20, 2026
Three `cast(...)` calls that mypy 1.20.1 reports as redundant, and which
are in fact redundant under 1.19.1 too (mypy already infers the narrowed
type from the surrounding `if unit in constants.SIZE_UNITS` etc. checks).

- `src/sentry/search/eap/columns.py`: drop `cast(constants.SearchType, ...)`
  inside a branch already guarded by membership in DURATION_TYPE/SIZE_TYPE.
- `src/sentry/search/events/builder/base.py`: drop `cast(constants.SizeUnit, unit)`
  and `cast(constants.DurationUnit, unit)` inside the corresponding membership-
  checked branches.
- `src/sentry/search/eap/trace_metrics/config.py`: drop
  `cast(TraceMetricType, metric_type)` where `metric_type` is already
  narrowed by `if ... not in ALLOWED_METRIC_TYPES` early-return.

Prep for the mypy 1.20 upgrade (#113419).

Agent transcript: https://claudescope.sentry.dev/share/rGGex5V3dtTetZqF-KULYaNyoOlBBLtcZWOK-0xI95U
JoshFerge added a commit that referenced this pull request Apr 20, 2026
Collected test-only type cleanups. All are preexisting latent issues that
stricter mypy (1.20.1) surfaces; these fixes are behavior-neutral under
the current checker too.

Two main patterns:

1. `x.refresh_from_db()` after an `assert x.field is None` (or similar) —
   refresh_from_db mutates in place, so mypy keeps the narrowing; later
   assertions against non-None values are therefore unreachable. Replace
   with `x = Model.objects.get(id=x.id)`, which is semantically equivalent
   (fresh row from db) and resets the type. Applies to:
   - tests/sentry/models/test_dashboard.py
   - tests/snuba/api/endpoints/test_organization_events_mep.py
   - tests/snuba/api/endpoints/test_organization_events_stats_mep.py
   - tests/sentry/monitors/logic/test_mark_ok.py
   - tests/sentry/grouping/seer_similarity/test_training_mode.py

2. Misc annotations/guards:
   - tests/sentry/snuba/metrics/test_naming_layer.py: assert parsed MRI is
     not None before dereferencing.
   - tests/sentry/integrations/vsts/test_provider.py: annotate
     `exchange_token` test result as `dict[str, Any]`. The declared return
     type is `dict[str, str]` but real OAuth responses include ints
     (`expires_in`); the annotation is a preexisting lie, but changing the
     api's signature is out of scope here.
   - tests/sentry/issues/test_occurrence_consumer.py: type a heterogeneous
     debug_meta case list as `list[dict[str, Any]]`.
   - tests/sentry/sentry_apps/api/endpoints/test_sentry_apps.py: guard
     `Mapping | None` before `**data`.
   - tests/sentry/notifications/notification_action/test_metric_alert_registry_handlers.py:
     compare `notification_context.integration_id` against an int (the
     dataclass field type) rather than a str.
   - tests/sentry/incidents/test_logic.py: compare `target_identifier`
     against `str(target_identifier)` since the model field is `str`.

Prep for the mypy 1.20 upgrade (#113419). Safe under 1.19.1.

Agent transcript: https://claudescope.sentry.dev/share/0y-hoZTElhR6ktlkfhGtYwf6fxuky_-5F7Fk4q7smC8
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 20, 2026

Backend Test Failures

Failures on 1b18fd5 in this run:

tests/sentry/incidents/test_logic.py::UpdateAlertRuleTriggerAction::test_pagerdutylog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/incidents/test_logic.py:2978: in test_pagerduty
    assert action.target_identifier == str(target_identifier)
E   AssertionError: assert 41 == '41'
E    +  where 41 = <AlertRuleTriggerAction at 0x7f0352adf3e0: id=12>.target_identifier
E    +  and   '41' = str(41)
tests/sentry/workflow_engine/endpoints/test_validators.py::TestBaseGroupTypeDetectorValidator::test_validate_type_validlog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/workflow_engine/endpoints/test_validators.py:118: in test_validate_type_valid
    class TestGroupType(GroupType):
src/sentry/issues/grouptype.py:282: in __init_subclass__
    registry.add(cls)
src/sentry/issues/grouptype.py:117: in add
    raise ValueError(
E   ValueError: A group type with the type_id 1 has already been registered.
tests/sentry/workflow_engine/endpoints/test_validators.py::TestBaseGroupTypeDetectorValidator::test_validate_type_incompatiblelog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/workflow_engine/endpoints/test_validators.py:142: in test_validate_type_incompatible
    class TestGroupType(GroupType):
src/sentry/issues/grouptype.py:282: in __init_subclass__
    registry.add(cls)
src/sentry/issues/grouptype.py:117: in add
    raise ValueError(
E   ValueError: A group type with the type_id 1 has already been registered.
tests/sentry/notifications/notification_action/test_metric_alert_registry_handlers.py::TestBaseMetricAlertHandler::test_build_notification_contextlog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/notifications/notification_action/test_metric_alert_registry_handlers.py:375: in test_build_notification_context
    assert notification_context.integration_id == 1234567890
E   AssertionError: assert '1234567890' == 1234567890
E    +  where '1234567890' = NotificationContext(id=33, integration_id='1234567890', target_identifier='channel456', target_display=None, target_type=None, sentry_app_config=None, sentry_app_id=None).integration_id

JoshFerge added a commit that referenced this pull request Apr 20, 2026
…arrowing

Tests like:

    assert widget.discover_widget_split is None   # narrows to None
    ...
    widget.refresh_from_db()                      # mypy keeps the narrowing
    assert widget.discover_widget_split == TRANSACTION_LIKE  # mypy: unreachable

`refresh_from_db()` mutates the object in place, so mypy does not reset the
narrowed attribute type — later assertions comparing against non-None values
become semantically unreachable under stricter checkers. Replace with
`x = Model.objects.get(id=x.id)`, which is semantically equivalent (fresh
row from the same table) and an assignment resets the inferred type.

Applied in:

- tests/sentry/models/test_dashboard.py
- tests/snuba/api/endpoints/test_organization_events_mep.py
- tests/snuba/api/endpoints/test_organization_events_stats_mep.py
- tests/sentry/monitors/logic/test_mark_ok.py
- tests/sentry/grouping/seer_similarity/test_training_mode.py

Prep for the mypy 1.20 upgrade (#113419). Safe under 1.19.1.

Agent transcript: https://claudescope.sentry.dev/share/hYewe-LTcVPdbrD9iGU6eeyfLBiHv5fKzkiUjiPAepc
JoshFerge added a commit that referenced this pull request Apr 20, 2026
Small one-off type hint corrections in tests. Each is behavior-neutral.

- test_naming_layer.py: `assert parsed_mri is not None` before
  dereferencing `.mri_string`.
- test_provider.py: annotate the `exchange_token` test result as
  `dict[str, Any]`. The declared return type is `dict[str, str]` but
  real OAuth responses include ints (`expires_in`); this is a
  preexisting lie in the production signature, so the test locally
  widens until that gets fixed separately.
- test_occurrence_consumer.py: annotate a heterogeneous debug_meta case
  list as `list[dict[str, Any]]`.
- test_sentry_apps.py: guard `Mapping | None` before `**data`.
- test_metric_alert_registry_handlers.py: compare
  `notification_context.integration_id` against an int (the dataclass
  field type) rather than a str.
- test_logic.py: compare `target_identifier` against
  `str(target_identifier)` since the model field is typed `str`.

Prep for the mypy 1.20 upgrade (#113419). Safe under 1.19.1.

Agent transcript: https://claudescope.sentry.dev/share/zQXk8MwFd_O336VWRbM_DtJHLs12UetlIk6Q3HiZFeY
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant