Skip to content

MAINT Fix canonical typing - #3064

Merged
Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
romanlutz:romanlutz-wider-typing-failures
Oct 13, 2026
Merged

Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
romanlutz:romanlutz-wider-typing-failures

Conversation

@romanlutz

@romanlutz Roman Lutz (romanlutz) commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Restore and enforce the complete production-plus-unit typing check, ty check pyrit tests/unit, without weakening checker rules. The original wider check failed even though production typing passed: test-helper imports, fixture and mock annotations, signature-erasing decorators, and several real test defects needed correction.

  • Align pytest and ty helper roots and normalize imports to unit.* and end_to_end.*, including the newer ProPILE and Execute unit tests.
  • Correct generator fixtures, mock return types, optional-value narrowing, and test input/output types. Preserve dedicated legacy-alias and invalid-input coverage.
  • Preserve argument and return types through the rate limiter and custom-result retry decorator without changing their runtime pacing or retry behavior. Make the embedding mock assertion execute and retain cancellation and partial-setup cleanup coverage.
  • Run make ty once in the existing Ubuntu pre-commit job, using its all-extras environment and Python 3.11, the minimum supported version. Enforce it for all pull requests, merge-queue runs, and pushes to main, including test-only changes. Keep the existing cross-platform production typing hook unchanged; do not duplicate the broader check across the 24 unit-test matrix jobs.

Production model/scorer APIs, checker severities, and directory-level overrides are unchanged. Two line-specific ty exemptions retain deliberately invalid positional construction and an intentionally unresolved annotation. Integration tests, partner-integration tests, and top-level build-script typing remain separate work and are not added to CI's canonical check.

The SQLite timing fix is already on main through #3121. That change supplies the deterministic deadline regression and non-cancelling test watchdog. The remaining SQLite changes here strengthen cancellation, native busy-timeout, and connection-restoration assertions. This PR primarily restores unit-test typing and adds enforcement; it is not a broad fix for flaky tests. Production SQLite deadlines and retry behavior are unchanged.

The branch includes main through 04e3f84de68c0ed186070ca3fdbd094a4d0cff02. Accepted review changes are published in 0b64fde1ace251f6a5ec4004846c1f2ba040eee1.

No breaking runtime API change is intended.

Tests and Documentation

Validation of the latest-main merge and accepted review changes used this worktree's Python 3.13.13, uv 0.11.8, ty 0.0.84, and all extras. UV_PYTHON selected the owned interpreter and UV_PYTHON_DOWNLOADS=never was set. The targeted tests used process-local Hugging Face, datasets, and Transformers offline settings.

Command Result
uv run --frozen --extra all --link-mode=copy ty check pyrit Passed, zero diagnostics
uv run --frozen --extra all --link-mode=copy -m ty check pyrit tests\unit Passed, zero diagnostics; this is the existing make ty recipe
uv run --frozen --extra all --link-mode=copy -m ruff check tests\unit\build_scripts\test_typing_ci_contract.py tests\unit\scenario\garak\test_propile.py tests\unit\scenario\scenarios\technique\test_execute.py Passed
uv run --frozen --extra all --link-mode=copy -m ruff format --check tests\unit\build_scripts\test_typing_ci_contract.py tests\unit\scenario\garak\test_propile.py tests\unit\scenario\scenarios\technique\test_execute.py Passed
uv run --frozen --extra all --link-mode=copy -m pytest -n 4 --dist=loadfile -q tests\unit\build_scripts\test_typing_ci_contract.py tests\unit\scenario\garak\test_propile.py tests\unit\scenario\scenarios\technique\test_execute.py tests\unit\setup\techniques\test_extra_techniques.py tests\unit\memory\test_attack_analytics_lock_retry.py 226 passed
uv run --frozen --extra all --link-mode=copy pre-commit run --files .github\workflows\build_and_test.yml doc\getting_started\install_local_dev.md tests\unit\build_scripts\test_typing_ci_contract.py tests\unit\scenario\garak\test_propile.py tests\unit\scenario\scenarios\technique\test_execute.py Passed, including workflow YAML validation; production-only hooks skipped for these files, so production typing ran explicitly above
Commit hooks Passed without bypasses

