perf(registry): parallelize independent MCP server-info fetches

[Sergio Sisternes (sergio-sisternes-epam)]

TL;DR

MCPServerOperations.batch_fetch_server_info no longer walks server_references one HTTP lookup at a time. Independent find_server_by_reference calls now run through the same bounded ThreadPoolExecutor(max_workers=4) already used by check_servers_needing_installation. Public result shape is unchanged: submission order, per-ref None on exception.

Note

Closes #2981. Same 5-ref / 200 ms mocked lookup: 1.02 s serial on HEAD → 0.41 s on this branch.

Problem (WHY)

• batch_fetch_server_info in src/apm_cli/registry/operations.py issued one find_server_by_reference per reference in a serial for loop, so wall time grew linearly with server count.
• The sibling method check_servers_needing_installation already parallelizes the identical fan-out with ThreadPoolExecutor(max_workers=4), so this path was an established-pattern miss, not a new concurrency design.
• [!] Install flows that call batch_fetch_server_info when no cache is present paid that serial cost on every multi-server project.

Why these matter: the accepted scan in #2981 names the defect as "O(n) sequential registry HTTP round-trips" and the type/performance implement lens requires a "BEFORE measurement on HEAD and an AFTER measurement on your branch" before calling the change a perf fix (implement-refactor.md).

Approach (WHAT)

#	Fix
1	Wrap each independent lookup in _fetch_one and collect via executor.map so insertion order matches server_references.
2	Cap workers at min(4, len(server_references)); skip the pool on an empty list.
3	Keep the existing per-ref except Exception: None contract so a single failed lookup cannot abort the batch.

Implementation (HOW)

• src/apm_cli/registry/operations.py -- batch_fetch_server_info gains an optional max_workers: int = 4 and uses thread_name_prefix="mcp-fetch". Call sites stay positional; no other methods were retuned.
• tests/unit/integration/test_mcp_registry_parallel.py -- wall-time trap (3 x 500 ms must finish under 1.0 s) plus order/exception characterization.
• CHANGELOG.md -- Unreleased Performance entry pointing at [perf-scan] 2026-09-15 -- performance opportunities found #2981.

Diagrams

Legend: each registry document fetch is independent; this PR replaces the serial chain with a four-worker pool that still writes the cache in input order.

flowchart LR
  subgraph serial [HEAD serial]
    S1[ref 1] --> S2[ref 2] --> Sn[ref n]
  end
  subgraph parallel [This PR bounded pool]
    P1[ref 1]
    P2[ref 2]
    Pn[ref n]
    Pool[ThreadPoolExecutor max_workers 4]
    Cache[server_info_cache]
    P1 --> Pool
    P2 --> Pool
    Pn --> Pool
    Pool --> Cache
  end
  classDef new stroke-dasharray: 5 5
  class P1,P2,Pn,Pool,Cache new

Loading

Trade-offs

• Reuse the existing pool pattern instead of asyncio. Chose ThreadPoolExecutor because check_servers_needing_installation already ships that shape in the same class; rejected asyncio to avoid a second concurrency model on a blocking HTTP client.
• Optional max_workers rather than a hard-coded 4. Callers can tighten tests; production keeps the WS2b cap of 4.
• Scenario evidence is included. This is a behavior-preserving performance change with new tests, not a docs-only or asset-bump skip.

Benefits

1. Same 5-ref / 200 ms mocked lookup: 1.02 s on HEAD vs 0.41 s after (~2.5x).
2. Three 500 ms lookups now finish under 1.0 s instead of 1.5 s serial (test_batch_fetch_server_info_parallel_wall_time).
3. Failed lookups still cache as None without aborting the rest of the batch.

Validation

Lint (CI-mirror):

All checks passed!
1866 files already formatted
Your code has been rated at 10.00/10
[+] auth-signal lint clean

Targeted pytest:

115 passed in 2.70s

Commands: uv run --extra dev ruff check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ · ruff format --check · pylint R0801 · bash scripts/lint-auth-signals.sh · uv run --extra dev pytest -q tests/unit/integration/test_mcp_registry_parallel.py tests/unit/test_registry_operations_state.py tests/unit/test_registry_operations_phase3.py

Hermetic before/after (5 refs, 200 ms sleep, one raising ref):

BEFORE HEAD serial: elapsed 1.02
AFTER this branch:  elapsed 0.41
keys ['a', 'b', 'c', 'boom', 'd']
none_for_boom True

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	Looking up several MCP servers does not wait for each registry round-trip in sequence.	DevX (pragmatic as npm)	tests/unit/integration/test_mcp_registry_parallel.py::TestParallelRegistryLookups::test_batch_fetch_server_info_parallel_wall_time	unit
2	A failed registry lookup for one server still returns the others, in the order they were requested, with None for the failure.	DevX (pragmatic as npm)	tests/unit/integration/test_mcp_registry_parallel.py::TestParallelRegistryLookups::test_batch_fetch_preserves_submission_order_and_exceptions tests/unit/test_registry_operations_state.py::TestBatchFetchServerInfo::test_returns_none_on_exception	unit

How to test

• Run uv run --extra dev pytest -q tests/unit/integration/test_mcp_registry_parallel.py -> all parallel wall-time tests pass, including test_batch_fetch_server_info_parallel_wall_time.
• Run uv run --extra dev pytest -q tests/unit/test_registry_operations_state.py::TestBatchFetchServerInfo -> empty list, success, and exception-to-None still hold.
• Confirm MCPServerOperations.batch_fetch_server_info(["a","b"]) still returns a dict keyed in input order with no CLI output change.

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

[Copilot]

🟡 Changes recommended

The worker limit must be enforced; the changelog heading also needs adjustment.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR parallelizes independent MCP server-info lookups while preserving ordering and exception handling.

