Skip to content

MAINT Fix canonical typing - #3064

Open
Roman Lutz (romanlutz) wants to merge 5 commits into
microsoft:mainfrom
romanlutz:romanlutz-wider-typing-failures
Open

Roman Lutz (romanlutz) wants to merge 5 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

The existing production-plus-unit typing check fails even when production typing passes. Unresolved test-helper imports account for much of the noise, but fixture annotations, mock return types, signature-erasing decorators, and several real test defects also need correction.

This restores the complete pyrit tests\unit check without weakening checker rules:

  • Align pytest and ty helper roots and normalize imports to unit.* and end_to_end.*. Keep make ty on its existing production-plus-unit scope with locked dependencies and all extras.
  • Correct generator fixtures, honest mock return types, optional values, and test input/output narrowing. Use canonical constructor fields in ordinary setup while retaining dedicated legacy-alias and invalid-input tests.
  • Preserve argument and return types through the rate limiter and custom-result retry decorator. Their runtime pacing and retry behavior are unchanged.
  • Make the embedding mock assertion actually execute, preserve cancellation and partial-setup cleanup, and add signature/result regression coverage.

Production model/scorer APIs, CI hook scope, checker severities, and directory-level overrides are unchanged. Two line-specific ty exemptions retain deliberately invalid positional construction and an intentionally unresolved annotation. Integration, partner-integration, and top-level build-script typing remediation remains separate and was not rerun.

The branch includes main through 0a5abbf99b. Follow-up commit b06f89e5d3 addresses a CI failure in the existing SQLite lock-deadline test: the expected timeout occurred, but total elapsed time was 1.26 seconds and failed an unrelated one-second stopwatch assertion. The replacement waits for a real SQLITE_BUSY retry before expiring the shared control, verifies native busy sleeps are disabled and the original timeout is restored, and checks successful connection reuse. It covers all five analytics projections and retains control/task cancellation coverage. Production deadlines and retry behavior are unchanged; the five-second completion wait is a test watchdog, not a production latency guarantee.

No breaking runtime API change is intended.

Tests and Documentation

Initial implementation validation in this worktree used Python 3.13.13, uv 0.11.8, ty 0.0.84, and all extras. These results predate the subsequent main merges and follow-up stabilization:

Command Result
uv run --frozen --no-sync ty check pyrit --output-format concise Passed, zero diagnostics
uv run --frozen --no-sync ty check pyrit tests\unit --output-format concise Passed, zero diagnostics; also rechecked after committing
uv run --frozen --no-sync ruff check $changed Passed for all changed Python files
uv run --frozen --no-sync ruff format --check $changed Passed for all changed Python files
uv run --frozen --no-sync -m pytest -n 4 --dist=loadfile -q --disable-warnings $files 3,193 passed, 13 warnings; $files contains all changed test modules
uv run --frozen --no-sync -m pytest -n 4 --dist=loadfile tests\unit -q --disable-warnings 24,855 passed, 1 failed, 21 skipped, 81 subtests passed, 397 warnings
Commit hooks, including uv run --frozen --extra all --link-mode=copy ty check pyrit Passed with UV_PYTHON set to the owned Python 3.13 interpreter; no hooks bypassed

The runtime runs used Hugging Face/Transformers/datasets offline settings. make is unavailable on this Windows host, so the full-suite command runs the Makefile's four-worker unit-test recipe while retaining the owned all-extras environment.

Initial full-suite caveat: The then-unchanged tests\unit\setup\test_reinitialization.py::test_replacement_precedence_interpolation_empty_and_omission mocked setting CentralMemory without installing a memory fixture. It passed in isolation without target configuration, but failed when endpoint, key, and model environment variables caused the real target initializer to construct a target. Process-local dummy configuration reproduced the same failure under both the current helper roots and the original pythonpath=.. This was not a green full-suite result or a clean-HEAD full-suite comparison; the typing implementation left the unrelated fixture issue unchanged. Subsequent main changes include SDK test-isolation work from #3070; the historical result above is not a result for the current PR head.

Follow-up stabilization validation for b06f89e5d3, with process-local UV_PYTHON selecting this worktree's Python 3.13 interpreter:

Command Result
uv run --frozen --extra all --link-mode=copy -m ruff check tests\unit\memory\test_attack_analytics.py tests\unit\memory\test_attack_analytics_lock_retry.py Passed
uv run --frozen --extra all --link-mode=copy -m ruff format --check tests\unit\memory\test_attack_analytics.py tests\unit\memory\test_attack_analytics_lock_retry.py Passed
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
uv run --frozen --extra all --link-mode=copy -m pytest -n 4 --dist=loadfile -q tests\unit\memory\test_attack_analytics.py tests\unit\memory\test_attack_analytics_lock_retry.py tests\unit\memory\test_attack_analytics_eval_identity.py tests\unit\memory\test_attack_analytics_metadata.py tests\unit\memory\test_sqlite_cancellation.py 230 passed
Commit hooks for the follow-up test changes Passed; production-only hooks correctly skipped for test-only changes; production typing ran explicitly above

Current-head GitHub CI has been triggered. A full local unit-suite rerun, live Azure SQL, and expanded typing were not run for this follow-up.

Updated the local development guide to distinguish production, canonical, and expanded typing commands and explain interpreter versus checker target versions. Added tests for typed decorator forwarding/results and optional-task cleanup; retained legacy constructor-alias, negative-input, and cancellation coverage.

JupyText was not run; no notebook or executable documentation example was changed by the typing or stabilization implementation.

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>

This branch has not been deployed

No deployments
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.

1 participant