perf(scaling-guards): add variant-key and cache-lookup guards, wire to CI integration gate

[sergio-sisternes-epam]

Description

Add 15 hermetic scaling guards covering several hot paths in the CLI, wire them into the CI integration gate, and fix an O(n^2) hook deduplication regression discovered during baseline measurement.

Each guard compares wall-clock time at two input sizes and asserts the ratio stays below a calibrated threshold, catching algorithmic regressions at merge-queue time without full benchmarking infrastructure.

Production fix: O(n^2) hook deduplication

Baseline measurement of guard 10 revealed a 64x ratio at 50->500 entries -- confirming an O(n^2) regression in _integrate_merged_hooks. Two fixes applied to hook_integrator.py:

• _reinject_apm_source_from_sidecar: replaced O(n*m) list.pop() scan with O(n) dict-based lookup using json.dumps(sort_keys=True) for canonical hashing
• _integrate_merged_hooks dedup: replaced O(n^2) list scan with O(n) set-based lookup; also fixed a duplicated append() call that was doubling every entry

Calibrated guard table

#	Guard	Small	Large	Growth	Complexity	Limit	Theoretical	Margin
1	ChildrenIndex	50	500	10x	O(n)	15	10.0x	50%
2	Discovery	100	1000	10x	O(n)	14	10.0x	40%
3	ConsoleSingleton	1000	10000	10x	O(1)	15	1.0x	amortised
4	ComputePackageHash	50	500	10x	O(n)	16	10.0x	60%
5	SemanticEquivalence	50	500	10x	O(n)	25	10.0x	150%
6	ShouldExclude	50	500	10x	sub-quadratic	2	~1x	~100%
7	VariantKey	200	2000	10x	O(n log n)	25	14.3x	74%
8	VariantLookup	50	500	10x	O(1)	2	1.0x	100%
9	SidecarReinjection	50	500	10x	O(n)	15	10.0x	50%
10	HookDedup	50	500	10x	O(n)	15	10.0x	50%
11	FlattenDependencies	50	500	10x	O(n log n)	21	15.9x	32%
12	JsonKeyDiff	100	1000	10x	O(n log n)	22	15.0x	47%
13	RenderInstructions	50	500	10x	O(n log n)	21	15.9x	32%
14	GetInstalledPaths	50	500	10x	O(n log n)	21	15.9x	32%
15	RewriteHooksData	50	1000	20x	O(E * M)	30	20.0x	50%

Hot paths covered (guards 9-15)

Guard	Hot path	Why it matters
9	_reinject_apm_source_from_sidecar	Runs on every install for schema-strict targets (Claude, Gemini). Was O(n*m) with list pool scan.
10	Hook entry dedup in _integrate_merged_hooks	Deduplicates same-package entries on every hook integration. Was O(n^2) with list scan -- confirmed at 64x ratio.
11	flatten_dependencies (BFS tree walk)	Core install-path function. BFS with per-level sort gives O(n log n).
12	json_key_diff	Drift detection on every config write. Sorted-union of keys gives O(n log n).
13	render_instructions_block	Compilation hot path. Groups instructions by apply_to pattern with sorting.
14	LockFile.get_installed_paths	Called during install/uninstall to resolve on-disk locations. Iterates all locked deps.
15	_rewrite_hooks_data	Path rewriting across all hook entries x matchers. Linear in total work (E * M).

CI wiring

The scaling-guards job runs hermetically (no binary, no secrets, no network) in ci-integration.yml and is wired into the fan-in gate alongside integration test shards.

Changes

• tests/benchmarks/test_scaling_guards.py -- 15 scaling guards + 1 correctness test (16 total)
• src/apm_cli/integration/hook_integrator.py -- O(n^2) -> O(n) dedup fix + sidecar reinject fix
• .github/workflows/ci-integration.yml -- scaling-guards job + fan-in wiring

Validation

