perf(http): pool remaining raw aiohttp.ClientSession sites onto shared client (#12979)

[mrveiss]

Thinking Path

Batch 9 of #12979 — read the issue's comment history for the consolidated recipe and per-batch gotchas (test-seam retargeting, raise_for_status() not implying get_json(), connector= pre-screen). Measured the true remaining set with the ceiling test's own raw_client_session_sites() AST walker (22 sites), pre-screened all candidate files for connector= (none found — no hidden SSRF pins in this batch), then read every site individually per the recipe's standing rule before classifying it tracked_request() vs RAW carve-out.

Two of the task's 12 candidate files turned out, on inspection, to be long-lived/returned-session carve-outs rather than convertible sites — services/npu_pipeline/dispatcher.py (instance-owned session reused across every worker in a pipeline pass, including the token-streaming leg) and services/npu_pipeline/npu_client.py (accepts an externally-owned session and fires concurrent asyncio.gather() calls over the same instance session) — both the same shape as services/npu_client.py's _get_session() carve-out from batch 7. Left RAW with in-code comments instead of converting.

voice_processing/realtime/openai_provider.py was flagged in the task as "very likely a WebSocket session" — on inspection it is a plain multipart HTTP POST (WebRTC negotiation happens client-side in the browser; the Python code only proxies the SDP offer/answer), so it converts normally.

What Changed

Converted to tracked_request() (10 files, 10 sites — 0% get_json()/post_json(), continuing every prior batch's finding that the ratio tracks the module's contract, not the codebase):

File	Site	Why tracked_request()
llm_shared/mock_providers.py	LocalLLM.generate()	reads .text() on failure branch before returning a sentinel
media/link/pipeline.py	_fetch_and_parse()	reads .text()/.headers/.status, branches on >=400
onboarding/doctor.py	_probe_http()	only inspects .status < 500, never raises (health probe — added suppress_error_log=True)
services/command_extraction_service.py	_slm_exec()	reads .json() unconditionally, no raise_for_status(), ssl=False forwards as a per-request kwarg
services/notification_service.py	_send_webhook()	reads .text() on >=400 before raise_for_status()
services/whatsapp_service.py	download_whatsapp_media()	two sequential requests (metadata GET → bytes GET), branches on .status, reads .read()
tools/description_compressor.py	_compress_via_ollama()	sentinel-encodes non-200 to None, json(content_type=None)
voice_processing/providers/cloud/base.py + deepgram_provider.py + assemblyai_provider.py	_make_session() factory (removed) + 4 call sites (_call_api, _upload, _submit, _poll)	base class returned a fresh per-call session to 3 sibling call sites — removed the factory entirely, callers now use tracked_request() with the shared _DEFAULT_TIMEOUT (600s) passed per-request
voice_processing/providers/lv/tilde_provider.py	_call_tilde_api()	reads .text() on failure, multipart FormData forwards fine as data=
voice_processing/realtime/openai_provider.py	_post()	plain HTTP POST (not WebSocket, despite the module name) — reads .read() bytes, custom status mapping

New RAW carve-outs documented (2 files, long-lived, found by reading — not in the task's scope list):

• services/npu_pipeline/dispatcher.py — PipelineDispatcher.__aenter__(): session held for the instance's full lifetime, reused across every worker in the pipeline plan including token streaming.
• services/npu_pipeline/npu_client.py — NPUClient.__aenter__(): accepts an externally-owned session and fires concurrent partial_forward() calls via asyncio.gather() over the same instance session.

Ceiling guard (tests/test_raw_client_session_ceiling_12992.py): MAX_RAW_CLIENT_SESSIONS 22 → 12, re-measured with the file's own AST walker against a tree freshly synced to origin/Dev_new_gui immediately before push (confirmed not behind). Added a "Remaining raw sites" list to the module docstring enumerating all 12 remaining sites and their carve-out category (5 SSRF-pinned-connector files, 4 long-lived files, 2 custom-TLS files), so the next reader sees "intentional" rather than "unfinished".

Test-seam retargeting (6 test files, all the "mock aiohttp.ClientSession directly" variant found via the repo-wide patch(.*ClientSession grep): media/link/pipeline_test.py (7 sites), services/command_extraction_service_test.py (2), tests/services/notification_service_test.py (2), voice_processing/providers/cloud/test_cloud_providers.py (3 — 2 deepgram + 1 assemblyai patch.object(_make_session)), voice_processing/realtime/registry_test.py (2), api/realtime_session_test.py (12 — the widest blast radius this batch, a router-level test file sharing no name with the converted openai_provider.py). All retargeted to patch.object(get_http_client(), "tracked_request", ...).

Pre-existing bugs found (filed, not fixed — different scope):

• test(npu_pipeline): test_dispatcher.py's async mock_post breaks dispatcher.py's sync-call contract #13004 — services/npu_pipeline/test_dispatcher.py's mock_post helpers are declared async def but called synchronously by dispatcher.py, breaking all 5 of that file's tests; confirmed pre-existing via cp-swap against origin/Dev_new_gui.
• test(voice_processing): test_get_speech_provider_convenience depends on import-order side effect, fails standalone #13005 — voice_processing/providers/test_registry.py::test_get_speech_provider_convenience depends on import-order side effects from another test module and fails when run standalone; confirmed pre-existing via cp-swap.

Verification

(a) Static checks, all 21 touched files:

py_compile: OK (21/21)
flake8 --max-line-length=120: 0 violations
black --check --line-length=120: 21 files unchanged

(b) Leak/counter exercise: each converted call path exercised locally against real tracked_request() semantics via the retargeted unit tests (success + failure branches per site).

(c) Baseline-vs-after diff (never trusted a passing suite alone): curated regression scope run before AND after conversion —

media/link/pipeline_test.py + services/command_extraction_service_test.py +
tests/services/notification_service_test.py + voice_processing/providers/cloud/test_cloud_providers.py +
voice_processing/realtime/registry_test.py + api/realtime_session_test.py + onboarding/doctor_test.py +
services/npu_pipeline/test_dispatcher.py + services/npu_pipeline/integration_test.py +
voice_processing/providers/test_registry.py + api/whatsapp_test.py
= 195 passed, 6 failed (identical failing set both before and after — #13004/#13005, confirmed pre-existing via cp-swap of the unmodified files against origin/Dev_new_gui)

Repo-wide grep -rln 'patch(.*ClientSession' re-run after all edits: only 5 files remain, all for carve-outs this batch didn't touch (test_dispatcher.py, test_external_importer_ssrf.py, ssrf_guard_test.py, knowledge_api_integration_test.py, helpers_reorganization_test.py) — confirmed unrelated to any converted module.

(d) Ceiling, measured not computed: raw_client_session_sites() AST walker on a tree freshly synced to origin/Dev_new_gui immediately before push (confirmed not behind): 22 → 12 (exactly the 10 files converted). Negative-case check: monkeypatching MAX_RAW_CLIENT_SESSIONS to 11 trips the assertion as expected.

Recommendation on exact-vs-ceiling: at 12, every remaining raw site is a documented, deliberate carve-out (SSRF-pinned connector / long-lived session / custom TLS) — none are backlog. I'd lean toward converting the assertion to == 12 now that the "moving base while batches are in flight" problem this ceiling was built to survive no longer applies (there's no more batch work in flight to race against). That said, an exact-equality assertion means any new raw ClientSession(...) anywhere in the tree — even a legitimate new carve-out — trips CI until the test file itself is updated, which is arguably the point of a gate. I'm flagging this rather than changing the assertion semantics unilaterally, per the task's instruction.

Model Used

Sonnet (claude-sonnet-5)

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!