Changes:

• Uses a bounded thread pool for registry lookups.
• Adds timing, ordering, and failure-handling tests.
• Documents the performance improvement.

File summaries

File	Review
tests/unit/integration/test_mcp_registry_parallel.py	Adds parallelism and regression coverage.
src/apm_cli/registry/operations.py	Moderate (2 votes): Clamp max_workers to the established four-worker cap.
CHANGELOG.md	Nit (1 vote): Place the entry under the existing Changed section.

Review details

Suppressed comments (1)

CHANGELOG.md:29

• The repository's changelog instruction only defines the Added, Changed, Deprecated, Removed, Fixed, and Security groups (.github/instructions/changelog.instructions.md:12-21). Performance is a new, non-canonical heading here, so this entry will not follow the established changelog structure; place the bullet under the existing Changed section instead.

### Performance

- `batch_fetch_server_info` now looks up independent MCP registry documents concurrently with a bounded `ThreadPoolExecutor` (max 4 workers), matching the existing install-check fan-out. Closes #2981. (#2983)

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Sergio Sisternes (sergio-sisternes-epam)]

PR triage recommendation

ready-for-review

Recommendation only, not merge or scope approval. A responsible
human maintainer must approve. Labels, automated advice, and
silence are not approval.

Linked issue

#2981 -- open, labelled status/accepted (also type/performance,
type/automation). Scanner-filed performance finding; this PR
states it closes that issue.

Proposed classification

type/performance -- matches the linked issue and the bounded
parallel registry lookup change.

Suggested next action

Wait for human CODEOWNERS review. A review request is already
on Daniel Meppiel (@danielmeppiel) (CODEOWNERS: Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam)).

Linked issue #2981 is labelled status/accepted, so this PR is in
scope for human review. CODEOWNERS owners are Daniel Meppiel (@danielmeppiel) and
Sergio Sisternes (@sergio-sisternes-epam); the existing review request on
Daniel Meppiel (@danielmeppiel) is left unchanged. Copilot review asked to clamp
max_workers and move the changelog entry under Changed; those
replies are on the thread. This comment is advisory classification
only.

---

Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

[Sergio Sisternes (sergio-sisternes-epam)]

Thanks for the PR, Sergio Sisternes (@sergio-sisternes-epam), and for the fix for #2981.

This branch had picked up a conflict with main after #3150 landed (CHANGELOG.md only). It has been resolved on this same branch with a merge from the latest main: both Unreleased changelog entries are kept, your commits and authorship are untouched, no history was rewritten, and the product changes in this PR are unchanged.

The PR is mergeable again and is waiting on code-owner review.

---

Generated by autopilot-pr-ready-takeover. This comment is AI-generated and may contain errors.

[Sergio Sisternes (sergio-sisternes-epam)]

APM Review Panel: ship_with_followups

batch_fetch_server_info now fans out independent MCP registry lookups through a bounded ThreadPoolExecutor (max 4 workers), matching the existing install-check parallelism, while preserving submission order and mapping per-ref exceptions to None.

panel-mode=lean; personas=python-architect,test-coverage-expert,doc-writer,apm-ceo

cc Daniel Meppiel (@danielmeppiel) -- a fresh advisory pass is ready for your review.

Conflict resolve on tip e20b7b27 restored MERGEABLE after merging main and keeping both the #2983 Changed entry and main's #3150 Changed/Fixed changelog rows. Copilot's earlier notes (clamp workers to four; place the changelog bullet under Changed) are already reflected on this tip. Unit coverage adds wall-time parallelism, order+exception mapping, and an explicit max_workers=1000 → 4 clamp assertion.

No panelist raised a blocking-severity finding. Soft follow-ups are documenting that the shared registry_client must remain thread-safe under concurrent find_server_by_reference, and optionally adding a one-line note in the method docstring that empty input returns immediately without opening a pool.

Aligned with: pragmatic_as_npm -- bounded fan-out mirrors the install-check pool adopters already rely on; secure_by_default -- worker count is hard-capped at four so callers cannot open an unbounded pool.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	0	Bounded executor.map preserves order and exception→None; document client thread-safety assumption under concurrent lookups.
Test Coverage Expert	0	0	0	Wall-time, order/exception, and clamp tests cover the new contract; no missing unit-tier floor found.
Doc Writer	0	1	0	CHANGELOG Changed entry is correct; optional docstring note for empty-input early return.

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

Top 2 follow-ups

1. [Python Architect] Document that registry_client.find_server_by_reference must be safe under concurrent calls from the bounded pool -- The new fan-out assumes the client has no shared mutable request state; a one-line docstring or module note locks that contract for future client changes.
2. [Doc Writer] Note the empty-input early return in the batch_fetch_server_info docstring -- Callers scanning the API surface should see that [] skips pool construction.

Recommendation

Ship this tip through normal human CODEOWNER review once checks are green. The parallelism matches an established in-repo pattern, the four-worker cap is tested, changelog placement is canonical, and no blocking findings remain. This is advisory prose for the reviewer, not approval or permission to merge.

---

Full per-persona findings

Python Architect

• [recommended] Document registry_client thread-safety under concurrent find_server_by_reference at src/apm_cli/registry/operations.py
batch_fetch_server_info now calls the client from worker threads. The implementation is sound if the client is immutable per-request; a short docstring note prevents a future shared-session client from silently racing.
  Suggested: Add one sentence to the method docstring: lookups may run concurrently; registry_client.find_server_by_reference must be thread-safe.

Test Coverage Expert

No findings.

Doc Writer

• [recommended] Mention empty-input early return in the method docstring at src/apm_cli/registry/operations.py
  The early if not server_references: return {} is correct and avoids opening a pool for no work; surface it in the docstring beside the max_workers clamp note.

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

---

Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.