[Daniel Meppiel (danielmeppiel)]

Description

fix(perf): consolidate reported import and lookup improvements

TL;DR

Address the four distinct findings repeated across all six open "performance opportunities found" reports. Defer two error-only imports, use the ZIP name index instead of allocating/scanning a name list, and deduplicate uninstall selections without comparing previous entries. Add 30 deterministic regression cases while preserving selection order, aliases, fallback behavior, and diagnostics.

Note

The import reports overstate startup impact: update/marketplace commands are already lazily registered, and other imports still load GitPython/PyYAML. This PR fixes the reported direct imports; it does not claim a measured cold-start speedup or make the entire uninstall/download operation constant-time.

Problem (WHY)

• The uninstall deduplication loop compares each selected entry against a growing list. The new regression reproduced 999,000 equality comparisons for 1,000 selected entries requested twice.
• Single-file archive fallback allocates namelist() and performs up to two linear membership scans despite ZipFile already indexing members.
• [!] GitCommandError and yaml are directly imported when their command modules load, although these references are only needed inside specific helpers.

The regression-first approach follows Agent Skills' validation loop: "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes.". The observations above are reproduced by tests rather than inferred wall-clock gains.

Approach (WHAT)

• Resolve archive members through getinfo() while retaining prefixed-before-bare precedence.
• Keep the ordered uninstall result list and a companion set of selected object IDs.
• Preserve the canonical selector as the authority for matching and ambiguity.
• Move the two direct error-only imports into the helpers that need them.
• Use operation counts and real ZIP bytes, not timing thresholds or live registry calls.

Implementation (HOW)

File	Change and boundary
src/apm_cli/deps/download_strategies.py	Reuse the existing member list for the first entry and perform at most two indexed lookups. Missing members continue to the next candidate; ZIP read errors retain existing handling.
src/apm_cli/commands/uninstall/engine.py	Deduplicate the original objects returned by the selector, including unhashable manifest mappings; preserve first-request order and repeated-match diagnostics.
src/apm_cli/commands/update.py	Import GitCommandError inside revision-pin resolution; retain exception rendering and exit codes.
src/apm_cli/commands/marketplace/plugin/__init__.py	Import yaml only when probing an existing manifest; retain the bounded YAML loader.
tests/unit/test_performance_scan_regressions.py	Add import-edge, ZIP lookup-count, fallback, YAML, ordering, alias, and deduplication regression cases.
docs/src/content/docs/contributing/development-guide.md	Document the focused command and the limits of operation-count/import-edge evidence.

Diagram

Dashed nodes show the new indexed lookups; archive retrieval, central-directory parsing, and fallback order remain unchanged.

flowchart LR
    subgraph Archive["Full-archive fallback"]
        A["HTTP archive bytes"] --> B["ZipFile parses central directory"]
    end
    subgraph Lookup["Indexed member selection"]
        B --> C["getinfo: prefixed file path"]
        C -->|"missing"| D["getinfo: bare file path"]
    end
    C -->|"found"| E["read ZipInfo and return bytes"]
    D -->|"found"| E
    D -->|"missing"| F["Try next archive URL"]
    classDef new stroke-dasharray: 5 5;
    class C,D new;

Loading

Trade-offs

• Object identity is invocation-local: the canonical selector returns original manifest entries retained in current_deps. This avoids reparsing identity, hashing dictionaries, or introducing a second matching authority.
• Archive parsing still scales with archive size, and canonical manifest selection still scans declarations. Only the redundant lookup/deduplication work is removed.
• Import tests guard the reported direct dependency edges, not the absence of these libraries from sys.modules; broader startup restructuring is outside these reports.
• Download tests use mocked HTTP with real ZIP bytes. No live registry benchmark, complete repository test run, or remote CI success is claimed.

Benefits

