MAINT: fix remaining pre-commit failures on main (ty + reST roles)#1938
Merged
romanlutz merged 3 commits intoJun 4, 2026
Merged
Conversation
Clears the ty (type check) hook (5 errors -> 0) and the Reject Sphinx reST cross-reference roles hook (12 hits -> 0) so main CI goes green and unblocks downstream PRs. ty errors: - pyrit/auth/copilot_authenticator.py: pre-initialize `page = None` alongside `browser`/`context` so the `finally` block does not dereference an unbound local when `chromium.launch` or `new_context` raises before line 391. - pyrit/output/score/pretty.py: guard `scorer_class_identifier` for `None` and fall back to `"Unknown"`, mirroring the existing convention in `pyrit/models/score.py`. - pyrit/scenario/scenarios/adaptive/text_adaptive.py: annotate `VERSION` as `ClassVar[int]` to match `AdaptiveScenario.VERSION: ClassVar[int]` and satisfy LSP. - pyrit/cli/pyrit_shell.py: suppress the soft-deprecation warning on `os.system` with `# type: ignore[ty:deprecated]` (PyRIT mypy-style convention; a `subprocess.run` rewrite would need `shell=True` for the Windows `cls` builtin and is strictly worse). reST role replacements (12 occurrences across 3 files): replace `:class:`, `:meth:`, and `:attr:` Sphinx roles with double-backtick code spans per PyRIT's MyST convention in: - pyrit/auxiliary_attacks/gcg/data.py - pyrit/auxiliary_attacks/gcg/experiments/run.py - pyrit/setup/initializers/components/scenario_techniques.py Verified locally: - pre-commit run ty-check --all-files (clean) - pre-commit run check-no-rest-roles --all-files (clean) - ruff check pyrit/ tests/ doc/ (clean) - pytest tests/unit -n 8 -q (9364 passed, 124 skipped) The 9 `unused-type-ignore-comment` warnings from the same ty run are intentionally deferred -- they are warnings, not errors, and do not fail the hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
hannahwestra25
approved these changes
Jun 4, 2026
The ternary fits on one line at the project's 120-char limit; ruff format requires the single-line form. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jsong468
approved these changes
Jun 4, 2026
rlundeen2
approved these changes
Jun 4, 2026
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
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.
Description
Unblocks
mainCI (currently RED on commit309b7a54) and downstream PRs that depend on it (notably #1884, the typing modernization sweep). Companion to #1935, which cleared the dominantmissing-override-decoratorbucket.Two pre-commit hooks fail on
maintoday; this PR clears both.ty (type check) -- 5 errors -> 0
pyrit/auth/copilot_authenticator.py: pre-initializepage = Nonealongside the existingbrowser/contextpre-init. Thefinallyblock already references all three, butpagewas only assigned inside thetry-- so an exception fromchromium.launchornew_context(before line 391) would dereference an unbound local. Real latent bug, not just a type lint.pyrit/output/score/pretty.py: guardscore.scorer_class_identifierforNoneand fall back to"Unknown", mirroring the existing convention inpyrit/models/score.py.pyrit/scenario/scenarios/adaptive/text_adaptive.py: annotateVERSIONasClassVar[int] = 1so it matches the parentAdaptiveScenario.VERSION: ClassVar[int]and satisfies LSP.ClassVarwas already imported.pyrit/cli/pyrit_shell.py: suppress theos.systemsoft-deprecation with# type: ignore[ty:deprecated](PyRIT's mypy-style convention; asubprocess.runrewrite would needshell=Truefor the Windowsclsbuiltin, which is strictly worse).Reject Sphinx reST cross-reference roles -- 12 hits -> 0
Replace
:class:,:meth:, and:attr:Sphinx roles with double-backtick code spans per PyRIT's MyST docstring convention:pyrit/auxiliary_attacks/gcg/data.py(2)pyrit/auxiliary_attacks/gcg/experiments/run.py(4)pyrit/setup/initializers/components/scenario_techniques.py(6)Deferred (out of scope)
The same
tyrun surfaces 9unused-type-ignore-commentwarnings acrosspyrit/datasets/seed_datasets/remote/{jbb_behaviors,sorry_bench,red_team_social_bias}_dataset.py,pyrit/prompt_target/openai/_openai_realtime_streaming_session.py, andpyrit/scenario/scenarios/foundry/red_team_agent.py. These are warnings (not errors) and do not fail the hook -- left for a follow-up sweep to keep this PR surgical.Tests and Documentation
No new tests; this is a CI / static-analysis fix. The
copilot_authenticator.pychange is a defensive pre-init for the exception path -- behavior on the happy path is unchanged.Verified locally:
uv run --link-mode=copy pre-commit run ty-check --all-files-- cleanuv run --link-mode=copy pre-commit run check-no-rest-roles --all-files-- cleanuv run --link-mode=copy ruff check pyrit/ tests/ doc/-- cleanuv run --link-mode=copy pytest tests/unit -n 8 -q-- 9364 passed, 124 skipped