Skip to content

feat: MVP port — CLI standardization, lazy imports, build/eval/inspect/perf enhancements#238

Closed
tezheng wants to merge 1 commit into
mainfrom
feat/mvp
Closed

feat: MVP port — CLI standardization, lazy imports, build/eval/inspect/perf enhancements#238
tezheng wants to merge 1 commit into
mainfrom
feat/mvp

Conversation

@tezheng

@tezheng tezheng commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Port of mvp/main work to gh/main (13 commits, squashed from 44 originals).

Features

  • CLI args standardization: shared decorator module (_options.py), global-only -v/-q, standardized --device/--ep/-m/-o/-t/-p across all 11 subcommands (10 ADRs in docs/design/cli/3_cli_args_spec.md)
  • Lazy imports: 88x startup speedup (37s → 0.4s for winml --help)
  • Build console: Rich StageLive progress, EP bar animation, summary sections
  • Eval command: Rich console display, EPContext support
  • Inspect: I/O extraction consolidation, inspect2 prototype
  • Perf: resolved device/task/IO tensor display, ONNX monitor fix
  • Analyzer: optim registry integration, EP pass-through

Fixes

  • Circular import onnx ↔ compiler ↔ session broken with lazy __getattr__
  • torch.device coercion in session
  • QNN compiler output suppression
  • ORT pre-process metadata injection
  • Warning suppression for transformers/torch
  • Pre-commit compliance (license headers, formatting)

Tests

  • 19 CLI spec enforcement tests (test_cli_spec.py)
  • 43 import-time regression tests (test_import_time.py)

@tezheng tezheng requested a review from a team as a code owner April 2, 2026 08:32
@timenick

timenick commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

static_analyzer -> analyzer? Since we rename it to analyzer in src/


# Resolution — read directly from the config object.
# No inference or reverse mapping — display what the config contains.
_ref_config = config_obj if not module else (configs[0] if configs else None)

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable Error

Local variable 'configs' may be used before it is initialized.

Copilot Autofix

AI 3 months ago

In general, this class of problem is fixed by ensuring the variable is definitely assigned on all code paths before its first use, or by guarding its use so that it is never read when uninitialized. For Python, another option is to rely on a non-local or global binding, but that would change the intended scope; here, configs is clearly meant to be a local variable representing some configuration list.

The single best way to fix this without changing the existing functionality is to avoid depending on configs when it might not exist, because its only use here is to obtain a reference configuration when module is truthy. In the provided code, _ref_config is set with:

_ref_config = config_obj if not module else (configs[0] if configs else None)

This line both references configs and implicitly assumes it is defined. A safe and behavior-preserving change is to first initialize _ref_config to config_obj, then, only if module is true and configs is known to exist, override _ref_config from configs[0]. We can do this by:

  1. Setting _ref_config = config_obj unconditionally.
  2. Wrapping the override in a try/except NameError block (or another guard) so that if configs was not defined earlier in the function, the NameError is caught and _ref_config simply remains config_obj.

This keeps the existing intent—use configs[0] when in module mode and configs is available—while guaranteeing that line 422 no longer directly references an uninitialized local. All changes are localized to the small region around lines 420–423 in src/winml/modelkit/commands/config.py, and no new imports or helpers are required.

Suggested changeset 1
src/winml/modelkit/commands/config.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/winml/modelkit/commands/config.py b/src/winml/modelkit/commands/config.py
--- a/src/winml/modelkit/commands/config.py
+++ b/src/winml/modelkit/commands/config.py
@@ -419,7 +419,14 @@
 
         # Resolution — read directly from the config object.
         # No inference or reverse mapping — display what the config contains.
-        _ref_config = config_obj if not module else (configs[0] if configs else None)
+        _ref_config = config_obj
+        if module:
+            # In module mode, prefer the first entry from `configs` if available.
+            try:
+                _ref_config = configs[0] if configs else None
+            except NameError:
+                # `configs` was never initialized on this code path; fall back to `config_obj`.
+                pass
         if _ref_config is not None:
             _quant = _ref_config.quant
 
EOF
@@ -419,7 +419,14 @@

# Resolution — read directly from the config object.
# No inference or reverse mapping — display what the config contains.
_ref_config = config_obj if not module else (configs[0] if configs else None)
_ref_config = config_obj
if module:
# In module mode, prefer the first entry from `configs` if available.
try:
_ref_config = configs[0] if configs else None
except NameError:
# `configs` was never initialized on this code path; fall back to `config_obj`.
pass
if _ref_config is not None:
_quant = _ref_config.quant

Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread src/winml/modelkit/build/common.py Fixed
Comment thread src/winml/modelkit/commands/build.py Fixed
Comment thread src/winml/modelkit/commands/build.py Fixed
Comment thread src/winml/modelkit/build/common.py Fixed
Comment thread src/winml/modelkit/core/onnx_utils.py Fixed
Comment thread src/winml/modelkit/export/io.py Fixed
Comment thread src/winml/modelkit/utils/console.py Fixed
Comment thread src/winml/modelkit/utils/console.py Fixed
Comment thread src/winml/modelkit/utils/console.py Fixed
@tezheng tezheng force-pushed the feat/mvp branch 8 times, most recently from cc21402 to 3b3cc65 Compare April 3, 2026 05:14
Comment thread src/winml/modelkit/commands/build.py Fixed
Comment thread src/winml/modelkit/commands/build.py Fixed
Comment thread src/winml/modelkit/commands/sys.py Fixed
@tezheng tezheng force-pushed the feat/mvp branch 7 times, most recently from ede2514 to efb372e Compare April 5, 2026 01:14
…t/perf enhancements

CLI Arguments:
- Shared decorator module (_options.py) for -m, -d, --ep, -o, -t, -p
- Global-only -v/-q verbosity (ADR-2), mutual exclusion
- Standardized --device (auto|cpu|gpu|npu, case-insensitive) across 8 commands
- --precision as string+validator for forward-compat (ADR-8)
- --no-X negation convention (ADR-10)
- 19 spec enforcement tests + 43 import-time regression tests

Performance:
- Lazy imports — 88x startup speedup (37s to 0.4s)
- Break circular import onnx↔compiler↔session with lazy __getattr__

Features:
- Build: StageLive Rich progress, EP bar animation, summary sections
- Eval: Rich console display, EPContext support
- Inspect: I/O extraction consolidation, --list-tasks
- Perf: resolved device/task/IO tensor display, ONNX monitor fix
- Analyzer: optim registry integration, EP pass-through
- Config: registry short-circuit, fusion flag cleanup

Fixes:
- torch.device coercion in session
- QNN compiler output suppression
- ORT pre-process metadata injection
- Warning suppression for transformers/torch
- EP re-registration log level (warning→debug)
- External data sidecar cleanup in optim pipes
@tezheng tezheng closed this Apr 13, 2026
@tezheng tezheng deleted the feat/mvp branch April 13, 2026 14:01
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