From e9661a8cf8145ecc98946823d5b9eac3131afde5 Mon Sep 17 00:00:00 2001 From: Omar Baradei Date: Thu, 9 Jul 2026 17:46:55 -0700 Subject: [PATCH 1/2] Refresh context-length zero-guard on current upstream/main Reapply the non-positive context-length guards onto the post-history-replacement mainline without carrying any stale branch history. save_context_length() now refuses to persist length <= 0 (keeping upstream's normalized _context_cache_key), and get_model_context_length() drops non-positive cache hits at the head of the invalidation chain (Codex/Kimi/MiniMax/Grok branches become elif) so a poisoned entry re-resolves instead of short-circuiting to 0. Refresh of PR #25812; original head d62ed5eb92f057d8c707ba937b44f168f2df0677. --- agent/model_metadata.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/agent/model_metadata.py b/agent/model_metadata.py index bd974af5e870..41c8fe6796aa 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -1452,6 +1452,15 @@ def save_context_length(model: str, base_url: str, length: int) -> None: Cache key is ``model@base_url`` so the same model name served from different providers can have different limits. """ + # Never persist non-positive values — a 0 or negative context length + # is always a bug and would poison the cache, causing downstream + # `get_model_context_length()` to return 0 (since `0 is not None`). + if length <= 0: + logger.warning( + "Refusing to cache non-positive context length %s -> %s tokens", + f"{model}@{base_url}", length, + ) + return key = _context_cache_key(model, base_url) cache = _load_context_cache() if cache.get(key) == length: @@ -2664,8 +2673,19 @@ def get_model_context_length( if base_url and not _skip_persistent_context_cache(base_url, provider): cached = get_cached_context_length(model, base_url) if cached is not None: + # Reject non-positive cached values — a 0 or negative value + # is always a bug (corrupted cache, probe failure, or manual + # edit). Without this guard, `0 is not None` short-circuits + # the resolution chain and the compressor gets context_length=0, + # breaking every status-bar and /usage display downstream. + if cached <= 0: + logger.warning( + "Dropping non-positive cache entry %s@%s -> %s; re-resolving", + model, base_url, cached, + ) + _invalidate_cached_context_length(model, base_url) # Invalidate stale 32k cache entries for Kimi-family models. - if cached <= 32768 and _model_name_suggests_kimi(model): + elif cached <= 32768 and _model_name_suggests_kimi(model): logger.info( "Dropping stale Kimi cache entry %s@%s -> %s (OpenRouter underreport); " "re-resolving via hardcoded defaults", From 064d23f6ddbe69c35e8433137d7ebe2d6a37c80b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:51:39 -0700 Subject: [PATCH 2/2] test: regression coverage for the non-positive context-cache guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #25812 — the original PR shipped without tests. --- tests/agent/test_model_metadata.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/agent/test_model_metadata.py b/tests/agent/test_model_metadata.py index cb73c608392d..78ba56c1675c 100644 --- a/tests/agent/test_model_metadata.py +++ b/tests/agent/test_model_metadata.py @@ -1145,6 +1145,33 @@ def test_get_context_length_from_vllm_max_model_len_error(self): class TestContextLengthCache: + def test_non_positive_lengths_never_persisted(self, tmp_path): + """save_context_length must refuse 0/negative values — a persisted 0 + short-circuits step 1 (``0 is not None``) and poisons the whole + resolution chain downstream (#25812).""" + cache_file = tmp_path / "cache.yaml" + with patch("agent.model_metadata._get_context_cache_path", return_value=cache_file): + save_context_length("test/model", "http://x", 0) + save_context_length("test/model", "http://x", -1) + assert get_cached_context_length("test/model", "http://x") is None + + @patch("agent.model_metadata.fetch_model_metadata") + def test_non_positive_cached_entry_dropped_and_reresolved(self, mock_fetch, tmp_path): + """A pre-existing 0 entry (corrupted cache / manual edit) must be + invalidated at step 1 and re-resolved instead of returned.""" + mock_fetch.return_value = {} + cache_file = tmp_path / "cache.yaml" + with patch("agent.model_metadata._get_context_cache_path", return_value=cache_file): + # Write the poison entry directly — save_context_length now refuses it. + cache_file.write_text( + "context_lengths:\n test/model@http://x: 0\n", encoding="utf-8" + ) + assert get_cached_context_length("test/model", "http://x") == 0 + result = get_model_context_length("test/model", base_url="http://x") + assert result > 0 + assert get_cached_context_length("test/model", "http://x") != 0 + + def test_null_context_lengths_key_returns_empty(self, tmp_path): """``context_lengths:`` with no value parses as None — must behave like an empty cache instead of crashing every caller (#47135)."""