MAINT: Enable 11 clean ty rules for production code#2096
Open
romanlutz wants to merge 7 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>
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 of those rules are actually 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 11 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-type,possibly-missing-attribute, andunused-type-ignore-comment.The small fixes:
jailbreak_dataset.py: cast thelist[SeedPrompt]to thelist[SeedUnion]theSeedDatasetconstructor expects (list invariance).tokenizer_template_normalizer.py: scoped# ty: ignore[possibly-missing-attribute]ontransformers.AutoTokenizer(optional dependency, dynamic attribute).# type: ignoredirectives across 5 files; kept the 2 that are still required.Because
[tool.ty.rules]is global and the gate runsty check pyrit tests/unit, rules that only fire on intentional test-double/fixture patterns intests/unitwere added to the existingtests/unitoverride rather than editing any tests.Two rules are intentionally left deferred, with comments in
pyproject.tomlexplaining why: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, which the lock pins for Python 3.10-3.13 including CI's 3.11; it is only clean under Python 3.14's numpy 2.4).Follows #2090 (now merged).
Tests and Documentation
No test files were modified; this is a production-code and config-only change.
uv run -m ty check pyrit tests/unitpasses under both Python 3.11 (CI's version, numpy 2.2.6) and Python 3.14 (numpy 2.4.4).ruff-format,ruff-check,ty) pass on all touched files; 2 property docstrings were reworded to satisfy ruff's previewproperty-docstring-starts-with-verbrule.