Skip to content

MAINT: suppress ty missing-override-decorator rule#1935

Merged
romanlutz merged 1 commit into
microsoft:mainfrom
romanlutz:romanlutz/fix-ty-missing-override
Jun 4, 2026
Merged

MAINT: suppress ty missing-override-decorator rule#1935
romanlutz merged 1 commit into
microsoft:mainfrom
romanlutz:romanlutz/fix-ty-missing-override

Conversation

@romanlutz
Copy link
Copy Markdown
Contributor

Description

#1919 bumped allganize/ty-pre-commit from v0.0.32 to v0.0.43, and the newer ty release activated the missing-override-decorator rule. Under PyRIT's existing [tool.ty.rules] all = "error" config that turned 735 method overrides across 287 files in pyrit/ into hard CI errors, which has been blocking main and every open PR (#1884 in particular).

This change suppresses the rule with a one-line entry under [tool.ty.rules], alongside the existing pragmatic ignores (unresolved-import, possibly-missing-attribute, empty-body). The accompanying comment explains why we don't just adopt @override everywhere instead:

  • typing.override is Python 3.12+. PyRIT supports 3.10+, so adopting it would require from typing_extensions import override on every site.
  • typing_extensions is currently installed only transitively (via Azure SDK, SQLAlchemy, pydantic, etc.) -- adopting would mean a new direct runtime dep.
  • It's an ongoing footgun: copy-pastes from 3.12-only examples that write from typing import override would silently break 3.10/3.11.

If the team later wants the override safety check back, that can be a separate, incremental effort (e.g., per-package PRs) -- at which point this single line is trivial to remove.

Note: this PR only addresses the missing-override-decorator fallout. The ty hook on main is also failing on a handful of unrelated diagnostics (possibly-unresolved-reference in pyrit/auth/copilot_authenticator.py, unresolved-attribute in pyrit/output/score/pretty.py, invalid-attribute-override in pyrit/scenario/scenarios/adaptive/text_adaptive.py, deprecated os.system in pyrit/cli/pyrit_shell.py). Those are pre-existing and out of scope here; happy to clean them up in a follow-up.

Tests and Documentation

No code changes, so no new tests or docs needed. Verified locally on this worktree:

  • uv run --link-mode=copy pre-commit run ty-check --all-files -- 0 missing-override-decorator errors (down from 735).
  • uv run --link-mode=copy ruff check pyrit/ tests/ doc/ clean.
  • uv run --link-mode=copy ruff format --check pyrit/ tests/ doc/ clean (1264 files).
  • uv run --link-mode=copy pytest tests/unit -n 8 -q -- 9359 passed, 124 skipped.

PR microsoft#1919 bumped allganize/ty-pre-commit from v0.0.32 to v0.0.43. The newer
ty release activated the missing-override-decorator rule, which under the
existing `[tool.ty.rules] all = "error"` config produced 735 hard CI
errors across pyrit/ (287 files) and turned main RED.

Adopting `@override` everywhere would require `typing_extensions`
(`typing.override` is 3.12+, PyRIT supports 3.10+), adding a new direct
runtime dependency plus the ongoing import-path footgun of contributors
copying `from typing import override` from 3.12-only examples. Suppressing
the rule is consistent with the file's other pragmatic ignores
(`unresolved-import`, `possibly-missing-attribute`, `empty-body`) and
keeps the project mergeable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@romanlutz romanlutz enabled auto-merge June 4, 2026 15:52
@romanlutz romanlutz added this pull request to the merge queue Jun 4, 2026
Merged via the queue into microsoft:main with commit 309b7a5 Jun 4, 2026
47 checks passed
@romanlutz romanlutz deleted the romanlutz/fix-ty-missing-override branch June 4, 2026 16:25
romanlutz pushed a commit to romanlutz/PyRIT that referenced this pull request Jun 4, 2026
PR microsoft#1935 just landed on main and globally suppressed ty's

missing-override-decorator rule rather than backfilling @OverRide

across pyrit/, citing the typing_extensions friction on Python 3.10/3.11.

Drop our two newly-added @OverRide decorators (and the typing_extensions

import) so this loader is consistent with the rest of the codebase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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