mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
8 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
石岳峰
|
b7f342c153
|
fix(wrap): verify proxy deps before mutating Codex config (#1628)
## Description \`headroom wrap codex\` now verifies that optional proxy dependencies (\`headroom-ai[proxy]\`) are installed before mutating Codex \`config.toml\`. If the check fails, the command exits with the same error message as \`headroom proxy\` and leaves Codex config untouched. Fixes #1614 (Bug 1: config mutated before proxy dependency check). ## 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 - Extract \`ensure_proxy_dependencies()\` in \`headroom/cli/proxy.py\` (shared with \`headroom proxy\`) - Call it at the start of \`wrap codex\` when \`not no_proxy\`, before config snapshot/injection - Add regression tests for prepare-only abort, \`--no-proxy\` skip, and import failure messaging ## Testing - [x] Unit tests pass (\`pytest\`) - [x] Linting passes (\`ruff check .\`) - [ ] Type checking passes (\`mypy headroom\`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output \`\`\`bash pytest tests/test_cli/test_wrap_codex.py::test_wrap_codex_aborts_before_mutating_config_when_proxy_deps_missing \ tests/test_cli/test_wrap_codex.py::test_wrap_codex_skips_proxy_dependency_check_with_no_proxy \ tests/test_cli/test_wrap_codex.py::test_ensure_proxy_dependencies_exits_when_server_import_fails -q # 3 passed ruff check headroom/cli/wrap.py headroom/cli/proxy.py tests/test_cli/test_wrap_codex.py ruff format --check headroom/cli/wrap.py headroom/cli/proxy.py tests/test_cli/test_wrap_codex.py \`\`\` ## Real Behavior Proof Environment: Linux (Ubuntu), Python 3.12, local checkout with \`PYTHONPATH\` pointed at patched sources. Exact command / steps: 1. Created a temp \`~/.codex/config.toml\` with \`model_provider = "openai"\`. 2. Patched \`headroom.cli.wrap.ensure_proxy_dependencies\` to raise \`SystemExit(1)\` (simulating missing \`[proxy]\` extra). 3. Ran \`headroom wrap codex --prepare-only --no-serena --port 8787\`. Observed result: exit code 1; \`config.toml\` unchanged; no \`config.toml.headroom-backup\` created; no \`[mcp_servers.headroom]\` block written. Also verified: \`headroom wrap codex --prepare-only --no-proxy ...\` does not invoke the dependency check. Not tested: Windows-specific proxy selector behavior (covered separately in #1655). ## 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 did not edit CHANGELOG.md; release notes are generated automatically --------- Co-authored-by: syf2211 <syf2211@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: JD Davis <jd@jds-macbook-air.tail2a279.ts.net> |
||
|
Shubham Srivastava
|
f9f3162d38
|
docs(proxy): document HEADROOM_SAVINGS_PROFILE and correct --mode default (#2031) (#2040)
## Description
`HEADROOM_SAVINGS_PROFILE` is an implemented env var
(`headroom/agent_savings.py`) that selects a named profile bundling
Headroom's whole compression posture (proxy mode, keep-ratio, which
messages are compressed, `force_kompress`, etc.) at proxy startup. It
was entirely undocumented — `grep` over `docs/` found zero mentions.
Related, the proxy docs were **misleading about the default optimization
mode**: `docs/content/docs/proxy.mdx` stated `--mode` defaults to
`token`, but the code default is `cache`:
```python
# headroom/cli/proxy.py — the Click option has no default
@click.option("--mode", default=None, ...)
# ... mode resolution (default is CACHE):
effective_mode = normalize_proxy_mode(mode or os.environ.get("HEADROOM_MODE") or PROXY_MODE_CACHE)
```
A bare `headroom proxy` (no `--mode`, no `HEADROOM_MODE`) runs in
**cache** mode, and the default `coding` savings profile also sets
`proxy_mode="cache"` — which is exactly what the issue reporter found
confusing.
This documents `HEADROOM_SAVINGS_PROFILE` and corrects the `--mode`
default rows so the doc is accurate and internally consistent.
Closes #2031
## 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)
- [x] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
`docs/content/docs/proxy.mdx` only:
- Corrected the `--mode` default in the Core-options table and the
Context-management table (`token` → `cache`), each pointing to the new
Savings profiles section for the reason.
- Added a `### Savings profiles` section documenting: the
`HEADROOM_SAVINGS_PROFILE` env var; a table of the four built-in
profiles (`coding` default, `balanced` fallback, `agent-90`, `general`)
with target savings, mode, and `force_kompress`; the unset→`coding`
default; the unknown-value→`balanced` warning-and-fallback (proxy never
fails to start); and the mode precedence (explicit `--mode` >
`HEADROOM_MODE` seeded by a profile > `cache` default), with an example.
No code change. Every documented value is pinned to
`headroom/agent_savings.py` (profile definitions) and
`headroom/cli/proxy.py` (default-mode resolution).
## Testing
- [x] Unit tests not run; docs-only source verification performed
- [x] Linting not run; docs-only MDX/source verification performed
- [x] Type checking not applicable; no Python code changed
- [x] New tests not applicable; documentation-only correction
- [x] Manual testing performed
### Test Output
Docs-only change; verification is cross-checking every documented value
against the source of truth:
```text
$ grep -n "DEFAULT_PROFILE = \|FALLBACK_PROFILE = " headroom/agent_savings.py
14:FALLBACK_PROFILE = "balanced"
18:DEFAULT_PROFILE = "coding"
# profile modes / knobs (agent_savings.py):
# coding → proxy_mode="cache", force_kompress=False, target_ratio=None (emergent)
# balanced → proxy_mode="token", force_kompress=False, target_ratio=0.30
# agent-90 → proxy_mode="token", force_kompress=True, target_ratio=0.10
# general → proxy_mode="token", force_kompress=False, target_ratio=None (emergent)
$ grep -n "effective_mode\|PROXY_MODE_CACHE" headroom/cli/proxy.py
# effective_mode = normalize_proxy_mode(mode or os.environ.get("HEADROOM_MODE") or PROXY_MODE_CACHE)
# → confirms the real default optimization mode is cache, not token
```
MDX sanity: code fences balance (even count) and the `### Savings
profiles` heading slugifies to `#savings-profiles`, matching the two
in-page anchor links added to the mode rows.
## Real Behavior Proof
- **Environment:** Windows 11; docs source inspected against the working
tree at the current `main` base.
- **Exact command / steps:** Each documented fact is grounded in code —
profile names, modes, `force_kompress`, and target ratios come from
`headroom/agent_savings.py:_PROFILES`; the default profile (`coding`)
from the `os.environ.get("HEADROOM_SAVINGS_PROFILE") or "coding"` reads
in `headroom/cli/proxy.py` and `headroom/proxy/server.py`; the `cache`
default mode from `headroom/cli/proxy.py`'s `mode or HEADROOM_MODE or
PROXY_MODE_CACHE`; the unknown-value fallback from
`get_agent_savings_profile` (`agent_savings.py`).
- **Observed result:** The new section's table and prose match those
sources exactly, and the previously-wrong `--mode` default rows now
state `cache`.
- **Not tested:** A live render of the Fumadocs/Next.js docs site (no
local docs build run here) — the change is MDX-syntax-valid (balanced
fences, well-formed table, standard heading-anchor slug).
## 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] Code comments not applicable; documentation-only change
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] Tests not applicable; docs-only facts verified against source
- [x] New and existing unit tests pass locally with my changes
- [x] CHANGELOG not applicable; documentation-only correction
## Screenshots (if applicable)
N/A (docs prose/table addition; a rendered screenshot can be added if
the docs site is built for preview).
## Additional Notes
- Test/tests-added checklist items are N/A — this is a
documentation-only change.
- Out of scope (intentionally): the `--mode` Click **help text** in
`headroom/cli/proxy.py` also says "default: token" and is likewise
inaccurate, but correcting Python help text is a code change beyond this
docs issue — noted as a possible follow-up.
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
|
||
|
Andrew Barnes
|
099c66432b
|
feat(proxy): expose retry delay configuration (#2077)
## Description Expose Headroom's existing retry-delay configuration through the proxy CLI and environment so operators can tune upstream backoff without changing code. Existing 1000 ms / 30000 ms defaults remain unchanged. Closes #2030 ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Changes Made - add CLI flags for initial and maximum upstream retry delays - support `HEADROOM_RETRY_BASE_DELAY_MS` and `HEADROOM_RETRY_MAX_DELAY_MS` - validate non-negative values and forward them into `ProxyConfig` - cover explicit CLI values and environment-variable wiring ## Testing - [x] Focused unit tests pass (`pytest`) - [x] Touched-file linting passes (`ruff check`) - [ ] Type checking passes (`mypy headroom`) — not run - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_cli_proxy_improvements.py::TestRetryDelayValidation tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring::test_headroom_retry_delays_from_env -q 4 passed uv run ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! uv run ruff format --check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py 2 files already formatted ``` ## Real Behavior Proof - Environment: local Python test environment - Exact command / steps: invoke the focused Click CLI tests with explicit delay flags and `HEADROOM_RETRY_*` environment variables - Observed result: all four cases passed; parsed values reached `ProxyConfig`, while invalid negative values were rejected - Not tested: live upstream retry timing or the full repository test suite ## 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] My changes generate no new warnings in the scoped checks - [x] I have added tests that prove the feature works - [x] Relevant existing and new unit tests pass locally - [ ] Documentation and changelog updates — not applicable for these self-documenting CLI options ## Additional Notes The existing runtime backoff helper still caps the base delay against the maximum. This change only exposes values already supported by `ProxyConfig`. Co-authored-by: Tejas Chopra <chopratejas@gmail.com> |
||
|
Andrew McFague
|
ebe0a3bd7b
|
feat(proxy): add provider-only HTTP proxy (#1807)
## Description Adds provider-only HTTP proxy configuration for upstream LLM calls without setting process-wide proxy environment variables. `--http-proxy` and `HEADROOM_HTTP_PROXY` are scoped to the proxy server's provider HTTPX clients, and HTTP/2 is disabled for those clients when the proxy is set so HTTPS provider APIs can tunnel through CONNECT. Using process env vars such as `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, or `NO_PROXY` would also affect HTTPX, but those vars are inherited by tool executions, so this keeps proxy routing out of the global environment. Closes: N/A ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added `--http-proxy` with `HEADROOM_HTTP_PROXY` fallback. - Passed the proxy URL only into provider HTTPX clients. - Disabled provider HTTP/2 when the proxy is configured. - Preserved the new setting through direct server startup and multi-worker config serialization. - Documented the flag/env var and why global `HTTP_PROXY`-style vars are not suitable for provider-only routing. - Added an Unreleased changelog entry. - Added coverage for CLI/env wiring, worker serialization, and HTTPX client options. ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [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 $ uv run --frozen pytest tests/test_cli_proxy_improvements.py tests/test_proxy_scalability.py ============================== 72 passed in 5.95s ============================== $ uv run --frozen ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_scalability.py All checks passed! $ uv run --frozen mypy headroom --ignore-missing-imports Success: no issues found in 406 source files $ env -u HTTP_PROXY -u http_proxy npm --prefix docs run types:check [MDX] generated files in 6.351916000000074ms Generating route types... [MDX] generated files in 5.813166999999794ms ✓ Types generated successfully $ git diff --check # no output ``` ## Real Behavior Proof - Environment: local provider setup that requires outbound LLM traffic through an HTTP proxy - Exact command / steps: ran focused pytest, Ruff, mypy, docs `types:check`, and `git diff --check` after rebasing the branch onto `origin/main`; reviewed the docs and changelog diffs; actively used the new proxy setting locally for a provider that requires proxied egress - Observed result: CLI/env/config tests passed; static checks passed; docs type generation passed; local provider traffic can be routed through the provider-only proxy setting without exporting global proxy variables to tool executions - Not tested: broad provider matrix across every supported upstream ## 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 ## Screenshots (if applicable) N/A. CLI/backend/docs update only. ## Additional Notes The branch keeps implementation, docs, changelog, and formatting changes in separate commits. |
||
|
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. |
||
|
gglucass
|
814ffa36a4
|
fix(proxy): wire --compression-max-workers / HEADROOM_COMPRESSION_MAX_WORKERS (#1632)
## Description `ProxyConfig.compression_max_workers` is documented as settable via `--compression-max-workers` / `HEADROOM_COMPRESSION_MAX_WORKERS` and is consumed by `HeadroomProxy.__init__` to bound the dedicated compression threadpool. But the proxy CLI never defined the option and never passed the value into `ProxyConfig`, so the field was permanently `None` and always resolved to the `min(32, (cpu_count or 1) * 4)` default. Neither the flag nor the env var had any effect. This matters under concurrent sessions: the compression pool runs CPU-bound Kompress work that releases the GIL, so `cpu*4` oversubscribes cores and there was no way to cap it despite the docs promising one. Closes # ## 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 - Added the `--compression-max-workers` click option (with `envvar="HEADROOM_COMPRESSION_MAX_WORKERS"`) to the `proxy` command, mirroring the existing `--anthropic-pre-upstream-concurrency` wiring. - Added the `compression_max_workers` parameter to the `proxy()` signature and passed it into the `ProxyConfig(...)` construction. - No change to `HeadroomProxy` — it already reads `config.compression_max_workers` and clamps `< 1` to 1. ## 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 $ pytest tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q 3 passed in 1.50s $ pytest tests/test_cli_proxy_improvements.py -q 48 passed in 5.04s $ ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! $ mypy headroom/cli/proxy.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS, Python 3.10.18, branch off upstream/main @ 0.28.0 - Exact command / steps: new tests assert the value reaches `ProxyConfig` via both `--compression-max-workers 3` (flag) and `HEADROOM_COMPRESSION_MAX_WORKERS=5` (env), and that it stays `None` when unset. - Observed result: flag -> `config.compression_max_workers == 3`; env -> `== 5`; unset -> `is None`. - Not tested: end-to-end proxy run under real concurrent load (the pool-sizing effect itself is already covered by existing `test_proxy_compression_executor.py`). ## 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes CHANGELOG left untouched: this makes existing documented behavior actually work rather than adding new surface. N/A: manual testing (covered by unit tests + existing executor tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Rod Boev
|
3be2526b76
|
fix(proxy): add an Anthropic buffered read-timeout override (#1331)
## Description Buffered Anthropic `/v1/messages` requests still use Headroom's generic 300-second read timeout, which can produce proxy-generated `502 ReadTimeout` errors on long turns. This adds a dedicated buffered Anthropic timeout, keeps it applied across CCR and memory continuations plus batch paths, and makes the direct server entrypoint enforce the same positive-integer contract as the Click CLI. Closes #1261. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `anthropic_buffered_request_timeout_seconds` for buffered Anthropic reads. - Routed `/v1/messages`, CCR continuation, memory continuation, batch create, batch passthrough, and batch results through that timeout. - Enforced the same positive-integer validation for `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS` and `--anthropic-buffered-request-timeout-seconds` in both startup paths. - Added focused regressions and updated `CHANGELOG.md`. ## Testing - [x] `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring` - [x] `uv run ruff check .` - [x] `uv run ruff format . --check` ### Test Output ```text $ uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring 17 passed in 3.42s $ uv run ruff check . All checks passed! $ uv run ruff format . --check 966 files already formatted ``` ## Real Behavior Proof - Environment: local FastAPI `TestClient` with stubbed retry and HTTP client seams - Exact command / steps: run `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring`; the tests build `ProxyConfig(request_timeout_seconds=7, connect_timeout_seconds=3, anthropic_buffered_request_timeout_seconds=19)`, drive `/v1/messages`, `/v1/messages/batches`, `/v1/messages/batches/{batch_id}/results`, a CCR continuation, and a memory continuation through `TestClient`, then verify `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS=0` falls back to `600`, `--anthropic-buffered-request-timeout-seconds 0` is rejected, and default proxy timeouts stay `read=300` and `write=300` - Observed result: buffered Anthropic paths use `httpx.Timeout(connect=3, read=19, write=7, pool=3)`, continuation requests stay on that same budget, invalid zero-valued startup config is rejected or ignored back to the default, and unrelated proxy timeout defaults stay unchanged - Not tested: live upstream Anthropic latency beyond the focused stubbed-timeout regression ## 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 added tests that prove the fix - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable |
||
|
Patrick A
|
028efabb4e
|
feat(cli): comprehensive help text, validation, and exception handling improvements (#640)
## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com> |