mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
7 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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 `
|
||
|
JD Davis
|
9bacf4810f
|
refactor(transforms): isolate mixed content parsing (#1939)
## Description Extracts mixed-content parsing out of the large `ContentRouter` module into a pure transform-domain module. The router still exports the existing compatibility names, but section typing, mixed-content indicators, section splitting, and JSON block extraction now live in a focused domain object/function layer. Closes # ## Type of Change - [ ] 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 - [x] Code refactoring (no functional changes) ## Changes Made - Added `headroom.transforms.mixed_content` with `ContentSection`, `mixed_content_indicators`, `is_mixed_content`, `split_into_sections`, and JSON block extraction. - Updated `ContentRouter` to delegate mixed-content debug indicators and parsing to the new module while preserving legacy imports from `content_router.py`. - Added direct unit coverage for mixed-content detection, section boundaries, and JSON delimiters inside string literals. - Included the LiteLLM callback signature compatibility shim needed for repo-wide mypy while the earlier architecture PRs are still open. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text python -m pytest tests/test_mixed_content_sections.py tests/test_transforms_content_router.py tests/test_litellm_callback.py tests/test_compress_api.py::TestLiteLLMCallback -q 50 passed in 6.82s python -m ruff check . All checks passed! python -m ruff format --check . 1095 files already formatted python -m mypy headroom --ignore-missing-imports headroom\proxy\server.py:1457: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] headroom\proxy\server.py:1468: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] Success: no issues found in 409 source files ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.13, clean worktree `C:\git\headroom-pr-slice6` - Exact command / steps: ran the pytest, Ruff, format, and mypy commands listed above. - Observed result: mixed-content parsing behavior remains covered through existing router tests and new direct tests; repo-wide lint/type checks pass. - Not tested: full pytest suite and Docker/native CI jobs are left to GitHub Actions. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [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 - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A. ## Additional Notes - Documentation, changelog, and screenshots are N/A for this internal refactor. - Manual UI testing is N/A; this is pure transform parsing logic. - Comment checklist is unchecked because the extracted functions are small and covered by direct tests. |
||
|
Rod Boev
|
0a3851b240
|
perf(proxy): cap compression workers to CPU count (#1803)
## Description The request-path compression executor currently uses asyncio-style I/O sizing for CPU-bound Kompress work. When `compression_max_workers` is unset, `HeadroomProxy.__init__` resolves the pool to `min(32, cpu * 4)`, so an eight-core host can run 32 simultaneous compression workers that all contend for real CPU. This changes only the automatic request-path default to one worker per reported CPU while preserving the existing explicit override path from `--compression-max-workers` and `HEADROOM_COMPRESSION_MAX_WORKERS`. Closes #1635 ## 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 - [x] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Cap the automatic request-path compression executor default at `max(1, os.cpu_count() or 1)`. - Preserve explicit `compression_max_workers` values, including the existing clamp to at least one worker. - Keep CLI help, `ProxyConfig` comments, and nearby test documentation aligned with the CPU-bound default. - Update the focused compression executor regression so the default contract documents CPU-bound sizing, and keep the existing Codex compression stress guard stable when p50 rounds to zero. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_codex_ws_compression_scheduler.py tests/test_proxy_compression_executor.py tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q`) - [x] Linting passes (`uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_compression_executor.py tests/test_codex_ws_compression_scheduler.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_codex_ws_compression_scheduler.py tests/test_proxy_compression_executor.py tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q 16 passed, 1 skipped, 1 warning in 6.13s $ uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_compression_executor.py tests/test_codex_ws_compression_scheduler.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows, Python environment from `uv sync --extra dev`, no provider credentials needed. - Exact command / steps: construct `HeadroomProxy` with `compression_max_workers=None`, inspect `proxy.compression_max_workers` and `/health` `runtime.compression_executor`. - Observed result: the automatic request-path pool resolves to reported CPU count, while explicit overrides still resolve to the configured value and report `source: explicit`. - Not tested: multi-session wall-clock benchmark under live Kompress load. ## 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 - [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 have updated the CHANGELOG.md if applicable ## Additional Notes No `CHANGELOG.md` edit: this repo generates changelog entries from conventional commits. This intentionally does not touch the background compression executor surface covered by #1633. |
||
|
Tejas Chopra
|
e94a36cb6d |
test(codex): de-flake semaphore-tail ratio check on fast runners
test_concurrent_compression_has_no_semaphore_tail computed p99/max(p50,1). On a fast/quiet CI runner p50 rounds to 0ms, so the ratio collapses to p99-in-ms and a few ms of ordinary scheduler jitter (p50=0ms, p99=5ms) read as ~4.8x, tripping the <4x gate — noise, not the semaphore-contention tail it targets (tens of ms, ~27x). Only enforce the ratio once p99 clears a 25ms scheduler-noise floor (assert ratio < 4.0 or p99 < 25ms). A real contention regression still trips it (large absolute tail + high ratio); sub-ms jitter no longer does. Verified locally: the test passes. |
||
|
chopratejas
|
c22342e1c7 |
fix(tests): make stress test CI-robust — uniform frame sizes, tighter ratio
Second CI failure on the same stress test, this time with the ratio threshold: AssertionError: p99/p50 ratio is 7.4× (p50=28406ms, p99=210468ms). Expected < 5× — wall=651s. Root cause: previous iteration used MIXED frame sizes (200 B → 16 KB) across 30 concurrent sessions on a 2-vCPU CI runner. The p99/p50 ratio captured TWO things: 1. The contention-tail signature we want to catch (≈27× pre-fix). 2. Size-variance compute spread (≈3–8× depending on hardware). On dev hardware the (2) component was small relative to the contention signal. On CI it dominated, masking the (1) detection. The fix is to remove (2) from the measurement entirely: * All 60 frames are now identical 4 KB plain-text payloads. * Concurrency dropped from 30 to 12 — still > the deleted 10-slot semaphore (so the bug pattern, if reintroduced, surfaces), but doesn't oversaturate the 2-vCPU CI runner with OS-scheduler noise. * Frames per session dropped from 12 to 5 → 60 total samples, still enough to compute a meaningful p99, with bounded runtime. * Threshold tightened from 5× to 4×. On uniform workload the only legitimate source of p99/p50 spread is OS-level scheduling noise (≈2–3×). 4× sits comfortably between that and the bug signature (≈27×). Local re-run: 60 frames, 0.59s wall, p50=108ms p99=198ms ratio=1.83× — well under the 4× ceiling, captures the bug shape unambiguously. Test design note added to docstring explaining the why so future CI hardware changes don't trip the threshold again. |
||
|
chopratejas
|
f82d61b163 |
fix(tests): make codex scheduler stress test machine-independent
CI failure on first attempt at the stress test: p99 per-frame elapsed_ms = 214020; expected < 1000 GitHub Actions runners (2 vCPU, shared) are 5–50× slower in absolute terms than the 12-CPU dev box this PR's baseline numbers were taken on. The absolute thresholds (p99<1000ms, wall<5s) intentionally caught the bug on dev hardware but force CI either to skip the test or to use thresholds so loose they stop catching the regression. The bug being guarded against creates a *bimodal* latency distribution (most fast, some catastrophic) via the deleted ``_CODEX_WS_UNIT_ROUTER_SEMAPHORE``. Pre-fix on dev: p50=91ms, p99=2433ms → ratio=27×. The contention *pattern* is invariant — if the semaphore tail comes back, the ratio explodes regardless of CPU speed. This commit: * Removes the machine-dependent absolute thresholds (p99<1000ms, wall<5s). * Keeps the p99/p50 ratio test (now strictly < 5×, no special floor). * Adds a `print()` of the full distribution so CI logs always show numbers — useful both for diagnosing failures and tracking drift. Local re-run: p50=264ms p99=492ms ratio=1.87× — well under the 5× ceiling and the test still proves the contention tail is gone. |
||
|
chopratejas
|
a167f5cc29 |
fix(proxy): unblock Codex WS compression — delete inner-pool + global semaphore
Production proxy logs (2026-05-14) showed 305 `TimeoutError: forwarding original frame` warnings and 12,905 `slow compression unit elapsed_ms>1s` log entries, with p99 unit elapsed_ms = 587 SECONDS, max = 1987 seconds, and WS session p90 duration = 48 minutes. The cause was a two-layer concurrency bug in `_compress_openai_responses_payload`: * `_CODEX_WS_UNIT_ROUTER_SEMAPHORE = threading.BoundedSemaphore(10)` — a process-global gate over every compression unit in every frame across every concurrent session. At ~3+ active Codex users it saturates; subsequent units block on acquisition. The 30s parent timeout fires; uncompressed frames forward but the user already waited 30s. * `time.perf_counter()` started BEFORE semaphore acquisition, so `elapsed_ms` conflated wait time with compute. A `strategy=passthrough` unit on 148 bytes (a no-op) showed `elapsed_ms=60917` in the log — 60 seconds of "compression" that was actually 60 seconds of queueing. * `concurrent.futures.ThreadPoolExecutor(max_workers=worker_count)` was created and torn down per frame, layered on top of the `self._compression_executor` proxy-wide pool. Pool-on-pool plus the global semaphore made the bug self-amplifying. Fix: delete all three. Process routed units serially within the frame- level worker thread. Frame-level parallelism is already provided by the existing `self._compression_executor` (32 workers, sized `min(32, cpu*4)`, instrumented). Bonus: add a structured PERF log emit from `handle_openai_responses_ws` so Codex traffic is no longer invisible to `headroom perf` — same visibility bug class as #327, fixed for Codex. Tier 3 replay against `scripts/replay_codex_ws_load.py` (30 concurrent sessions × 30 frames = 900 frames, 4.6MB) — same machine, before vs after: | metric | pre-fix (main) | post-fix | Δ | |---------------------|-----------------|----------------|------------| | p50 per-frame | 91 ms | 258 ms | +183 % | | p99 per-frame | 2 434 ms | 275 ms | −89 % | | max per-frame | 2 681 ms | 368 ms | −86 % | | p99 / p50 ratio | 27 × | 1.06 × | tail gone | | wall time | 7.54 s | 7.09 s | −6 % | | errors | 0 | 0 | — | The median rises modestly at high load (the cost of KISS: serial units instead of intra-frame parallelism, documented in EC2 of the design). That trade is right: the catastrophic p99 contention tail is what users felt, and it collapses 9×. At low load (10c × 20f) the fix is strictly equal-or-better on every metric — the trade is invisible until the semaphore was actually the binding constraint. Tests * tests/test_codex_ws_compression_scheduler.py — three regression guards: source-level assertions that `_CODEX_WS_UNIT_ROUTER_SEMAPHORE` and `concurrent.futures.ThreadPoolExecutor` cannot reappear in handlers/openai.py, plus a concurrency stress test asserting p99 < 1000ms and p99/p50 < 5× at 30 concurrent sessions. * All 95 existing Codex/streaming/cache tests pass with zero regressions. Removed surface * Deleted `_CODEX_WS_UNIT_ROUTER_MAX_WORKERS`, `_CODEX_WS_UNIT_ROUTER_SEMAPHORE`, `_codex_ws_unit_worker_count`, and the `HEADROOM_CODEX_WS_UNIT_WORKERS` env knob. Net −13 module- level lines + one undocumented env var gone from the public surface. |