make is unavailable on this Windows host. Local execution of the equivalent command on Python 3.13 does not substitute for Ubuntu/Python 3.11 execution; the new CI step provides that execution. Fresh CI is triggered by the pushed head, but its result is not asserted here. The complete local unit suite, live Azure SQL, and expanded typing were not rerun for this review follow-up.

Historical validation is not a result for the current head: the initial implementation had 3,193 changed-module tests passing, and its full offline suite had 24,855 passed, 1 failed, and 21 skipped. The disclosed failure was an unrelated environment-sensitive reinitialization fixture test. A later main integration had 709 targeted tests passing and green GitHub CI on ee78428616856b9ce92c086a5f0d70d728616326, before this latest merge and review follow-up.

Updated the local development guide to distinguish production, canonical, and expanded typing and document the Ubuntu-only CI enforcement. Added workflow contract tests for the single Ubuntu execution, minimum Python and all-extras setup, event coverage without path filters, canonical scope, and unchanged production hook. Existing typed-decorator, negative-input, optional-task cleanup, and cancellation coverage is retained.

JupyText was not run; no notebook or executable documentation example was changed by this PR's implementation. Notebook changes included by merging main are upstream changes.

Roman Lutz (romanlutz) and others added 3 commits October 9, 2026 14:58
Align test-helper import roots, correct fixture and mock contracts, and preserve decorator signatures without relaxing checker rules. Keep legacy validation and teardown behavior covered.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Observe real SQLITE_BUSY retries before expiring the shared control. Check native busy-timeout settings, cleanup under deadline and cancellation, and successful connection reuse across every analytics projection.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Inspect the request-owned driver while the writer lock is held, so the regression checks the actual setting rather than only configuration calls.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Integrate main through 40ac8cc, preserving newer test lifecycle and lock-deadline coverage. Narrow the configured-provider fixture parameter to its declared boolean type.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread tests/unit/setup/techniques/test_extra_techniques.py
@richlundeen

Copy link
Copy Markdown
Contributor

I suggest adding CI enforcement in this PR. It restores the production-plus-unit typing check, so CI should prevent that check from breaking again.

  • Run make ty once in the existing Ubuntu pre-commit job, using its all-extras environment and Python 3.11, the minimum supported version.
  • Run it even for test-only PRs, and retain enforcement for pull requests, merge-queue runs, and pushes to main.
  • Keep the scope at pyrit plus unit tests. Integration tests and build scripts still have known typing errors and should remain separate work.
  • Keep the existing cross-platform production typing hook. There is no need to repeat the broader check across the 24 unit-test matrix jobs.

Also, please clarify in the description that the SQLite timing fix is already on main through #3121. The remaining changes here primarily restore unit-test typing and strengthen cancellation/restoration assertions; they are not a broad fix for flaky tests.

Drafted with GitHub Copilot.

@jsong468 Justin Song (jsong468) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Is there a reason we dont do ty check pyrit and ty check pyrit tests/unit (and maybe also build_scripts) as part of CI?

Enforce production-plus-unit typing once in Ubuntu CI, normalize remaining unit helper imports, and cover the CI scope with contract tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@romanlutz

Copy link
Copy Markdown
Contributor Author

Accepted and implemented in 0b64fde. The existing Ubuntu pre-commit job now runs make ty once with all extras and Python 3.11, including test-only PRs, merge-queue runs, and pushes to main. The cross-platform production hook and the pyrit plus unit-test scope are unchanged. Added contract tests and documented the enforcement. The description now explicitly credits #3121 for the SQLite timing fix already on main and limits the remaining SQLite work here to cancellation/restoration assertions. Production and canonical typing pass locally, along with 226 related tests. Fresh CI is running.

Integrate Attack analytics REST while preserving production-plus-unit typing and reviewed cancellation/restoration assertions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 13, 2026
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 13, 2026
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 13, 2026
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 13, 2026
Merged via the queue into microsoft:main with commit 8d17e0d Oct 13, 2026
55 checks passed
@romanlutz
Roman Lutz (romanlutz) deleted the romanlutz-wider-typing-failures branch October 13, 2026 07:06
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