1. Zero equality comparisons against previously selected uninstall entries in the 10- and 1,000-entry regression cases.
2. At most two indexed member lookups per archive, with no namelist() allocation, including archives with 1,000 unrelated entries.
3. Zero direct imports of the two reported error-only dependencies during command-module loading.
4. Thirty regression cases preserve behavior and bound the optimized work.

Issue and approved scope

Issue: #3365 is the canonical aggregate reference; the six open matching reports are all completed by this PR.

Human scope-approval comment: none exists on these issues. Daniel Meppiel (@danielmeppiel) explicitly requested this consolidated implementation and tested PR in the originating Copilot session. This records that direct request, not a fabricated issue approval or an inference from automation labels.

Bounded scope: the four distinct findings in these six reports, regression tests, and directly related documentation. Done when all four are addressed and the affected tests and local lint mirror pass. Excluded: unrelated performance issues, a CLI-wide import redesign, transport/authentication changes, and end-to-end latency claims. Review contact: Daniel Meppiel (@danielmeppiel); review availability is not asserted.

Fixes #3264
Fixes #3269
Fixes #3287
Fixes #3306
Fixes #3340
Fixes #3365

Reports	Completed finding
#3264, #3287, #3340, #3365	Deferred GitCommandError import
#3264, #3340, #3365	Deferred yaml import
#3269	Ordered, constant-time membership for uninstall deduplication
#3306	Indexed ZIP member selection

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

Validation

The full repository suite was not run. The selected Python suites passed 660 tests, and the docs link-checker suite passed 14 tests. Before the source fixes, the new regression file produced 12 failed, 18 passed; afterward all 30 cases passed. Existing pytest temporary-directory cleanup warnings were emitted after successful runs.

Exact commands and results

uv run --frozen --extra dev pytest -q tests/unit/test_performance_scan_regressions.py --tb=short

30 passed in 1.53s

uv run --extra dev pytest -q tests/unit/test_performance_scan_regressions.py tests/unit/commands/test_update_command.py tests/unit/commands/test_marketplace_plugin.py tests/unit/deps/test_download_strategies_phase3.py tests/unit/test_uninstall_engine_validation.py tests/unit/commands/test_uninstall_atomic_selection.py tests/unit/models/test_dependency_uninstall_selection.py tests/unit/test_cli_lazy_dispatch.py --tb=short

330 passed in 9.44s

uv run --extra dev pytest -q tests/integration/test_download_strategies_phase3w5.py tests/integration/test_uninstall_marketplace.py tests/integration/test_marketplace_plugin_integration.py tests/unit/test_uninstall_engine_helpers.py tests/unit/test_uninstall_engine_phase3w5.py --tb=short

169 passed in 7.92s

uv run --extra dev pytest -p no:cacheprovider -q tests/quality --tb=short

161 passed in 111.51s (0:01:51)

uv run --frozen python scripts/check_test_assertions.py

[+] assertion-quality ratchet clean: AQ001=4, AQ002=12

uv run --frozen python scripts/check_exact_test_duplicates.py

[+] exact test duplicate ratchet clean: 1415 files, 0 allowed duplicate group(s)

uv run --extra dev ruff check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py

All checks passed!

uv run --extra dev ruff format --check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py

2092 files already formatted

uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py

Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

bash scripts/lint-auth-signals.sh — passed, exit 0 ([+] auth-signal lint clean).

bash scripts/lint-architecture-boundaries.sh — passed, exit 0 with no output.

The YAML I/O, 2,100-line, and portable-relative-path CI guards also passed using the same patterns over their configured paths (Python equivalent of GNU grep on macOS). No dependency changes are included.

npm --prefix docs run test:links — passed: 14 tests, 0 failures. Its negative fixtures intentionally print a broken-link diagnostic; this is not a generated-site link audit.

mmdc -i zip-lookup.mmd -o zip-lookup.svg --quiet — passed, exit 0 (session artifact directory).

