From 1f5c9348bf19d001014bea851ecfcb925741b50e Mon Sep 17 00:00:00 2001 From: zhenchaoni Date: Thu, 25 Jun 2026 17:19:37 +0800 Subject: [PATCH 1/2] Fix multiple consistency bugs --- .../modelkit/analyze/core/runtime_checker.py | 2 +- src/winml/modelkit/cli.py | 12 ++-- src/winml/modelkit/commands/analyze.py | 22 +++---- src/winml/modelkit/commands/compile.py | 9 +-- src/winml/modelkit/commands/eval.py | 8 +-- src/winml/modelkit/commands/optimize.py | 2 +- src/winml/modelkit/utils/cli.py | 24 +++++++- tests/cli/test_help_cli.py | 57 ++++++++++++++++++- 8 files changed, 97 insertions(+), 39 deletions(-) diff --git a/src/winml/modelkit/analyze/core/runtime_checker.py b/src/winml/modelkit/analyze/core/runtime_checker.py index 4a766c56b..1a8711346 100644 --- a/src/winml/modelkit/analyze/core/runtime_checker.py +++ b/src/winml/modelkit/analyze/core/runtime_checker.py @@ -206,7 +206,7 @@ def op_support( if not self._has_any_rule_data: logger.warning( "No runtime check data found. Follow " - "https://github.com/microsoft/WinML-ModelKit/blob/main/CONTRIBUTING.md " + "https://github.com/microsoft/winml-cli/blob/main/CONTRIBUTING.md " "to set up runtime check files." ) else: diff --git a/src/winml/modelkit/cli.py b/src/winml/modelkit/cli.py index 2e4745950..90c087eb7 100644 --- a/src/winml/modelkit/cli.py +++ b/src/winml/modelkit/cli.py @@ -222,17 +222,13 @@ def format_commands(self, ctx: click.Context, formatter: click.HelpFormatter) -> commands = [] for cmd_name in self.list_commands(ctx): help_text = _parse_click_help(_COMMANDS_DIR / f"{cmd_name}.py") - commands.append((cmd_name, help_text)) + commands.append((cmd_name, help_text or "")) if commands: - limit = max(1, formatter.width - 6 - max(len(name) for name, _ in commands)) - rows = [] - for name, help_text in commands: - short = help_text[:limit].rstrip() if help_text else "" - rows.append((name, short)) - + # Pass full help to write_dl; Click wraps the description column to + # the formatter width (with hanging indent) instead of clipping it. with formatter.section("Commands"): - formatter.write_dl(rows) + formatter.write_dl(commands) @click.group( diff --git a/src/winml/modelkit/commands/analyze.py b/src/winml/modelkit/commands/analyze.py index 07c618f6e..ef7ae95bd 100644 --- a/src/winml/modelkit/commands/analyze.py +++ b/src/winml/modelkit/commands/analyze.py @@ -38,7 +38,6 @@ from ..utils import cli as cli_utils from ..utils.constants import ( - ALL_EP_NAMES, DEVICE_TYPE_TO_DEVICE, EP_SUPPORTED_DEVICES, SUPPORTED_DEVICES, @@ -716,27 +715,22 @@ def _build_runtime_debug_output_path(model_path: Path, ep_name: str, device_name @click.command(name="analyze") @cli_utils.model_path_option(required=True) -@click.option( - "--ep", - "--execution-provider", +@cli_utils.ep_option( required=False, default="auto", - show_default=True, - type=click.Choice([*ALL_EP_NAMES, "all", "auto"], case_sensitive=False), - help=( - "Target execution provider. Supports canonical names, aliases, and all/auto. " + include_auto=True, + include_all=True, + optional_message=( "all = evaluate all rule-data-backed EPs; " "auto = infer a single best target from local availability" ), ) -@click.option( - "--device", +@cli_utils.device_option( required=False, default="auto", - show_default=True, - type=click.Choice([*SUPPORTED_DEVICES, "all", "auto"], case_sensitive=False), - help=( - "Target device type. Supports CPU/GPU/NPU and all/auto. " + include_auto=True, + include_all=True, + optional_message=( "all = all rule-data-backed devices; " "auto = infer a single best target from local availability" ), diff --git a/src/winml/modelkit/commands/compile.py b/src/winml/modelkit/commands/compile.py index 895d6a4d8..b5e82a641 100644 --- a/src/winml/modelkit/commands/compile.py +++ b/src/winml/modelkit/commands/compile.py @@ -57,13 +57,10 @@ default=None, help="Output directory (default: same as input model)", ) -@click.option( - "--device", - "-d", - type=click.Choice(["auto", "npu", "gpu", "cpu"], case_sensitive=False), +@cli_utils.device_option( + required=False, default="auto", - show_default=True, - help="Target device", + include_auto=True, ) @cli_utils.ep_option( required=False, diff --git a/src/winml/modelkit/commands/eval.py b/src/winml/modelkit/commands/eval.py index fb13096c2..7f26101a4 100644 --- a/src/winml/modelkit/commands/eval.py +++ b/src/winml/modelkit/commands/eval.py @@ -74,12 +74,10 @@ default=None, help="Task (e.g. 'image-classification'). Auto-detected from --model-id.", ) -@click.option( - "--device", - type=click.Choice(["auto", "cpu", "gpu", "npu"], case_sensitive=False), +@cli_utils.device_option( + required=False, default="auto", - show_default=True, - help="Device to run on. 'auto' detects the best available device.", + include_auto=True, ) @cli_utils.ep_option(required=False) @cli_utils.precision_option( diff --git a/src/winml/modelkit/commands/optimize.py b/src/winml/modelkit/commands/optimize.py index 287f9a423..a9bf01ffb 100644 --- a/src/winml/modelkit/commands/optimize.py +++ b/src/winml/modelkit/commands/optimize.py @@ -323,7 +323,7 @@ def optimize( else: console.print(f" source: {group.sources[0]}") console.print(f" target: {group.target_class}") - console.print(f" rule: modelkit/pattern/rules/{rule_file}") + console.print(f" rule: winml/modelkit/pattern/rules/{rule_file}") if is_multi: for src in group.sources: src_flag = f"--enable-{source_flag_name(src, group.target_class)}" diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index e0ef63d87..c21367b77 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -155,7 +155,13 @@ def format_option( ) -def ep_option(required: bool = True, optional_message: str | None = None) -> Callable[[F], F]: +def ep_option( + required: bool = True, + optional_message: str | None = None, + default: str | None = None, + include_auto: bool = False, + include_all: bool = False, +) -> Callable[[F], F]: """Add --ep (execution provider) option to a Click command. Args: @@ -163,6 +169,11 @@ def ep_option(required: bool = True, optional_message: str | None = None) -> Cal optional_message: Message to append to help text when optional (e.g., "If not specified, analyzes all supported EPs.") + default: Default value when optional (default: None) + include_auto: Whether to include "auto" as a valid choice + (default: False). + include_all: Whether to include "all" as a valid choice + (default: False). Returns: Decorator function @@ -176,13 +187,16 @@ def ep_option(required: bool = True, optional_message: str | None = None) -> Cal help_text = f"{help_text}. {optional_message}" ep_choices = [name for name in ALL_EP_NAMES if name not in ("cuda", "CUDAExecutionProvider")] + choices = ["auto", *ep_choices] if include_auto else ep_choices + choices = ["all", *choices] if include_all else choices return click.option( "--ep", "--execution-provider", required=required, - default=None, - type=click.Choice(ep_choices, case_sensitive=False), + default=default if not required else None, + show_default=True, + type=click.Choice(choices, case_sensitive=False), help=help_text, ) @@ -262,6 +276,7 @@ def device_option( optional_message: str | None = None, default: str | None = "NPU", include_auto: bool = False, + include_all: bool = False, ) -> Callable[[F], F]: """Add --device option to a Click command. @@ -273,12 +288,15 @@ def device_option( default: Default value when optional (default: "NPU") include_auto: Whether to include "auto" as a valid choice (default: False). + include_all: Whether to include "all" as a valid choice + (default: False). Returns: Decorator function """ device_choices = [device.lower() for device in SUPPORTED_DEVICES] choices = ["auto", *device_choices] if include_auto else device_choices + choices = ["all", *choices] if include_all else choices help_text = f"Target device type ({', '.join(choices)})" if optional_message: help_text = f"{help_text}. {optional_message}" diff --git a/tests/cli/test_help_cli.py b/tests/cli/test_help_cli.py index 346594311..d58568086 100644 --- a/tests/cli/test_help_cli.py +++ b/tests/cli/test_help_cli.py @@ -34,11 +34,12 @@ from __future__ import annotations +import click import pytest from click.testing import CliRunner, Result from winml.modelkit import __version__ -from winml.modelkit.cli import _COMMANDS_DIR, _DISABLED_COMMANDS, main +from winml.modelkit.cli import _COMMANDS_DIR, _DISABLED_COMMANDS, _parse_click_help, main # --------------------------------------------------------------------------- @@ -212,3 +213,57 @@ def test_version_flag_executes(self) -> None: result = _invoke("--version") assert result.exit_code == 0 assert __version__ in result.output + + +# =========================================================================== +# Commands section wrapping (LazyGroup.format_commands) +# =========================================================================== + +# Longest command help text, derived from production docstrings (not hardcoded). +_CMD_HELP: dict[str, str] = { + name: _parse_click_help(_COMMANDS_DIR / f"{name}.py") for name in ENABLED_COMMANDS +} +_LONGEST_CMD, _LONGEST_HELP = max(_CMD_HELP.items(), key=lambda kv: len(kv[1])) +assert _LONGEST_HELP, "Longest command help text is empty — AST extraction failed." + + +def _render_commands(width: int) -> str: + """Render only the Commands section at a fixed formatter width.""" + formatter = click.HelpFormatter(width=width) + ctx = click.Context(main, info_name="winml") + main.format_commands(ctx, formatter) + return formatter.getvalue() + + +def _norm(text: str) -> str: + """Collapse all runs of whitespace to single spaces.""" + return " ".join(text.split()) + + +class TestCommandHelpWrapping: + """``LazyGroup.format_commands`` wraps long help instead of clipping it.""" + + def test_full_help_on_single_line_when_wide(self) -> None: + """At a generous width the longest help renders untruncated on one line.""" + out = _render_commands(width=len(_LONGEST_HELP) + 40) + row_line = next( + ln for ln in out.splitlines() if ln.strip().startswith(_LONGEST_CMD + " ") + ) + assert _LONGEST_HELP in row_line + + def test_long_help_preserved_when_narrow(self) -> None: + """At a narrow width the full help text survives (wrapped, never clipped).""" + out = _render_commands(width=40) + assert _norm(_LONGEST_HELP) in _norm(out) + + def test_long_help_spans_multiple_lines_when_narrow(self) -> None: + """At a narrow width the longest help wraps onto a continuation line.""" + lines = _render_commands(width=40).splitlines() + row_idx = next( + i for i, ln in enumerate(lines) if ln.strip().startswith(_LONGEST_CMD + " ") + ) + last_word = _LONGEST_HELP.split()[-1] + # The whole help cannot fit on the row line at 40 cols ... + assert last_word not in lines[row_idx] + # ... so its tail must appear on a following continuation line. + assert any(last_word in ln for ln in lines[row_idx + 1 :]) From 2819044285775b1bc964710a6c5153214886b8c7 Mon Sep 17 00:00:00 2001 From: zhenchaoni Date: Fri, 26 Jun 2026 10:58:46 +0800 Subject: [PATCH 2/2] Resolve comments --- src/winml/modelkit/cli.py | 12 +++++--- tests/cli/test_help_cli.py | 57 +------------------------------------- 2 files changed, 9 insertions(+), 60 deletions(-) diff --git a/src/winml/modelkit/cli.py b/src/winml/modelkit/cli.py index 90c087eb7..2e4745950 100644 --- a/src/winml/modelkit/cli.py +++ b/src/winml/modelkit/cli.py @@ -222,13 +222,17 @@ def format_commands(self, ctx: click.Context, formatter: click.HelpFormatter) -> commands = [] for cmd_name in self.list_commands(ctx): help_text = _parse_click_help(_COMMANDS_DIR / f"{cmd_name}.py") - commands.append((cmd_name, help_text or "")) + commands.append((cmd_name, help_text)) if commands: - # Pass full help to write_dl; Click wraps the description column to - # the formatter width (with hanging indent) instead of clipping it. + limit = max(1, formatter.width - 6 - max(len(name) for name, _ in commands)) + rows = [] + for name, help_text in commands: + short = help_text[:limit].rstrip() if help_text else "" + rows.append((name, short)) + with formatter.section("Commands"): - formatter.write_dl(commands) + formatter.write_dl(rows) @click.group( diff --git a/tests/cli/test_help_cli.py b/tests/cli/test_help_cli.py index d58568086..346594311 100644 --- a/tests/cli/test_help_cli.py +++ b/tests/cli/test_help_cli.py @@ -34,12 +34,11 @@ from __future__ import annotations -import click import pytest from click.testing import CliRunner, Result from winml.modelkit import __version__ -from winml.modelkit.cli import _COMMANDS_DIR, _DISABLED_COMMANDS, _parse_click_help, main +from winml.modelkit.cli import _COMMANDS_DIR, _DISABLED_COMMANDS, main # --------------------------------------------------------------------------- @@ -213,57 +212,3 @@ def test_version_flag_executes(self) -> None: result = _invoke("--version") assert result.exit_code == 0 assert __version__ in result.output - - -# =========================================================================== -# Commands section wrapping (LazyGroup.format_commands) -# =========================================================================== - -# Longest command help text, derived from production docstrings (not hardcoded). -_CMD_HELP: dict[str, str] = { - name: _parse_click_help(_COMMANDS_DIR / f"{name}.py") for name in ENABLED_COMMANDS -} -_LONGEST_CMD, _LONGEST_HELP = max(_CMD_HELP.items(), key=lambda kv: len(kv[1])) -assert _LONGEST_HELP, "Longest command help text is empty — AST extraction failed." - - -def _render_commands(width: int) -> str: - """Render only the Commands section at a fixed formatter width.""" - formatter = click.HelpFormatter(width=width) - ctx = click.Context(main, info_name="winml") - main.format_commands(ctx, formatter) - return formatter.getvalue() - - -def _norm(text: str) -> str: - """Collapse all runs of whitespace to single spaces.""" - return " ".join(text.split()) - - -class TestCommandHelpWrapping: - """``LazyGroup.format_commands`` wraps long help instead of clipping it.""" - - def test_full_help_on_single_line_when_wide(self) -> None: - """At a generous width the longest help renders untruncated on one line.""" - out = _render_commands(width=len(_LONGEST_HELP) + 40) - row_line = next( - ln for ln in out.splitlines() if ln.strip().startswith(_LONGEST_CMD + " ") - ) - assert _LONGEST_HELP in row_line - - def test_long_help_preserved_when_narrow(self) -> None: - """At a narrow width the full help text survives (wrapped, never clipped).""" - out = _render_commands(width=40) - assert _norm(_LONGEST_HELP) in _norm(out) - - def test_long_help_spans_multiple_lines_when_narrow(self) -> None: - """At a narrow width the longest help wraps onto a continuation line.""" - lines = _render_commands(width=40).splitlines() - row_idx = next( - i for i, ln in enumerate(lines) if ln.strip().startswith(_LONGEST_CMD + " ") - ) - last_word = _LONGEST_HELP.split()[-1] - # The whole help cannot fit on the row line at 40 cols ... - assert last_word not in lines[row_idx] - # ... so its tail must appear on a following continuation line. - assert any(last_word in ln for ln in lines[row_idx + 1 :])