perf: consolidate six performance scan reports

docs/src/content/docs/contributing/development-guide.md

@@ -201,6 +201,15 @@ uv run pytest tests/unit -x -v
 to force serial execution. The default selection in `pyproject.toml` excludes
 `benchmark` and `live` tests.
 
+For performance changes, prefer deterministic operation-count regressions over
+wall-clock thresholds. Run the consolidated scan regressions with
+`uv run --extra dev pytest -q tests/unit/test_performance_scan_regressions.py`.
+They check deferred error-only imports, at most two indexed ZIP member lookups,
+and uninstall deduplication without comparing previously selected entries.
+Archive parsing and canonical dependency matching retain their existing costs.
+Deferring a direct import does not prove a cold-start speedup: other modules may
+still load the same dependency transitively.
+
 Without `uv`, use a standard Python venv and pip:
 
 ```bash

src/apm_cli/commands/marketplace/plugin/__init__.py

@@ -7,7 +7,6 @@
 from pathlib import Path
 
 import click
-import yaml
 
 from ....core.command_logger import CommandLogger
 from ....marketplace.errors import (
@@ -60,6 +59,8 @@ def _has_marketplace_block(apm_path: Path) -> bool:
     """Return True when *apm_path* has a populated ``marketplace:`` block."""
     if not apm_path.exists():
         return False
+    import yaml
+
     try:
         # Bounded loader so a hostile apm.yml cannot wedge this probe with a
         # merge/alias expansion bomb (fails closed as yaml.YAMLError).

src/apm_cli/commands/uninstall/engine.py

@@ -663,6 +663,7 @@ def _validate_uninstall_packages(
         )
 
     packages_to_remove = []
+    seen_entry_ids: set[int] = set()
     packages_not_found = []
 
     for package in packages:
@@ -702,7 +703,10 @@ def _validate_uninstall_packages(
         selection = select_manifest_dependency(canonical_for_match, current_deps, lockfile)
         if selection.status is DependencySelectionStatus.MATCHED:
             matched_dep = selection.manifest_entry
-            if matched_dep not in packages_to_remove:
+            # Selection returns the original manifest object, including unhashable mappings.
+            entry_id = id(matched_dep)
+            if entry_id not in seen_entry_ids:
+                seen_entry_ids.add(entry_id)
                 packages_to_remove.append(matched_dep)
             if not log_matches:
                 continue

src/apm_cli/commands/update.py

@@ -59,7 +59,6 @@
 from typing import TYPE_CHECKING, Any
 
 import click
-from git.exc import GitCommandError
 
 from ..core.auth import AuthResolver
 from ..core.command_logger import InstallLogger
@@ -156,6 +155,8 @@ def _resolve_and_stage_revision_pin_updates(
     against the new SHAs after the user's consent decision while dry-run and
     decline paths leave the original manifest model untouched.
     """
+    from git.exc import GitCommandError
+
     only_set = set(only_packages) if only_packages is not None else None
     logger.progress("Checking upstream for revision-pin freshness...", symbol="running")
 

src/apm_cli/deps/download_strategies.py

@@ -549,13 +549,14 @@ def download_file_from_artifactory(
                 if resp.status_code != 200:
                     continue
                 with zipfile.ZipFile(io.BytesIO(resp.content)) as zf:
-                    names = zf.namelist()
-                    root_prefix = names[0] if names else ""
-                    target_name = root_prefix + file_path
-                    if target_name in names:
-                        return zf.read(target_name)
-                    if file_path in names:
-                        return zf.read(file_path)
+                    entries = zf.infolist()
+                    root_prefix = entries[0].filename if entries else ""
+                    for name in (root_prefix + file_path, file_path):
+                        try:
+                            entry = zf.getinfo(name)
+                        except KeyError:
+                            continue
+                        return zf.read(entry)
             except (zipfile.BadZipFile, requests.RequestException):
                 continue
 

tests/unit/test_performance_scan_regressions.py

@@ -0,0 +1,261 @@
+"""Deterministic regressions for the consolidated October performance scans."""
+
+from __future__ import annotations
+
+import io
+import json
+import subprocess
+import sys
+import zipfile
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from apm_cli.commands.marketplace.plugin import _has_marketplace_block
+from apm_cli.commands.uninstall.engine import _validate_uninstall_packages
+from apm_cli.deps.download_strategies import DownloadDelegate
+from apm_cli.deps.lockfile import LockedDependency, LockFile
+from apm_cli.models.dependency.reference import DependencyReference
+from apm_cli.models.dependency.selection import DependencySelection, DependencySelectionStatus
+
+pytestmark = pytest.mark.component
+
+
+@pytest.mark.parametrize(
+    ("module", "dependency"),
+    [
+        ("apm_cli.commands.update", "git.exc"),
+        ("apm_cli.commands.marketplace.plugin", "yaml"),
+    ],
+)
+def test_command_import_defers_its_error_only_dependency(module: str, dependency: str) -> None:
+    """Check the reported import edge, not unrelated transitive imports."""
+    code = """
+import builtins
+import importlib
+import json
+import sys
+
+module, dependency = sys.argv[1:]
+original_import = builtins.__import__
+attempts = []
+def track_import(name, globals=None, locals=None, fromlist=(), level=0):
+    if name == dependency and globals and globals.get("__name__") == module:
+        attempts.append(name)
+    return original_import(name, globals, locals, fromlist, level)
+builtins.__import__ = track_import
+importlib.import_module(module)
+print(json.dumps(attempts))
+"""
+    result = subprocess.run(
+        [sys.executable, "-c", code, module, dependency],
+        capture_output=True,
+        text=True,
+        check=False,
+        timeout=30,
+    )
+    assert result.returncode == 0, result.stderr
+    assert json.loads(result.stdout) == []
+
+
+@pytest.mark.parametrize(
+    ("content", "expected"),
+    [
+        ("marketplace:\n  name: example\n", True),
+        ("marketplace: {}\n", True),
+        ("marketplace: null\n", False),
+        ("name: example\n", False),
+        ("", False),
+        ("- marketplace\n", False),
+        ("marketplace: [\n", False),
+    ],
+)
+def test_marketplace_probe_retains_yaml_behavior(
+    tmp_path: Path, content: str, expected: bool
+) -> None:
+    manifest = tmp_path / "apm.yml"
+    manifest.write_text(content, encoding="utf-8")
+    assert _has_marketplace_block(manifest) is expected
+
+
+def test_marketplace_probe_retains_missing_and_unreadable_behavior(tmp_path: Path) -> None:
+    manifest = tmp_path / "apm.yml"
+    assert _has_marketplace_block(manifest) is False
+    manifest.write_text("marketplace: {}\n", encoding="utf-8")
+    with patch("apm_cli.utils.yaml_io.load_yaml", side_effect=OSError("permission denied")):
+        assert _has_marketplace_block(manifest) is False
+
+
+def _archive(entries: dict[str, bytes]) -> bytes:
+    """Build real ZIP bytes without touching disk or a remote registry."""
+    buffer = io.BytesIO()
+    with zipfile.ZipFile(buffer, "w") as archive:
+        for name, content in entries.items():
+            archive.writestr(name, content)
+    return buffer.getvalue()
+
+
+@pytest.mark.parametrize("unrelated_count", [0, 1000])
+@pytest.mark.parametrize(
+    ("entries", "expected", "lookups"),
+    [
+        ({"repo/": b"", "repo/apm.yml": b"prefixed", "apm.yml": b"bare"}, b"prefixed", 1),
+        ({"repo/": b"", "apm.yml": b"bare"}, b"bare", 2),
+        ({"apm.yml": b"bare"}, b"bare", 2),
+        ({}, None, 2),
+    ],
+)
+def test_archive_member_lookup_is_bounded_and_preserves_precedence(
+    entries: dict[str, bytes], expected: bytes | None, lookups: int, unrelated_count: int
+) -> None:
+    """No namelist allocation or linear scan, regardless of archive size."""
+    archive_bytes = _archive(
+        {**entries, **{f"other/{index}.txt": b"filler" for index in range(unrelated_count)}}
+    )
+    host = SimpleNamespace(
+        registry_config=None,
+        artifactory_token=None,
+        _resilient_get=MagicMock(
+            return_value=SimpleNamespace(status_code=200, content=archive_bytes)
+        ),
+    )
+    original_getinfo = zipfile.ZipFile.getinfo
+    queried_names: list[str] = []
+
+    def getinfo(archive: zipfile.ZipFile, name: str) -> zipfile.ZipInfo:
+        queried_names.append(name)
+        return original_getinfo(archive, name)
+
+    with (
+        patch("apm_cli.deps.artifactory_entry.fetch_entry_from_archive", return_value=None),
+        patch(
+            "apm_cli.deps.download_strategies.build_artifactory_archive_url",
+            return_value=["https://registry.example/archive.zip"],
+        ),
+        patch.object(zipfile.ZipFile, "namelist", side_effect=AssertionError("linear scan")),
+        patch.object(zipfile.ZipFile, "getinfo", getinfo),
+    ):
+        delegate = DownloadDelegate(host)
+        if expected is None:
+            with pytest.raises(RuntimeError, match="Failed to download file"):
+                delegate.download_file_from_artifactory(
+                    "registry.example", "apm", "owner", "repo", "apm.yml", "main"
+                )
+        else:
+            assert (
+                delegate.download_file_from_artifactory(
+                    "registry.example", "apm", "owner", "repo", "apm.yml", "main"
+                )
+                == expected
+            )
+    assert len(queried_names) == lookups
+    assert queried_names[-1] == ("repo/apm.yml" if lookups == 1 else "apm.yml")
+
+
+@pytest.mark.parametrize(
+    "first_content",
+    [b"not a zip", _archive({}), _archive({"other.yml": b"other"})],
+    ids=["invalid-zip", "empty-zip", "missing-member"],
+)
+def test_archive_missing_member_continues_to_next_candidate(first_content: bytes) -> None:
+    host = SimpleNamespace(
+        registry_config=None,
+        artifactory_token=None,
+        _resilient_get=MagicMock(
+            side_effect=[
+                SimpleNamespace(status_code=200, content=first_content),
+                SimpleNamespace(status_code=200, content=_archive({"apm.yml": b"found"})),
+            ]
+        ),
+    )
+    with (
+        patch("apm_cli.deps.artifactory_entry.fetch_entry_from_archive", return_value=None),
+        patch(
+            "apm_cli.deps.download_strategies.build_artifactory_archive_url",
+            return_value=[
+                "https://registry.example/first.zip",
+                "https://registry.example/second.zip",
+            ],
+        ),
+    ):
+        result = DownloadDelegate(host).download_file_from_artifactory(
+            "registry.example", "apm", "owner", "repo", "apm.yml", "main"
+        )
+    assert result == b"found"
+    assert host._resilient_get.call_count == 2
+
+
+@pytest.mark.parametrize("entry_kind", ["string", "mapping", "model"])
+@pytest.mark.parametrize("log_matches", [False, True])
+def test_uninstall_deduplication_retains_manifest_objects_and_argument_order(
+    entry_kind: str, log_matches: bool
+) -> None:
+    declarations = ["org/first", "org/second"]
+    if entry_kind == "mapping":
+        entries = [{"git": name, "ref": "main"} for name in declarations]
+    elif entry_kind == "model":
+        entries = [DependencyReference.parse(name) for name in declarations]
+    else:
+        entries = declarations
+    logger = MagicMock()
+    removed, missing = _validate_uninstall_packages(
+        ["org/second", "org/first", "org/second", "org/missing"],
+        entries,
+        logger,
+        log_matches=log_matches,
+    )
+    assert len(removed) == 2
+    assert removed[0] is entries[1]
+    assert removed[1] is entries[0]
+    assert missing == ["org/missing"]
+    assert logger.progress.call_count == (3 if log_matches else 0)
+
+
+def test_uninstall_alias_and_exact_path_remove_one_unhashable_entry() -> None:
+    entry = {"path": "../packages/example"}
+    locked = LockedDependency(
+        repo_url="_local/example", source="local", local_path="../packages/example"
+    )
+    lockfile = LockFile(dependencies={locked.get_unique_key(): locked})
+    removed, missing = _validate_uninstall_packages(
+        ["_local/example", "../packages/example", "_local/example"],
+        [entry],
+        MagicMock(),
+        lockfile=lockfile,
+    )
+    assert len(removed) == 1
+    assert removed[0] is entry
+    assert missing == []
+
+
+@pytest.mark.parametrize("count", [10, 1000])
+def test_uninstall_deduplication_does_not_compare_previously_selected_entries(count: int) -> None:
+    """Isolate deduplication cost from the unchanged canonical selection cost."""
+    comparisons = 0
+
+    class CountedEntry(dict):
+        def __eq__(self, other: object) -> bool:
+            nonlocal comparisons
+            comparisons += 1
+            return super().__eq__(other)
+
+    entries = [CountedEntry(git=f"org/package-{index}") for index in range(count)]
+    selections = [
+        DependencySelection(DependencySelectionStatus.MATCHED, entry, 1) for entry in entries
+    ]
+    with patch(
+        "apm_cli.commands.uninstall.engine.select_manifest_dependency",
+        side_effect=selections + selections,
+    ):
+        removed, missing = _validate_uninstall_packages(
+            [entry["git"] for entry in entries] * 2,
+            entries,
+            MagicMock(),
+            log_matches=False,
+        )
+    assert len(removed) == count
+    assert all(actual is expected for actual, expected in zip(removed, entries, strict=True))
+    assert missing == []
+    assert comparisons == 0