Skip to content

ref(typing): Convert RegressionDetector + AttributeHandler config to ClassVar#113422

Merged
JoshFerge merged 1 commit into
masterfrom
joshferge/ref/classvar-regression-detector-attribute-handler
Apr 24, 2026
Merged

ref(typing): Convert RegressionDetector + AttributeHandler config to ClassVar#113422
JoshFerge merged 1 commit into
masterfrom
joshferge/ref/classvar-regression-detector-attribute-handler

Conversation

@JoshFerge

@JoshFerge JoshFerge commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Both RegressionDetector (src/sentry/statistical_detectors/detector.py) and AttributeHandler (src/sentry/rules/conditions/event_attribute.py) 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, etc.). The classes are never instantiated; subclasses override the fields as class-level attributes (source = "transaction", minimum_path_length = 1).

Convert the fields to ClassVar[T] and drop the no-op @dataclass(frozen=True) decorators. No runtime behavior change.

Why this PR exists

Prep for the mypy upgrade to 1.20.1 in #113419. 1.20.1 adds a new [misc] check, "Cannot access instance-only attribute on class object," which surfaces this pattern. Splitting the fix out so each targeted reviewer only has to look at their own area.

This PR is safe under the current mypy (1.19.1).

Agent transcript: https://claudescope.sentry.dev/share/8Gzqw1K3cHTGBqYv-FAytvqDyA86B3vsiZyoJkHL7DM

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
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 20, 2026
@JoshFerge
JoshFerge marked this pull request as ready for review April 22, 2026 16:33
@JoshFerge
JoshFerge requested review from a team as code owners April 22, 2026 16:33
@JoshFerge
JoshFerge enabled auto-merge (squash) April 22, 2026 16:35
@JoshFerge
JoshFerge requested review from a team April 23, 2026 13:44

@armenzg armenzg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Super!

@JoshFerge
JoshFerge merged commit bd09f19 into master Apr 24, 2026
60 checks passed
@JoshFerge
JoshFerge deleted the joshferge/ref/classvar-regression-detector-attribute-handler branch April 24, 2026 14:17
JoshFerge added a commit that referenced this pull request Apr 24, 2026
Bumps the mypy pin to 1.20.1. Prep PRs for the bulk
class-level-attribute fixes have already merged; what remains here is
the pin bump, the `GroupType` ClassVar + `__init_subclass__` migration,
a handful of small typing cleanups, and targeted `# type: ignore`
comments for cases that will get proper fixes in the follow-up PRs
below.

### Prep PRs (already merged)

- #113422 — `ref(typing)`: `RegressionDetector` + `AttributeHandler` →
`ClassVar`
- #113423 — `ref(issues)`: `GroupType` → `ClassVar` + move validation to
`__init_subclass__`
- #113424 — `fix(typing)`: `group_status` args are `int`, not
`GroupStatus`
- #113427 — `test(typing)`: refetch via queryset instead of
`refresh_from_db` after narrowing
- #113428 — `test(typing)`: misc test-side type hint fixes

### What's in this PR

- Bump `mypy>=1.19.1` → `mypy>=1.20.1` (+ `uv.lock`)
- Drop the now-stale `kombu.*` entry from `[[tool.mypy.overrides]]`
- `src/sentry/issues/grouptype.py` — convert `GroupType` fields to
`ClassVar` and move the category-validation check from the dead
`__post_init__` into `__init_subclass__`. Plus the corresponding test
updates in `tests/sentry/issues/test_grouptype.py` and
`tests/sentry/workflow_engine/endpoints/test_validators.py`.
- Small typing fixes that don't change runtime behavior:
- `digests/backends/base.py` — widen options map to `Mapping[str, Any]`
- `discover/compare_timeseries.py` — heterogeneous mismatches dict typed
as `dict[int, dict[str, Any]]`
- `explore/endpoints/explore_saved_queries.py` — drop unused `# type:
ignore`
- `tests/sentry/incidents/test_logic.py` — stringify both sides of the
`target_identifier` comparison so mypy stops flagging it as unreachable
- Targeted `# type: ignore` on six files where the proper fix is being
landed as a follow-up PR (see below)
- `.github/workflows/scripts/bootstrap-snuba.py` — `# type:
ignore[arg-type]` on `int(workers_str)` (single-file mypy can't see
through `sys.exit`; follow-up can replace with `NoReturn`)

### Follow-up PRs (drafts, based on this branch)

Each one removes one of the `# type: ignore` comments and applies the
proper fix. They'll merge after this lands.

- #113927 — `fix(typing)`: narrow `organization_id` in `from_auth`
- #113928 — `fix(typing)`: guard None options in `ReconnectingMemcache`
- #113929 — `docs(typing)`: annotate the `save_team_assignments` ignore
with a FIXME
- #113930 — `ref(typing)`: drop redundant cast in trace metric search
type
- #113932 — `ref(typing)`: drop redundant cast on
`TraceMetric.metric_type`
- #113933 — `ref(typing)`: drop redundant casts in
`resolve_measurement_value`

### Performance

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

### CI expectation

- `mypy` job: clean (0 errors)
- `uv` jobs: resolve against `pypi.devinfra.sentry.io` (1.20.1 has been
published there)
@github-actions github-actions Bot locked and limited conversation to collaborators May 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants