fix analyze coverage bugs

CHANGELOG.md

@@ -112,7 +112,7 @@ Expand-Archive -Path .\rules-v0.0.3.zip -DestinationPath src\winml\modelkit\anal
 
 `gh release download` skips pre-releases unless you pass `--tag`, so the explicit `v0.0.3` is required.
 
-If you set `MODELKIT_RULES_DIR` anywhere (shell profile, CI pipeline, user env), rename it to `WINMLCLI_RULES_DIR`. Same `os.pathsep`-separated multi-directory semantics; relative paths still resolve from `src/winml/modelkit/analyze/utils/`.
+If you set `MODELKIT_RULES_DIR` anywhere (shell profile, CI pipeline, user env), rename it to `WINMLCLI_RULES_DIR`. It points to a single rules directory (not split on `os.pathsep`); relative paths still resolve from `src/winml/modelkit/analyze/utils/`.
 
 Related PRs: #411 (Parquet migration), #600 (rules zip in release), #627 (versioned filename), #587 (env var rename as part of ModelKit → WinML CLI Wave 1).
 

src/winml/modelkit/analyze/core/runtime_checker.py

@@ -221,12 +221,11 @@ def op_support(
         run_for_node_total_ms = 0
         callback_total_ms = 0
 
-        # Get all nodes from model
-        model_proto = self._model.get_model()
         # Get cached RuntimeCheckerQuery
         query = self._get_query()
+        # Use the same graph snapshot as RuntimeCheckerQuery (post shape inference).
+        nodes = query.model_proto.graph.node
         # Use tqdm for progress unless caller provides a callback
-        nodes = model_proto.graph.node
         iterator = nodes if on_node_result else tqdm.tqdm(nodes)
         for node in iterator:
             node_start = time.perf_counter()

src/winml/modelkit/analyze/core/runtime_checker_query.py

@@ -62,7 +62,9 @@
     shape_and_dtype_from_valueinfo,
 )
 from ..utils.node_key_utils import build_node_key_by_node_id, resolve_stable_node_key
-from ..utils.rule_loader import resolve_rule_parquet_path
+from ..utils.rule_loader import (
+    resolve_rule_parquet_path,
+)
 from ..utils.timing_utils import make_timing_logger
 from .node_checkers.base import NodeChecker
 from .node_checkers.registry import NodeCheckerRegistry
