microsoft / microsoft/winml-cli

Refactor: extract common CLI options into shared decorators

Open
#369 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

P2 refactor triaged
Dominant language
Python
Stars
40
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
50

Description

Summary

Extract commonly duplicated CLI options (--model, --device, --ep, --task, --output, --verbose) into shared reusable decorators in src/winml/modelkit/utils/cli.py, and adopt them across all commands to eliminate inconsistencies.

Context

Raised in PR #354 review by @xieofxie: "should we add common options for others that are reused across different commands?" — @DingmaomaooBJTU confirmed this should be done.

A shared utility module already exists at src/winml/modelkit/utils/cli.py with four decorators (model_option, ep_option, device_option, verbosity_options), but only analyze.py uses them. The remaining 11 commands each define their own copies of these options with divergent types, defaults, help text, and casing.

Current State

Existing shared decorators in src/winml/modelkit/utils/cli.py
  • model_option(required=True)--model/-m as click.Path(exists=True, path_type=Path)
  • ep_option(required=True, optional_message=None)--ep as click.Choice(ALL_EP_NAMES)
  • device_option(required=True, optional_message=None, default="NPU")--device as click.Choice(SUPPORTED_DEVICES)
  • verbosity_options(f) — stackable --verbose/-v (count) + --quiet/-q (flag)
Only consumer: analyze.py

All other commands duplicate these options inline.

Key inconsistencies across commands
Option Commands using it Inconsistencies
--model/-m analyze, build, compile, eval, export, hub, inspect, perf, quantize (9) Type varies: click.Path vs str (for HF model IDs); required flag differs
--device/-d analyze, build, compile, config, eval, perf (6) Default varies: "NPU" (analyze), "npu" (compile), "auto" (config/eval/perf); case sensitivity differs
--ep analyze, build, compile, config, perf (5) Choice source differs: ALL_EP_NAMES vs VALID_EPS from config module vs manual strings
--task/-t config, eval, export, inspect, perf, quantize (6) All optional str with auto-detection — duplicated definition
--output/-o analyze, build, compile, config, eval, export, optimize, perf, quantize (9) Some use --output, others --output-dir; type varies (Path vs str)
--verbose/-v 11 of 12 commands analyze uses stackable count=True; all others use simple is_flag=True
--precision/-p config, perf, quantize (3) All type=str with similar values; duplicated

Desired State

  1. Expand src/winml/modelkit/utils/cli.py with additional shared decorators covering all commonly reused options (at minimum: task_option, output_option, precision_option).
  2. Evolve existing decorators to handle the two major model input patterns:
    • model_path_option — strict click.Path(exists=True) for commands that require a local .onnx file (compile, optimize, quantize)
    • model_option — flexible str type for commands that accept HF model IDs or local paths (build, config, eval, export, perf)
  3. Normalize --device — unify to a single set of choices (["auto", "cpu", "gpu", "npu"]) with case_sensitive=False and a configurable default (most commands should default to "auto").
  4. Normalize --ep — single canonical choice list from constants.py; remove manual string lists in individual commands.
  5. Normalize --verbose — decide whether all commands should use the stackable count form or the simple flag. If stackable is chosen, adopt verbosity_options everywhere.
  6. Adopt shared decorators in all 12 command files — replace inline @click.option(...) calls with the shared decorator calls.
  7. No behavioral changes — this is a pure refactor; CLI interface and option names must remain backward-compatible.

Acceptance Criteria

  • All duplicated options across commands are replaced with shared decorator calls from cli.py
  • --device choices and defaults are consistent across all commands
  • --ep choice list comes from a single source (constants.py)
  • --verbose behavior is consistent (all stackable or all flag-based)
  • No CLI-visible breaking changes (option names, short flags, defaults, help text semantics are preserved)
  • uv run ruff check --fix passes
  • uv run pytest tests/ passes

Technical Notes

  • The model_option decorator currently uses click.Path(exists=True), which rejects HuggingFace model IDs. Commands like build, eval, export, and perf accept both HF IDs and local paths, so the shared decorator needs a type parameter or two separate decorators (model_option for flexible input, model_path_option for strict path validation).
  • compile.py sources its EP choices from config/precision.py:VALID_EPS which may differ from constants.py:ALL_EP_NAMES. These should be reconciled or the compile command should add its own valid subset via the shared decorator's parameters.
  • The verbosity_options decorator adds both --verbose and --quiet, but no command other than analyze currently supports --quiet. Adopting this everywhere would add --quiet to all commands — decide if this is desired.
  • Some commands use "--model/-m" as a single string (slash syntax), while others use "--model", "-m" as separate args. The shared decorator uses separate args, which is correct Click usage.
  • hub.py uses -t for --model-type not --task, so its short flag conflicts — handle this when adopting shared task_option.

Related Files

  • src/winml/modelkit/utils/cli.py — existing shared decorators (expand here)
  • src/winml/modelkit/utils/constants.pyALL_EP_NAMES, SUPPORTED_DEVICES
  • src/winml/modelkit/commands/analyze.py — only current consumer of shared decorators (reference pattern)
  • src/winml/modelkit/commands/build.py — has its own --ep, --device, --model, --verbose
  • src/winml/modelkit/commands/compile.py — has its own --ep, --device, --model, --verbose; sources EP list from config/precision.py
  • src/winml/modelkit/commands/config.py — has its own --ep, --device, --model, --task, --verbose
  • src/winml/modelkit/commands/eval.py — has its own --device, --model, --task, --output, --verbose
  • src/winml/modelkit/commands/export.py — has its own --model, --task, --output, --verbose
  • src/winml/modelkit/commands/hub.py — has --model, --output (short flag conflict with --task)
  • src/winml/modelkit/commands/inspect.py — has its own --model, --task, --verbose
  • src/winml/modelkit/commands/optimize.py — has its own --model, --output, --verbose
  • src/winml/modelkit/commands/perf.py — has its own --ep, --device, --model, --task, --output, --verbose, --precision
  • src/winml/modelkit/commands/quantize.py — has its own --model, --task, --output, --verbose, --precision
  • src/winml/modelkit/commands/sys.py — has its own --verbose

References

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/winml/modelkit/utils/cli.py and analyze.py to understand the existing decorator pattern, then compare the option definitions across the command files listed in the issue and the constants in constants.py. Run uv run pytest tests/ and uv run ruff check --fix while checking that all 12 commands retain their option names and compatible behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.