From b699bedf95286138b1dda444ebc5f86bf7041f5a Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Sat, 11 Jul 2026 09:37:31 +0530 Subject: [PATCH] fix(models): version-boundary longest-prefix match in ModelRegistry.get (#1658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `ModelRegistry.get()` has a prefix fallback for versioned model ids. It accepted **any** registered name as a bare `str.startswith` prefix and returned the **first** match in dict-insertion order: ```python for name, info in _MODELS.items(): if model_lower.startswith(name): return info ``` Two concrete failures fall out of that: - `gpt-4` is registered before `gpt-4-32k`, so `get("gpt-4-32k-0613")` matches `gpt-4` first and returns an **8192**-token window instead of `gpt-4-32k`'s **32768**. - `gpt-4.1` / `gpt-4.5-preview` aren't registered, so they also match `gpt-4` and inherit its **8192**-token window — even though they're much larger, distinct models. `get_context_limit()` reads straight from `get()` (no LiteLLM fallback), so both cases make the proxy believe a nearly-empty context is almost full and compress far too aggressively — or reject — on requests that are actually small. This is silent: no error, just a wrong number driving every downstream compression decision for those models. ## Fix The fallback now: 1. Only matches when the registered name ends at a **version boundary** in the query — the next character must be a separator (`-`, `/`, `:`, `@`, `_`) — so `gpt-4.1`'s `.` no longer matches `gpt-4` (it falls through to the caller's default instead of a wrong 8192). 2. Picks the **longest** qualifying name, so `gpt-4-32k-0613` → `gpt-4-32k`. Exact and alias lookups are unchanged, and boundary-separated variants like `gpt-4o-new-version` still resolve to `gpt-4o`. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/models/registry.py`: replace the first-match `startswith` prefix loop in `ModelRegistry.get` with a longest-prefix-at-a-version-boundary match. - `tests/test_models.py`: add regression tests — `gpt-4-32k-0613` → `gpt-4-32k` (32768), and `gpt-4.1`/`gpt-4.5-preview` no longer resolve to gpt-4's 8192 window. - `CHANGELOG.md`: Bug Fixes entry under Unreleased. ## Testing - [x] New tests added for the fixed behavior (`tests/test_models.py`) - [x] Linting passes (`ruff check`) and formatting is clean (`ruff format --check`) - [ ] Full `pytest` run deferred to CI — see Real Behavior Proof for why I verify the logic with a dependency-free script locally. ```text $ uv run ruff check headroom/models/registry.py tests/test_models.py All checks passed! $ uv run ruff format --check headroom/models/registry.py tests/test_models.py 2 files already formatted ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, headroom built from this branch (`uv sync --extra dev`). Importing `headroom` pulls in the torch/transformers stack; a full `pytest` run exhausts memory and gets OOM-killed on this box, so I verify the matching logic with a dependency-free script (only stdlib) and leave the full pytest to CI. - Exact command / steps: replicated the relevant `_MODELS` registration order (`gpt-4o`, `gpt-4-turbo`, `gpt-4`, `gpt-4-32k`) and the new longest-prefix-with-boundary loop in a standalone script (no `headroom` import), then asserted the resolved context windows. - Observed result: `gpt-4-32k-0613` resolves to 32768 (was 8192 under first-match), `gpt-4.1`/`gpt-4.5-preview` fall through to the caller default (no longer 8192), and `gpt-4o-new-version` / `gpt-4` / `gpt-4-0613` resolve exactly as before: ```text OK: gpt-4-32k-0613 -> 32768 (was 8192 under old first-prefix-wins) OK: gpt-4.1 / gpt-4.5-preview -> default (not 8192) OK: gpt-4o-new-version, gpt-4, gpt-4-0613 still resolve as before REGISTRY LOGIC VERIFIED ``` - Not tested: I did not add explicit registry entries for `gpt-4.1`/`gpt-4.5` (their real windows) — that's a data addition, separate from this matching-logic fix; today they fall back to the caller's default, which is honest for an unregistered model and strictly better than the previous wrong 8192. Full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] 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 - [ ] New and existing unit tests pass locally with my changes — ran lint + a standalone logic check; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - No new dependencies; pure logic change in one function plus tests. - Found via a read-through of the registry while looking at how context limits drive compression decisions. --- CHANGELOG.md | 1 + headroom/models/registry.py | 25 +++++++++++++++++++++---- tests/test_models.py | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b3630e1e..b91c005f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **memory/sync:** stop the Claude Code sync adapter from clobbering distinct memories that share a first line. `write_memories` derived each file name from the first line of the content only (`headroom_{slug}.md`), so two different DB memories whose first lines slugify identically wrote to the same file and the second silently overwrote the first — and because the loser never landed on disk, the next sync re-exported it, ping-ponging the pair forever. When the slug is already taken by a *different* memory (distinct `headroom_id`) the file name is now disambiguated with a content-hash suffix; an update to the same memory still rewrites its slug file in place, so existing file names are unchanged. * **cli/proxy:** honor `HEADROOM_MIN_TOKENS=0` / `HEADROOM_MAX_ITEMS=0`. The Click `proxy` command built these with `_get_env_int_optional(name) or 500`/`or 50`, so an explicit `0` — a legitimate value (`min_tokens_to_crush=0` means "crush every item") — was treated as falsy and silently replaced with the default. The `headroom proxy` argparse path already preserved `0` via `_get_env_int`, so the two entry points disagreed. The Click path now uses the same None-checking helper. * **proxy:** strip the inbound `Content-Encoding`/`Transfer-Encoding` request headers on the Anthropic `/v1/messages` and OpenAI `/v1/chat/completions` paths before forwarding upstream. `read_request_json_with_bytes` already decompresses the inbound body (zstd/gzip/deflate/br), so the bytes forwarded upstream are plain JSON — but these two handlers left the original `content-encoding` header in place, so a client (or an edge proxy like a Cloudflare Worker) that sent a compressed body got its request rejected with upstream HTTP 400 because the provider tried to decompress already-decoded JSON. The `/v1/responses` handler already carried this fix (#1542); it is now applied to the messages and chat paths too. +* **models:** fix the model registry's prefix fallback silently returning the wrong context window. `ModelRegistry.get` accepted any registered name as a `str.startswith` prefix and returned the *first* match, so `gpt-4-32k-0613` resolved to `gpt-4` (8192) instead of `gpt-4-32k` (32768), and unregistered ids like `gpt-4.1`/`gpt-4.5` inherited `gpt-4`'s 8192-token window — making the proxy think a nearly-empty context was almost full and compress far too aggressively. The fallback now requires the registered name to end at a version boundary in the query (so `gpt-4.1` no longer matches `gpt-4`) and picks the longest qualifying name (so `gpt-4-32k-0613` → `gpt-4-32k`). * **proxy:** include the system prompt, tools, and the response-shaping request fields in the SemanticCache key. `_compute_key` hashed only `{model, messages}`, so two non-streaming requests with identical messages but a different top-level `system` prompt, tool set, sampling config, or output-shaping field collided on one key and the second caller was served the first's cached response — generated under different request semantics, in the default config (`cache_enabled` defaults on). The key now folds the request fields that shape generation — `temperature`/`top_p`/`top_k`/`max_tokens`/`stop`, plus OpenAI `tool_choice`/`response_format`/`parallel_tool_calls`/`seed`/`presence_penalty`/`frequency_penalty`/`logit_bias`/`n`/`logprobs`/`top_logprobs`/`reasoning_effort`/`verbosity`/`modalities` and Anthropic `thinking`/`tool_choice`/`output_config` — canonicalizing `system`/`tools` so a moved `cache_control` breakpoint does not fragment it, and the handlers snapshot the fields once at the cache read and reuse them at write so a body mutated by the pipeline cannot diverge the key. Non-streaming path only. * **learn (verbosity):** `--verbosity --apply --all` now aggregates the savings baseline across every project instead of overwriting it per project (last-project-wins), which previously left the output shaper with a tiny, unrepresentative baseline. The applied verbosity level comes from the project with the most samples ([#1288](https://github.com/headroomlabs-ai/headroom/pull/1288)). * **proxy/anthropic:** restore token-mode compression on continued Claude Code turns with a frozen prefix and deferred CCR tool injection. Token mode now runs request-side compression even when the client did not pre-register `headroom_retrieve`, relying on the existing marker-triggered injection override to keep emitted CCR markers redeemable ([#1487](https://github.com/headroomlabs-ai/headroom/issues/1487)). diff --git a/headroom/models/registry.py b/headroom/models/registry.py index 4a0863bf2..6526c9c2a 100644 --- a/headroom/models/registry.py +++ b/headroom/models/registry.py @@ -631,12 +631,29 @@ class ModelRegistry: if model_lower in _ALIASES: return _MODELS[_ALIASES[model_lower]] - # Prefix matching + # Prefix matching. Two rules keep this from silently mis-resolving + # newer or larger variants: + # 1. The registered name must end at a version boundary in the query + # (next char is a separator like ``-``/``/``/``:``/``@``/``_``), so + # ``gpt-4.1`` — a distinct model, not a variant of ``gpt-4`` — does + # not inherit gpt-4's 8192-token window (the ``.`` is not a + # boundary, so it no longer matches). + # 2. When several names qualify, the LONGEST wins, so + # ``gpt-4-32k-0613`` resolves to ``gpt-4-32k`` (32768) rather than + # the shorter ``gpt-4`` (8192) that happens to be registered first. + best: ModelInfo | None = None + best_len = -1 for name, info in _MODELS.items(): - if model_lower.startswith(name): - return info + if not model_lower.startswith(name): + continue + rest = model_lower[len(name) :] + if rest and rest[0] not in ("-", "/", ":", "@", "_"): + continue + if len(name) > best_len: + best = info + best_len = len(name) - return None + return best @classmethod def resolve( diff --git a/tests/test_models.py b/tests/test_models.py index 578e3674a..69b3e0c85 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -90,6 +90,24 @@ class TestModelRegistry: assert info is not None assert info.name == "gpt-4o" + def test_get_prefix_matching_prefers_longest_registered_name(self): + """`gpt-4-32k-0613` must resolve to `gpt-4-32k` (32768), not the + shorter `gpt-4` (8192) that is registered first.""" + info = ModelRegistry.get("gpt-4-32k-0613") + assert info is not None + assert info.name == "gpt-4-32k" + assert ModelRegistry.get_context_limit("gpt-4-32k-0613") == 32768 + + def test_get_prefix_matching_requires_version_boundary(self): + """`gpt-4.1`/`gpt-4.5` are distinct models, not variants of `gpt-4`. + A `.`-separated suffix must not match `gpt-4`, so they no longer + inherit gpt-4's 8192-token window (they fall back to the default).""" + assert ModelRegistry.get("gpt-4.1") is None + assert ModelRegistry.get("gpt-4.5-preview") is None + # Not silently reported as an 8192-token model: + assert ModelRegistry.get_context_limit("gpt-4.1") != 8192 + assert ModelRegistry.get_context_limit("gpt-4.1", default=100) == 100 + def test_resolve_future_google_family_fallback(self): """Resolve should return provider-scoped fallbacks for plausible future models.""" with patch("headroom.models.registry.get_model_pricing", return_value=None):