diff --git a/strix/config/settings.py b/strix/config/settings.py index 84339218..af70fc04 100644 --- a/strix/config/settings.py +++ b/strix/config/settings.py @@ -40,6 +40,15 @@ class LlmSettings(BaseSettings): default=False, alias="STRIX_FORCE_REQUIRED_TOOL_CHOICE", ) + # Explicit output-token ceiling. Leave unset (None) to let make_model_settings + # decide: Anthropic/Claude models get a sensible default (they need one on + # Bedrock Converse — adaptive-thinking models otherwise send no maxTokens and + # truncate long tool calls), every other provider keeps its own default so + # local users on smaller-ceiling models (gpt/ollama/gemini) aren't forced past + # their limit. Any value set here is clamped to the model's known ceiling. + # Must be positive: 0 or negative would yield empty output or a provider 400, + # so reject it at load time rather than pass it through to a request. + max_tokens: int | None = Field(default=None, gt=0, alias="STRIX_MAX_TOKENS") timeout: int = Field(default=300, alias="LLM_TIMEOUT") diff --git a/strix/core/inputs.py b/strix/core/inputs.py index de922d07..1ec803a7 100644 --- a/strix/core/inputs.py +++ b/strix/core/inputs.py @@ -122,18 +122,103 @@ def build_scope_context(scan_config: dict[str, Any]) -> dict[str, Any]: } +# Output ceiling for Claude models when the user hasn't set one. Adaptive-thinking +# Claude on Bedrock Converse otherwise sends no maxTokens and truncates long tool +# calls mid-JSON (the arguments blob is cut a brace short → next-turn history replay +# 400s in litellm's openai→bedrock converter). Every current Claude tier accepts at +# least this much output; the value is clamped to the model's own ceiling below, so +# it is safe even on older Claude with a smaller limit. +_CLAUDE_DEFAULT_MAX_TOKENS = 32000 + + +def resolve_max_tokens(model_name: str, configured: int | None) -> int | None: + """Decide the output ceiling for a model. + + - Non-Claude models: return ``configured`` untouched (``None`` → provider + default). We do not impose a ceiling on gpt/ollama/gemini/etc. — a local + user on a small-context model must not be forced past its limit. + - Claude models: use ``configured`` or a family default, then clamp to the + model's known output ceiling so the value can never exceed what the + provider accepts (older Claude tops out at 4096-8192). + + Gated on the Claude family rather than a version/tier list: the bug this + guards (adaptive-thinking truncation) spans opus-4-6/4-8 and sonnet-4-6 today + and will grow, while opus-4-1 is not adaptive — so tier is the wrong axis. + """ + if not _is_claude_model(model_name): + return configured + + ceiling = _model_output_ceiling(model_name) + if configured is not None: + # Respect an explicit user value; clamp only if we know the ceiling. + return min(configured, ceiling) if ceiling else configured + # No explicit value: only inject the family default when we know it fits the + # model. If the ceiling is unknown (model missing from litellm's cost map), + # fall back to None (provider default) rather than guess a value that could + # exceed the true limit — the adaptive models that need the fix all have a + # known ceiling, so nothing is lost. + if ceiling is None: + return None + return min(_CLAUDE_DEFAULT_MAX_TOKENS, ceiling) + + +def _is_claude_model(model_name: str) -> bool: + return "claude" in model_name.strip().lower() + + +def _model_output_ceiling(model_name: str) -> int | None: + """Look up a model's max output tokens in litellm's cost map. + + litellm keys the same model under several names (``global.anthropic.claude- + opus-4-8``, ``anthropic.claude-opus-4-8``, ``claude-opus-4-8``), and not every + region/provider-prefixed variant is present for every model. Try the name as + given, then progressively strip the SDK route prefix, an ``owner/`` segment, + and leading dotted segments (region like ``global.``/``us.``, then provider + like ``anthropic.``) so a prefixed name still resolves to the bare key. + """ + import litellm + + name = model_name.strip().lower() + for prefix in ("litellm/", "any-llm/", "openai/", "bedrock/"): + if name.startswith(prefix): + name = name[len(prefix) :] + break + + candidates = [name] + if "/" in name: + candidates.append(name.rsplit("/", 1)[1]) + # Strip leading dotted segments one at a time: global.anthropic.claude-x + # -> anthropic.claude-x -> claude-x. Each intermediate form is a real key + # for some models, so try them all. + for cand in list(candidates): + rest = cand + while "." in rest: + rest = rest.split(".", 1)[1] + candidates.append(rest) + + for cand in candidates: + entry = litellm.model_cost.get(cand) + if entry: + ceiling = entry.get("max_output_tokens") or entry.get("max_tokens") + if isinstance(ceiling, int): + return ceiling + return None + + def make_model_settings( reasoning_effort: ReasoningEffort | None, *, model_name: str, force_required_tool_choice: bool = False, request_timeout: float | None = None, + max_tokens: int | None = None, ) -> ModelSettings: model_settings = ModelSettings( parallel_tool_calls=False, retry=DEFAULT_MODEL_RETRY, include_usage=True, extra_args=request_timeout_extra_args(request_timeout), + max_tokens=resolve_max_tokens(model_name, max_tokens), ) if ( reasoning_effort is not None diff --git a/strix/core/runner.py b/strix/core/runner.py index 153d16dd..4bc4375d 100644 --- a/strix/core/runner.py +++ b/strix/core/runner.py @@ -216,6 +216,7 @@ async def run_strix_scan( model_name=resolved_model, force_required_tool_choice=settings.llm.force_required_tool_choice, request_timeout=settings.llm.timeout, + max_tokens=settings.llm.max_tokens, ) run_config = RunConfig( model=resolved_model, diff --git a/tests/test_inputs.py b/tests/test_inputs.py index bde83bd9..a0720b73 100644 --- a/tests/test_inputs.py +++ b/tests/test_inputs.py @@ -7,7 +7,12 @@ from typing import Any import pytest -from strix.core.inputs import build_root_task, child_initial_input, make_model_settings +from strix.core.inputs import ( + build_root_task, + child_initial_input, + make_model_settings, + resolve_max_tokens, +) def _child_kwargs(parent_history: list[Any]) -> dict[str, Any]: @@ -185,3 +190,81 @@ def test_make_model_settings_timeout_survives_reasoning_resolve() -> None: assert settings.extra_args is not None assert settings.extra_args["timeout"] == 120.0 + + +@pytest.mark.parametrize( + "model_name", + [ + "openai/gpt-5.4", + "gpt-4o", + "gemini/gemini-1.5-flash", + "ollama/llama3", + "bedrock/us.amazon.nova-lite-v1:0", + ], +) +def test_resolve_max_tokens_non_claude_is_untouched(model_name: str) -> None: + # No ceiling is imposed on non-Claude models: None stays None (provider + # default) and an explicit value passes through verbatim. + assert resolve_max_tokens(model_name, None) is None + assert resolve_max_tokens(model_name, 16000) == 16000 + + +@pytest.mark.parametrize( + "model_name", + [ + "bedrock/global.anthropic.claude-opus-4-8", + "bedrock/us.anthropic.claude-opus-4-8", + "bedrock/global.anthropic.claude-sonnet-4-6", + ], +) +def test_resolve_max_tokens_adaptive_claude_gets_default(model_name: str) -> None: + # The models the fix targets: unset -> family default, well under their + # (>=64k) ceiling so the value is the default itself, regardless of the + # region/provider prefix on the model id. + assert resolve_max_tokens(model_name, None) == 32000 + + +def test_resolve_max_tokens_clamps_default_to_small_ceiling() -> None: + # Older Claude tops out below the family default; the returned value must be + # the model's own ceiling, never the larger default (which would 400). + result = resolve_max_tokens("bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0", None) + assert result is not None + assert result <= 8192 + + +def test_resolve_max_tokens_clamps_explicit_override_to_ceiling() -> None: + # An over-large explicit value is clamped down to what the model accepts. + assert resolve_max_tokens("bedrock/global.anthropic.claude-opus-4-8", 500_000) == 128_000 + + +def test_resolve_max_tokens_unknown_claude_without_config_returns_none() -> None: + # A Claude model absent from litellm's cost map has no known ceiling; rather + # than guess a value that might exceed the real limit, fall back to None. + assert resolve_max_tokens("claude-does-not-exist-9", None) is None + + +def test_resolve_max_tokens_unknown_claude_respects_explicit_config() -> None: + # If the ceiling is unknown but the user set a value, honor it as-is. + assert resolve_max_tokens("claude-does-not-exist-9", 12345) == 12345 + + +@pytest.mark.parametrize("bad_value", ["0", "-1", "-4096"]) +def test_llm_settings_rejects_nonpositive_max_tokens( + monkeypatch: pytest.MonkeyPatch, bad_value: str +) -> None: + # A nonpositive STRIX_MAX_TOKENS would yield empty output or a provider 400; + # it must fail at settings-load rather than reach a request. + from pydantic import ValidationError + + from strix.config.settings import LlmSettings + + monkeypatch.setenv("STRIX_MAX_TOKENS", bad_value) + with pytest.raises(ValidationError): + LlmSettings() + + +def test_llm_settings_accepts_positive_max_tokens(monkeypatch: pytest.MonkeyPatch) -> None: + from strix.config.settings import LlmSettings + + monkeypatch.setenv("STRIX_MAX_TOKENS", "4096") + assert LlmSettings().max_tokens == 4096