@@ -1935,13 +1937,13 @@ def _load_parquet_rule_table(
         op_since_version: int,
         is_qdq: bool,
         for_debug: bool = False,
-    ) -> tuple[pd.DataFrame | None, Path | None, _ParquetConditionTree | None]:
+    ) -> tuple[Path, pd.DataFrame | None, _ParquetConditionTree | None]:
         """Load per-op parquet rule table with cache.
 
         Returns:
-            tuple[pd.DataFrame | None, Path | None, _ParquetConditionTree | None]:
-                Loaded dataframe when available, otherwise None,
-                the resolved parquet path used for lookup when found,
+            tuple[Path, pd.DataFrame | None, _ParquetConditionTree | None]:
+                The resolved or expected parquet path for lookup,
+                loaded dataframe when available, otherwise None,
                 and optional pre-built condition tree.
         """
         parquet_name = (
@@ -1950,26 +1952,30 @@ def _load_parquet_rule_table(
         )
         parquet_path = resolve_rule_parquet_path(parquet_name, for_debug=for_debug)
 
+        # This per-instance cache assumes a stable rules location for the query's
+        # lifetime: the rule-dir env vars must not change between calls. The path
+        # is recomputed each call (so reporting reflects the current location),
+        # but a cached None is reused without re-probing the filesystem.
         cache_key = (op_name, op_domain.value, op_since_version, is_qdq)
         if cache_key in self._parquet_rule_table_cache:
-            if parquet_path is not None:
+            if self._parquet_rule_table_cache[cache_key] is not None:
                 _log_parquet_cache_hit(parquet_path, scope="instance")
             return (
-                self._parquet_rule_table_cache[cache_key],
                 parquet_path,
+                self._parquet_rule_table_cache[cache_key],
                 self._parquet_condition_tree_cache.get(cache_key),
             )
 
-        if parquet_path is None:
+        if not parquet_path.exists():
             self._parquet_rule_table_cache[cache_key] = None
             self._parquet_condition_tree_cache[cache_key] = None
-            return None, None, None
+            return parquet_path, None, None
 
         table_df = _get_or_load_parquet_table_global(parquet_path)
         condition_tree = _build_condition_tree(table_df)
         self._parquet_rule_table_cache[cache_key] = table_df
         self._parquet_condition_tree_cache[cache_key] = condition_tree
-        return table_df, parquet_path, condition_tree
+        return parquet_path, table_df, condition_tree
 
     def _run_for_node_with_parquet_rules(
         self,
@@ -2023,18 +2029,16 @@ def _finish(result: PatternRuntime, outcome: str, **extra: Any) -> PatternRuntim
         since_version_ms = _elapsed_ms(since_version_start)
 
         load_table_start = time.perf_counter()
-        table_df, parquet_path, condition_tree = self._load_parquet_rule_table(
+        parquet_path, table_df, condition_tree = self._load_parquet_rule_table(
             node.op_type,
             op_domain,
             op_since_version,
             is_qdq,
             for_debug=for_debug,
         )
         load_table_ms = _elapsed_ms(load_table_start)
-        parquet_file = parquet_path.name if parquet_path is not None else None
-        parquet_path_norm = (
-            _normalize_table_path(parquet_path) if parquet_path is not None else None
-        )
+        parquet_file = parquet_path.name
+        parquet_path_norm = _normalize_table_path(parquet_path)
 
         if table_df is None:
             if run_unknown_op:

src/winml/modelkit/analyze/rules/runtime_check_rules/README.md

@@ -56,25 +56,26 @@ Copy all runtime rule parquet files from:
 
 ### Option 4: Use external rules directories via environment variable
 
-Set `WINMLCLI_RULES_DIR` to one or more directories containing parquet rule artifacts.
+Set `WINMLCLI_RULES_DIR` to a single directory containing parquet rule artifacts.
 
 Important: relative paths are resolved from `src/winml/modelkit/analyze/utils/` (the
 directory of `rule_loader.py`), not from the current terminal working directory.
 
 - Windows (PowerShell, user-level absolute path): `[Environment]::SetEnvironmentVariable("WINMLCLI_RULES_DIR", "C:\*path*\rules", "User")`
 - Windows (PowerShell, user-level repo-relative path): `[Environment]::SetEnvironmentVariable("WINMLCLI_RULES_DIR", "..\..\..\..\..\..\ModelKitArtifacts\rules", "User")`
 
-Multiple directories are supported using `os.pathsep` (`;` on Windows, `:` on Unix-like systems).
+Only one directory is supported. The value is not split on `os.pathsep`; it is treated
+as a single literal directory path.
 
 ## Rule lookup order
 
-The analyzer searches directories in this order:
+`WINMLCLI_RULES_DIR` overrides — it does not augment — the embedded default:
 
-1. Directories listed in `WINMLCLI_RULES_DIR` (left to right)
-2. Embedded default directory: `src/winml/modelkit/analyze/rules/runtime_check_rules/`
-
-`WINMLCLI_RULES_DIR` takes precedence over the embedded default when the same parquet file
-exists in multiple locations.
+- If `WINMLCLI_RULES_DIR` is set, only that single directory is searched. The embedded
+  default directory is **not** consulted, so that directory must contain every parquet
+  rule you need.
+- If `WINMLCLI_RULES_DIR` is unset or empty, only the embedded default directory is searched:
+  `src/winml/modelkit/analyze/rules/runtime_check_rules/`.
 
 ## What happens if parquet rules are missing
 

src/winml/modelkit/analyze/utils/rule_loader.py

@@ -17,12 +17,12 @@
 
 logger = logging.getLogger(__name__)
 
-#: Environment variable for additional runtime check rules directories.
-#: Use ``os.pathsep`` (`;` on Windows, `:` on Unix) to separate multiple paths.
+#: Environment variable for the runtime check rules directory.
+#: Holds a single directory path; it is not split on ``os.pathsep``.
 WINMLCLI_RULES_DIR_ENV = "WINMLCLI_RULES_DIR"
 
-#: Environment variable for additional runtime debug rule directories.
-#: Use ``os.pathsep`` (`;` on Windows, `:` on Unix) to separate multiple paths.
+#: Environment variable for the runtime debug rule directory.
+#: Holds a single directory path; it is not split on ``os.pathsep``.
 WINMLCLI_RULES_DIR_FOR_DEBUG_ENV = "WINMLCLI_RULES_DIR_FOR_DEBUG"
 
 # Directory containing this module file. Relative env-var entries are resolved from here.
@@ -46,54 +46,59 @@ def _resolve_env_rules_dir_entry(entry: str) -> Path:
     return (_RULE_LOADER_DIR / entry_path).resolve()
 
 
-def _get_env_rules_dirs(env_name: str) -> list[Path]:
-    """Parse ``os.pathsep``-separated env var values into absolute paths."""
-    dirs: list[Path] = []
+def _get_env_rules_dir(env_name: str) -> Path | None:
+    """Resolve the single directory configured in ``env_name``.
+
+    The value is treated as one directory path and is intentionally not split
+    on ``os.pathsep`` -- only a single rules directory is supported. Returns
+    ``None`` when the env var is unset or blank.
+    """
     env_val = os.environ.get(env_name, "").strip()
-    if env_val:
-        for entry in env_val.split(os.pathsep):
-            entry = entry.strip()
-            if entry:
-                dirs.append(_resolve_env_rules_dir_entry(entry))
-    return dirs
+    if not env_val:
+        return None
+    return _resolve_env_rules_dir_entry(env_val)
 
 
 def get_runtime_rules_search_dirs() -> list[Path]:
-    """Return ordered list of directories to search for runtime rule artifacts.
+    """Return the directory to search for runtime rule artifacts.
 
-    The search order is:
-        1. Any extra directories listed in the :data:`WINMLCLI_RULES_DIR` env var
-            (separated by ``os.pathsep``). Absolute paths are used directly;
-            relative paths are resolved relative to this module file directory.
-      2. Default embedded directory (``src/winml/modelkit/analyze/rules/runtime_check_rules/``)
+    Selection behavior:
+        1. If :data:`WINMLCLI_RULES_DIR` is set, use only that directory.
+            Absolute paths are used directly; a relative path is resolved
+            relative to this module file directory.
+        2. If :data:`WINMLCLI_RULES_DIR` is unset/empty, use the embedded default
+            directory (``src/winml/modelkit/analyze/rules/runtime_check_rules/``).
 
     Returns:
-        List of directory Paths (may include non-existent ones; callers filter).
+        Single-element list with the selected directory (the embedded default
+        when the env var is unset). The directory may not exist; callers filter.
     """
-    dirs = _get_env_rules_dirs(WINMLCLI_RULES_DIR_ENV)
-    dirs.append(_DEFAULT_RUNTIME_RULES_DIR)
-    return dirs
+    env_dir = _get_env_rules_dir(WINMLCLI_RULES_DIR_ENV)
+    if env_dir is not None:
+        return [env_dir]
+    return [_DEFAULT_RUNTIME_RULES_DIR]
 
 
 def get_runtime_rules_debug_search_dirs() -> list[Path]:
-    """Return ordered debug-rule directories from env var only.
+    """Return the debug-rule directory from the env var only.
 
     Unlike :func:`get_runtime_rules_search_dirs`, this intentionally has no
-    embedded default fallback directory.
+    embedded default fallback: an empty list is returned when
+    :data:`WINMLCLI_RULES_DIR_FOR_DEBUG` is unset.
     """
-    return _get_env_rules_dirs(WINMLCLI_RULES_DIR_FOR_DEBUG_ENV)
+    env_dir = _get_env_rules_dir(WINMLCLI_RULES_DIR_FOR_DEBUG_ENV)
+    return [env_dir] if env_dir is not None else []
 
 
-def resolve_rule_parquet_path(parquet_filename: str, for_debug: bool = False) -> Path | None:
-    """Resolve a parquet runtime-rule artifact from ``<EP>_<DEVICE>/`` subdirs.
+def resolve_rule_parquet_path(parquet_filename: str, for_debug: bool = False) -> Path:
+    """Resolve preferred parquet runtime-rule path from ``<EP>_<DEVICE>/`` subdirs.
 
     Args:
         parquet_filename: Bare file name, e.g.
             ``Split_QNNExecutionProvider_NPU_ai.onnx_opset13.parquet``
 
     Returns:
-        Resolved Path to the parquet file if found in provider subdirectories;
-        otherwise ``None``.
+        Preferred candidate Path in search order. Existence is not checked here.
     """
 
     def _infer_ep_device_subdir(filename: str) -> str | None:
@@ -108,21 +113,22 @@ def _infer_ep_device_subdir(filename: str) -> str | None:
         return f"{match.group('ep')}_{match.group('device')}"
 
     ep_device_subdir = _infer_ep_device_subdir(parquet_filename)
-    if ep_device_subdir is None:
-        return None
+    relative_path = (
+        Path(ep_device_subdir) / parquet_filename
+        if ep_device_subdir is not None
+        else Path(parquet_filename)
+    )
 
     if for_debug:
-        for debug_dir in get_runtime_rules_debug_search_dirs():
-            candidate_in_subdir = debug_dir / ep_device_subdir / parquet_filename
-            if candidate_in_subdir.exists():
-                return candidate_in_subdir
+        debug_dirs = get_runtime_rules_debug_search_dirs()
+        if debug_dirs:
+            return debug_dirs[0] / relative_path
 
-    for search_dir in get_runtime_rules_search_dirs():
-        candidate_in_subdir = search_dir / ep_device_subdir / parquet_filename
-        if candidate_in_subdir.exists():
-            return candidate_in_subdir
+    search_dirs = get_runtime_rules_search_dirs()
+    if search_dirs:
+        return search_dirs[0] / relative_path
 
-    return None
+    return relative_path
 
 
 class RuleLoader:

src/winml/modelkit/commands/analyze.py

@@ -928,45 +928,60 @@ def analyze(
             devices = []
         devices = sorted(d.upper() for d in devices)
 
-        eps: list[EPName | None]
-        if ep == "all":
-            eps = list(SUPPORTED_EPS)
-        elif ep == "auto":
-            # Single highest-priority EP available on the target device. With
-            # device == "all" there is no single device context, so fall back to
-            # the best available device purely for EP selection.
-            if device == "all":
-                try:
-                    ref_device, _ = resolve_device(device="auto")
-                except (ValueError, RuntimeError) as e:
-                    logger.error("Could not auto-select an execution provider: %s", e)
+        execution_pairs: list[tuple[EPName, str]]
+        if ep == "auto" and device == "all":
+            # auto + all: resolve the best available EP per device rather than
+            # picking a single EP from one ref device and fanning it across
+            # unrelated devices. resolve_eps() already returns only EPs that are
+            # valid and locally available for the given device, so the resulting
+            # pairs need no further EP_SUPPORTED_DEVICES filtering.
+            execution_pairs = _sort_ep_device_pairs(
+                [
+                    (device_eps[0], target_device)
+                    for target_device in devices
+                    if (device_eps := resolve_eps(target_device))
+                ]
+            )
+        else:
+            eps: list[EPName | None]
+            if ep == "all":
+                eps = list(SUPPORTED_EPS)
+            elif ep == "auto":
+                # Single highest-priority EP available on the target device.
+                # device == "all" is handled above, so a concrete device context
+                # exists here -- but guard against an empty device list (e.g. a
+                # programmatic ``device=None`` call) so we exit cleanly instead
+                # of raising an unguarded IndexError on ``devices[0]``.
+                ref_device = devices[0] if devices else None
+                if not ref_device:
+                    logger.error("No device context available for EP auto-resolution.")
+                    sys.exit(2)
+                compatible_eps = resolve_eps(ref_device)
+                if not compatible_eps:
+                    logger.error(
+                        "No execution provider is available for device '%s'.", ref_device
+                    )
                     sys.exit(2)
+                eps = [compatible_eps[0]]
             else:
-                ref_device = devices[0]
-            compatible_eps = resolve_eps(ref_device)
-            if not compatible_eps:
-                logger.error("No execution provider is available for device '%s'.", ref_device)
-                sys.exit(2)
-            eps = [compatible_eps[0]]
-        else:
-            # ep is a specific EP or alias
-            eps = [normalize_ep_name(ep)]
-
-        # Build with a for-loop rather than a single nested comprehension so
-        # the `candidate_ep is not None and ... in EP_SUPPORTED_DEVICES`
-        # narrowing carries through to the appended tuple's type (EPName,
-        # not str). The inner generator stays a comprehension to satisfy
-        # ruff PERF401.
-        execution_pairs: list[tuple[EPName, str]] = []
-        for candidate_ep in eps:
-            if candidate_ep is None or candidate_ep not in EP_SUPPORTED_DEVICES:
-                continue
-            execution_pairs.extend(
-                (candidate_ep, candidate_device)
-                for candidate_device in devices
-                if candidate_device.lower() in EP_SUPPORTED_DEVICES[candidate_ep]
-            )
-        execution_pairs = _sort_ep_device_pairs(execution_pairs)
+                # ep is a specific EP or alias
+                eps = [normalize_ep_name(ep)]
+
+            # Build with a for-loop rather than a single nested comprehension so
+            # the `candidate_ep is not None and ... in EP_SUPPORTED_DEVICES`
+            # narrowing carries through to the appended tuple's type (EPName,
+            # not str). The inner generator stays a comprehension to satisfy
+            # ruff PERF401.
+            execution_pairs = []
+            for candidate_ep in eps:
+                if candidate_ep is None or candidate_ep not in EP_SUPPORTED_DEVICES:
+                    continue
+                execution_pairs.extend(
+                    (candidate_ep, candidate_device)
+                    for candidate_device in devices
+                    if candidate_device.lower() in EP_SUPPORTED_DEVICES[candidate_ep]
+                )
+            execution_pairs = _sort_ep_device_pairs(execution_pairs)
 
         # Local pairs are still needed to gate --run-unknown-op probing
         # (_resolve_run_unknown_op). Single-target `auto` selection is already

src/winml/modelkit/pattern/op_input_gen/pad_input_generator.py

@@ -86,6 +86,11 @@ def get_input_and_infinite_attribute_combinations(
                 "pads": InputValueConstraint(np.array([0, 0, 1, 1, 0, 0, 1, 1], dtype=np.int64)),
                 "constant_value": InputValueConstraint(np.array(0.0, dtype=np.float32)),
             },
+            {
+                "data": InputShapeConstraint((2, 3, 4, 5)),
+                "pads": InputValueConstraint(np.array([0, 1, 2, 0, 1, 0, 0, 2], dtype=np.int64)),
+                "constant_value": InputValueConstraint(np.array(0.0, dtype=np.float32)),
+            },
             # ===== 5D Input =====
             {
                 "data": InputShapeConstraint((2, 3, 4, 4, 5)),