MAINT: Enable 10 clean ty rules for production code#2096
Open
romanlutz wants to merge 14 commits into
Open
Conversation
Remove the global [tool.ty.rules] ignores for nine rules that already have zero findings in pyrit/, so they are now enforced under `all = "error"`: invalid-method-override, invalid-return-type, missing-type-argument, no-matching-overload, possibly-missing-import, unresolved-attribute, unresolved-import, unused-ignore-comment, and empty-body. These rules are global, so they also apply to the `ty check pyrit tests/unit` gate. Four of them fire only on test-double/fixture patterns in tests/unit (unresolved-attribute, missing-type-argument, invalid-method-override, no-matching-overload); add those to the existing tests/unit override so the gate stays green while production type checking tightens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for pyrit Fix the two singleton production findings and remove their global ignores: - jailbreak_dataset.py: cast list[SeedPrompt] to list[SeedUnion] at SeedDataset construction (list invariance), matching the existing cast convention in seed_dataset.py. - tokenizer_template_normalizer.py: scoped ty: ignore[possibly-missing-attribute] for transformers.AutoTokenizer (optional-dependency dynamic attribute). invalid-argument-type fires only on intentional validation-failure patterns in tests/unit, so it is added to the tests/unit override (config, not test edits). possibly-missing-attribute is clean everywhere after the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove 8 unused `# type: ignore[ty:...]` directives in pyrit/ and enable the rule globally. Each was verified unused under both Python 3.11 (CI) and 3.14, with transformers installed, so none are environment-defensive: - hugging_face_chat_target.py:286,295 (invalid-argument-type) - already covered by the kept pyrit/prompt_target/hugging_face/** override. - openai_error_handling.py:100,101 (invalid-argument-type, no-matching-overload) - guarded by isinstance narrowing. - registry.py:182, red_team_agent.py:564, pyrit_initializer.py:349,360 (invalid-assignment). Used directives (registry.py:183, red_team_agent.py:573) are left in place. unused-type-ignore-comment fires on intentional directives in tests/unit, which are not edited in this cleanup, so the rule is relaxed in the tests/unit override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Batch 1 enabled missing-type-argument globally after seeing 0 findings, but that was only validated under Python 3.14 (numpy 2.4.4). CI runs the ty pre-commit hook under Python 3.11, where the lock pins numpy 2.2.6; under that numpy, ty reports 38 missing-type-argument errors on bare `np.ndarray` generics across 5 production files (analytics/conversation_analytics, prompt_converter/transparency_attack_converter, score/scorer_evaluation/*). Because the hook runs `pre-commit run --all-files` on main, leaving the rule enabled would turn main red post-merge even though incremental PR runs stay green. Move missing-type-argument back to the deferred (globally ignored) bucket with an explanatory comment, and drop the now-redundant tests/unit override entry. The other 8 rules enabled in Batch 1 remain clean under both Python 3.11 and 3.14. Full gate (ty check pyrit tests/unit) verified green under both versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Batches 2-3 modified jailbreak_dataset.py and red_team_agent.py for the ty cleanup. Both files carry pre-existing property docstrings starting with "Return", which ruff v0.15.19's preview rule property-docstring-starts-with-verb flags. Because CI runs pre-commit incrementally over whole changed files, these pre-existing violations would fail the ruff-check hook now that our change touches these files. Reword both docstrings as noun phrases (correct for properties) so the touched files pass ruff cleanly. The broader repo-wide backlog of this preview rule is left for a separate sweep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR microsoft#2090 (the original base) was squash-merged into main. Merge main so this branch targets main directly and its diff shows only the ty rule-enablement work. Resolved the pyproject.toml [tool.ty] conflict by keeping the evolved rule configuration (production rules enforced; tests/unit override extended). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lean-rules # Conflicts: # pyrit/datasets/seed_datasets/local/jailbreak_dataset.py
…p ignores) CI installs `uv sync --extra all`, so optional dependencies (torch, playwright, etc.) are present. With them installed, `# type: ignore[ty:unresolved-import]` directives on optional imports become "unused", which the now-enabled unused-type-ignore-comment rule flags across several unrelated modules (auth/copilot_authenticator, executor/benchmark/fairness_bias, prompt_target/playwright_*). The incremental PR pre-commit run only checks changed files, but the on-main `--all-files` run would fail. The same directive is "used" locally when the extra is not installed, making the rule environment -dependent and not cleanly enableable without refactoring optional imports repo-wide. Defer the rule (global ignore with rationale) like missing-type-argument and missing-override-decorator, and revert the torch import workaround from the prior commit since it is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ario Enabling ty `invalid-argument-type` surfaced three findings in the WebInjection scenario (added on main): - `default_dataset_config` was constructed with the base `DatasetConfiguration`, but `Scenario.__init__` expects `DatasetAttackConfiguration` (the subclass every other scenario passes, and the one that resolves seeds into attack groups). Switch to `DatasetAttackConfiguration`; constructor args are unchanged. - `_build_prompts_for_strategy` and `_scoring_config_for_strategy` take a `WebInjectionStrategy`, but the loop iterated `self._scenario_strategies`, typed as `list[ScenarioStrategy]` on the base class. This scenario only ever populates it with its own `WebInjectionStrategy` members, so narrow with a single cast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanlutz
changed the title
Merging latest main surfaced three findings in rules this PR enables (all in main's code, ignored on main today): - invalid-method-override in models/parameter.py: microsoft#2112 made `Parameter` a pydantic `BaseModel`, so its existing `validate()` instance method now shadows the deprecated `BaseModel.validate` classmethod. Apply the same `# type: ignore[ty:invalid-method-override]` the sibling models (Message, Score, MessagePiece) already use for this intentional pattern. - invalid-argument-type (x2) in scenario/core/scenario.py and scenarios/benchmark/adversarial.py: both pass `context.seed_groups_by_dataset` (typed `Mapping[...]`) to `MatrixAtomicAttackBuilder.build`, whose `dataset_groups` param was `dict[...]`. The builder only reads the mapping (`.items()`/`.values()`), so widen the parameter to `Mapping[...]`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
pyproject.tomldisables a broad set oftyrules under[tool.ty.rules] all = "error"as a pragmatic baseline. Many are already clean for production code, so this PR audits each one and enforces the ones that pass, tightening type checking onpyrit/without code churn.Enabled 10 previously-ignored rules across
pyrit/:invalid-method-override,invalid-return-type,no-matching-overload,possibly-missing-import,unresolved-attribute,unresolved-import,unused-ignore-comment,empty-body.invalid-argument-typeandpossibly-missing-attribute.The small fixes:
jailbreak_dataset.py: castlist[SeedPrompt]to thelist[SeedUnion]theSeedDatasetconstructor expects (list invariance).tokenizer_template_normalizer.py: scoped# ty: ignore[possibly-missing-attribute]ontransformers.AutoTokenizer(optional dependency, dynamic attribute).garak/web_injection.py: this scenario (added on main) passed the baseDatasetConfigurationwhereScenario.__init__expectsDatasetAttackConfiguration(the subclass every other scenario uses), and iterated the base-typed_scenario_strategiesinto methods that takeWebInjectionStrategy. Switched toDatasetAttackConfigurationand narrowed the strategy loop with a single cast.# type: ignoredirectives across 5 files; kept the 2 still required.Because
[tool.ty.rules]is global and the gate also coverstests/unit, rules that only fire on intentional test-double/fixture patterns there were added to the existingtests/unitoverride rather than editing any tests.Three rules are intentionally left deferred, with rationale in
pyproject.toml:missing-override-decorator(~695 methods; adopting@overrideneeds thetyping_extensionsbackport on Python 3.10/3.11).missing-type-argument(38 barenp.ndarraygenerics under numpy 2.2.6, pinned for Python 3.10-3.13 incl. CI's 3.11; clean only under Python 3.14's numpy 2.4).unused-type-ignore-commentis environment-dependent:# type: ignoreon optional-dependency imports (torch, playwright, etc.) is "used" when the extra is absent but "unused" once installed. CI runs with--extra all, so enabling it would flag such directives across several unrelated modules. Cleanly enabling it requires refactoring optional imports repo-wide, so it is deferred for a follow-up.Tests and Documentation
No test files were modified; this is a production-code and config-only change.
uv run pre-commit run --all-filespasses (ruff-format, ruff-check, ty) on the full repo withuv sync --extra all, matching CI.ty check pyrit tests/unitpasses under both Python 3.11 (CI's version, numpy 2.2.6) and Python 3.14.web_injection.py's unit tests (16) pass after theDatasetAttackConfigurationchange.