Run TP4 batches concurrently

[elliottwaves-20]

Closes #730

What

_check_tp4 now runs its batches through run_async(analyzer.arun_batches_detailed(batches)) instead of the serial run_batches_detailed loop, the same way the semantic analyzers and the meta-review already fan out.

• The shared limiter keeps honouring SKILLSPECTOR_MAX_LLM_CONCURRENCY; 1 still serializes.
• arun_batches_detailed records failures per batch exactly like the sync path (structured-response retries, runtime limit, provider failures) and returns successes in batch order, so the TP4_MAX_FINDINGS cap and the ledger projection are unchanged.

Tests

• New TestTP4Concurrency: four TP4 batches with SKILLSPECTOR_MAX_LLM_CONCURRENCY=2 reach two requests in flight and all four complete. It fails on the previous code, which never reaches the async path.
• The TP4 retry tests now patch _asleep_before_retry, since retries back off through the async path; the structured-model double gained ainvoke_with_usage.
• test_graph.py: the failing TP4 analyzer double implements arun_batches_detailed.
• pytest tests/test_mcp_tool_poisoning.py tests/integration/test_graph.py: 79 passed. Unit suite (default -m "not integration and not provider", Windows 11, Python 3.13): 8410 passed. The remaining 18 failures and 2 errors fail identically on upstream/main in the same environment (release script, CLI rendering, timing and path tests); four tests that differed between the two parallel runs pass on both trees when run serially. ruff check and ruff format --check clean.

Note: on Windows, tests/nodes/test_security_end_to_end.py does not finish within five minutes, with and without this change, so I ran the unit suite without it.

Measurement

Provider codex_cli, one skill with 46 LLM calls (26 of them TP4): TP4 span ~270 s → 75 s with SKILLSPECTOR_MAX_LLM_CONCURRENCY=4, whole scan 320 s → 206 s. Details in #730.

🤖 Generated with Claude Code

[rng1995]

[SkillSpector Review]

Hi @elliottwaves-20, thank you for measuring the TP4 long tail and moving it onto the shared async path with a focused concurrency test!

Value and readiness: _check_tp4 now runs its batches through run_async(analyzer.arun_batches_detailed(batches)), the same path the semantic analyzers and the meta-review already use. Per-batch failure classification, retries, deadline handling and result order are unchanged. It also closes a quiet gap. The serial loop never took the process-wide limiter, so TP4 could add one request beyond SKILLSPECTOR_MAX_LLM_CONCURRENCY, even with =1. It now shares the single process-wide budget described in the 2.11.1 release notes. CI is green and it merges cleanly with main. Ready for final maintainer review; see the merge-order note for #691 below.

Material findings

1. [Non-blocking] src/skillspector/llm_analyzer_base.py:1425-1455: a batch can end in ValueError or NotImplementedError, for example a refusal that returns None, which _TP4Analyzer.parse_response rejects. The async path then raises only after every batch has finished, while the serial loop stopped at the first one. The final TP4 ledger is the same (every batch FAILED), but a model that refuses every batch now spends up to TP4_MAX_BATCHES (64) calls instead of one. The semantic analyzers already behave this way, so no change is needed here. It is worth keeping in mind for #691's Opus 5 refusal handling.
2. [Non-blocking] tests/test_mcp_tool_poisoning.py:1715: the new test proves fan-out under the shared limit (peak == 2 is deterministic) and fails on main. One TP4-level case with mixed outcomes would pin the rest of the contract. Make batch 0 finish last and batch 1 raise, then assert that ledger rows and findings follow batch order, batch 1 gets a FAILED row, the others are COMPLETED, and llm_call_log is ok: False. gather() already guarantees the ordering, and the shared layer tests failure isolation (test_detailed_outcome_preserves_failed_batch), so this is hardening, not a gap.

PIC tradeoffs: TP4 now draws on the same SKILLSPECTOR_MAX_LLM_CONCURRENCY pool (default 10) as the semantic analyzers. Scans finish sooner, but TP4 sends more requests per minute. Low-RPM tiers and local servers such as Ollama may see more 429s or queued timeouts unless users lower the limit, and those show up as partial TP4 coverage. A release note would help. The limiter is FIFO across analyzers, so under a tight workflow deadline, which analyzer's batches get cut now depends on arrival order. Before, TP4 effectively had its own lane.

Verification and gaps:

• Compared run_batches_detailed() (llm_analyzer_base.py:1300) with arun_batches_detailed() (:1397). Both use the same structured-response retries, provider retry and backoff (including Retry-After), RUNTIME_LIMIT and provider-failure classification, and ValueError/NotImplementedError propagation. gather(..., return_exceptions=True) isolates failures per batch, and results are walked in batch order. Successes, failures, the TP4_MAX_FINDINGS cap, dedup and the llm_call_log error class therefore stay deterministic. Per-call inference_usage records now follow completion order, as they already do for the semantic analyzers.
• Deadlines: each batch re-checks the shared deadline in _model_for_call() after it gets a permit, and retry sleeps are capped at the remaining time. With a workflow deadline, provider timeouts are retargeted to the remaining time on each call. Queued batches fail fast with RUNTIME_LIMIT (PARTIAL) once time runs out, and an in-flight call overshoots by at most its capped timeout.
• Concurrency safety: _GlobalLLMLimiter is shared across event loops (covered by test_three_nodes_on_three_loops_share_one_budget), and InferenceUsageCollector is lock-protected. CLI providers' async calls are asyncio.to_thread(self.invoke...), so no provider-side serialization is bypassed. run_async() handles callers that already have a running loop. TP4's whole-run except Exception still marks unfinished batches FAILED, which the updated test_graph.py double exercises.
• semantic_security_discovery.py:218 is now the only analyzer still on the serial path outside the limiter (out of scope here).
• Overlaps: #693 merges cleanly with this PR. Its tests use the shared _FakeStructuredLLM, which gains ainvoke_with_usage here. #691 conflicts on the adjacent skillspector.llm_utils import line, which is trivial to resolve. Suggested order: land this PR first, then update #691.
• #691 will also need test changes once this lands, because two of its new TP4 tests stub sync-only hooks. test_wrapped_json_schema_response_stays_incomplete patches llm_analyzer_base.time.sleep, so its retries would really sleep (about 3.5 s). test_opus5_function_calling_refusal_stays_incomplete patches ChatOpenAI._generate, but the async path calls _agenerate. It would attempt a real request to example.invalid instead of exercising the refusal path. Patching _agenerate and LLMAnalyzerBase._asleep_before_retry fixes both.
• CI: all 6 checks passed on this head, and main has not touched the changed files since. Tests and the PR's timing figures were not reproduced locally, per review policy.

---

Decision: Approved (reviewed head 9919c159597e62016214a1d97ae1eb559e547b11)

[elliottwaves-20]

Thank you for the review and the merge. I can follow up with the TP4-level mixed-outcome test you described: batch 0 finishes last, batch 1 raises, ledger rows and findings stay in batch order, batch 1 is FAILED and llm_call_log reports ok: False. I would send it as a small separate PR unless you would rather leave it.