diff --git a/CHANGELOG.md b/CHANGELOG.md index c3559494e..467829582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **tokenizers:** resolve HuggingFace tokenizer names by the most-specific prefix. `get_tokenizer_name` scanned `MODEL_TO_TOKENIZER` in dict-insertion order and returned the first key the model merely starts with, so a short family key shadowed a more-specific one — `qwen2-7b-instruct` matched `qwen` before `qwen2`/`qwen2-7b` and resolved to the Qwen1 tokenizer (a different vocabulary, hence wrong token counts); `qwen2.5-*` and `deepseek-v2.x` were mis-resolved the same way. It now picks the longest matching prefix, mirroring the order-dependent-prefix guard the sibling tiktoken `get_encoding_for_model` already documents. * **pricing:** map retired `claude-3-sonnet-20240229` to a Sonnet-tier price instead of Haiku. When LiteLLM's cost DB lacks the retired model, resolution falls through to `MODEL_ALIASES`, which pointed Claude 3 Sonnet (a $3/$15-per-1M model) at `claude-3-haiku-20240307` ($0.25/$1.25) — a different tier that underpriced every cost/savings figure for that model ~12x on both input and output. It now aliases to `claude-sonnet-4-20250514`, the same-price target the other retired-Sonnet aliases already use. * **cache/semantic:** don't evict an unrelated entry when re-storing a key that is already cached. `SemanticCache.put` ran its at-capacity eviction loop before computing the entry's key, so overwriting a key that was already present (a duplicate or retried store) still evicted the LRU-oldest distinct entry even though an in-place update grows nothing. That silently dropped a live entry and turned a later lookup for it into a false cache miss. The key is now computed first and the eviction loop only runs when the key is genuinely new (mirroring `CompressionCache.store_compressed`, which deletes-then-inserts). +* **wrap/claude:** don't raise `UnboundLocalError` in the cleanup `finally` when the proxy fails to start. `claude()` referenced `_wrap_settings_path` in its `finally` block but only assigned it inside the `try`, after `_ensure_proxy` (which raises on port exhaustion or a failed proxy start). An early failure therefore made the `finally` raise `UnboundLocalError` — replacing the real error with a raw traceback and, because the `finally` aborted before `cleanup()`, skipping proxy cleanup and wrap-marker clearing. `_wrap_settings_path` is now bound before the `try` alongside the other cleanup holders (`proxy_holder`, `_saved_base_url`, …), so the `finally` is always safe. * **wrap/codex:** export the detected custom upstream base URL so Codex actually routes to it. `_inject_codex_provider_config` detects an OpenAI-compatible gateway declared in `~/.codex/config.toml` and writes an `X-Headroom-Base-Url` header mapped to `HEADROOM_CODEX_UPSTREAM_BASE_URL`, and it returns that URL for the caller to export. But `_prepare_codex_wrap_state` discarded the return and `_run_codex_wrap` never set the env var, so Codex emitted no header, the proxy fell back to `api.openai.com`, and the user's gateway key was sent to OpenAI (which rejects it). This restores the wiring a later refactor dropped: `_prepare_codex_wrap_state` now returns the URL and `_run_codex_wrap` exports it into the launch env when set (a user-provided value still wins) (regression of [#1614](https://github.com/chopratejas/headroom/issues/1614)). * **cache:** normalize embeddings before the semantic dynamic-content similarity check. `SemanticDetector` scored sentences with `np.dot` against exemplar embeddings and compared the result to `semantic_threshold` (a 0-1 cosine value), but `sentence_transformers.encode(..., convert_to_numpy=True)` does not normalize, so the dot product was an unbounded inner product (vector norms ~5-15) rather than a cosine similarity. Nearly every sentence cleared the 0.7 threshold, so the semantic tier flagged almost all text as dynamic and stripped static content, busting the cache it is meant to protect (a standalone repro scores an unrelated sentence, true cosine ~0.1, at a raw dot of ~9). Both `encode` calls now pass `normalize_embeddings=True`, matching the siblings in `prediction/feature_extractor.py` and `memory/adapters/embedders.py`, so the dot product is a true cosine in [-1, 1]. * **subscription:** cap `HeadroomContribution.efficiency_pct` at a real removal ratio so it can't exceed 100%. The numerator used `total_saved()` (which includes `tokens_saved_cache_reads`) while the denominator `raw_without_headroom()` excludes cache reads, so a contribution with large prefix-cache reads and small forwarded input reported impossible values (e.g. `tokens_submitted=100`, `tokens_saved_cache_reads=1000` → `1000.0%` on the dashboard). Cache reads are a provider-side discount on tokens that were still forwarded, not tokens Headroom removed, so the ratio now uses the sibling `compression_saved()` (compression + CLI filtering) as the numerator — bounded by its own denominator. `total_saved()` is unchanged for its other callers. diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index d69ac7ce3..7f54b570a 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -4016,6 +4016,11 @@ def claude( _settings_foundry: list[bool] = [False] port_holder: list[int] = [port] _settings_vertex: list[bool] = [False] + # Bind before the try so the finally can always reference it. It is otherwise + # only assigned inside the try (after _ensure_proxy, which can raise), so an + # early proxy-start failure would make the finally raise UnboundLocalError, + # masking the real error and skipping cleanup(). Mirrors the holders above. + _wrap_settings_path = Path.cwd() / ".claude" / "settings.local.json" cleanup = _make_cleanup(proxy_holder, port_holder) signal.signal(signal.SIGINT, _ignore_child_sigint) signal.signal(signal.SIGTERM, cleanup) @@ -4219,7 +4224,8 @@ def claude( # daemon's environment) also route through Headroom. _settings_vertex[0] = bool(use_vertex) _settings_foundry[0] = bool(foundry_upstream) and not _settings_vertex[0] - _wrap_settings_path = Path.cwd() / ".claude" / "settings.local.json" + # _wrap_settings_path is bound before the try (above) so the finally is + # always safe; the value is unchanged here. _check_and_clear_stale_wrap_marker( _wrap_settings_path, key=_claude_wrap_base_url_env_key( diff --git a/tests/test_cli/test_wrap_claude_finally_unbound.py b/tests/test_cli/test_wrap_claude_finally_unbound.py new file mode 100644 index 000000000..398717eab --- /dev/null +++ b/tests/test_cli/test_wrap_claude_finally_unbound.py @@ -0,0 +1,70 @@ +"""Regression: `wrap claude`'s finally must not raise UnboundLocalError when the +proxy fails to start before `_wrap_settings_path` is assigned inside the try.""" + +from __future__ import annotations + +import pytest +from click.testing import CliRunner + +from headroom.cli import wrap as wrap_mod +from headroom.cli.main import main + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +def test_finally_survives_early_proxy_start_failure( + runner: CliRunner, monkeypatch: pytest.MonkeyPatch +) -> None: + state = {"restore_called": False, "cleanup_called": False} + + for key in ( + "ANTHROPIC_BASE_URL", + "ANTHROPIC_VERTEX_BASE_URL", + "ANTHROPIC_FOUNDRY_BASE_URL", + "ANTHROPIC_FOUNDRY_RESOURCE", + "CLAUDE_CODE_USE_VERTEX", + "CLAUDE_CODE_USE_FOUNDRY", + "VERTEX_TARGET_API_URL", + ): + monkeypatch.delenv(key, raising=False) + + monkeypatch.setattr(wrap_mod.shutil, "which", lambda _name: "/usr/bin/claude") + monkeypatch.setattr(wrap_mod, "_register_proxy_client", lambda _port: None) + monkeypatch.setattr(wrap_mod.signal, "signal", lambda *_a, **_k: None) + monkeypatch.setattr(wrap_mod, "_push_runtime_env", lambda *_a, **_k: None) + monkeypatch.setattr(wrap_mod, "_setup_coding_compressor", lambda *_a, **_k: None) + monkeypatch.setattr(wrap_mod, "_print_telemetry_notice", lambda: None) + monkeypatch.setattr(wrap_mod, "_write_claude_wrap_base_url", lambda *_a, **_k: None) + + def _fake_make_cleanup(_holder, _port): + def _cleanup() -> None: + state["cleanup_called"] = True + + return _cleanup + + monkeypatch.setattr(wrap_mod, "_make_cleanup", _fake_make_cleanup) + + def _fake_restore(*_a, **_k) -> None: + state["restore_called"] = True + + monkeypatch.setattr(wrap_mod, "_restore_claude_wrap_base_url", _fake_restore) + + def _boom(*_a, **_k): + raise RuntimeError("proxy failed to start") + + monkeypatch.setattr(wrap_mod, "_ensure_proxy", _boom) + + result = runner.invoke( + main, + ["wrap", "claude", "--no-context-tool", "--no-mcp", "--no-tokensave", "--no-serena"], + ) + + # The finally must complete: no UnboundLocalError masking the real failure, + # and both restore and cleanup ran even though the proxy failed before + # _wrap_settings_path was assigned inside the try. + assert not isinstance(result.exception, UnboundLocalError), result.output + assert state["restore_called"] is True + assert state["cleanup_called"] is True