From 18e78653c132ffad65114e30b8f1daf483c473f3 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Tue, 21 Apr 2026 11:47:40 +0800 Subject: [PATCH 1/7] =?UTF-8?q?feat(telemetry):=20Phase=202=20=E2=80=94=20?= =?UTF-8?q?core=20telemetry=20logic=20(consent,=20scrub,=20singleton)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the ModelKit-specific telemetry layer on top of Phase 1's library/ and deviceid/: - constants.py: empty INSTRUMENTATION_KEY placeholder (official builds inject) - utils.py: exception scrubbing (path trim / 200-char cap / PII regex), structured traceback extraction, base64 cache encoding, msvcrt file lock - consent.py: accept-by-default first-run prompt, JSON config at %USERPROFILE%\.modelkit\config.json, CI auto-detection (fail-closed) - telemetry.py: lazy singleton wiring LoggerProvider + Resource, log_heartbeat / log_action / log_error with per-event allowlist, defensive try/except on every public method, idempotent shutdown Still not wired to the CLI — ActionGroup lands in Phase 3. --- src/winml/modelkit/telemetry/__init__.py | 11 + src/winml/modelkit/telemetry/consent.py | 171 +++++++++++++++ src/winml/modelkit/telemetry/constants.py | 13 ++ src/winml/modelkit/telemetry/telemetry.py | 207 ++++++++++++++++++ src/winml/modelkit/telemetry/utils.py | 205 +++++++++++++++++ tests/unit/telemetry/conftest.py | 36 +++ tests/unit/telemetry/test_consent.py | 163 ++++++++++++++ tests/unit/telemetry/test_constants.py | 13 ++ tests/unit/telemetry/test_public_api.py | 19 ++ tests/unit/telemetry/test_telemetry_emit.py | 131 +++++++++++ tests/unit/telemetry/test_telemetry_init.py | 49 +++++ .../unit/telemetry/test_telemetry_shutdown.py | 94 ++++++++ tests/unit/telemetry/test_utils_cache.py | 73 ++++++ tests/unit/telemetry/test_utils_scrubbing.py | 112 ++++++++++ tests/unit/telemetry/test_utils_stack.py | 58 +++++ 15 files changed, 1355 insertions(+) create mode 100644 src/winml/modelkit/telemetry/consent.py create mode 100644 src/winml/modelkit/telemetry/constants.py create mode 100644 src/winml/modelkit/telemetry/telemetry.py create mode 100644 src/winml/modelkit/telemetry/utils.py create mode 100644 tests/unit/telemetry/conftest.py create mode 100644 tests/unit/telemetry/test_consent.py create mode 100644 tests/unit/telemetry/test_constants.py create mode 100644 tests/unit/telemetry/test_public_api.py create mode 100644 tests/unit/telemetry/test_telemetry_emit.py create mode 100644 tests/unit/telemetry/test_telemetry_init.py create mode 100644 tests/unit/telemetry/test_telemetry_shutdown.py create mode 100644 tests/unit/telemetry/test_utils_cache.py create mode 100644 tests/unit/telemetry/test_utils_scrubbing.py create mode 100644 tests/unit/telemetry/test_utils_stack.py diff --git a/src/winml/modelkit/telemetry/__init__.py b/src/winml/modelkit/telemetry/__init__.py index 862c45ce3..f91e89dce 100644 --- a/src/winml/modelkit/telemetry/__init__.py +++ b/src/winml/modelkit/telemetry/__init__.py @@ -2,3 +2,14 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. # -------------------------------------------------------------------------- + +"""ModelKit telemetry - OneCollector-backed CLI usage and error reporting. + +Public surface consists of :class:`Telemetry` only. The ``ActionGroup`` +auto-wrap (Phase 3) will be re-exported here as well when it lands. +""" + +from .telemetry import Telemetry + + +__all__ = ["Telemetry"] diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py new file mode 100644 index 000000000..4aa2fd682 --- /dev/null +++ b/src/winml/modelkit/telemetry/consent.py @@ -0,0 +1,171 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +r"""Consent decision for ModelKit telemetry. + +A first-run interactive prompt collects user consent (default: accept) +and persists it to ``%USERPROFILE%\.modelkit\config.json``. This module +owns: CI/CD detection, config-file read/write, and the prompt. There +are **no** environment-variable overrides and **no** ``winml telemetry`` +subcommands - to change consent after first run, users edit the config +file directly. +""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +from pathlib import Path +from typing import Literal + + +_CONFIG_PATH: Path = Path(os.environ.get("USERPROFILE", "")) / ".modelkit" / "config.json" + +_CI_ENV_VARS = ( + "CI", + "TF_BUILD", + "GITHUB_ACTIONS", + "JENKINS_URL", + "CODEBUILD_BUILD_ID", + "BUILDKITE", + "SYSTEM_TEAMFOUNDATIONCOLLECTIONURI", +) + +Consent = Literal["enabled", "disabled"] + + +def _is_ci_environment() -> bool: + return any(os.environ.get(v) for v in _CI_ENV_VARS) + + +def _load_config() -> dict: + """Read the full config.json. Return ``{}`` if missing or unreadable.""" + try: + raw = _CONFIG_PATH.read_text(encoding="utf-8") + except FileNotFoundError: + return {} + except OSError: + return {} + try: + data = json.loads(raw) + except json.JSONDecodeError: + return {} + return data if isinstance(data, dict) else {} + + +def _read_stored_consent() -> Consent | None: + """Return the stored consent value, or ``None`` if not recorded. + + ``None`` is returned for missing / malformed / unknown values. + """ + data = _load_config() + tele = data.get("telemetry") + if not isinstance(tele, dict): + return None + value = tele.get("consent") + if value in ("enabled", "disabled"): + return value # type: ignore[return-value] + return None + + +def _write_stored_consent(value: Consent) -> None: + """Persist consent to config.json. Atomic: temp file + replace. + + Preserves any unrelated top-level keys the user (or future features) + may have added. + """ + data = _load_config() + tele = data.get("telemetry") if isinstance(data.get("telemetry"), dict) else {} + tele["consent"] = value + data["telemetry"] = tele + + _CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) + # Write to a temp file in the same directory, then atomic replace. + fd, tmp_name = tempfile.mkstemp(prefix=".config-", suffix=".json.tmp", dir=_CONFIG_PATH.parent) + tmp_path = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + json.dump(data, f, indent=2) + tmp_path.replace(_CONFIG_PATH) + except Exception: + try: + tmp_path.unlink() + except OSError: + pass + raise + + +_PROMPT_TEXT = """\ +ModelKit can collect anonymous usage data to help improve the product. + +What is collected: + - Command name, duration, success/failure + - Target device/EP (when the command specifies them) + - OS, architecture, ModelKit version + - Unhandled exception types, code locations, and scrubbed error + messages (paths trimmed, length capped, PII patterns scrubbed) + +What is never collected: + - File paths, model contents, command arguments, credentials + +Enable telemetry? [Y/n]: """ + + +def _prompt_for_consent() -> Consent: + """Show the first-run prompt and return the user's decision. + + Default on empty / unknown input is ``'enabled'`` (accept-by-default). + Only an explicit ``n`` / ``no`` declines. + """ + try: + sys.stdout.write(_PROMPT_TEXT) + sys.stdout.flush() + answer = sys.stdin.readline().strip().lower() + except (OSError, EOFError): + # If we can't read the answer, fall back to disabled - silent + # environments must not default to emission. + return "disabled" + if answer in ("n", "no"): + return "disabled" + return "enabled" + + +def resolve_consent() -> Consent: + r"""Compute the effective consent decision for this invocation. + + Precedence (first match wins): + + 1. CI environment -> ``disabled`` (does not touch stored state) + 2. Non-TTY stdin -> ``disabled`` (does not touch stored state) + 3. Stored decision -> honored + 4. Interactive prompt -> asks user, persists answer + + Accept-by-default applies **only** to step 4. Steps 1 and 2 fail + closed - silent environments never default to emission even though + interactive users see an accept-by-default prompt. + + To change a stored decision, users edit ``telemetry.consent`` in + ``%USERPROFILE%\.modelkit\config.json`` directly. There are no CLI + subcommands for this. + """ + if _is_ci_environment(): + return "disabled" + + if not sys.stdin.isatty(): + return "disabled" + + stored = _read_stored_consent() + if stored is not None: + return stored + + answer = _prompt_for_consent() + try: + _write_stored_consent(answer) + except Exception: + # Never crash the CLI on storage failure - next run will re-prompt. + pass + return answer diff --git a/src/winml/modelkit/telemetry/constants.py b/src/winml/modelkit/telemetry/constants.py new file mode 100644 index 000000000..1aea68c41 --- /dev/null +++ b/src/winml/modelkit/telemetry/constants.py @@ -0,0 +1,13 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +"""Telemetry constants. + +INSTRUMENTATION_KEY is intentionally empty in source. The official build +pipeline replaces it with the real iKey before packaging the wheel. Tests +that need to exercise the emission path monkeypatch this constant. +""" + +INSTRUMENTATION_KEY = "" diff --git a/src/winml/modelkit/telemetry/telemetry.py b/src/winml/modelkit/telemetry/telemetry.py new file mode 100644 index 000000000..b83e0dee6 --- /dev/null +++ b/src/winml/modelkit/telemetry/telemetry.py @@ -0,0 +1,207 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +"""ModelKit telemetry singleton. + +Lazily constructed: on first call, consults :func:`consent.resolve_consent`, +builds a :class:`LoggerProvider` with device ID + OS + app context, and +exposes :meth:`log_heartbeat` / :meth:`log_action` / :meth:`log_error`. If +consent is disabled OR the iKey is empty, ``disabled=True`` and all +emission calls are no-ops. +""" + +from __future__ import annotations + +import logging +import platform +import sys +import time +import uuid +from typing import Any + +from . import consent as consent_mod +from . import constants +from .deviceid import get_or_create_device_id +from .utils import _extract_exception_stack, _format_exception_message + + +_LOGGER = logging.getLogger(__name__) + +_INSTANCE: Telemetry | None = None + +_HEARTBEAT_EVENT = "ModelKitHeartbeat" +_ACTION_EVENT = "ModelKitAction" +_ERROR_EVENT = "ModelKitError" + +_ALLOWED_KEYS: dict[str, set[str]] = { + _HEARTBEAT_EVENT: set(), + _ACTION_EVENT: { + "invoked_from", + "action_name", + "device", + "ep", + "duration_ms", + "success", + }, + _ERROR_EVENT: { + "exception_type", + "exception_message", + "exception_stack", + }, +} + + +def _filter_allowlist(event_name: str, attrs: dict[str, Any]) -> dict[str, Any]: + allowed = _ALLOWED_KEYS[event_name] + return {k: v for k, v in attrs.items() if k in allowed} + + +class Telemetry: + """Process-wide telemetry singleton. Use :meth:`get_or_init` to access.""" + + def __init__(self) -> None: + self._logger = None # set when enabled; None when disabled + self._provider = None + self._disabled = True # set to False only after successful init + self._init_ts = time.time() + self._app_instance_id = str(uuid.uuid4()) + self._invoked_from = "Script" if not sys.stdin.isatty() else "Interactive" + self._try_init() + + @classmethod + def get_or_init(cls) -> Telemetry: + """Return the cached singleton, constructing it on first call.""" + global _INSTANCE + if _INSTANCE is None: + _INSTANCE = cls() + return _INSTANCE + + @property + def disabled(self) -> bool: + """Whether telemetry is inactive (consent off, empty iKey, or init failed).""" + return self._disabled + + def _try_init(self) -> None: + """Wire up the LoggerProvider, swallowing any setup error. + + Every step that can fail is guarded - a telemetry init failure + must never propagate to the CLI. + """ + try: + if not constants.INSTRUMENTATION_KEY: + return + if consent_mod.resolve_consent() != "enabled": + return + # Import lazily so tests that never reach this branch don't + # pay the OTel SDK import cost. + from .library import create_logger_provider + + resource = self._build_resource() + self._provider = create_logger_provider( + ikey=constants.INSTRUMENTATION_KEY, + resource=resource, + ) + self._logger = self._provider.get_logger("modelkit") + self._disabled = False + except Exception: + _LOGGER.debug("telemetry init failed", exc_info=True) + self._logger = None + self._provider = None + self._disabled = True + + def _build_resource(self): + from opentelemetry.sdk.resources import Resource + + device_id, id_status = get_or_create_device_id() + uname = platform.uname() + try: + from winml.modelkit import __version__ as app_version + except Exception: + app_version = "0.0.0" + return Resource.create( + { + "device_id": device_id, + "id_status": id_status, + "os.name": uname.system, + "os.version": uname.version, + "os.release": uname.release, + "os.arch": uname.machine, + "app_version": app_version, + "app_instance_id": self._app_instance_id, + "initTs": self._init_ts, + } + ) + + def log_heartbeat(self) -> None: + """Emit a ``ModelKitHeartbeat`` event (session-started signal).""" + try: + if self._logger is None: + return + self._emit(_HEARTBEAT_EVENT, {}) + except Exception: + _LOGGER.debug("log_heartbeat failed", exc_info=True) + + def log_action( + self, + action_name: str, + device: str | None, + ep: str | None, + duration_ms: int, + success: bool, + **_unused: Any, + ) -> None: + """Emit a ``ModelKitAction`` event for a completed CLI command.""" + try: + if self._logger is None: + return + attrs = { + "invoked_from": self._invoked_from, + "action_name": action_name, + "device": device, + "ep": ep, + "duration_ms": duration_ms, + "success": success, + } + self._emit(_ACTION_EVENT, attrs) + except Exception: + _LOGGER.debug("log_action failed", exc_info=True) + + def log_error(self, exc: BaseException) -> None: + """Emit a ``ModelKitError`` event for an unhandled exception.""" + try: + if self._logger is None: + return + attrs = { + "exception_type": type(exc).__name__, + "exception_message": _format_exception_message(str(exc)), + "exception_stack": _extract_exception_stack(exc.__traceback__), + } + self._emit(_ERROR_EVENT, attrs) + except Exception: + _LOGGER.debug("log_error failed", exc_info=True) + + def _emit(self, event_name: str, attrs: dict[str, Any]) -> None: + from opentelemetry._logs import LogRecord + + filtered = _filter_allowlist(event_name, attrs) + record = LogRecord( + timestamp=time.time_ns(), + body=event_name, + attributes=filtered, + ) + self._logger.emit(record) + + def shutdown(self) -> None: + """Flush the provider and mark the instance disabled. Idempotent.""" + try: + if self._provider is None: + return + self._provider.shutdown() + except Exception: + _LOGGER.debug("telemetry shutdown failed", exc_info=True) + finally: + self._logger = None + self._provider = None + self._disabled = True diff --git a/src/winml/modelkit/telemetry/utils.py b/src/winml/modelkit/telemetry/utils.py new file mode 100644 index 000000000..fb2bbdd8f --- /dev/null +++ b/src/winml/modelkit/telemetry/utils.py @@ -0,0 +1,205 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +"""Utility helpers for the telemetry module. + +Exception scrubbing, structured traceback extraction, file locking, and +cache encoding. Private-by-convention (``_``-prefixed) functions are +exported so ``telemetry.py`` can use them; they are not part of the +public package API. +""" + +from __future__ import annotations + +import base64 +import json +import os +import re +import traceback +from pathlib import Path +from typing import Any + + +_PACKAGE_ROOT = "winml/modelkit" + + +def _trim_path(path: str) -> str: + """Rewrite a path to a package-relative form (forward slashes). + + If the path contains the package root, return the slice from that root + onwards. Otherwise fall back to the basename (stdlib / third-party + frames). Empty input returns empty. + """ + if not path: + return "" + normalized = path.replace("\\", "/") + idx = normalized.find(_PACKAGE_ROOT) + if idx >= 0: + return normalized[idx:] + # No package prefix - basename only (e.g., stdlib or third-party frames). + return normalized.rsplit("/", 1)[-1] + + +_PII_PATTERNS: list[re.Pattern[str]] = [ + # Email + re.compile(r"[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}"), + # GUID (8-4-4-4-12 hex) + re.compile( + r"\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}" + r"-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b" + ), + # IPv4 + re.compile(r"\b(?:\d{1,3}\.){3}\d{1,3}\b"), + # IPv6. Two alternatives: + # 1) compressed form with "::" (e.g. 2001:db8::1) + # 2) full uncompressed form (e.g. 2001:db8:85a3:0:0:8a2e:370:7334) + re.compile( + r"\b[0-9a-fA-F]{1,4}(?::[0-9a-fA-F]{0,4}){1,7}::[0-9a-fA-F:]*\b" + r"|\b(?:[0-9a-fA-F]{1,4}:){2,7}[0-9a-fA-F]{1,4}\b" + r"|\b[0-9a-fA-F]{1,4}::[0-9a-fA-F:]*\b" + ), + # Long opaque token (API keys, JWTs, base64 blobs). Runs last so it + # doesn't swallow already-scrubbed markers. + re.compile(r"\b[A-Za-z0-9_\-]{24,}\b"), +] + +_SCRUB_PLACEHOLDER = "" + + +def _scrub_pii(text: str) -> str: + """Replace known PII / secret patterns with ````. + + Order matters: specific patterns (email, GUID, IP) run before the + generic long-token pattern so a specific match is not subsumed. + """ + if not text: + return text + for pattern in _PII_PATTERNS: + text = pattern.sub(_SCRUB_PLACEHOLDER, text) + return text + + +_MESSAGE_CAP = 200 + + +def _format_exception_message(message: str | None) -> str: + """Run the scrubbing pipeline: path trim -> length cap -> PII scrub. + + Path trim is a token-level operation that only rewrites absolute paths + found in the message. Length cap truncates to ``_MESSAGE_CAP`` chars + with a trailing ``…`` marker. PII scrub replaces matching patterns + with ````. + """ + if not message: + return "" + # Trim absolute paths token-by-token (keeps surrounding text intact). + tokens = [_trim_path(tok) if _looks_like_path(tok) else tok for tok in message.split(" ")] + result = " ".join(tokens) + # Length cap first (so PII runs on a bounded string; also prevents huge + # messages from dominating the regex cost). + if len(result) > _MESSAGE_CAP: + result = result[: _MESSAGE_CAP - 1] + "…" + # PII scrub last (so truncation won't reveal partial PII). + return _scrub_pii(result) + + +def _looks_like_path(token: str) -> bool: + # Heuristic: absolute path if it has a drive letter, starts with /, + # or contains both a separator and the package root fragment. + if not token: + return False + if re.match(r"^[A-Za-z]:[\\/]", token): + return True + if token.startswith("/"): + return True + return "\\" in token and _PACKAGE_ROOT in token.replace("\\", "/") + + +def _encode_cache_entry(entry: dict[str, Any]) -> str: + """Encode a cache entry to a single-line string: ``base64(json(dict))``.""" + raw = json.dumps(entry, ensure_ascii=False).encode("utf-8") + return base64.b64encode(raw).decode("ascii") + + +def _decode_cache_entry(line: str) -> dict[str, Any] | None: + """Decode a line produced by :func:`_encode_cache_entry`. + + Returns ``None`` on any failure (malformed base64 or JSON). Callers + treat ``None`` as "skip this entry" - a single bad line must not + disturb the rest of the cache. + """ + if not line: + return None + try: + raw = base64.b64decode(line.encode("ascii"), validate=True) + except (ValueError, UnicodeEncodeError): + return None + try: + decoded = json.loads(raw.decode("utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError): + return None + if not isinstance(decoded, dict): + return None + return decoded + + +class _ExclusiveFileLock: + """Windows-only multi-process-safe exclusive lock on a lockfile. + + Uses ``msvcrt.locking``. If lock acquisition fails, the underlying + file handle is closed before the exception propagates (no handle leak). + """ + + def __init__(self, lockfile: Path) -> None: + self._path = Path(lockfile) + self._fd: int | None = None + + def __enter__(self) -> _ExclusiveFileLock: + import msvcrt + + self._path.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(self._path, os.O_RDWR | os.O_CREAT, 0o600) + try: + msvcrt.locking(fd, msvcrt.LK_LOCK, 1) + except Exception: + os.close(fd) + raise + self._fd = fd + return self + + def __exit__(self, exc_type, exc, tb) -> None: + import msvcrt + + if self._fd is None: + return + try: + try: + msvcrt.locking(self._fd, msvcrt.LK_UNLCK, 1) + except OSError: + # Already unlocked (e.g. process exit); close anyway. + pass + finally: + os.close(self._fd) + self._fd = None + + +def _extract_exception_stack(tb: Any) -> list[dict[str, Any]]: + """Return a list of ``{file, line, function}`` dicts for ``tb``. + + Contains only structural info: no exception message, no source line + text, no local variable values. File paths are trimmed to package- + relative form via :func:`_trim_path`. + """ + if tb is None: + return [] + frames = traceback.extract_tb(tb) + return [ + { + "file": _trim_path(frame.filename or ""), + "line": frame.lineno or 0, + "function": frame.name or "", + } + for frame in frames + ] diff --git a/tests/unit/telemetry/conftest.py b/tests/unit/telemetry/conftest.py new file mode 100644 index 000000000..8d1154430 --- /dev/null +++ b/tests/unit/telemetry/conftest.py @@ -0,0 +1,36 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +"""Shared fixtures for the telemetry unit test package.""" + +import pytest + + +@pytest.fixture +def isolated_config(monkeypatch, tmp_path): + """Redirect consent storage to a per-test temp config file. + + Consumed by test_consent.py and the Telemetry singleton tests. + """ + from winml.modelkit.telemetry import consent as consent_mod + + path = tmp_path / "config.json" + monkeypatch.setattr(consent_mod, "_CONFIG_PATH", path) + return path + + +@pytest.fixture +def clean_env(monkeypatch): + """Remove telemetry-relevant env vars so tests see a known-empty env.""" + for var in ( + "CI", + "TF_BUILD", + "GITHUB_ACTIONS", + "JENKINS_URL", + "CODEBUILD_BUILD_ID", + "BUILDKITE", + "SYSTEM_TEAMFOUNDATIONCOLLECTIONURI", + ): + monkeypatch.delenv(var, raising=False) diff --git a/tests/unit/telemetry/test_consent.py b/tests/unit/telemetry/test_consent.py new file mode 100644 index 000000000..8dbdf5f68 --- /dev/null +++ b/tests/unit/telemetry/test_consent.py @@ -0,0 +1,163 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +import io +import json + +import pytest + +from winml.modelkit.telemetry import consent + + +# --- CI detection -------------------------------------------------------- + + +@pytest.mark.parametrize( + "var", + [ + "CI", + "TF_BUILD", + "GITHUB_ACTIONS", + "JENKINS_URL", + "CODEBUILD_BUILD_ID", + "BUILDKITE", + "SYSTEM_TEAMFOUNDATIONCOLLECTIONURI", + ], +) +def test_is_ci_environment_detects_known_vars(var, clean_env, monkeypatch): + monkeypatch.setenv(var, "1") + assert consent._is_ci_environment() is True + + +def test_is_ci_environment_empty_env_returns_false(clean_env): + assert consent._is_ci_environment() is False + + +# --- config-file read / write ------------------------------------------- + + +def test_read_stored_consent_missing_file_returns_none(isolated_config): + assert consent._read_stored_consent() is None + + +def test_write_stored_consent_creates_file_and_roundtrips(isolated_config): + consent._write_stored_consent("enabled") + assert consent._read_stored_consent() == "enabled" + consent._write_stored_consent("disabled") + assert consent._read_stored_consent() == "disabled" + + +def test_write_persists_nested_schema(isolated_config): + consent._write_stored_consent("enabled") + payload = json.loads(isolated_config.read_text()) + assert payload == {"telemetry": {"consent": "enabled"}} + + +def test_read_preserves_unrelated_config_on_write(isolated_config): + # A user may have added unrelated keys; we must not clobber them. + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps( + { + "unrelated": {"foo": 1}, + "telemetry": {"consent": "enabled"}, + } + ) + ) + consent._write_stored_consent("disabled") + payload = json.loads(isolated_config.read_text()) + assert payload["unrelated"] == {"foo": 1} + assert payload["telemetry"]["consent"] == "disabled" + + +def test_read_unknown_value_returns_none(isolated_config): + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text(json.dumps({"telemetry": {"consent": "sometimes"}})) + assert consent._read_stored_consent() is None + + +def test_read_missing_telemetry_field_returns_none(isolated_config): + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text(json.dumps({"unrelated": {"foo": 1}})) + assert consent._read_stored_consent() is None + + +def test_read_malformed_json_returns_none(isolated_config): + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text("{ not valid json") + assert consent._read_stored_consent() is None + + +# --- first-run prompt ---------------------------------------------------- + + +def test_prompt_accept_returns_enabled(monkeypatch, capsys): + monkeypatch.setattr("sys.stdin", io.StringIO("y\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent._prompt_for_consent() == "enabled" + captured = capsys.readouterr() + assert "Enable telemetry?" in captured.out + assert "[Y/n]" in captured.out + + +def test_prompt_decline_returns_disabled(monkeypatch): + monkeypatch.setattr("sys.stdin", io.StringIO("n\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent._prompt_for_consent() == "disabled" + + +def test_prompt_empty_defaults_to_enabled(monkeypatch): + # Default is [Y/n] - empty input = Y (accept). + monkeypatch.setattr("sys.stdin", io.StringIO("\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent._prompt_for_consent() == "enabled" + + +def test_prompt_case_insensitive_decline(monkeypatch): + monkeypatch.setattr("sys.stdin", io.StringIO("N\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent._prompt_for_consent() == "disabled" + + +def test_prompt_garbage_input_defaults_to_enabled(monkeypatch): + # Unknown input falls through to the default (accept). Only explicit + # 'n' / 'no' declines. + monkeypatch.setattr("sys.stdin", io.StringIO("banana\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent._prompt_for_consent() == "enabled" + + +# --- resolve_consent ----------------------------------------------------- + + +def test_resolve_consent_ci_defaults_to_disabled_no_prompt(clean_env, isolated_config, monkeypatch): + monkeypatch.setenv("CI", "1") + assert consent.resolve_consent() == "disabled" + # Must NOT have touched the config file (CI is per-invocation). + assert not isolated_config.exists() + + +def test_resolve_consent_non_tty_defaults_to_disabled_no_prompt( + clean_env, isolated_config, monkeypatch +): + monkeypatch.setattr("sys.stdin.isatty", lambda: False) + assert consent.resolve_consent() == "disabled" + assert not isolated_config.exists() + + +def test_resolve_consent_stored_decision_honored(clean_env, isolated_config, monkeypatch): + consent._write_stored_consent("enabled") + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent.resolve_consent() == "enabled" + + +def test_resolve_consent_first_run_empty_input_accepts_and_persists( + clean_env, isolated_config, monkeypatch +): + # Accept-by-default: pressing Enter enables telemetry. + monkeypatch.setattr("sys.stdin", io.StringIO("\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + assert consent.resolve_consent() == "enabled" + assert consent._read_stored_consent() == "enabled" diff --git a/tests/unit/telemetry/test_constants.py b/tests/unit/telemetry/test_constants.py new file mode 100644 index 000000000..851ee920f --- /dev/null +++ b/tests/unit/telemetry/test_constants.py @@ -0,0 +1,13 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + + +def test_instrumentation_key_is_empty_in_source(): + """In source / dev installs the iKey is empty. Only official builds + inject it. This test guards against an iKey being accidentally committed. + """ + from winml.modelkit.telemetry import constants + + assert constants.INSTRUMENTATION_KEY == "" diff --git a/tests/unit/telemetry/test_public_api.py b/tests/unit/telemetry/test_public_api.py new file mode 100644 index 000000000..dd0e99919 --- /dev/null +++ b/tests/unit/telemetry/test_public_api.py @@ -0,0 +1,19 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + + +def test_public_api_exposes_telemetry(): + from winml.modelkit.telemetry import Telemetry + + assert Telemetry is not None + + +def test_private_submodules_not_in_all(): + import winml.modelkit.telemetry as pkg + + for name in ("_store", "consent", "constants", "utils", "library", "deviceid"): + assert name not in getattr(pkg, "__all__", []), ( + f"internal submodule {name!r} leaked into public __all__" + ) diff --git a/tests/unit/telemetry/test_telemetry_emit.py b/tests/unit/telemetry/test_telemetry_emit.py new file mode 100644 index 000000000..b3a4954e4 --- /dev/null +++ b/tests/unit/telemetry/test_telemetry_emit.py @@ -0,0 +1,131 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +from unittest.mock import MagicMock + +import pytest + +from winml.modelkit.telemetry import consent as consent_mod +from winml.modelkit.telemetry import telemetry as telemetry_mod +from winml.modelkit.telemetry.telemetry import Telemetry + + +@pytest.fixture(autouse=True) +def _reset_singleton(): + telemetry_mod._INSTANCE = None + yield + telemetry_mod._INSTANCE = None + + +@pytest.fixture +def enabled_telemetry(monkeypatch, isolated_config, clean_env): + """Fully-enabled Telemetry with a mock underlying logger. + + `isolated_config` and `clean_env` come from conftest.py. + """ + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "o:test-key") + consent_mod._write_stored_consent("enabled") + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + + t = Telemetry.get_or_init() + assert t.disabled is False + # Replace the underlying logger with a mock that records emit() calls. + t._logger = MagicMock() + return t + + +def test_log_heartbeat_emits_event_with_no_data(enabled_telemetry): + t = enabled_telemetry + t.log_heartbeat() + t._logger.emit.assert_called_once() + log_record = t._logger.emit.call_args.args[0] + assert str(log_record.body) == "ModelKitHeartbeat" + assert dict(log_record.attributes) == {} + + +def test_log_action_emits_with_whitelisted_attrs(enabled_telemetry): + t = enabled_telemetry + t.log_action( + action_name="build", + device="NPU", + ep="QNNExecutionProvider", + duration_ms=1234, + success=True, + ) + t._logger.emit.assert_called_once() + log_record = t._logger.emit.call_args.args[0] + assert str(log_record.body) == "ModelKitAction" + attrs = dict(log_record.attributes) + assert attrs["action_name"] == "build" + assert attrs["device"] == "NPU" + assert attrs["ep"] == "QNNExecutionProvider" + assert attrs["duration_ms"] == 1234 + assert attrs["success"] is True + assert attrs["invoked_from"] in ("Script", "Interactive") + + +def test_log_action_drops_unknown_attrs(enabled_telemetry): + """Regression: attributes outside the whitelist are silently dropped, + never sent.""" + t = enabled_telemetry + t.log_action( + action_name="build", + device=None, + ep=None, + duration_ms=10, + success=True, + # Extra kwarg NOT in the whitelist - should be dropped. + leaked_field=r"C:\Users\Alice\somewhere", + ) + log_record = t._logger.emit.call_args.args[0] + attrs = dict(log_record.attributes) + assert "leaked_field" not in attrs + + +def test_log_error_scrubs_message_and_extracts_stack(enabled_telemetry): + t = enabled_telemetry + try: + raise ValueError( + "failed on alice@example.com at 10.0.0.1 for GUID 12345678-1234-5678-1234-567812345678" + ) + except ValueError as exc: + t.log_error(exc) + + t._logger.emit.assert_called_once() + log_record = t._logger.emit.call_args.args[0] + assert str(log_record.body) == "ModelKitError" + attrs = dict(log_record.attributes) + assert attrs["exception_type"] == "ValueError" + # Message scrubbed: no email, no IP, no GUID + assert "alice@example.com" not in attrs["exception_message"] + assert "10.0.0.1" not in attrs["exception_message"] + assert "12345678-1234-5678-1234-567812345678" not in attrs["exception_message"] + assert "" in attrs["exception_message"] + # Stack is a list of {file, line, function} + stack = attrs["exception_stack"] + assert isinstance(stack, list) + assert stack # at least one frame + for frame in stack: + assert set(frame.keys()) == {"file", "line", "function"} + + +def test_disabled_telemetry_noops_all_emits(monkeypatch, tmp_path): + telemetry_mod._INSTANCE = None + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "") + t = Telemetry.get_or_init() + assert t.disabled is True + # These must not raise and must not hit a logger (there is none). + t.log_heartbeat() + t.log_action( + action_name="build", + device=None, + ep=None, + duration_ms=0, + success=True, + ) + try: + raise RuntimeError("x") + except RuntimeError as e: + t.log_error(e) diff --git a/tests/unit/telemetry/test_telemetry_init.py b/tests/unit/telemetry/test_telemetry_init.py new file mode 100644 index 000000000..c081eb6cd --- /dev/null +++ b/tests/unit/telemetry/test_telemetry_init.py @@ -0,0 +1,49 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +import pytest + +from winml.modelkit.telemetry import consent as consent_mod +from winml.modelkit.telemetry import telemetry as telemetry_mod +from winml.modelkit.telemetry.telemetry import Telemetry + + +@pytest.fixture(autouse=True) +def _reset_singleton(): + """Ensure each test starts with no cached Telemetry instance.""" + telemetry_mod._INSTANCE = None + yield + telemetry_mod._INSTANCE = None + + +# `isolated_config` and `clean_env` are provided by +# tests/unit/telemetry/conftest.py. + + +def test_empty_ikey_makes_telemetry_disabled(clean_env, isolated_config, monkeypatch): + """Regression: in dev installs / source checkouts, INSTRUMENTATION_KEY + is empty and telemetry must stay off even if consent says enabled.""" + consent_mod._write_stored_consent("enabled") + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + t = Telemetry.get_or_init() + assert t.disabled is True + # No logger was even constructed. + assert t._logger is None + + +def test_consent_disabled_makes_telemetry_disabled(clean_env, isolated_config, monkeypatch): + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "o:test-key") + consent_mod._write_stored_consent("disabled") + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + t = Telemetry.get_or_init() + assert t.disabled is True + assert t._logger is None + + +def test_singleton_is_cached(clean_env, isolated_config, monkeypatch): + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "") + t1 = Telemetry.get_or_init() + t2 = Telemetry.get_or_init() + assert t1 is t2 diff --git a/tests/unit/telemetry/test_telemetry_shutdown.py b/tests/unit/telemetry/test_telemetry_shutdown.py new file mode 100644 index 000000000..738ebe80e --- /dev/null +++ b/tests/unit/telemetry/test_telemetry_shutdown.py @@ -0,0 +1,94 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +from unittest.mock import MagicMock + +import pytest + +from winml.modelkit.telemetry import consent as consent_mod +from winml.modelkit.telemetry import telemetry as telemetry_mod +from winml.modelkit.telemetry.telemetry import Telemetry + + +@pytest.fixture(autouse=True) +def _reset_singleton(): + telemetry_mod._INSTANCE = None + yield + telemetry_mod._INSTANCE = None + + +@pytest.fixture +def enabled_telemetry(monkeypatch, isolated_config, clean_env): + """Fully-enabled Telemetry with a mock provider (for shutdown assertions).""" + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "o:test-key") + consent_mod._write_stored_consent("enabled") + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + + t = Telemetry.get_or_init() + assert t.disabled is False + t._provider = MagicMock() + return t + + +def test_shutdown_flushes_provider(enabled_telemetry): + t = enabled_telemetry + provider = t._provider + t.shutdown() + provider.shutdown.assert_called_once() + + +def test_shutdown_clears_logger_for_post_shutdown_noop(enabled_telemetry): + t = enabled_telemetry + provider = t._provider + t.shutdown() + # After shutdown the instance must be in a disabled state. + assert t.disabled is True + assert t._logger is None + # And the provider reference is dropped so we can't double-shutdown the + # same object by accident. + assert t._provider is None + # Further emits are no-ops (no crash, no provider interaction). + t.log_heartbeat() + provider.shutdown.assert_called_once() # still just one call + + +def test_shutdown_is_idempotent(enabled_telemetry): + t = enabled_telemetry + provider = t._provider + t.shutdown() + t.shutdown() # must not raise + assert provider.shutdown.call_count == 1 + + +def test_shutdown_on_disabled_is_noop(monkeypatch): + telemetry_mod._INSTANCE = None + monkeypatch.setattr("winml.modelkit.telemetry.constants.INSTRUMENTATION_KEY", "") + t = Telemetry.get_or_init() + assert t.disabled is True + t.shutdown() # no crash; provider is None to begin with + + +def test_public_methods_never_raise_even_if_logger_broken(enabled_telemetry): + """Regression: a telemetry subsystem failure must never take down the + CLI. If the logger / provider object starts raising, the public + methods continue to return normally.""" + t = enabled_telemetry + t._logger = MagicMock() + t._logger.emit.side_effect = RuntimeError("downstream blew up") + + # None of these should raise. + t.log_heartbeat() + t.log_action( + action_name="x", + device=None, + ep=None, + duration_ms=0, + success=True, + ) + try: + raise ValueError("y") + except ValueError as e: + t.log_error(e) + t.shutdown() diff --git a/tests/unit/telemetry/test_utils_cache.py b/tests/unit/telemetry/test_utils_cache.py new file mode 100644 index 000000000..38c7ce68c --- /dev/null +++ b/tests/unit/telemetry/test_utils_cache.py @@ -0,0 +1,73 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +import base64 + +import pytest + +from winml.modelkit.telemetry.utils import ( + _decode_cache_entry, + _encode_cache_entry, + _ExclusiveFileLock, +) + + +def test_encode_decode_roundtrip(): + original = {"name": "ModelKitAction", "data": {"success": True}, "n": 42} + encoded = _encode_cache_entry(original) + assert isinstance(encoded, str) # storable as a single line + decoded = _decode_cache_entry(encoded) + assert decoded == original + + +def test_encode_then_decode_unicode(): + original = {"note": "café - λ"} + assert _decode_cache_entry(_encode_cache_entry(original)) == original + + +def test_decode_invalid_input_returns_none(): + # Cache read path must be robust to malformed entries (base64 garbage, + # truncated lines). Loss of one entry is acceptable; crashing the + # process is not. + assert _decode_cache_entry("not-base64!!!") is None + assert _decode_cache_entry("") is None + + +def test_decode_json_parse_error_returns_none(): + # Regression guard: the decode path must call json.loads AFTER base64 + # decode, not treat the decoded bytes as the final value. + bogus = base64.b64encode(b"this is not json").decode("ascii") + assert _decode_cache_entry(bogus) is None + + +def test_exclusive_file_lock_acquires_and_releases(tmp_path): + lock_file = tmp_path / "telemetry.lock" + with _ExclusiveFileLock(lock_file): + assert lock_file.exists() + # After exit the lock is released - re-acquiring proves it. + with _ExclusiveFileLock(lock_file): + pass + + +def test_exclusive_file_lock_closes_handle_on_lock_failure(tmp_path, monkeypatch): + """If lock acquisition fails, the underlying file handle must still be + closed (otherwise Windows would refuse to delete the lockfile).""" + lock_file = tmp_path / "telemetry.lock" + + import msvcrt + + def failing_locking(fd, mode, nbytes): + raise OSError("simulated lock failure") + + monkeypatch.setattr(msvcrt, "locking", failing_locking) + + with pytest.raises(OSError), _ExclusiveFileLock(lock_file): + pass # pragma: no cover - should not be entered + + # File handle not leaked - unlink would fail with PermissionError on leak. + try: + lock_file.unlink() + except PermissionError: # pragma: no cover - would indicate a leak + pytest.fail("file handle was leaked on lock failure") diff --git a/tests/unit/telemetry/test_utils_scrubbing.py b/tests/unit/telemetry/test_utils_scrubbing.py new file mode 100644 index 000000000..9bf29cc5e --- /dev/null +++ b/tests/unit/telemetry/test_utils_scrubbing.py @@ -0,0 +1,112 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +import pytest + +from winml.modelkit.telemetry.utils import ( + _format_exception_message, + _scrub_pii, + _trim_path, +) + + +@pytest.mark.parametrize( + "input_path,expected", + [ + # Windows absolute path -> package-relative + ( + r"C:\Users\Alice\src\winml\modelkit\commands\build.py", + "winml/modelkit/commands/build.py", + ), + # Already-relative stays relative, slashes normalized + ( + r"winml\modelkit\commands\build.py", + "winml/modelkit/commands/build.py", + ), + # Path without the package prefix falls back to basename + ( + r"C:\Users\Alice\scripts\external.py", + "external.py", + ), + # Empty / missing -> empty + ("", ""), + ], +) +def test_trim_path(input_path, expected): + assert _trim_path(input_path) == expected + + +@pytest.mark.parametrize( + "before,after", + [ + # Email + ("contact alice@example.com for help", "contact for help"), + # GUID (case-insensitive) + ( + "job id 12345678-1234-5678-1234-567812345678 failed", + "job id failed", + ), + ( + "JOB id 12345678-1234-5678-1234-567812345678 failed", + "JOB id failed", + ), + # IPv4 + ( + "cannot reach 192.168.1.100 port 8080", + "cannot reach port 8080", + ), + # IPv6 (common compressed form) + ( + "cannot reach 2001:db8::1 port 443", + "cannot reach port 443", + ), + # Long opaque token (24+ alphanumeric/underscore/dash) + ( + "Authorization: Bearer abcdef0123456789_abcdef_9999", + "Authorization: Bearer ", + ), + # Nothing sensitive -> unchanged + ("Invalid argument: expected int", "Invalid argument: expected int"), + # Multiple hits + ( + "user alice@example.com at 10.0.0.1", + "user at ", + ), + ], +) +def test_scrub_pii(before, after): + assert _scrub_pii(before) == after + + +def test_format_exception_message_applies_path_trim_and_pii_and_length(): + # Path + email + length - all three pipeline stages exercised. + msg = ( + "FileNotFoundError: " + r"C:\Users\Alice\src\winml\modelkit\commands\build.py " + "email: alice@example.com " + "x" * 500 + ) + result = _format_exception_message(msg) + # Path trimmed + assert "winml/modelkit/commands/build.py" in result + # PII scrubbed + assert "alice@example.com" not in result + assert "" in result + # Length capped at 200, with truncation marker + assert len(result) <= 200 + assert result.endswith("…") + + +def test_format_exception_message_short_passes_through(): + # Short, no PII, no absolute paths. + assert _format_exception_message("ValueError: expected int") == "ValueError: expected int" + + +def test_format_exception_message_empty_is_empty(): + assert _format_exception_message("") == "" + + +def test_format_exception_message_none_safe(): + # Defensive: accepts None by returning empty string. + assert _format_exception_message(None) == "" diff --git a/tests/unit/telemetry/test_utils_stack.py b/tests/unit/telemetry/test_utils_stack.py new file mode 100644 index 000000000..4b60669ad --- /dev/null +++ b/tests/unit/telemetry/test_utils_stack.py @@ -0,0 +1,58 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +# -------------------------------------------------------------------------- + +from winml.modelkit.telemetry.utils import _extract_exception_stack + + +def _raise_chain(): + def inner(): + raise ValueError("boom") + + def outer(): + inner() + + try: + outer() + except ValueError as e: + return e + + +def test_extract_exception_stack_returns_list_of_dicts(): + exc = _raise_chain() + frames = _extract_exception_stack(exc.__traceback__) + assert isinstance(frames, list) + assert len(frames) >= 2 # outer + inner + for frame in frames: + assert set(frame.keys()) == {"file", "line", "function"} + assert isinstance(frame["line"], int) + assert isinstance(frame["file"], str) + assert isinstance(frame["function"], str) + + +def test_extract_exception_stack_trims_paths(): + exc = _raise_chain() + frames = _extract_exception_stack(exc.__traceback__) + # Every frame's file is either package-relative or a basename - + # never a user-specific absolute path. + for frame in frames: + # No drive letter (e.g., "C:") in the first segment. + assert ":" not in frame["file"].split("/")[0] + # No absolute-Windows-path prefix. + assert not frame["file"].startswith(("C:", "c:")) + + +def test_extract_exception_stack_contains_no_message_or_locals(): + exc = _raise_chain() + frames = _extract_exception_stack(exc.__traceback__) + # No surprise fields - strictly {file, line, function}. + for frame in frames: + assert "locals" not in frame + assert "source" not in frame + assert "message" not in frame + assert "line_text" not in frame + + +def test_extract_exception_stack_on_none_returns_empty(): + assert _extract_exception_stack(None) == [] From 0b37cdacbeae143e49daa52a17ae10ccc9972fa6 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Tue, 21 Apr 2026 12:32:23 +0800 Subject: [PATCH 2/7] chore(telemetry): address CodeQL findings on PR 371 - consent.py: add explanatory comment to empty except OSError in _write_stored_consent temp-file cleanup path - test_utils_stack.py: add unreachable explicit return None in _raise_chain to avoid mixed implicit/explicit return warning --- src/winml/modelkit/telemetry/consent.py | 2 ++ tests/unit/telemetry/test_utils_stack.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py index 4aa2fd682..c2196746f 100644 --- a/src/winml/modelkit/telemetry/consent.py +++ b/src/winml/modelkit/telemetry/consent.py @@ -95,6 +95,8 @@ def _write_stored_consent(value: Consent) -> None: try: tmp_path.unlink() except OSError: + # Best-effort cleanup; temp file may already be gone or on a + # read-only volume. The real failure is re-raised below. pass raise diff --git a/tests/unit/telemetry/test_utils_stack.py b/tests/unit/telemetry/test_utils_stack.py index 4b60669ad..36325e54e 100644 --- a/tests/unit/telemetry/test_utils_stack.py +++ b/tests/unit/telemetry/test_utils_stack.py @@ -17,6 +17,8 @@ def outer(): outer() except ValueError as e: return e + # Unreachable: outer() always raises. Explicit return keeps CodeQL happy. + return None # pragma: no cover def test_extract_exception_stack_returns_list_of_dicts(): From ff41e353dcb8a9811233315c23c220e46fe24a70 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Tue, 21 Apr 2026 15:22:06 +0800 Subject: [PATCH 3/7] refactor(telemetry): address PR 371 review feedback - utils.py: require a digit in the long-token regex so Pythonic class names (e.g. WinMLImageFeatureExtractionEvaluator) are not scrubbed - utils.py: validate IPv4 octets to 0-255; document the remaining 4-part-version false positive as an accepted trade-off - consent.py: fall back to expanduser("~") if USERPROFILE is unset so the config never silently resolves to a CWD-relative path - telemetry.py: comment that invoked_from="Script" is forward-compat under the current consent rules (non-TTY -> disabled) - telemetry.py: note in Telemetry docstring that callers own shutdown lifecycle; Phase 3 ActionGroup will own it in its teardown path - tests: regression cases for class-name preservation, digit-gated long token, and out-of-range IPv4 --- src/winml/modelkit/telemetry/consent.py | 7 ++++++- src/winml/modelkit/telemetry/telemetry.py | 14 +++++++++++++- src/winml/modelkit/telemetry/utils.py | 19 +++++++++++++++---- tests/unit/telemetry/test_utils_scrubbing.py | 17 +++++++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py index c2196746f..e46f57f21 100644 --- a/src/winml/modelkit/telemetry/consent.py +++ b/src/winml/modelkit/telemetry/consent.py @@ -23,7 +23,12 @@ from typing import Literal -_CONFIG_PATH: Path = Path(os.environ.get("USERPROFILE", "")) / ".modelkit" / "config.json" +# `USERPROFILE` is virtually always set on Windows, but can be missing in +# minimal service accounts / containers. Fall back to `expanduser("~")`, +# which honors `HOMEDRIVE+HOMEPATH` or `HOME` if either is set, so we +# never silently resolve to a CWD-relative `.modelkit/config.json`. +_USER_HOME = Path(os.environ.get("USERPROFILE") or Path("~").expanduser()) +_CONFIG_PATH: Path = _USER_HOME / ".modelkit" / "config.json" _CI_ENV_VARS = ( "CI", diff --git a/src/winml/modelkit/telemetry/telemetry.py b/src/winml/modelkit/telemetry/telemetry.py index b83e0dee6..6def5f3d1 100644 --- a/src/winml/modelkit/telemetry/telemetry.py +++ b/src/winml/modelkit/telemetry/telemetry.py @@ -59,7 +59,14 @@ def _filter_allowlist(event_name: str, attrs: dict[str, Any]) -> dict[str, Any]: class Telemetry: - """Process-wide telemetry singleton. Use :meth:`get_or_init` to access.""" + """Process-wide telemetry singleton. Use :meth:`get_or_init` to access. + + The caller owns the lifecycle: :meth:`shutdown` must be invoked before + the process exits to flush any events still queued in the underlying + ``BatchLogRecordProcessor``. Phase 3's ``ActionGroup`` will own this + in its Click teardown path; until then, direct callers risk losing + the last batch on process exit. + """ def __init__(self) -> None: self._logger = None # set when enabled; None when disabled @@ -67,6 +74,11 @@ def __init__(self) -> None: self._disabled = True # set to False only after successful init self._init_ts = time.time() self._app_instance_id = str(uuid.uuid4()) + # Kept in the event schema for forward-compat: today + # `consent.resolve_consent()` returns "disabled" for non-TTY, so + # only "Interactive" reaches the wire. If Phase 3 relaxes that + # rule (e.g. honors stored consent in non-TTY), no schema change + # is needed. self._invoked_from = "Script" if not sys.stdin.isatty() else "Interactive" self._try_init() diff --git a/src/winml/modelkit/telemetry/utils.py b/src/winml/modelkit/telemetry/utils.py index fb2bbdd8f..80a38a06a 100644 --- a/src/winml/modelkit/telemetry/utils.py +++ b/src/winml/modelkit/telemetry/utils.py @@ -50,8 +50,16 @@ def _trim_path(path: str) -> str: r"\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}" r"-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b" ), - # IPv4 - re.compile(r"\b(?:\d{1,3}\.){3}\d{1,3}\b"), + # IPv4. Octets are validated to the 0-255 range so obvious nonsense + # (e.g. "999.0.0.1") is not scrubbed. 4-part version strings like + # "1.18.0.1" will still match - we accept this false positive per + # the spec's "conservative over leaky" philosophy; version numbers + # in error messages are secondary signal and the user still has + # exception_type + stack for triage. + re.compile( + r"\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}" + r"(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b" + ), # IPv6. Two alternatives: # 1) compressed form with "::" (e.g. 2001:db8::1) # 2) full uncompressed form (e.g. 2001:db8:85a3:0:0:8a2e:370:7334) @@ -61,8 +69,11 @@ def _trim_path(path: str) -> str: r"|\b[0-9a-fA-F]{1,4}::[0-9a-fA-F:]*\b" ), # Long opaque token (API keys, JWTs, base64 blobs). Runs last so it - # doesn't swallow already-scrubbed markers. - re.compile(r"\b[A-Za-z0-9_\-]{24,}\b"), + # doesn't swallow already-scrubbed markers. Requires at least one + # digit so we don't scrub long Pythonic class names (e.g. + # "WinMLImageFeatureExtractionEvaluator") that carry diagnostic value + # in error messages. + re.compile(r"\b(?=[A-Za-z0-9_\-]*\d)[A-Za-z0-9_\-]{24,}\b"), ] _SCRUB_PLACEHOLDER = "" diff --git a/tests/unit/telemetry/test_utils_scrubbing.py b/tests/unit/telemetry/test_utils_scrubbing.py index 9bf29cc5e..0575dd0b4 100644 --- a/tests/unit/telemetry/test_utils_scrubbing.py +++ b/tests/unit/telemetry/test_utils_scrubbing.py @@ -74,6 +74,23 @@ def test_trim_path(input_path, expected): "user alice@example.com at 10.0.0.1", "user at ", ), + # Long all-letter identifiers (class / function names) must NOT + # be scrubbed - they're diagnostic, not secrets. Regression for + # PR 371 review feedback. + ( + "TypeError in WinMLImageFeatureExtractionEvaluator.compute", + "TypeError in WinMLImageFeatureExtractionEvaluator.compute", + ), + # Long token WITH at least one digit is still scrubbed. + ( + "token=abcdef0123456789abcdefghijk", + "token=", + ), + # Invalid-octet IPv4 is NOT scrubbed (octet > 255 is not a real IP). + ( + "build number 999.888.777.666", + "build number 999.888.777.666", + ), ], ) def test_scrub_pii(before, after): From 72887073e9e90f0b5c7672f38ac96cc13da09419 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Tue, 21 Apr 2026 15:23:16 +0800 Subject: [PATCH 4/7] refactor(telemetry): replace expanduser fallback with Windows-native vars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit expanduser("~") is stdlib cross-platform — it checks POSIX HOME on non-Windows. ModelKit is Windows-only, so use USERPROFILE with a fallback to HOMEDRIVE+HOMEPATH (both native Windows vars) instead. --- src/winml/modelkit/telemetry/consent.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py index e46f57f21..33395fbcf 100644 --- a/src/winml/modelkit/telemetry/consent.py +++ b/src/winml/modelkit/telemetry/consent.py @@ -24,11 +24,19 @@ # `USERPROFILE` is virtually always set on Windows, but can be missing in -# minimal service accounts / containers. Fall back to `expanduser("~")`, -# which honors `HOMEDRIVE+HOMEPATH` or `HOME` if either is set, so we -# never silently resolve to a CWD-relative `.modelkit/config.json`. -_USER_HOME = Path(os.environ.get("USERPROFILE") or Path("~").expanduser()) -_CONFIG_PATH: Path = _USER_HOME / ".modelkit" / "config.json" +# minimal service accounts / containers. Fall back to the Windows-native +# `HOMEDRIVE + HOMEPATH` pair so we never silently resolve to a +# CWD-relative `.modelkit/config.json`. +def _resolve_user_home() -> str: + profile = os.environ.get("USERPROFILE") + if profile: + return profile + drive = os.environ.get("HOMEDRIVE", "") + path = os.environ.get("HOMEPATH", "") + return drive + path # empty string if neither is set + + +_CONFIG_PATH: Path = Path(_resolve_user_home()) / ".modelkit" / "config.json" _CI_ENV_VARS = ( "CI", From bf48fbf55da9d277ec11a7e7301273b68e364346 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Wed, 22 Apr 2026 14:46:56 +0800 Subject: [PATCH 5/7] feat(telemetry): version the stored consent record Introduces `consent_version` in `%USERPROFILE%\.modelkit\config.json` so a materially changed notice text can trigger a re-prompt on the next invocation. A stored version strictly older than `_CONSENT_VERSION` is treated as unrecorded on read; records without the field are grandfathered as the current version so existing users are not re-prompted by the introduction of the field itself. Addresses xieofxie's review on PR #371. --- src/winml/modelkit/telemetry/consent.py | 29 ++++++-- tests/unit/telemetry/test_consent.py | 89 ++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 5 deletions(-) diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py index 33395fbcf..40d2e01ed 100644 --- a/src/winml/modelkit/telemetry/consent.py +++ b/src/winml/modelkit/telemetry/consent.py @@ -38,6 +38,12 @@ def _resolve_user_home() -> str: _CONFIG_PATH: Path = Path(_resolve_user_home()) / ".modelkit" / "config.json" +# Bumped when the consent notice materially changes (new data category, +# changed scope). Stored records with an older version are treated as +# unrecorded on read so the user sees the updated notice and re-consents. +# Records predating this field are grandfathered as the current version. +_CONSENT_VERSION: int = 1 + _CI_ENV_VARS = ( "CI", "TF_BUILD", @@ -73,16 +79,30 @@ def _load_config() -> dict: def _read_stored_consent() -> Consent | None: """Return the stored consent value, or ``None`` if not recorded. - ``None`` is returned for missing / malformed / unknown values. + ``None`` is returned for missing / malformed / unknown values, or + when the stored ``consent_version`` is strictly older than the + current ``_CONSENT_VERSION`` (triggers a re-prompt after a notice + update). Records without a ``consent_version`` field are grandfathered + as the current version so introducing the field doesn't re-prompt + existing users. """ data = _load_config() tele = data.get("telemetry") if not isinstance(tele, dict): return None value = tele.get("consent") - if value in ("enabled", "disabled"): - return value # type: ignore[return-value] - return None + if value not in ("enabled", "disabled"): + return None + stored_version = tele.get("consent_version") + # `bool` is a subclass of `int` in Python; exclude explicitly so a + # stray `True` isn't silently interpreted as version 1. + if ( + isinstance(stored_version, int) + and not isinstance(stored_version, bool) + and stored_version < _CONSENT_VERSION + ): + return None + return value # type: ignore[return-value] def _write_stored_consent(value: Consent) -> None: @@ -94,6 +114,7 @@ def _write_stored_consent(value: Consent) -> None: data = _load_config() tele = data.get("telemetry") if isinstance(data.get("telemetry"), dict) else {} tele["consent"] = value + tele["consent_version"] = _CONSENT_VERSION data["telemetry"] = tele _CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/unit/telemetry/test_consent.py b/tests/unit/telemetry/test_consent.py index 8dbdf5f68..eba0220bf 100644 --- a/tests/unit/telemetry/test_consent.py +++ b/tests/unit/telemetry/test_consent.py @@ -52,7 +52,9 @@ def test_write_stored_consent_creates_file_and_roundtrips(isolated_config): def test_write_persists_nested_schema(isolated_config): consent._write_stored_consent("enabled") payload = json.loads(isolated_config.read_text()) - assert payload == {"telemetry": {"consent": "enabled"}} + assert payload == { + "telemetry": {"consent": "enabled", "consent_version": consent._CONSENT_VERSION} + } def test_read_preserves_unrelated_config_on_write(isolated_config): @@ -90,6 +92,73 @@ def test_read_malformed_json_returns_none(isolated_config): assert consent._read_stored_consent() is None +# --- consent_version ---------------------------------------------------- + + +def test_read_without_version_field_is_grandfathered(isolated_config): + # Records predating the version field must not trigger a re-prompt. + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text(json.dumps({"telemetry": {"consent": "enabled"}})) + assert consent._read_stored_consent() == "enabled" + + +def test_read_older_version_returns_none(isolated_config, monkeypatch): + monkeypatch.setattr(consent, "_CONSENT_VERSION", 2) + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps({"telemetry": {"consent": "enabled", "consent_version": 1}}) + ) + assert consent._read_stored_consent() is None + + +def test_read_same_version_honored(isolated_config): + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps( + { + "telemetry": { + "consent": "disabled", + "consent_version": consent._CONSENT_VERSION, + } + } + ) + ) + assert consent._read_stored_consent() == "disabled" + + +def test_read_newer_version_honored(isolated_config): + # A config written by a newer ModelKit (higher version) must be + # honored, not silently re-prompted. + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps( + { + "telemetry": { + "consent": "enabled", + "consent_version": consent._CONSENT_VERSION + 5, + } + } + ) + ) + assert consent._read_stored_consent() == "enabled" + + +def test_read_malformed_version_is_grandfathered(isolated_config): + # Non-int version (string, None, bool) is ignored rather than + # causing a re-prompt. + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps({"telemetry": {"consent": "enabled", "consent_version": "1"}}) + ) + assert consent._read_stored_consent() == "enabled" + + +def test_write_stamps_current_version(isolated_config): + consent._write_stored_consent("enabled") + payload = json.loads(isolated_config.read_text()) + assert payload["telemetry"]["consent_version"] == consent._CONSENT_VERSION + + # --- first-run prompt ---------------------------------------------------- @@ -161,3 +230,21 @@ def test_resolve_consent_first_run_empty_input_accepts_and_persists( monkeypatch.setattr("sys.stdin.isatty", lambda: True) assert consent.resolve_consent() == "enabled" assert consent._read_stored_consent() == "enabled" + + +def test_resolve_consent_reprompts_when_notice_version_bumped( + clean_env, isolated_config, monkeypatch +): + # Simulate an older stored decision + a newer notice version. The + # user should see the prompt again and the decision should be re-stamped. + isolated_config.parent.mkdir(parents=True, exist_ok=True) + isolated_config.write_text( + json.dumps({"telemetry": {"consent": "enabled", "consent_version": 1}}) + ) + monkeypatch.setattr(consent, "_CONSENT_VERSION", 2) + monkeypatch.setattr("sys.stdin", io.StringIO("n\n")) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + + assert consent.resolve_consent() == "disabled" + payload = json.loads(isolated_config.read_text()) + assert payload["telemetry"] == {"consent": "disabled", "consent_version": 2} From d7913e93fec783de6173a448a61090499807a649 Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Thu, 23 Apr 2026 11:46:07 +0800 Subject: [PATCH 6/7] refactor(telemetry): co-locate _PROMPT_TEXT with _CONSENT_VERSION MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two constants are a pair — version bumps when prompt text's scope changes — so keep them adjacent and document the coupling. --- src/winml/modelkit/telemetry/consent.py | 43 +++++++++++++------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/winml/modelkit/telemetry/consent.py b/src/winml/modelkit/telemetry/consent.py index 40d2e01ed..87bf2331d 100644 --- a/src/winml/modelkit/telemetry/consent.py +++ b/src/winml/modelkit/telemetry/consent.py @@ -38,12 +38,31 @@ def _resolve_user_home() -> str: _CONFIG_PATH: Path = Path(_resolve_user_home()) / ".modelkit" / "config.json" -# Bumped when the consent notice materially changes (new data category, -# changed scope). Stored records with an older version are treated as -# unrecorded on read so the user sees the updated notice and re-consents. -# Records predating this field are grandfathered as the current version. +# Consent notice version + text are a pair: bump the version whenever +# _PROMPT_TEXT's scope materially changes (new data category, widened +# scope). The prompt always describes the full current scope - it is +# NOT a delta vs. prior versions - so whatever vN's text lists is +# exactly what the user consents to when they answer. On a bump, +# stored records with an older version are treated as unrecorded on +# read so the user sees the updated notice and re-consents. Records +# predating the version field are grandfathered as the current version. _CONSENT_VERSION: int = 1 +_PROMPT_TEXT = """\ +ModelKit can collect anonymous usage data to help improve the product. + +What is collected: + - Command name, duration, success/failure + - Target device/EP (when the command specifies them) + - OS, architecture, ModelKit version + - Unhandled exception types, code locations, and scrubbed error + messages (paths trimmed, length capped, PII patterns scrubbed) + +What is never collected: + - File paths, model contents, command arguments, credentials + +Enable telemetry? [Y/n]: """ + _CI_ENV_VARS = ( "CI", "TF_BUILD", @@ -135,22 +154,6 @@ def _write_stored_consent(value: Consent) -> None: raise -_PROMPT_TEXT = """\ -ModelKit can collect anonymous usage data to help improve the product. - -What is collected: - - Command name, duration, success/failure - - Target device/EP (when the command specifies them) - - OS, architecture, ModelKit version - - Unhandled exception types, code locations, and scrubbed error - messages (paths trimmed, length capped, PII patterns scrubbed) - -What is never collected: - - File paths, model contents, command arguments, credentials - -Enable telemetry? [Y/n]: """ - - def _prompt_for_consent() -> Consent: """Show the first-run prompt and return the user's decision. From 08ba3af3044c780a76d4d2a45a5497450fd5b61d Mon Sep 17 00:00:00 2001 From: Zhipeng Wang Date: Fri, 24 Apr 2026 14:51:40 +0800 Subject: [PATCH 7/7] =?UTF-8?q?docs:=20refresh=20naming=20convention=20?= =?UTF-8?q?=E2=80=94=20drop=20stale=20violations,=20list=20all=20EPs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove Section 7 'Current Violations': all 9 classes already renamed. - Acronym table: fix HTP example (real classes), drop HF row (not a class prefix). - Add canonical EP table covering CPU, CUDA, DML, MIGraphX, NvTensorRTRTX, OpenVINO, QNN, VitisAI with ORT full names and casing notes. - Move HF_ constant prefix to a separate 'Other Canonical Identifiers' section. --- docs/naming-convention.md | 40 +++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/docs/naming-convention.md b/docs/naming-convention.md index c70d67edf..7a93fabe1 100644 --- a/docs/naming-convention.md +++ b/docs/naming-convention.md @@ -16,8 +16,7 @@ Domain acronyms in PascalCase class names **retain their uppercase form**, excep | QNN | Qualcomm Neural Network | `QNN` | `QNNMonitor` | | Op | Operator (2-letter prefix) | `Op` | `OpUnsupportedError` | | IO | Input/Output | `IO` | `IOConfigInfo` | -| HF | HuggingFace | `HF` | `HF_MODEL_CLASS_MAPPING` | -| HTP | Hexagon Tensor Processor | `HTP` | (directory/module level) | +| HTP | Hexagon Tensor Processor | `HTP` | `HTPConfig`, `HTPExporter`, `HTPMetadataBuilder` | ### Why `Op` Not `OP` @@ -29,6 +28,27 @@ Two-letter acronyms used as **class name prefixes** use PascalCase: All-caps is acceptable in **constants** (e.g., `SUPPORTED_OPS`). +### Canonical Execution Provider Names + +Execution providers appear mainly in constants, EP-name strings, and config keys rather than as class prefixes. Each EP has a fixed canonical short name (used in our code) and an ORT full name (the `*ExecutionProvider` symbol). + +| Short name | ORT full name | Device | Vendor / Notes | +|------------|---------------|--------|----------------| +| `CPU` | `CPUExecutionProvider` | CPU | Default fallback. | +| `CUDA` | `CUDAExecutionProvider` | GPU | NVIDIA. All caps. | +| `DML` | `DmlExecutionProvider` | GPU | DirectML. Use `DML` in our code; do not write `DirectML` as the EP name. | +| `MIGraphX` | `MIGraphXExecutionProvider` | GPU | AMD. Exact casing (mixed case). | +| `NvTensorRTRTX` | `NvTensorRTRTXExecutionProvider` | GPU | NVIDIA TensorRT-RTX. Exact casing; do not shorten to `TensorRT`. | +| `OpenVINO` | `OpenVINOExecutionProvider` | CPU / GPU / NPU | Intel. Exact casing. Alias: `ov`. | +| `QNN` | `QNNExecutionProvider` | NPU | Qualcomm. All caps. | +| `VitisAI` | `VitisAIExecutionProvider` | NPU | AMD Ryzen AI. Exact casing. Alias: `vitis`. | + +### Other Canonical Identifiers + +| Token | Meaning | Notes | +|-------|---------|-------| +| `HF_` | HuggingFace (constant/variable prefix) | e.g., `HF_MODEL_CLASS_MAPPING`, `HF_TASK_DEFAULTS`. Not used as a class prefix. | + ## 2. Module and Package Names Follow PEP 8: all lowercase with underscores. @@ -82,19 +102,3 @@ Known collisions to be aware of: | `utils` | `modelkit/utils/`, `analyze/utils/` | no shared content | | `pattern` | `modelkit/pattern/`, `analyze/pattern/` | active vs near-empty | | `inspect` | `modelkit/inspect/` | shadows Python stdlib | - -## 7. Current Violations - -The following classes violate the acronym naming rules and should be renamed: - -| Current | Correct | File | -|---|---|---| -| `OnnxOP` | `ONNXOp` | `src/winml/modelkit/analyze/models/onnx_op.py` | -| `OnnxConfigNotFoundError` | `ONNXConfigNotFoundError` | `src/winml/modelkit/export/io.py` | -| `OnnxModelOutput` | `ONNXModelOutput` | `src/winml/modelkit/export/htp/metadata_builder.py` | -| `EpContextNodeChecker` | `EPContextNodeChecker` | `src/winml/modelkit/analyze/core/node_checkers/ep_context_node_checker.py` | -| `EpPackage` | `EPPackage` | `src/winml/modelkit/sysinfo/software.py` | -| `QdqFixResult` | `QDQFixResult` | `src/winml/modelkit/quant/qdq_fix.py` | -| `OPOptionalInputSupportError` | `OpOptionalInputSupportError` | `src/winml/modelkit/analyze/exceptions.py` | -| `OPLackOfRequiredInformationError` | `OpLackOfRequiredInformationError` | `src/winml/modelkit/analyze/exceptions.py` | -| `OPUnsupportedError` | `OpUnsupportedError` | `src/winml/modelkit/analyze/exceptions.py` |