• All 16 scaling guard tests pass locally
• All 156 hook integrator unit tests pass (no regressions from production fix)
• Lint clean (ruff check + ruff format --check)

[Copilot]

Pull request overview

Adds new performance regression guards for the sparse-cone checkout “variant” code paths (introduced in #1436) and wires them into the merge-queue Tier 2 integration workflow so algorithmic regressions are caught before auto-merge.

Changes:

• Add two new scaling guards covering _variant_key() complexity and variant directory existence lookup complexity.
• Add a new scaling-guards job to the merge-queue integration workflow and make the existing fan-in Integration Tests (Linux) job fail if scaling guards fail.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/benchmarks/test_scaling_guards.py	Adds Guard 7 (variant key scaling + canonicalization) and Guard 8 (variant directory lookup scaling).
.github/workflows/ci-integration.yml	Introduces a new Scaling Guards (Linux) job and aggregates its result into the existing integration gate job.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

[github-actions]

APM Review Panel: ship_with_followups

O(n^2)->O(n) production fixes are correct and ship-ready; Guard 10 tests a local copy not the production path -- close the divergence gap as a followup.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

This PR delivers two confirmed algorithmic fixes: a 64x-ratio O(n^2)->O(n) regression in hook deduplication and an O(n*m)->O(n) fix in sidecar reinject for Claude/Gemini targets. Both fixes are structurally sound, and the 15-guard CI harness correctly enforces wall-clock ratio thresholds on the production functions. The wiring of scaling-guards into the integration-tests fan-in gate means future regressions cannot merge silently. This is exactly the engineering-rigor signal APM needs at this stage of community trust-building.

The one finding that warrants tracking -- not blocking -- is the Guard 10 shadow-copy problem. Both python-architect and test-coverage-expert independently flagged that guard 10 defines _dedup_hook_entries inline in the test file rather than importing the production helper. The guard proves O(n) complexity of a local copy, not the production path. However, production-path regression coverage does exist via test_hook_integrator.py::test_content_dedup_same_package (asserts len==1 after dedup), so there is no uncovered regression window today. The risk is maintenance drift: the test-local copy can silently diverge from the production implementation on any future edit to _integrate_merged_hooks. The fix is a single extraction into a module-level helper in hook_integrator.py -- a low-cost refactor that closes the gap permanently and is the correct followup action before the next feature PR touches that code path.

The oss-growth-hacker growth signal is concrete and actionable: a 64x confirmed regression fix on every apm hook integration call is enterprise-evaluator proof content and should appear in CHANGELOG under a 'Performance' section. The PR title buries the lede (variant-key and cache-lookup guards) while the headline value is the production fix. This is a CHANGELOG and release-note framing call, not a code change -- low cost, high conversion yield.

Dissent. No cross-panelist disagreements on severity classification. python-architect and test-coverage-expert converged independently on the Guard 10 shadow-copy finding; both classified it recommended (not blocking), consistent with production-path coverage existing via test_hook_integrator.py. test-coverage-expert finding 2 carries outcome=unknown on the integration-tier duplicate-append trap -- per arbitration policy, unknown carries no weight and is downweighted to opinion only.

Aligned with: governed_by_policy -- the scaling-guards CI gate means algorithmic regressions can no longer merge without tripping the fan-in gate; multi_harness_multi_host -- O(n*m)->O(n) sidecar-reinject fix benefits every Claude and Gemini install; oss_community_driven -- 15 hermetic guards lower the bar for external contributors to detect perf regressions without understanding internals; pragmatic_as_npm -- a 64x regression caught and fixed proactively, with a CI harness preventing recurrence, is the 'works reliably at scale' pragmatism APM must demonstrate.

Growth signal. Add a CHANGELOG 'Performance' section entry for the 64x hook-dedup fix and the O(n*m)->O(n) sidecar-reinject fix. Frame the scaling-guard CI harness as APM's automated performance contract -- a differentiator no incumbent package manager ships. One sentence in the next release post: 'APM now enforces algorithmic complexity bounds in CI; a 64x O(n^2) regression in hook deduplication was caught and fixed before reaching any user.'

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	O(n)->O(1) fixes are correct; 2 latent-import antipatterns; guard 10 tests a test-local copy, not production code -- coverage gap.
DevX UX Expert	0	0	1	No CLI surface changes; perf fix is invisible to users. One nit: CI job name could hint at user impact for changelog readers.
Supply Chain Security	0	0	1	No new supply-chain surface introduced; CI actions follow existing tag-pin pattern already present in the workflow file.
OSS Growth Hacker	0	1	1	Perf infra + silent O(n^2) fix: strong engineering signal, zero conversion surface touched. One story beat worth surfacing in CHANGELOG.
Test Coverage Expert	0	2	1	Duplicate-append bug has regression trap in test_content_dedup_same_package; Guard 10 covers a standalone helper not the production path, but production-path integration tests exist.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

1. [Python Architect + Test Coverage Expert] Extract the inline dedup logic from _integrate_merged_hooks into a named module-level helper in hook_integrator.py; update guard 10 to import and exercise that helper rather than a test-local copy. -- Guard 10 currently proves O(n) complexity on a shadow copy. If production dedup regresses, guard 10 still passes. Evidence=missing on the integration tier for this guard path. One extraction closes the divergence permanently.
2. [OSS Growth Hacker] Add a CHANGELOG 'Performance' section documenting the 64x O(n^2)->O(n) hook-dedup fix and O(n*m)->O(n) sidecar-reinject fix with the measured ratios. -- Confirmed algorithmic fixes at this magnitude are enterprise-evaluator proof content. Invisible today; one paragraph captures the signal permanently and feeds release post copy.
3. [Python Architect] Remove redundant function-level import json and from collections import deque inside _reinject_apm_source_from_sidecar and _integrate_merged_hooks; use module-level imports throughout. -- Redundant imports in hot loops add minor overhead and create misleading signals for future readers about whether the module-level import is safe to remove.
4. [Test Coverage Expert] Add an integration-tier regression test that exercises the old double-append symptom path through the full install flow, not just the unit-tier len==1 assertion. -- The unit trap exists but a full-flow integration test would survive refactors that break the unit boundary.
5. [DevX UX Expert] Rename CI job from 'Scaling Guards (Linux)' to 'Perf Scaling Guards (Linux)' for contributor discoverability in workflow logs. -- Low-cost, self-documenting change that communicates intent to external contributors scanning CI output.

Architecture

classDiagram
    direction LR
    class BaseIntegrator {
      <<Abstract>>
      +find_hook_files(path) list
      +_hook_entry_content_key(entry) str
    }
    class HookIntegrator {
      <<ConcreteIntegrator>>
      +_integrate_merged_hooks(config, ...) HookIntegrationResult
      +_should_remove_prior_merged_entry(...) bool
      +_rewrite_hooks_data(...) tuple
    }
    class HookIntegrationResult {
      <<ValueObject>>
      +files_integrated int
      +files_updated int
      +files_skipped int
      +target_paths list
    }
    class IntegrationResult {
      <<ValueObject>>
      +files_integrated int
    }
    class _MergeHookConfig {
      <<ValueObject>>
      +target_key str
      +config_filename str
      +schema_strict bool
      +require_dir bool
    }
    class _reinject_apm_source_from_sidecar {
      <<Pure>>
      +__call__(hooks, sidecar_data) None
    }
    class _dedup_hook_entries_testlocal {
      <<Pure>>
      note: defined only in test file not imported from src
    }
    BaseIntegrator <|-- HookIntegrator
    IntegrationResult <|-- HookIntegrationResult
    HookIntegrator ..> HookIntegrationResult : returns
    HookIntegrator ..> _MergeHookConfig : reads
    HookIntegrator ..> _reinject_apm_source_from_sidecar : calls
    class HookIntegrator:::touched
    class _reinject_apm_source_from_sidecar:::touched
    class _dedup_hook_entries_testlocal:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A[apm install CLI entry] --> B[HookIntegrator._integrate_merged_hooks]
    B --> C{sidecar file exists?}
    C -- yes --> D[read sidecar JSON]
    D --> E[_reinject_apm_source_from_sidecar]
    E --> E1[Build pool: dict-of-deques O-n]
    E1 --> E2[Match disk entries via popleft O-n]
    C -- no --> F[per hook_file loop]
    E2 --> F
    F --> G[idempotent upsert: strip prior entries]
    G --> H[extend event list with fresh entries]
    H --> I[inline dedup: set-based seen_keys O-n]
    I --> J[write merged JSON config]
    J --> K[copy referenced scripts]
    K --> L[return HookIntegrationResult]
    style E1 fill:#d4edda
    style E2 fill:#d4edda
    style I fill:#fff3b0

Loading

Recommendation

Ship. The production algorithmic fixes are correct, the CI gate is wired, and existing integration tests provide regression coverage on the dedup path today. The Guard 10 shadow-copy issue is real maintenance debt and should be resolved before the next PR modifies _integrate_merged_hooks -- open a followup issue and assign it before merge. The CHANGELOG entry for the 64x fix is a one-paragraph addition that should land in this PR or immediately after; do not let this signal ship dark. All other findings are nits with no correctness impact.

---

Full per-persona findings

Python Architect

• [recommended] Guard 10 (HookDedup) tests a test-local copy of the dedup logic, not the production code path at tests/benchmarks/test_scaling_guards.py:523
  The guard defines _dedup_hook_entries in test_scaling_guards.py as a verbatim copy of the inline dedup logic from _integrate_merged_hooks. If the production dedup regresses (e.g. reverts to a list scan), guard 10 still passes because it exercises the local copy, not the source. Extracting the dedup into a module-level function in hook_integrator.py and importing it in both the call site and the test would close this gap with one refactor.
  Suggested: Extract the dedup block into a module-level helper _dedup_hook_entries(entries) in hook_integrator.py. Import it in the guard test instead of redefining it locally.

• [nit] Function-level import json / from collections import deque inside _reinject_apm_source_from_sidecar are redundant at src/apm_cli/integration/hook_integrator.py:216
json is already imported at module top (line 46). The function-level imports add a dict-lookup overhead on every call and mislead readers into thinking these are lazy imports for circular-dependency reasons.
  Suggested: Move from collections import deque to the module-level import block. Remove the redundant import json inside the function body.

• [nit] import json as _json inside _integrate_merged_hooks hot loop is redundant at src/apm_cli/integration/hook_integrator.py:1190
json is already available at module scope. The inline alias inside a hot per-event loop adds unnecessary overhead.
  Suggested: Remove import json as _json; replace _json.dumps with json.dumps (already in scope).

CLI Logging Expert -- inactive

No CLI output, logging, or user-facing message changes in this PR. No findings from cli-logging-expert.

DevX UX Expert

• [nit] CI job name 'Scaling Guards (Linux)' is opaque to external contributors scanning workflow logs at .github/workflows/ci-integration.yml
  Developers reading CI output see 'Scaling Guards (Linux)' with no hint that this guards install-time performance. A self-documenting label costs nothing.
  Suggested: name: Perf Scaling Guards (Linux)

Supply Chain Security

• [nit] GitHub Actions not SHA-pinned (pre-existing repo-wide pattern) at .github/workflows/ci-integration.yml
  The new scaling-guards job uses mutable tags (@v4/@v5/@v6) consistent with the rest of the file. A compromised tag could inject malicious code into CI. This is pre-existing and not introduced by this PR.
  Suggested: Pin all action refs to full commit SHAs and track updates via Dependabot or Renovate. Address repo-wide as a separate hardening PR.

OSS Growth Hacker

• [recommended] 64x hook-dedup fix deserves a CHANGELOG entry and a story beat -- it is invisible today
  A confirmed O(n^2)->O(n) regression fix at 64x measured ratio on a hot path (every hook integration) is exactly the kind of proof point that converts skeptical power users into advocates. It is currently buried in PR body prose. Adding a CHANGELOG entry under a 'Performance' section costs one line and pays forward into release notes, social copy, and the 'why apm is fast' narrative. Silent perf fixes are the most common form of growth waste in OSS projects.

• [nit] PR title mentions 'variant-key and cache-lookup guards' but the headline value is the O(n^2) production fix -- consider reordering the story for future release-note drafting.

Auth Expert -- inactive

Changed files (.github/workflows/ci-integration.yml, src/apm_cli/integration/hook_integrator.py, tests/benchmarks/test_scaling_guards.py) do not touch any auth trigger paths; PR is a performance fix for hook deduplication with no credential resolution, token management, or host classification changes.

Doc Writer -- inactive

No changed files match doc-writer trigger paths (docs/src/content/docs/, README.md, CHANGELOG.md, MANIFESTO.md, .apm/skills//*.md, packages/apm-guide/**, etc.). The PR touches CI workflow, an internal hook deduplication fix, and benchmark tests -- none of which are user-facing documentation or natural-language artifacts.

Test Coverage Expert

• [recommended] Guard 10 (HookDedup) tests a standalone extracted helper, not the production _integrate_merged_hooks path at tests/benchmarks/test_scaling_guards.py
  test_scaling_guards.py defines a local _dedup_hook_entries function that reproduces the dedup logic from _integrate_merged_hooks, then benchmarks it. The scaling guard proves O(n) complexity of the extracted copy, not the production function. If the production dedup logic drifts, Guard 10 will not catch it. Production-path coverage does exist via test_hook_integrator.py::test_content_dedup_same_package (calls integrate_package_hooks_claude and asserts len(entries) == 1), so the user promise is defended -- but the scaling guard is testing a shadow copy, which is maintenance debt: the two can silently diverge.
  Suggested: Either import the dedup logic directly from hook_integrator (if extracted to a named helper), or add a comment in the guard that explicitly names the production symbol it mirrors.
  Proof (missing): tests/benchmarks/test_scaling_guards.py::test_hook_dedup_scaling -- proves: That the dedup scaling guard exercises the actual production dedup code path, not a shadow copy

• [recommended] Duplicate-append bug fix has a unit-tier regression trap but no integration-tier test specifically targeting the old double-append symptom at tests/unit/integration/test_hook_integrator.py
test_content_dedup_same_package (line 3589) asserts len==1 after installing a package with two identical hook files -- a valid regression trap at unit tier. The floor tier for install-pipeline surfaces is integration-with-fixtures. test_reinstall_heals_preexisting_duplicates (line 659) seeds duplicates in a real settings.json and checks they collapse, which is close but pre-existing, not PR-authored.
  Suggested: Confirm that test_reinstall_heals_preexisting_duplicates (line 659) runs cleanly against the new code -- it seeds a real settings.json with three duplicate entries and asserts they collapse, which is the strongest regression trap for this bug.
  Proof (unknown): tests/unit/integration/test_hook_integrator.py::test_reinstall_heals_preexisting_duplicates -- proves: That identical entries from the same package appear exactly once after integration

• [nit] PR body claims '156 hook integrator unit tests pass' -- count is unverifiable without running the suite; no Scenario Evidence table present
  The behavioral changes (FIFO order preservation via deque, dedup via set+json.dumps key) are not mapped to APM principles. Minor authoring gap; underlying test coverage appears adequate from file inspection.
  Suggested: Add a Scenario Evidence table to the PR body mapping the two behavioral changes to their scenario, principle, and test path.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

Generated by PR Review Panel for issue #1439 · ● 2.4M · ◷