From ae61abb59eb63670a0f5ca02d147bd745050b9b1 Mon Sep 17 00:00:00 2001 From: Herik Webb Date: Sun, 5 Jul 2026 09:08:28 -0400 Subject: [PATCH 1/5] feat(claude): modernize model defaults and bound inline-completion output - Default chat fallback moves to claude-sonnet-5 (current Sonnet tier); the previous claude-sonnet-4-5 default is a generation behind. - Inline completion falls back to claude-haiku-4-5: a suggestion has to beat the user's next keystroke and fires on every pause in typing, so the fastest/cheapest tier is the right default there. - Cap inline-completion max_tokens at 1024 (override with NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS); the old 10K ceiling let a rambling response generate for many seconds before the extraction regex threw most of it away. - fetch_claude_models prefers the Models API's max_input_tokens over litellm's static database, which lags new model releases; litellm remains the fallback when the field is absent. --- notebook_intelligence/claude.py | 24 +++++++-- tests/test_claude_models.py | 87 +++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index 262b6a3..6153c47 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -36,8 +36,17 @@ def _extract_text_from_content(content) -> str: CLAUDE_CODE_ICON_SVG = '' CLAUDE_CODE_ICON_URL = f"data:image/svg+xml;base64,{base64.b64encode(CLAUDE_CODE_ICON_SVG.encode('utf-8')).decode('utf-8')}" -CLAUDE_DEFAULT_CHAT_MODEL = "claude-sonnet-4-5" -CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL = "claude-sonnet-4-5" +# Fallbacks when the user hasn't picked a model. Chat wants the current +# Sonnet-tier balance of quality and speed; autocomplete wants the fastest, +# cheapest tier — a suggestion has to beat the user's next keystroke, and a +# Sonnet-class model is both slower and several times the cost per token +# for a surface that fires on every pause in typing. +CLAUDE_DEFAULT_CHAT_MODEL = "claude-sonnet-5" +CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL = "claude-haiku-4-5" +# Autocomplete suggestions are at most a few dozen lines; the previous +# 10K-token ceiling let a rambling response generate for many seconds +# before the extraction regex threw most of it away. +CLAUDE_INLINE_COMPLETION_MAX_TOKENS = int(os.getenv("NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS", "1024")) CLAUDE_CODE_CHAT_PARTICIPANT_ID = "claude-code" CLAUDE_CODE_MAX_BUFFER_SIZE = 20 * 1024 * 1024 # 20MB @@ -547,10 +556,17 @@ def fetch_claude_models(api_key: str = None, base_url: str = None) -> list[dict] page = client.models.list(limit=100) models = [] for model in page.data: + # The Models API reports the context window directly on + # newer SDK/service versions (max_input_tokens). Prefer it + # over litellm's static database, which lags new model + # releases and mislabels unknown ids with the 200K default. + context_window = getattr(model, "max_input_tokens", None) + if not isinstance(context_window, int) or context_window <= 0: + context_window = _get_context_window(model.id) models.append({ "id": model.id, "name": model.display_name, - "context_window": _get_context_window(model.id), + "context_window": context_window, }) _claude_models_cache.clear() _claude_models_cache.extend(models) @@ -702,7 +718,7 @@ def inline_completions(self, prefix, suffix, language, filename, context: Comple message = self._client.messages.create( model=self._model_id, - max_tokens=10000, + max_tokens=CLAUDE_INLINE_COMPLETION_MAX_TOKENS, system=f"""You are a code completion assistant. Your task is to generate intelligent autocomplete suggestions for the code at the cursor position for given language and active file type. This is not an interactive session, don't ask for clarifying questions, always generate a suggestion. Don't include any explanations for your response, just generate the code. Don't return any thinking or reasoning, just generate the code. You are given a code snippet with a prefix and a suffix. You need to generate a suggestion for the code that fits best in place of . You should return only the code that fits best in place of . You should provide multiline code if needed. Enclose the code in triple backticks, just return the code in language. You should not return any other text, just the code. DO NOT INCLUDE THE PREFIX OR SUFFIX IN THE RESPONSE. .ipynb files are Jupyter notebook files and for notebook files, you generate suggestions for a cell within the notebook. A cell can be a code cell with code or a markdown cell with markdown text. If the language is markdown, only return markdown text. If you need to install a Python package within a notebook cell code (for .ipynb files), use %pip install instead of !pip install . Follow the tags very carefully for proper spacing and indentations.""", messages=[ {"role": "user", "content": f"""Generate a single suggestion that fits best in place of cursor. The code is below in between tags and is the placeholder for the code to be filled in. Current language is {language} and the active file is {filename}. diff --git a/tests/test_claude_models.py b/tests/test_claude_models.py index 0db2eb1..fd4cdf9 100644 --- a/tests/test_claude_models.py +++ b/tests/test_claude_models.py @@ -175,3 +175,90 @@ def test_empty_api_key_passed_as_none(self, mock_anthropic_cls, mock_ctx_window) mock_anthropic_cls.assert_called_once_with( api_key=None, base_url=None, default_headers=ANY ) + + +class TestModelDefaults: + """Pin the fallback model choices. Chat falls back to the current + Sonnet tier; autocomplete falls back to the fastest/cheapest tier — + a suggestion has to beat the user's next keystroke, and it fires on + every pause in typing.""" + + def test_default_chat_model_is_current_sonnet(self): + from notebook_intelligence.claude import CLAUDE_DEFAULT_CHAT_MODEL + assert CLAUDE_DEFAULT_CHAT_MODEL == "claude-sonnet-5" + + def test_default_inline_completion_model_is_haiku(self): + from notebook_intelligence.claude import CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL + assert CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL == "claude-haiku-4-5" + + def test_inline_completion_max_tokens_is_bounded(self): + # Autocomplete output is a few dozen lines at most; the old + # 10K-token ceiling let a rambling response generate for many + # seconds before extraction threw most of it away. + from notebook_intelligence.claude import CLAUDE_INLINE_COMPLETION_MAX_TOKENS + assert 0 < CLAUDE_INLINE_COMPLETION_MAX_TOKENS <= 4096 + + @patch("anthropic.Anthropic") + def test_inline_completion_request_uses_bounded_max_tokens(self, mock_anthropic_cls): + from notebook_intelligence.claude import ( + CLAUDE_INLINE_COMPLETION_MAX_TOKENS, + ClaudeCodeInlineCompletionModel, + ) + + mock_message = Mock() + mock_message.content = [] + mock_anthropic_cls.return_value.messages.create.return_value = mock_message + + model = ClaudeCodeInlineCompletionModel("", api_key="test-key") + assert model.id == "claude-haiku-4-5" + + cancel_token = Mock() + cancel_token.is_cancel_requested = False + model.inline_completions("prefix", "suffix", "python", "nb.ipynb", None, cancel_token) + + kwargs = mock_anthropic_cls.return_value.messages.create.call_args.kwargs + assert kwargs["max_tokens"] == CLAUDE_INLINE_COMPLETION_MAX_TOKENS + assert kwargs["model"] == "claude-haiku-4-5" + + +class TestFetchClaudeModelsContextWindow: + def _make_mock_model(self, model_id, display_name, max_input_tokens=None): + m = Mock() + m.id = model_id + m.display_name = display_name + m.max_input_tokens = max_input_tokens + return m + + @patch("notebook_intelligence.claude._get_context_window", return_value=150000) + @patch("anthropic.Anthropic") + def test_prefers_models_api_context_window(self, mock_anthropic_cls, mock_ctx_window): + """When the Models API reports max_input_tokens, litellm's static + database (which lags new releases) must not be consulted.""" + from notebook_intelligence.claude import fetch_claude_models + + mock_page = Mock() + mock_page.data = [ + self._make_mock_model("claude-sonnet-5", "Claude Sonnet 5", max_input_tokens=1000000) + ] + mock_anthropic_cls.return_value.models.list.return_value = mock_page + + result = fetch_claude_models(api_key="test-key") + + assert result[0]["context_window"] == 1000000 + mock_ctx_window.assert_not_called() + + @patch("notebook_intelligence.claude._get_context_window", return_value=150000) + @patch("anthropic.Anthropic") + def test_falls_back_to_litellm_when_api_omits_window(self, mock_anthropic_cls, mock_ctx_window): + from notebook_intelligence.claude import fetch_claude_models + + mock_page = Mock() + mock_page.data = [ + self._make_mock_model("claude-sonnet-4-6", "Claude Sonnet 4.6", max_input_tokens=None) + ] + mock_anthropic_cls.return_value.models.list.return_value = mock_page + + result = fetch_claude_models(api_key="test-key") + + assert result[0]["context_window"] == 150000 + mock_ctx_window.assert_called_once_with("claude-sonnet-4-6") From 2dece2daf56ff71a376befb05b1364a2199def5e Mon Sep 17 00:00:00 2001 From: Herik Webb Date: Sun, 5 Jul 2026 09:26:20 -0400 Subject: [PATCH 2/5] fix(claude): validate and clamp the inline-completion max-tokens override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (PR #46): NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS was passed to the API unvalidated — a non-integer raised ValueError at import time and blocked the backend from starting, 0/negative values produced failing API requests, and a large value silently restored the unbounded behavior the cap exists to prevent. Parse via _bounded_int_env: malformed values warn and fall back to the 1024 default; out-of-range values are clamped to [1, 4096] with a warning. --- notebook_intelligence/claude.py | 38 +++++++++++++++++++++++++++++++-- tests/test_claude_models.py | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index 6153c47..e6eb5d7 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -43,10 +43,44 @@ def _extract_text_from_content(content) -> str: # for a surface that fires on every pause in typing. CLAUDE_DEFAULT_CHAT_MODEL = "claude-sonnet-5" CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL = "claude-haiku-4-5" +def _bounded_int_env(name: str, default: int, minimum: int, maximum: int) -> int: + """Parse an int env var defensively and clamp it to a sane range. + + This runs at import time: a malformed value must degrade to the + default with a warning, not raise ``ValueError`` and block the + backend from starting. Out-of-range values are clamped so an + operator typo (``0``, ``-5``, ``10000``) can't produce failing API + requests or restore the unbounded behavior the cap exists to + prevent. + """ + raw = os.getenv(name) + if raw is None or raw.strip() == "": + return default + try: + value = int(raw) + except ValueError: + log.warning( + "Invalid %s=%r (expected an integer); using default %d", + name, raw, default, + ) + return default + clamped = max(minimum, min(maximum, value)) + if clamped != value: + log.warning( + "%s=%d is outside the supported range [%d, %d]; clamping to %d", + name, value, minimum, maximum, clamped, + ) + return clamped + + # Autocomplete suggestions are at most a few dozen lines; the previous # 10K-token ceiling let a rambling response generate for many seconds -# before the extraction regex threw most of it away. -CLAUDE_INLINE_COMPLETION_MAX_TOKENS = int(os.getenv("NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS", "1024")) +# before the extraction regex threw most of it away. The override is +# clamped to [1, 4096] so it can't reintroduce that behavior or produce +# a max_tokens value the API rejects. +CLAUDE_INLINE_COMPLETION_MAX_TOKENS = _bounded_int_env( + "NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS", 1024, 1, 4096 +) CLAUDE_CODE_CHAT_PARTICIPANT_ID = "claude-code" CLAUDE_CODE_MAX_BUFFER_SIZE = 20 * 1024 * 1024 # 20MB diff --git a/tests/test_claude_models.py b/tests/test_claude_models.py index fd4cdf9..285a38d 100644 --- a/tests/test_claude_models.py +++ b/tests/test_claude_models.py @@ -262,3 +262,41 @@ def test_falls_back_to_litellm_when_api_omits_window(self, mock_anthropic_cls, m assert result[0]["context_window"] == 150000 mock_ctx_window.assert_called_once_with("claude-sonnet-4-6") + + +class TestBoundedIntEnv: + """NBI_CLAUDE_INLINE_COMPLETION_MAX_TOKENS is parsed at import time — + malformed values must degrade to the default (never raise and block + startup), and out-of-range values are clamped so an operator typo + can't restore unbounded autocomplete output or break API requests.""" + + def _parse(self, monkeypatch, raw): + from notebook_intelligence.claude import _bounded_int_env + if raw is None: + monkeypatch.delenv('NBI_TEST_INT', raising=False) + else: + monkeypatch.setenv('NBI_TEST_INT', raw) + return _bounded_int_env('NBI_TEST_INT', 1024, 1, 4096) + + def test_unset_returns_default(self, monkeypatch): + assert self._parse(monkeypatch, None) == 1024 + + def test_valid_value_parsed(self, monkeypatch): + assert self._parse(monkeypatch, '2048') == 2048 + + def test_empty_string_returns_default(self, monkeypatch): + assert self._parse(monkeypatch, ' ') == 1024 + + def test_malformed_value_returns_default(self, monkeypatch): + assert self._parse(monkeypatch, '1k') == 1024 + + def test_too_high_is_clamped_to_maximum(self, monkeypatch): + # 10000 would restore the old unbounded behavior the cap prevents. + assert self._parse(monkeypatch, '10000') == 4096 + + def test_zero_is_clamped_to_minimum(self, monkeypatch): + # max_tokens=0 would fail the Anthropic request outright. + assert self._parse(monkeypatch, '0') == 1 + + def test_negative_is_clamped_to_minimum(self, monkeypatch): + assert self._parse(monkeypatch, '-5') == 1 From 47a94d9cd19fd91d7cdc28884ef5e0a19609270b Mon Sep 17 00:00:00 2001 From: Herik Webb Date: Sun, 5 Jul 2026 09:33:53 -0400 Subject: [PATCH 3/5] fix(claude): resolve default model ids against the fetched model list Review follow-up (PR #46): the CLAUDE_DEFAULT_* constants name current first-party aliases, but a custom base_url can front an endpoint that serves a different catalog. resolve_default_model prefers the constant only when the endpoint lists it, then falls back to the newest id in the same tier, then to the first listed model. An empty cache (fetch not yet run, or failed) trusts the constant. --- notebook_intelligence/claude.py | 39 ++++++++++++++++++++++++++-- tests/test_claude_models.py | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index e6eb5d7..35ab375 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -538,6 +538,41 @@ def get_claude_models() -> list[dict]: """Return the cached list of available Claude models.""" return _claude_models_cache + +def resolve_default_model(preferred: str, tier_prefix: str) -> str: + """Pick a default model id, validated against the fetched model list. + + The ``CLAUDE_DEFAULT_*`` constants name current first-party aliases, + but a custom ``base_url`` can front an endpoint that serves a + different catalog. When the model cache has entries, prefer the + constant only if the endpoint actually lists it; otherwise fall back + to the newest id in the same tier (``claude-sonnet``/``claude-haiku`` + prefix), then to the first listed model. An empty cache (fetch not + yet run, or failed) trusts the constant — first-party endpoints + always serve it. + """ + models = get_claude_models() + if not models: + return preferred + ids = [m["id"] for m in models if isinstance(m.get("id"), str)] + if preferred in ids: + return preferred + tier_matches = sorted(i for i in ids if i.startswith(tier_prefix)) + if tier_matches: + # Anthropic ids sort by recency within a tier well enough for a + # fallback ('claude-sonnet-5' > 'claude-sonnet-4-5'). + fallback = tier_matches[-1] + elif ids: + fallback = ids[0] + else: + return preferred + log.info( + "Default model %r is not served by the configured endpoint; using %r", + preferred, fallback, + ) + return fallback + + def _get_context_window(model_id: str) -> int: """Get context window size for a model using litellm's model database.""" try: @@ -616,7 +651,7 @@ class ClaudeChatModel(ChatModel): def __init__(self, model_id: str, api_key: str = None, base_url: str = None): super().__init__(provider=None) if model_id == "": - model_id = CLAUDE_DEFAULT_CHAT_MODEL + model_id = resolve_default_model(CLAUDE_DEFAULT_CHAT_MODEL, "claude-sonnet") model_info = model_info_from_id(model_id) self._model_id = model_id @@ -700,7 +735,7 @@ class ClaudeCodeInlineCompletionModel(InlineCompletionModel): def __init__(self, model_id: str, api_key: str = None, base_url: str = None): super().__init__(provider=None) if model_id == "": - model_id = CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL + model_id = resolve_default_model(CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL, "claude-haiku") model_info = model_info_from_id(model_id) self._model_id = model_id diff --git a/tests/test_claude_models.py b/tests/test_claude_models.py index 285a38d..905239b 100644 --- a/tests/test_claude_models.py +++ b/tests/test_claude_models.py @@ -300,3 +300,48 @@ def test_zero_is_clamped_to_minimum(self, monkeypatch): def test_negative_is_clamped_to_minimum(self, monkeypatch): assert self._parse(monkeypatch, '-5') == 1 + + +class TestResolveDefaultModel: + """Defaults are validated against the fetched model list so a custom + base_url that serves a different catalog never gets a model id it + doesn't recognize. An empty cache trusts the constant — first-party + endpoints always serve the current aliases.""" + + def test_empty_cache_trusts_the_constant(self): + from notebook_intelligence.claude import resolve_default_model + assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-5" + + def test_preferred_id_used_when_endpoint_lists_it(self): + from notebook_intelligence.claude import resolve_default_model, _claude_models_cache + _claude_models_cache.extend([ + {"id": "claude-sonnet-5", "name": "Claude Sonnet 5", "context_window": 1000000}, + {"id": "claude-haiku-4-5", "name": "Claude Haiku 4.5", "context_window": 200000}, + ]) + assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-5" + + def test_falls_back_to_newest_same_tier_model(self): + from notebook_intelligence.claude import resolve_default_model, _claude_models_cache + _claude_models_cache.extend([ + {"id": "claude-sonnet-4-5", "name": "Claude Sonnet 4.5", "context_window": 200000}, + {"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}, + {"id": "claude-haiku-4-5", "name": "Claude Haiku 4.5", "context_window": 200000}, + ]) + assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-4-6" + + def test_falls_back_to_first_listed_model_without_tier_match(self): + from notebook_intelligence.claude import resolve_default_model, _claude_models_cache + _claude_models_cache.extend([ + {"id": "custom-proxy-model", "name": "Proxy", "context_window": 128000}, + ]) + assert resolve_default_model("claude-haiku-4-5", "claude-haiku") == "custom-proxy-model" + + def test_chat_model_constructor_resolves_against_cache(self): + from unittest.mock import patch as _patch + from notebook_intelligence.claude import ClaudeChatModel, _claude_models_cache + _claude_models_cache.extend([ + {"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}, + ]) + with _patch("anthropic.Anthropic"): + model = ClaudeChatModel("", api_key="test-key") + assert model.id == "claude-sonnet-4-6" From 6d75a8743b7b7e2d7f802d67e948e1b114321640 Mon Sep 17 00:00:00 2001 From: Herik Webb Date: Sun, 5 Jul 2026 09:37:25 -0400 Subject: [PATCH 4/5] fix(claude): scope the model cache to the endpoint it was fetched from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (PR #46): resolve_default_model validated defaults against a global cache with no endpoint identity, so after switching to a custom base_url (or while its refresh was in flight) the default could resolve against another endpoint's catalog and send it a model id it doesn't serve. fetch_claude_models now stamps the cache with the normalized (api_key, base_url) it fetched from, and resolve_default_model treats the cache as authoritative only for a matching endpoint — otherwise it trusts the constant until the endpoint's own fetch lands. Credential normalization makes ''/None equivalent, matching the settings-panel empty-field behavior. --- notebook_intelligence/claude.py | 36 +++++++++---- tests/test_claude_models.py | 92 +++++++++++++++++++++++++++------ 2 files changed, 103 insertions(+), 25 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index 35ab375..f507345 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -532,6 +532,12 @@ def model_info_from_id(model_id: str) -> dict: # another doomed thread, hammering api.anthropic.com in parallel when the # api_key is missing or wrong. _claude_models_cache: list[dict] = [] +# Identity of the endpoint the cache was fetched from — (api_key, +# base_url) after credential normalization. The cache is only +# authoritative for that endpoint: a user can switch to a custom +# base_url whose catalog differs, and resolving a default against +# another endpoint's model list would send it an id it doesn't serve. +_claude_models_cache_endpoint: Optional[tuple] = None _claude_models_fetch_lock = threading.Lock() def get_claude_models() -> list[dict]: @@ -539,21 +545,31 @@ def get_claude_models() -> list[dict]: return _claude_models_cache -def resolve_default_model(preferred: str, tier_prefix: str) -> str: +def _endpoint_identity(api_key: str = None, base_url: str = None) -> tuple: + return ( + _normalize_anthropic_credential(api_key), + _normalize_anthropic_credential(base_url), + ) + + +def resolve_default_model(preferred: str, tier_prefix: str, api_key: str = None, base_url: str = None) -> str: """Pick a default model id, validated against the fetched model list. The ``CLAUDE_DEFAULT_*`` constants name current first-party aliases, but a custom ``base_url`` can front an endpoint that serves a - different catalog. When the model cache has entries, prefer the - constant only if the endpoint actually lists it; otherwise fall back - to the newest id in the same tier (``claude-sonnet``/``claude-haiku`` - prefix), then to the first listed model. An empty cache (fetch not - yet run, or failed) trusts the constant — first-party endpoints - always serve it. + different catalog. When the model cache holds this endpoint's + catalog, prefer the constant only if the endpoint actually lists it; + otherwise fall back to the newest id in the same tier + (``claude-sonnet``/``claude-haiku`` prefix), then to the first + listed model. When the cache is empty — or was fetched from a + *different* endpoint (credentials changed, refresh still in flight) + — trust the constant rather than another endpoint's catalog. """ models = get_claude_models() if not models: return preferred + if _claude_models_cache_endpoint != _endpoint_identity(api_key, base_url): + return preferred ids = [m["id"] for m in models if isinstance(m.get("id"), str)] if preferred in ids: return preferred @@ -637,8 +653,10 @@ def fetch_claude_models(api_key: str = None, base_url: str = None) -> list[dict] "name": model.display_name, "context_window": context_window, }) + global _claude_models_cache_endpoint _claude_models_cache.clear() _claude_models_cache.extend(models) + _claude_models_cache_endpoint = _endpoint_identity(api_key, base_url) log.info(f"Fetched {len(models)} Claude models: {[m['id'] + ' (' + m['name'] + ')' for m in models]}") return models except Exception as e: @@ -651,7 +669,7 @@ class ClaudeChatModel(ChatModel): def __init__(self, model_id: str, api_key: str = None, base_url: str = None): super().__init__(provider=None) if model_id == "": - model_id = resolve_default_model(CLAUDE_DEFAULT_CHAT_MODEL, "claude-sonnet") + model_id = resolve_default_model(CLAUDE_DEFAULT_CHAT_MODEL, "claude-sonnet", api_key, base_url) model_info = model_info_from_id(model_id) self._model_id = model_id @@ -735,7 +753,7 @@ class ClaudeCodeInlineCompletionModel(InlineCompletionModel): def __init__(self, model_id: str, api_key: str = None, base_url: str = None): super().__init__(provider=None) if model_id == "": - model_id = resolve_default_model(CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL, "claude-haiku") + model_id = resolve_default_model(CLAUDE_DEFAULT_INLINE_COMPLETION_MODEL, "claude-haiku", api_key, base_url) model_info = model_info_from_id(model_id) self._model_id = model_id diff --git a/tests/test_claude_models.py b/tests/test_claude_models.py index 905239b..28b9412 100644 --- a/tests/test_claude_models.py +++ b/tests/test_claude_models.py @@ -7,11 +7,13 @@ @pytest.fixture(autouse=True) def clear_cache(): - """Clear the model cache before and after each test.""" - from notebook_intelligence.claude import _claude_models_cache - _claude_models_cache.clear() + """Clear the model cache (and its endpoint marker) around each test.""" + import notebook_intelligence.claude as claude_module + claude_module._claude_models_cache.clear() + claude_module._claude_models_cache_endpoint = None yield - _claude_models_cache.clear() + claude_module._claude_models_cache.clear() + claude_module._claude_models_cache_endpoint = None class TestGetClaudeModels: @@ -305,24 +307,32 @@ def test_negative_is_clamped_to_minimum(self, monkeypatch): class TestResolveDefaultModel: """Defaults are validated against the fetched model list so a custom base_url that serves a different catalog never gets a model id it - doesn't recognize. An empty cache trusts the constant — first-party - endpoints always serve the current aliases.""" + doesn't recognize. The cache is only authoritative for the endpoint + it was fetched from; an empty or foreign-endpoint cache trusts the + constant — first-party endpoints always serve the current aliases.""" + + def _fill_cache(self, models, api_key=None, base_url=None): + import notebook_intelligence.claude as claude_module + claude_module._claude_models_cache.extend(models) + claude_module._claude_models_cache_endpoint = ( + claude_module._endpoint_identity(api_key, base_url) + ) def test_empty_cache_trusts_the_constant(self): from notebook_intelligence.claude import resolve_default_model assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-5" def test_preferred_id_used_when_endpoint_lists_it(self): - from notebook_intelligence.claude import resolve_default_model, _claude_models_cache - _claude_models_cache.extend([ + from notebook_intelligence.claude import resolve_default_model + self._fill_cache([ {"id": "claude-sonnet-5", "name": "Claude Sonnet 5", "context_window": 1000000}, {"id": "claude-haiku-4-5", "name": "Claude Haiku 4.5", "context_window": 200000}, ]) assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-5" def test_falls_back_to_newest_same_tier_model(self): - from notebook_intelligence.claude import resolve_default_model, _claude_models_cache - _claude_models_cache.extend([ + from notebook_intelligence.claude import resolve_default_model + self._fill_cache([ {"id": "claude-sonnet-4-5", "name": "Claude Sonnet 4.5", "context_window": 200000}, {"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}, {"id": "claude-haiku-4-5", "name": "Claude Haiku 4.5", "context_window": 200000}, @@ -330,18 +340,68 @@ def test_falls_back_to_newest_same_tier_model(self): assert resolve_default_model("claude-sonnet-5", "claude-sonnet") == "claude-sonnet-4-6" def test_falls_back_to_first_listed_model_without_tier_match(self): - from notebook_intelligence.claude import resolve_default_model, _claude_models_cache - _claude_models_cache.extend([ + from notebook_intelligence.claude import resolve_default_model + self._fill_cache([ {"id": "custom-proxy-model", "name": "Proxy", "context_window": 128000}, ]) assert resolve_default_model("claude-haiku-4-5", "claude-haiku") == "custom-proxy-model" + def test_cache_from_another_endpoint_is_not_authoritative(self): + # Endpoint A's catalog must not pick the default for endpoint B: + # the id might not be served there at all. The constant is the + # safer bet until B's own fetch lands. + from notebook_intelligence.claude import resolve_default_model + self._fill_cache( + [{"id": "endpoint-a-only-model", "name": "A", "context_window": 128000}], + base_url="https://endpoint-a.example.com", + ) + resolved = resolve_default_model( + "claude-sonnet-5", "claude-sonnet", + base_url="https://endpoint-b.example.com", + ) + assert resolved == "claude-sonnet-5" + + def test_credential_normalization_applies_to_identity(self): + # '' and None are the same identity (settings-panel empty fields + # save as ''), so a cache fetched with no explicit credentials is + # authoritative for a constructor called with empty strings. + from notebook_intelligence.claude import resolve_default_model + self._fill_cache( + [{"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}], + api_key=None, base_url=None, + ) + resolved = resolve_default_model( + "claude-sonnet-5", "claude-sonnet", api_key="", base_url=" ", + ) + assert resolved == "claude-sonnet-4-6" + def test_chat_model_constructor_resolves_against_cache(self): from unittest.mock import patch as _patch - from notebook_intelligence.claude import ClaudeChatModel, _claude_models_cache - _claude_models_cache.extend([ - {"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}, - ]) + from notebook_intelligence.claude import ClaudeChatModel + self._fill_cache( + [{"id": "claude-sonnet-4-6", "name": "Claude Sonnet 4.6", "context_window": 200000}], + api_key="test-key", + ) with _patch("anthropic.Anthropic"): model = ClaudeChatModel("", api_key="test-key") assert model.id == "claude-sonnet-4-6" + + @patch("notebook_intelligence.claude._get_context_window", return_value=200000) + @patch("anthropic.Anthropic") + def test_fetch_stamps_the_endpoint_identity(self, mock_anthropic_cls, mock_ctx_window): + import notebook_intelligence.claude as claude_module + from notebook_intelligence.claude import fetch_claude_models + + mock_model = Mock() + mock_model.id = "claude-sonnet-4-6" + mock_model.display_name = "Claude Sonnet 4.6" + mock_model.max_input_tokens = None + mock_page = Mock() + mock_page.data = [mock_model] + mock_anthropic_cls.return_value.models.list.return_value = mock_page + + fetch_claude_models(api_key="key-a", base_url="https://a.example.com") + + assert claude_module._claude_models_cache_endpoint == ( + "key-a", "https://a.example.com" + ) From b8379a86e85061471d2e83489524729710edfd70 Mon Sep 17 00:00:00 2001 From: Herik Webb Date: Sun, 5 Jul 2026 09:47:19 -0400 Subject: [PATCH 5/5] fix(claude): version-aware tier fallback and stale-cache refresh Review follow-ups (PR #46, round 4): - The 'newest same tier' fallback sorted model ids lexicographically, which ranks claude-sonnet-4-6 above claude-sonnet-4-10. Sort by the extracted numeric version components instead ([4,6] < [4,10] < [5]), id as tiebreak. - When the cache demonstrably belongs to another endpoint (credentials changed), resolve_default_model now kicks off a background catalog refresh so subsequent resolutions converge on the new endpoint's catalog. It deliberately never blocks on the network: it runs in model constructors on the request path, and a slow or dead custom endpoint must degrade to one possibly-failing request, not a hung chat turn. A never-stamped cache is left to the startup fetch that update_models_from_config already fires. --- notebook_intelligence/claude.py | 44 +++++++++++++--- tests/test_claude_models.py | 93 +++++++++++++++++++++++++++++++-- 2 files changed, 126 insertions(+), 11 deletions(-) diff --git a/notebook_intelligence/claude.py b/notebook_intelligence/claude.py index f507345..42af4e2 100644 --- a/notebook_intelligence/claude.py +++ b/notebook_intelligence/claude.py @@ -552,6 +552,17 @@ def _endpoint_identity(api_key: str = None, base_url: str = None) -> tuple: ) +def _model_version_key(model_id: str) -> tuple: + """Sort key ordering model ids by their numeric version components. + + Plain lexicographic order gets version-like ids wrong — + ``claude-sonnet-4-6`` sorts *after* ``claude-sonnet-4-10``. Compare + the extracted number sequences instead ([4, 6] < [4, 10] < [5]), + with the id itself as a deterministic tiebreak. + """ + return ([int(part) for part in re.findall(r"\d+", model_id)], model_id) + + def resolve_default_model(preferred: str, tier_prefix: str, api_key: str = None, base_url: str = None) -> str: """Pick a default model id, validated against the fetched model list. @@ -563,20 +574,39 @@ def resolve_default_model(preferred: str, tier_prefix: str, api_key: str = None, (``claude-sonnet``/``claude-haiku`` prefix), then to the first listed model. When the cache is empty — or was fetched from a *different* endpoint (credentials changed, refresh still in flight) - — trust the constant rather than another endpoint's catalog. + — trust the constant rather than another endpoint's catalog, and + trigger a background refresh so later resolutions converge on this + endpoint's catalog. """ models = get_claude_models() - if not models: - return preferred - if _claude_models_cache_endpoint != _endpoint_identity(api_key, base_url): + endpoint_matches = _claude_models_cache_endpoint == _endpoint_identity(api_key, base_url) + if not models or not endpoint_matches: + if not endpoint_matches and _claude_models_cache_endpoint is not None: + # The cache demonstrably belongs to another endpoint (the + # credentials changed): kick off a background refresh so + # subsequent resolutions see this endpoint's catalog. A + # never-stamped cache is left to the startup fetch that + # update_models_from_config already fires. The single-flight + # lock inside fetch_claude_models dedupes concurrent + # attempts; failures leave the cache as-is. Deliberately + # never block on the network here — this runs in model + # constructors on the request path, and a slow or dead + # custom endpoint must degrade to one possibly-failing + # request, not a hung chat turn. + threading.Thread( + target=fetch_claude_models, + kwargs={"api_key": api_key, "base_url": base_url}, + name="nbi-claude-models-refresh", + daemon=True, + ).start() return preferred ids = [m["id"] for m in models if isinstance(m.get("id"), str)] if preferred in ids: return preferred - tier_matches = sorted(i for i in ids if i.startswith(tier_prefix)) + tier_matches = sorted( + (i for i in ids if i.startswith(tier_prefix)), key=_model_version_key + ) if tier_matches: - # Anthropic ids sort by recency within a tier well enough for a - # fallback ('claude-sonnet-5' > 'claude-sonnet-4-5'). fallback = tier_matches[-1] elif ids: fallback = ids[0] diff --git a/tests/test_claude_models.py b/tests/test_claude_models.py index 28b9412..8a0b9b3 100644 --- a/tests/test_claude_models.py +++ b/tests/test_claude_models.py @@ -355,10 +355,14 @@ def test_cache_from_another_endpoint_is_not_authoritative(self): [{"id": "endpoint-a-only-model", "name": "A", "context_window": 128000}], base_url="https://endpoint-a.example.com", ) - resolved = resolve_default_model( - "claude-sonnet-5", "claude-sonnet", - base_url="https://endpoint-b.example.com", - ) + # The stale-cache path spawns a background catalog refresh; + # patch it so the test doesn't leak a real network thread that + # holds the single-flight lock into later tests. + with patch("notebook_intelligence.claude.threading.Thread"): + resolved = resolve_default_model( + "claude-sonnet-5", "claude-sonnet", + base_url="https://endpoint-b.example.com", + ) assert resolved == "claude-sonnet-5" def test_credential_normalization_applies_to_identity(self): @@ -405,3 +409,84 @@ def test_fetch_stamps_the_endpoint_identity(self, mock_anthropic_cls, mock_ctx_w assert claude_module._claude_models_cache_endpoint == ( "key-a", "https://a.example.com" ) + + +class TestResolveDefaultModelRefreshAndOrdering: + def _fill_cache(self, models, api_key=None, base_url=None): + import notebook_intelligence.claude as claude_module + claude_module._claude_models_cache.extend(models) + claude_module._claude_models_cache_endpoint = ( + claude_module._endpoint_identity(api_key, base_url) + ) + + def test_foreign_endpoint_triggers_background_refresh(self): + from notebook_intelligence.claude import resolve_default_model + self._fill_cache( + [{"id": "endpoint-a-model", "name": "A", "context_window": 128000}], + base_url="https://endpoint-a.example.com", + ) + with patch("notebook_intelligence.claude.threading.Thread") as mock_thread: + resolved = resolve_default_model( + "claude-sonnet-5", "claude-sonnet", + base_url="https://endpoint-b.example.com", + ) + assert resolved == "claude-sonnet-5" + mock_thread.assert_called_once() + kwargs = mock_thread.call_args.kwargs + assert kwargs["kwargs"] == { + "api_key": None, "base_url": "https://endpoint-b.example.com" + } + mock_thread.return_value.start.assert_called_once() + + def test_matching_endpoint_triggers_no_refresh(self): + from notebook_intelligence.claude import resolve_default_model + self._fill_cache( + [{"id": "claude-sonnet-5", "name": "S", "context_window": 1000000}] + ) + with patch("notebook_intelligence.claude.threading.Thread") as mock_thread: + resolve_default_model("claude-sonnet-5", "claude-sonnet") + mock_thread.assert_not_called() + + def test_empty_cache_with_matching_identity_triggers_no_refresh(self): + import notebook_intelligence.claude as claude_module + from notebook_intelligence.claude import resolve_default_model + claude_module._claude_models_cache_endpoint = ( + claude_module._endpoint_identity(None, None) + ) + with patch("notebook_intelligence.claude.threading.Thread") as mock_thread: + resolved = resolve_default_model("claude-sonnet-5", "claude-sonnet") + assert resolved == "claude-sonnet-5" + mock_thread.assert_not_called() + + def test_never_stamped_cache_triggers_no_refresh(self): + # Nothing fetched yet at all (marker is None): the startup fetch + # that update_models_from_config fires owns that case. Spawning + # here too would make every model constructor a network call in + # fresh processes (and in tests). + from notebook_intelligence.claude import resolve_default_model + with patch("notebook_intelligence.claude.threading.Thread") as mock_thread: + resolved = resolve_default_model( + "claude-haiku-4-5", "claude-haiku", api_key="test-key" + ) + assert resolved == "claude-haiku-4-5" + mock_thread.assert_not_called() + + def test_tier_fallback_orders_versions_numerically(self): + # Lexicographic order would pick 4-6 over 4-10; numeric version + # comparison must pick 4-10 (and a bare 5 over both). + from notebook_intelligence.claude import resolve_default_model + self._fill_cache([ + {"id": "claude-sonnet-4-10", "name": "S", "context_window": 200000}, + {"id": "claude-sonnet-4-6", "name": "S", "context_window": 200000}, + ]) + assert resolve_default_model("claude-sonnet-9", "claude-sonnet") == "claude-sonnet-4-10" + + def test_bare_major_version_beats_dated_snapshot(self): + from notebook_intelligence.claude import _model_version_key + ordered = sorted( + ["claude-sonnet-4-5-20250929", "claude-sonnet-5", "claude-sonnet-4-10"], + key=_model_version_key, + ) + assert ordered == [ + "claude-sonnet-4-5-20250929", "claude-sonnet-4-10", "claude-sonnet-5" + ]