headroom/tests/test_codex_ws_compression_scheduler.py
JD Davis a708c0571e
fix(ci): prevent native detector from hanging test shards (#2996)
## Description

CI shard 4 was not merely slow: after thousands of fast tests it parked
indefinitely inside `headroom._core.detect_content_type` at 0% CPU. The
router watchdogged only the first native call and then permanently
trusted direct calls via `_detect_native_verified`. Earlier suite
activity can change ORT/native state after that first success, making a
later call deadlock until GitHub cancels the job.

This keeps every native call bounded by the existing watchdog, activates
the process-wide pure-Python circuit breaker after a timeout, restores
the test-job ceiling to 30 minutes, and removes a separate wall-clock
scheduler assertion that generated false shard-1 failures despite the
structural regression guards passing.

No issue is auto-closed by this infrastructure repair.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- Removed the unsafe process-lifetime `_detect_native_verified` fast
path.
- Kept every native detection call behind the existing bounded watchdog.
- Preserved the process-wide fallback circuit breaker so only the first
wedged call consumes the watchdog budget.
- Added a success-then-hang regression test.
- Isolated native circuit-breaker state in fallback exception tests.
- Restored the CI test timeout from the temporary 90-minute diagnostic
ceiling to 30 minutes.
- Replaced the Codex scheduler's noise-sensitive p99/p50 assertion with
its meaningful absolute regression ceiling while retaining source-level
guards against the removed semaphore and nested executor.
- Corrected import order and formatting defects inherited from current
main so the synthetic merge commit passes repository-wide lint.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

```text
Exact local shard-4 command with coverage:
2723 passed, 172 skipped, 8661 deselected in 108.40s

Focused detector/router suite:
62 passed

Codex scheduler suite:
3 passed, 1 skipped

ruff check .
All checks passed!

ruff format --check .
1411 files already formatted

mypy headroom/transforms/content_router.py
Success: no issues found in 1 source file
```

Exact-head GitHub CI on `28f284c7a1` is
entirely green. Test jobs 1–4, test-extras, test-agno, build, wheel,
lint, CodeQL, dependency audit, secret scan, smoke, governance, and
conflict checks all passed. Remaining skips are path-filtered jobs not
applicable to this diff.

## Real Behavior Proof

- Environment: macOS arm64/Python 3.13 locally; GitHub-hosted
Ubuntu/Python 3.12 using the production CI workflow and prebuilt wheel.
- Exact command / steps: reproduced `pytest tests scripts/tests --splits
4 --group 4 ...` hanging in native detection; sampled the parked
process; reran with `pytest-timeout` to locate `_rust_detect`; applied
the correction; reran the exact shard locally and all four CI shards
remotely.
- Observed result: local shard 4 completed in 1:48. GitHub shard 4's
pytest step completed in 5:45 and its full job in 8:06 under the
restored 30-minute ceiling. All four shards passed on the same head.
- Not tested: deliberately wedging a real production ORT runtime outside
the deterministic mocked regression; the watchdog behavior is covered
with a native-call fake that succeeds once and then never returns.

## Runtime Rollout Safety

- Rollout-managed feature(s): native content detection watchdog and
fallback only.
- Minimum rollout channel: normal patch release; no staged feature flag
required.
- Stable/default behavior changed: every native detection call remains
watchdog-bounded instead of only the first successful call.
- Kill switch / disable path: `HEADROOM_DETECT_BACKEND=python` bypasses
native detection; `HEADROOM_DETECT_TIMEOUT_SECS` controls the watchdog
budget.
- Unsafe override required: none.
- Qualification impact: full Python CI matrix must remain green; exact
shard-4 completion is the primary qualification evidence.
- Rollback path: human revert of this PR if bounded calls cause an
unexpected regression; setting the Python backend provides an immediate
operational fallback without code rollback.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation — inline
lifecycle documentation and PR operational notes; no user-facing docs
change is needed
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I did **not** edit `CHANGELOG.md` — it is generated by
release-please from my Conventional Commit PR title (a CI guard enforces
this)

## Screenshots (if applicable)

Not applicable; no UI change.

## Additional Notes

Human review only. No merge or auto-merge action has been configured.
The branch includes current main and preserves the MCP SDK compatibility
cap `mcp>=1.28.1,<2.0.0`.
2026-08-13 20:47:55 -05:00

313 lines
13 KiB
Python
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

"""P2 — Codex compression scheduler regression coverage.
The pre-fix code throttled all concurrent Codex WS compression units
through a process-global ``threading.BoundedSemaphore(10)`` and created
a fresh ``ThreadPoolExecutor`` per frame. Under realistic concurrent
load (≥10 sessions) the semaphore saturated, ``elapsed_ms`` was measured
INCLUDING the wait time, and frames hit the parent 30s timeout.
The fix:
* Deletes the module-global ``_CODEX_WS_UNIT_ROUTER_SEMAPHORE``.
* Deletes the per-call inner ``ThreadPoolExecutor``.
* Processes routed units serially inside the frame-level worker thread
(``self._compression_executor`` already provides frame-level parallelism
via the proxy-wide bounded executor).
* Adds a ``PERF`` log emission from ``handle_openai_responses_ws`` so
Codex traffic is no longer invisible to ``headroom perf``.
These tests verify that future contributors cannot silently re-introduce
either bottleneck.
"""
from __future__ import annotations
import concurrent.futures
import logging
import sys
import time
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
OPENAI_HANDLER = REPO_ROOT / "headroom" / "proxy" / "handlers" / "openai.py"
# ── Source-level regression guards ──────────────────────────────────────
def test_module_global_unit_semaphore_is_removed() -> None:
"""The 10-slot global semaphore that caused 30s frame timeouts must stay gone.
Read the source file directly — imported module state is not authoritative
because Python caches bytecode independently. The regression we are
guarding against is "someone reintroduces a module-level semaphore on
the Codex WS dispatch path" — that is detectable in source.
"""
source = OPENAI_HANDLER.read_text()
assert "_CODEX_WS_UNIT_ROUTER_SEMAPHORE" not in source, (
"Module-global semaphore on Codex WS path reintroduced. The P2 fix "
"deleted it because it saturated at 10 concurrent units and caused "
"the production cascade documented in issue #327's sibling slowness "
"report. Use `self._compression_executor` (the proxy-wide bounded "
"pool) for any new concurrency needs."
)
assert "_CODEX_WS_UNIT_ROUTER_MAX_WORKERS" not in source, (
"Module-global slot count for the (deleted) Codex unit semaphore reintroduced."
)
assert "_codex_ws_unit_worker_count" not in source, (
"The per-call inner-pool worker-count helper was deleted because the "
"inner pool was deleted. Reintroducing it suggests the inner pool "
"is back too — re-read docs/superpowers/specs/P2-codex-scheduler-fix.md."
)
assert "HEADROOM_CODEX_WS_UNIT_WORKERS" not in source, (
"The HEADROOM_CODEX_WS_UNIT_WORKERS env knob was removed. It only "
"existed to tune around the semaphore bottleneck, which is gone."
)
def test_no_per_call_threadpool_inside_compress_routed_units() -> None:
"""The inner ``ThreadPoolExecutor`` created per frame must stay gone.
Pre-fix, every call to ``_compress_openai_responses_payload`` created
and tore down a ``ThreadPoolExecutor(max_workers=worker_count)`` to run
routed units, layered on top of ``self._compression_executor``. That
pool-on-pool pattern added latency variance, fought for OS threads,
and made the global semaphore the binding constraint.
The exact phrase ``concurrent.futures.ThreadPoolExecutor`` should not
appear anywhere in openai.py — the dispatch uses the proxy's shared
bounded executor instead.
"""
source = OPENAI_HANDLER.read_text()
assert "concurrent.futures.ThreadPoolExecutor" not in source, (
"Per-call ThreadPoolExecutor reintroduced in handlers/openai.py. "
"Submit work to `self._compression_executor` (instrumented and "
"lifecycle-managed) instead of creating a new pool per frame."
)
# ── PERF log emission from the Codex WS path ────────────────────────────
#
# Codex WS traffic was invisible to ``headroom perf`` pre-fix because
# ``handle_openai_responses_ws`` emitted no PERF line. This is structurally
# the same bug class as #327's "Cache write: 0" for backend-routed
# streaming — the request is processed correctly but the operator can't
# see it. The new PERF emit closes that visibility gap.
class _DirectLogCapture(logging.Handler):
"""Direct handler attached to ``headroom.proxy`` so the proxy's
propagation flip in ``_setup_file_logging`` does not strip records.
Same pattern as ``tests/test_backend_streaming_cache_metrics.py`` —
see that file for the rationale.
"""
def __init__(self) -> None:
super().__init__(level=logging.INFO)
self.records: list[logging.LogRecord] = []
def emit(self, record: logging.LogRecord) -> None:
self.records.append(record)
def _attach_proxy_log_capture() -> tuple[_DirectLogCapture, logging.Logger, int]:
handler = _DirectLogCapture()
target = logging.getLogger("headroom.proxy")
target.addHandler(handler)
prior_level = target.level
target.setLevel(logging.INFO)
return handler, target, prior_level
def _detach_proxy_log_capture(handler, target, prior_level) -> None:
target.removeHandler(handler)
target.setLevel(prior_level)
def _make_perf_log_test_handler():
"""Build a minimal handler that lets us drive the PERF emit code path
of ``handle_openai_responses_ws`` end-to-end without a real upstream.
Imported lazily so a collection-time import error in the proxy module
does not break the source-level regression guards above.
"""
from headroom.proxy.handlers.openai import OpenAIHandlerMixin
from headroom.proxy.ws_session_registry import WebSocketSessionRegistry
class _M(OpenAIHandlerMixin):
OPENAI_API_URL = "https://api.openai.com"
def __init__(self) -> None:
self.rate_limiter = None
self.metrics = SimpleNamespace(
record_request=lambda **kw: None,
record_stage_timings=lambda *a, **kw: None,
inc_active_ws_sessions=lambda: None,
dec_active_ws_sessions=lambda: None,
inc_active_relay_tasks=lambda n=1: None,
dec_active_relay_tasks=lambda n=1: None,
record_ws_session_duration=lambda *a, **kw: None,
record_codex_ws_unit=lambda **kw: None,
)
self.config = SimpleNamespace(
optimize=True,
retry_max_attempts=1,
retry_base_delay_ms=1,
retry_max_delay_ms=1,
connect_timeout_seconds=10,
log_full_messages=False,
)
self.usage_reporter = None
self.openai_provider = SimpleNamespace(
get_context_limit=lambda model: 128_000,
get_token_counter=lambda model: SimpleNamespace(
count_text=lambda text: max(1, len(text) // 4),
count_messages=lambda *a, **k: 0,
),
)
self.openai_pipeline = SimpleNamespace(apply=MagicMock(), transforms=[])
self.anthropic_backend = None
self.cost_tracker = None
self.memory_handler = None
self.ws_sessions = WebSocketSessionRegistry()
self.logger = None
self.compression_executor_calls = 0
async def _next_request_id(self) -> str:
return "req-perf-emit-test"
async def _run_compression_in_executor(self, fn, *, timeout: float):
self.compression_executor_calls += 1
return fn()
return _M()
@pytest.mark.asyncio
async def test_codex_ws_emits_perf_log_with_cache_keys() -> None:
"""``handle_openai_responses_ws`` must emit a PERF line so ``headroom
perf`` counts Codex traffic instead of reporting it as zero requests.
Asserts on the structured-PERF kv fragment used by ``headroom/perf/
analyzer.py`` (``cache_read=`` / ``cache_write=`` / ``cache_hit_pct=``)
so the analyzer parser actually picks it up.
"""
pytest.skip(
"Pending: full WS lifecycle harness for handle_openai_responses_ws "
"needs a fuller FakeWebSocket+FakeUpstream wire-up than this file "
"owns. The PERF emit is verified via Tier-3 replay + Tier-4 manual "
"smoke; the source-level guards above prevent the emit from being "
"removed silently. Re-enable when the WS lifecycle harness in "
"test_openai_codex_ws_lifecycle.py is reused as a fixture."
)
# ── Concurrency stress (Tier 2) ─────────────────────────────────────────
#
# The smoking gun: with the old code, 30 concurrent calls to
# ``_compress_openai_responses_payload`` produced p99 per-call latency of
# ~2.4s on a 12-CPU machine because of the 10-slot global semaphore. After
# the fix, units run serially within the frame-level worker, but the
# frame-level compression executor lets 30 frames run in parallel without contention.
#
# Pass criteria mirror docs/superpowers/specs/P2-codex-scheduler-fix.md
# "Success criteria":
# - p99 per-frame < 250ms (vs baseline 2433ms)
# - p99/p50 < 3× (vs baseline 24×)
# - errors == 0
@pytest.mark.slow
def test_concurrent_compression_has_no_semaphore_tail() -> None:
"""Probe the 10-slot semaphore boundary with uniform-size workload.
Design notes — addresses a CI-vs-dev hardware skew that bit the
first iteration of this test:
* **12 concurrent sessions** (> the deleted 10-slot semaphore size).
Enough to saturate the gate if it ever reappears; small enough
that a 2-vCPU CI runner doesn't drown in OS-level scheduler
noise.
* **All frames the same size (4 KB)** so size-induced compute
variance cancels out. Pre-refactor the bug produced bimodal
latency (waiters vs holders) regardless of frame size; this
test must measure THAT, not size variance.
* **5 frames per session** = 60 total. Enough samples to make
the p99 statistic meaningful. Bounded runtime even on slow CI.
* **Threshold ratio < 4×.** On uniform-size workload the only
sources of p99/p50 spread are (a) the deleted semaphore tail
(≈27×) or (b) OS-level scheduler noise (≈23×). 4× sits
comfortably between the two — catches the bug, tolerates
hardware. (First iteration tried 5× with mixed sizes, which
let size-variance push CI ratios to 7.4×.) The ratio is only
enforced once p99 clears a scheduler-noise floor — on very fast
runners p50 rounds to 0ms and the ratio becomes pure jitter.
Marked ``slow`` so a normal ``pytest`` run can skip it via
``-m 'not slow'``. CI matrix runs all marks.
"""
sys.path.insert(0, str(REPO_ROOT))
from scripts.replay_codex_ws_load import ( # noqa: E402
Frame,
Scenario,
boot_proxy,
replay_session,
warmup,
)
proxy = boot_proxy()
warmup_ms = warmup(proxy)
assert warmup_ms < 30_000, (
f"Warmup took {warmup_ms:.0f}ms — Kompress model failed to load? "
"Subsequent timing assertions are meaningless without a warm router."
)
# 12 sessions × 5 frames = 60 total. Uniform 4 KB plain-text
# payload — each frame's compute time should be identical modulo
# scheduler noise.
UNIFORM_FRAME = Frame(bytes_estimate=4096, text_shape="plain_text_like")
scenarios = [
Scenario(
request_id=f"stress-{i:02d}",
frames=[UNIFORM_FRAME] * 5,
)
for i in range(12)
]
results: list = []
started = time.perf_counter()
with concurrent.futures.ThreadPoolExecutor(max_workers=12) as pool:
futures = [pool.submit(replay_session, proxy, s, "gpt-4o-mini") for s in scenarios]
for fut in concurrent.futures.as_completed(futures):
results.extend(fut.result())
wall_s = time.perf_counter() - started
elapsed = sorted(r.elapsed_ms for r in results)
p50 = elapsed[len(elapsed) // 2]
p99 = elapsed[int(len(elapsed) * 0.99)]
errors = [r for r in results if r.error]
# Always print the distribution so CI logs show numbers for
# diagnosing failures and tracking drift across runs.
print(
f"\n[stress] frames={len(results)} wall={wall_s:.2f}s "
f"p50={p50:.0f}ms p99={p99:.0f}ms ratio={p99 / max(p50, 1):.2f}× errors={len(errors)}"
)
assert not errors, f"Got {len(errors)} errors; first: {errors[0].error}"
SEMAPHORE_P99_CEILING_MS = 1_000.0
assert p99 < SEMAPHORE_P99_CEILING_MS, (
f"p99 is {p99:.0f}ms; expected < {SEMAPHORE_P99_CEILING_MS:.0f}ms on "
"uniform-size workload. The pre-fix semaphore baseline was ~2433ms."
)
# Do not add a p99/p50 wall-clock ratio here. A hosted runner can park one
# worker independently of this code path, making an otherwise healthy
# 2ms/76ms distribution look like a 35x contention tail. The property is
# covered structurally above (the semaphore and nested executor must stay
# absent), while this absolute ceiling still rejects the measured 2433ms
# pre-fix behavior without pretending scheduler jitter is product state.