git diff --check — passed, exit 0.

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	Fetch the same file whether an archive uses a root directory or bare paths; continue to another candidate when missing	Vendor-neutral	tests/unit/test_performance_scan_regressions.py::test_archive_member_lookup_is_bounded_and_preserves_precedence (regression-trap for #3306); ::test_archive_missing_member_continues_to_next_candidate	component
2	Repeat package identifiers or local aliases without duplicate removal or changed ordering	DevX (pragmatic as npm)	tests/unit/test_performance_scan_regressions.py::test_uninstall_deduplication_does_not_compare_previously_selected_entries (regression-trap for #3269); ::test_uninstall_alias_and_exact_path_remove_one_unhashable_entry; tests/unit/commands/test_uninstall_atomic_selection.py::test_duplicate_identifier_removes_manifest_entry_once	component
3	Keep marketplace package operations and update failure diagnostics working after import deferral	DevX (pragmatic as npm)	tests/unit/test_performance_scan_regressions.py::test_command_import_defers_its_error_only_dependency; ::test_marketplace_probe_retains_yaml_behavior; tests/unit/commands/test_update_command.py::TestRevisionPinResolutionErrors; tests/integration/test_marketplace_plugin_integration.py	component / integration

How to test

• Run uv run --frozen --extra dev pytest -q tests/unit/test_performance_scan_regressions.py; expect 30 passes.
• Run the affected-command and related integration commands above; expect 330 and 169 passes.
• Run the quality and lint commands above; expect no gate failures.

Spec conformance (OpenAPM v0.1)

If this PR changes behaviour that an OpenAPM v0.1 req-XXX covers,
confirm the three-step ritual in the
development guide:

• Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
  (new/changed <a id="req-XXX"></a> anchor + prose + Appendix C
  row).
• Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml
  updated.
• Test edit: a @pytest.mark.req("req-XXX") test under
  tests/spec_conformance/ added or extended.
• CONFORMANCE.{md,json} regenerated via
  uv run --extra dev python -m tests.spec_conformance.gen_statement
  and committed.
• N/A -- this PR does not change OpenAPM-observable behaviour.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Daniel Meppiel (danielmeppiel)]

Docs sync advisory

Verdict: no_change | Pages requiring changes: 0 | Classifier tasks: 1/15 (unavailable; direct L1 fallback)

No additional documentation impact detected. The changes preserve commands, flags, schemas, diagnostics, and package-selection behavior. The development guide already documents the deterministic regression command and explicitly limits the performance claims. No README or apm-usage reference updates are needed.

The classifier task lacked file tools; the orchestrator completed the docs-index and source-diff checks directly. No panel was spawned.

---

Generated by docs-sync. This comment is AI-generated and may contain errors.

[Copilot]

🟢 Approval recommended

The changes are localized, preserve existing behavior, and have focused regression coverage with no identified blocking issues.

No open issues 🎉

What changed in this PR

Consolidates four performance improvements reported across six scan issues, removing redundant work without changing command behavior or claiming measured startup gains.

Changes:

• Defer two helper-only imports in update and marketplace commands.
• Use indexed ZIP lookups and identity-based uninstall deduplication while preserving precedence and selection order.
• Add 30 deterministic regression cases and document their scope and limitations.

File	Description
tests/​unit/​test_performance_scan_regressions.py	Adds import, lookup-count, fallback, and selection regressions.
src/​apm_cli/​deps/​download_strategies.py	Replaces ZIP name-list scans with indexed member lookups.
src/​apm_cli/​commands/​update.py	Defers GitCommandError import to revision-pin resolution.
src/​apm_cli/​commands/​uninstall/​engine.py	Deduplicates selected objects using an ID set while retaining order.
src/​apm_cli/​commands/​marketplace/​plugin/​__init__.py	Defers the YAML import until an existing manifest is probed.
docs/​src/​content/​docs/​contributing/​development-guide.md	Documents deterministic performance tests and evidence limits.

_{Review effort: Balanced}