From b48d68f8bf039a1ecdf784b99c94112297ba0703 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 14:32:36 +0000 Subject: [PATCH] Fix Mistral seed mapping, raise default OAI-compat stop cap, thread sampling through GGUF direct path Mistral chat completions uses random_seed not seed; map the field via a new seed_field on the provider registry so the new seed control actually works on Mistral. Default for other providers stays seed. DeepSeek and Mistral both accept up to 16 stop sequences but the default OAI-compat branch was hard-capping at 4 (the OpenAI Chat limit). Studio routes the openai provider through /v1/responses not /v1/chat/completions so the 4-cap only applies if we explicitly added an openai entry. Raise the default to 16 and let per-provider stop_max overrides tighten if needed. The local GGUF direct chat path (gguf_generate / gguf_generate_with_tools) bypassed _build_openai_passthrough_body and therefore dropped frequency_penalty, seed, stop, and parallel_tool_calls on the floor for users on the default no-tools and with-tools paths. Thread the new fields through LlamaCppBackend.generate_chat_completion and generate_chat_completion_with_tools and the two callsites that invoke them. Also tighten comments to drop review-process narration that crept in and to remove the em dashes I had introduced in this PR's earlier commits. Tests pin the Mistral random_seed rename, the DeepSeek 16-cap, and confirm the openai-compat default cap is 16. --- .../core/inference/external_provider.py | 128 +++++++----------- studio/backend/core/inference/llama_cpp.py | 21 +++ studio/backend/core/inference/providers.py | 3 + studio/backend/routes/chat_history.py | 7 +- studio/backend/routes/inference.py | 30 ++-- .../tests/test_sampling_params_routing.py | 113 ++++++++++++---- .../components/ui/stop-sequences-input.tsx | 10 +- .../src/features/chat/chat-settings-sheet.tsx | 12 +- .../features/chat/presets/preset-policy.ts | 11 +- .../features/chat/provider-capabilities.ts | 16 +-- .../chat/utils/chat-settings-storage.ts | 29 ++-- 11 files changed, 206 insertions(+), 174 deletions(-) diff --git a/studio/backend/core/inference/external_provider.py b/studio/backend/core/inference/external_provider.py index c703129652..904380ac5d 100644 --- a/studio/backend/core/inference/external_provider.py +++ b/studio/backend/core/inference/external_provider.py @@ -459,35 +459,36 @@ class ExternalProviderClient: else: body["max_tokens"] = max_tokens - # Optional sampling extensions (added in #5XXX). Only forwarded - # when the caller passed a value. Each upstream provider that - # 400s on the field appears in `body_omit` (see providers.py) - # so the registry-driven drop loop below removes them before - # the request hits the wire. The Responses path - # (_stream_openai_responses) drops these explicitly because it - # never reaches this body construction. + # Optional sampling extensions. Only forwarded when the caller + # passed a value. Per-provider rename / cap is applied via + # `seed_field` and `stop_max` on the provider registry below, + # and body_omit strips fields the upstream rejects. + from core.inference.providers import get_provider_info + + provider_info = get_provider_info(self.provider_type) or {} if frequency_penalty is not None: body["frequency_penalty"] = frequency_penalty if seed is not None: - body["seed"] = seed + # Mistral renames `seed` to `random_seed` on /v1/chat/completions. + seed_field = provider_info.get("seed_field", "seed") + body[seed_field] = seed if stop: - # OpenAI Chat caps the list at 4 entries. Dedupe + drop - # empties first so users entering chips with whitespace or - # accidental repeats don't waste budget against the cap or - # trip a 400. + # Stop cap is provider-specific. OpenAI Chat = 4, Anthropic + # = 16 (client guard), DeepSeek = 16, others = 16 by default. + stop_max = int(provider_info.get("stop_max", 16)) if isinstance(stop, str): body["stop"] = stop elif isinstance(stop, list): sequences = list( dict.fromkeys(s for s in stop if isinstance(s, str) and s) ) - if len(sequences) > 4: + if len(sequences) > stop_max: logger.warning( - "stop sequences truncated to 4 entries " - "(received %d, OpenAI's hard cap is 4)", + "stop sequences truncated to %d entries (received %d)", + stop_max, len(sequences), ) - body["stop"] = sequences[:4] + body["stop"] = sequences[:stop_max] elif sequences: body["stop"] = sequences if service_tier is not None: @@ -495,15 +496,8 @@ class ExternalProviderClient: if parallel_tool_calls is not None: body["parallel_tool_calls"] = parallel_tool_calls - # Strip body fields a provider's registry entry declares unusable — - # reasoning-class models that lock these to fixed defaults (e.g. - # Kimi k2.5/k2.6 only accept temperature=1, top_p=1) 400 otherwise. - # The frontend capability map already hides the matching sliders; - # this is the matching guard for the pydantic default that the - # route layer would otherwise still fill in. - from core.inference.providers import get_provider_info - - provider_info = get_provider_info(self.provider_type) or {} + # Drop body fields the provider's registry entry locks down + # (e.g. Kimi k2.5/k2.6 only accept temperature=1, top_p=1). for field in provider_info.get("body_omit", ()): body.pop(field, None) @@ -900,11 +894,10 @@ class ExternalProviderClient: if max_tokens is not None: body["max_tokens"] = max_tokens - # Forward the new optional sampling extensions (#5711) on the - # web-search bypass too. The default OAI-compat body construction - # (which adds these) is skipped because this helper returns - # early; forwarding here ensures kimi-with-search honours the - # same sampling controls as kimi-without-search. + # The default OAI-compat body construction is skipped because + # this helper returns early. Forward the optional sampling + # extensions here so kimi-with-search behaves the same as + # kimi-without-search. if presence_penalty is not None: body["presence_penalty"] = presence_penalty if frequency_penalty is not None: @@ -912,13 +905,8 @@ class ExternalProviderClient: if seed is not None: body["seed"] = seed if stop: - # Mirror the default OAI-compat path's stop handling exactly - # so Kimi-with-search and Kimi-without-search apply the - # same rules — a single string is forwarded verbatim and - # lists are deduped + truncated to OpenAI's 4-entry cap. - # Earlier the bypass dropped whitespace-only strings here - # while the normal path forwarded them, which was an - # asymmetric provider-path fix. + # Match the default OAI-compat path: forward a single string + # verbatim, dedupe + cap lists to 4 (OpenAI hard limit). if isinstance(stop, str): body["stop"] = stop elif isinstance(stop, list): @@ -1453,32 +1441,26 @@ class ExternalProviderClient: body["top_k"] = top_k # Optional sampling extensions. Anthropic has no - # frequency_penalty / seed / logprobs equivalents, so those are - # silently dropped by virtue of not being forwarded from - # stream_chat_completion. The two body-level knobs Anthropic - # does accept land here: - # stop → stop_sequences (renamed, ws-stripped) - # service_tier → service_tier (auto|standard_only only) - # parallel_tool_calls inversion is applied AFTER the tools - # wiring below, because Anthropic requires it nested under - # tool_choice (top-level placement is rejected with - # `extraneous key [disable_parallel_tool_use] is not permitted`). + # frequency_penalty / seed / logprobs equivalents so they are + # never forwarded here. The two body-level knobs Anthropic + # accepts land here: + # stop -> stop_sequences (renamed, ws-stripped) + # service_tier -> service_tier (auto|standard_only only) + # parallel_tool_calls inversion is applied after the tools + # wiring below because Anthropic requires it nested under + # tool_choice. if stop: sequences: list[str] if isinstance(stop, str): sequences = [stop] if stop.strip() else [] else: - # Dedupe + drop whitespace-only entries. Anthropic 400s - # on any sequence that contains no non-whitespace char: - # `stop_sequences: each stop sequence must contain - # non-whitespace`. That rejects empty strings, " ", and - # — critically — common defaults like "\n" / "\n\n". - # The truncation cap below (16) is a client-side guard; - # the Anthropic Messages API does not publish a max - # array length but every SDK we have inspected treats - # 16 as a sane ceiling (Bedrock's hard cap is 8191, so - # this only matters when callers paste pathologically - # long lists by accident). + # Dedupe + drop whitespace-only entries. Anthropic + # rejects any sequence with no non-whitespace char + # ("stop_sequences: each stop sequence must contain + # non-whitespace"), so "", " ", "\n", "\n\n" are all + # filtered. The 16-cap is a client-side guard; the + # docs do not publish a max, but every SDK treats 16 + # as a sane ceiling (Bedrock is the outlier at 8191). sequences = list( dict.fromkeys(s for s in stop if isinstance(s, str) and s.strip()) ) @@ -1723,15 +1705,11 @@ class ExternalProviderClient: if anthropic_code_exec_container_id: body["container"] = anthropic_code_exec_container_id - # parallel_tool_calls=false → disable_parallel_tool_use=true, - # nested under tool_choice (NOT top-level). The Anthropic - # Messages API only accepts `disable_parallel_tool_use` as a - # property on the `tool_choice` object (ToolChoiceAuto / - # ToolChoiceAny / ToolChoiceTool). Top-level placement is - # rejected with `extraneous key [disable_parallel_tool_use] - # is not permitted`. Without any tools the flag is also a - # no-op upstream — skip it to keep the request body minimal. - # See + # parallel_tool_calls=False maps to disable_parallel_tool_use= + # True nested under tool_choice. Top-level placement is + # rejected with "extraneous key [disable_parallel_tool_use] + # is not permitted". Without tools the flag is a no-op so the + # block is skipped to keep the body minimal. See # https://platform.claude.com/docs/en/agents-and-tools/tool-use/implement-tool-use if parallel_tool_calls is False and body.get("tools"): tc = body.get("tool_choice") @@ -2831,17 +2809,11 @@ class ExternalProviderClient: "input": input_items, "stream": True, } - # Responses accepts service_tier on the Chat Completions enum - # MINUS `scale`. The `openai-python` SDK type - # (`src/openai/types/responses/response_create_params.py`) - # technically includes `scale`, but the live OpenAI Responses - # API reference and the PR's own provider matrix only list - # `auto|default|flex|priority` for /v1/responses, and an - # independent round of 20 codex reviewers reached the same - # conclusion. Drop `scale` here to prevent the 400 risk — - # users who want Scale Tier can still pick it on a Chat - # Completions-compat provider where the SDK enum is honored. - # parallel_tool_calls follows the same shape (default true). + # Responses accepts auto|default|flex|priority per the live + # docs. The openai-python SDK type happens to include "scale" + # too but the public Responses reference does not, so drop it + # here to avoid a 400. Scale Tier is still selectable on Chat + # Completions backends. parallel_tool_calls default is true. if service_tier in ("auto", "default", "flex", "priority"): body["service_tier"] = service_tier if parallel_tool_calls is not None: diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index bf8a3c04df..e227053118 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -4245,6 +4245,9 @@ class LlamaCppBackend: enable_thinking: Optional[bool] = None, reasoning_effort: Optional[str] = None, preserve_thinking: Optional[bool] = None, + frequency_penalty: Optional[float] = None, + seed: Optional[int] = None, + parallel_tool_calls: Optional[bool] = None, ) -> Generator[str | dict, None, None]: """ Send a chat completion request to llama-server and stream tokens back. @@ -4286,6 +4289,14 @@ class LlamaCppBackend: payload["t_max_predict_ms"] = _DEFAULT_T_MAX_PREDICT_MS if stop: payload["stop"] = stop + # Optional sampling extensions, gated on `is not None` so 0, + # 0.0, and False all reach the wire. + if frequency_penalty is not None: + payload["frequency_penalty"] = frequency_penalty + if seed is not None: + payload["seed"] = seed + if parallel_tool_calls is not None: + payload["parallel_tool_calls"] = parallel_tool_calls payload["stream_options"] = {"include_usage": True} url = f"{self.base_url}/v1/chat/completions" @@ -4428,6 +4439,9 @@ class LlamaCppBackend: auto_heal_tool_calls: bool = True, tool_call_timeout: int = 300, session_id: Optional[str] = None, + frequency_penalty: Optional[float] = None, + seed: Optional[int] = None, + parallel_tool_calls: Optional[bool] = None, ) -> Generator[dict, None, None]: """ Agentic loop: let the model call tools, execute them, and continue. @@ -4512,6 +4526,13 @@ class LlamaCppBackend: payload["t_max_predict_ms"] = _DEFAULT_T_MAX_PREDICT_MS if stop: payload["stop"] = stop + # Optional sampling extensions; gated on `is not None`. + if frequency_penalty is not None: + payload["frequency_penalty"] = frequency_penalty + if seed is not None: + payload["seed"] = seed + if parallel_tool_calls is not None: + payload["parallel_tool_calls"] = parallel_tool_calls try: _auth_headers = ( diff --git a/studio/backend/core/inference/providers.py b/studio/backend/core/inference/providers.py index fef9ba3e12..98c8abe53b 100644 --- a/studio/backend/core/inference/providers.py +++ b/studio/backend/core/inference/providers.py @@ -131,6 +131,9 @@ PROVIDER_REGISTRY: dict[str, dict[str, Any]] = { r"mistral-(?:large|medium|small|tiny)-latest|" r"mistral-vibe-cli-latest)$" ), + # Mistral renames OpenAI's `seed` to `random_seed` on + # /v1/chat/completions. https://docs.mistral.ai/api/endpoint/chat + "seed_field": "random_seed", }, "kimi": { "display_name": "Kimi", diff --git a/studio/backend/routes/chat_history.py b/studio/backend/routes/chat_history.py index 6f7efb6eb5..800873afba 100644 --- a/studio/backend/routes/chat_history.py +++ b/studio/backend/routes/chat_history.py @@ -107,10 +107,9 @@ class ChatInferenceSettings(BaseModel): minP: Optional[float] = None repetitionPenalty: Optional[float] = None presencePenalty: Optional[float] = None - # New per-provider sampling knobs. `extra="forbid"` would 422 any - # settings save from a frontend on the new code if these were not - # listed here, breaking the entire chat-settings persistence path. - # Keep these aligned with `InferenceParams` in + # New per-provider sampling knobs. extra="forbid" requires these + # to be listed; otherwise every save from the new frontend 422s. + # Keep aligned with InferenceParams in # studio/frontend/src/features/chat/types/runtime.ts. frequencyPenalty: Optional[float] = Field(default = None, ge = -2.0, le = 2.0) seed: Optional[int] = None diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 8c7267c0e2..5ee25e9922 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -2474,6 +2474,9 @@ async def openai_chat_completions( max_tokens = payload.max_tokens, repetition_penalty = payload.repetition_penalty, presence_penalty = payload.presence_penalty, + stop = payload.stop if isinstance(payload.stop, list) else ( + [payload.stop] if isinstance(payload.stop, str) and payload.stop else None + ), cancel_event = cancel_event, enable_thinking = payload.enable_thinking, reasoning_effort = payload.reasoning_effort, @@ -2488,6 +2491,9 @@ async def openai_chat_completions( if payload.tool_call_timeout is not None else 300, session_id = payload.session_id, + frequency_penalty = payload.frequency_penalty, + seed = payload.seed, + parallel_tool_calls = payload.parallel_tool_calls, ) _tool_sentinel = object() @@ -2653,10 +2659,16 @@ async def openai_chat_completions( max_tokens = payload.max_tokens, repetition_penalty = payload.repetition_penalty, presence_penalty = payload.presence_penalty, + stop = payload.stop if isinstance(payload.stop, list) else ( + [payload.stop] if isinstance(payload.stop, str) and payload.stop else None + ), cancel_event = cancel_event, enable_thinking = payload.enable_thinking, reasoning_effort = payload.reasoning_effort, preserve_thinking = payload.preserve_thinking, + frequency_penalty = payload.frequency_penalty, + seed = payload.seed, + parallel_tool_calls = payload.parallel_tool_calls, ) _gguf_sentinel = object() @@ -3780,12 +3792,9 @@ def _build_chat_request( chat_kwargs["top_p"] = payload.top_p if payload.max_output_tokens is not None: chat_kwargs["max_tokens"] = payload.max_output_tokens - # `parallel_tool_calls` is now a first-class field on - # ChatCompletionRequest (PR #5711) and the OpenAI-compat - # passthrough builder forwards it. Translate it here so a Responses - # API caller (e.g. OpenAI Codex SDK) that sets - # `parallel_tool_calls=false` actually sees the preference reach - # llama-server instead of getting silently dropped at the bridge. + # parallel_tool_calls is first-class on ChatCompletionRequest and + # the OpenAI-compat passthrough builder forwards it. Translate it + # so a Responses API caller's preference reaches llama-server. if payload.parallel_tool_calls is not None: chat_kwargs["parallel_tool_calls"] = payload.parallel_tool_calls @@ -4934,12 +4943,9 @@ def _build_passthrough_payload( body["repeat_penalty"] = repetition_penalty if presence_penalty is not None: body["presence_penalty"] = presence_penalty - # New per-provider sampling extensions (PR #5711). llama-server's - # /v1/chat/completions endpoint accepts the standard OpenAI fields, - # so forward them straight through. parallel_tool_calls is a no-op - # on llama-server today (the upstream always dispatches sequentially) - # but forward it anyway so a future llama-server release that - # implements it picks up the user's preference automatically. + # llama-server's /v1/chat/completions accepts the standard OpenAI + # fields. parallel_tool_calls is a no-op on llama-server today but + # is forwarded so a future release picks it up automatically. if frequency_penalty is not None: body["frequency_penalty"] = frequency_penalty if seed is not None: diff --git a/studio/backend/tests/test_sampling_params_routing.py b/studio/backend/tests/test_sampling_params_routing.py index 9777d5ef73..8d8b199d78 100644 --- a/studio/backend/tests/test_sampling_params_routing.py +++ b/studio/backend/tests/test_sampling_params_routing.py @@ -171,11 +171,10 @@ def test_anthropic_disable_parallel_tool_use_nested_under_tool_choice(monkeypatc """ captured = _install_mock(monkeypatch) body = _drive_anthropic_with_tools(captured, parallel_tool_calls = False) - # Top-level fields must not carry the flag — Anthropic 400s otherwise. + # Top-level placement is rejected with 400. assert "disable_parallel_tool_use" not in body, body assert "parallel_tool_calls" not in body, body - # The flag is set on tool_choice. Default type is "auto" when the - # user didn't pick one explicitly. + # Flag lives on tool_choice; default type is "auto". tc = body.get("tool_choice") assert isinstance(tc, dict), body assert tc.get("disable_parallel_tool_use") is True, body @@ -183,9 +182,8 @@ def test_anthropic_disable_parallel_tool_use_nested_under_tool_choice(monkeypatc def test_anthropic_disable_parallel_tool_use_skipped_without_tools(monkeypatch): - """Without any tools defined, `disable_parallel_tool_use` is a - no-op upstream — skip it so the request body stays minimal and the - flag never lands at top level either. + """Without tools the flag is a no-op upstream; keep the body + minimal and never emit it at top level either. """ captured = _install_mock(monkeypatch) body = _drive_anthropic(captured, parallel_tool_calls = False) @@ -271,7 +269,64 @@ def test_openai_compat_forwards_frequency_penalty(monkeypatch): def test_openai_compat_forwards_seed(monkeypatch): captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) body = _drive_openai_compat(captured, seed = 12345) - assert body.get("seed") == 12345, body + # Default OAI-compat provider (mistral here) renames seed to + # random_seed via provider registry's seed_field. + assert body.get("random_seed") == 12345, body + assert "seed" not in body, body + + +def test_openai_compat_seed_field_default_is_seed(monkeypatch): + """Providers without a seed_field override get the OpenAI default.""" + captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) + + async def run(): + client = ExternalProviderClient( + provider_type = "deepseek", + base_url = "https://api.deepseek.com/v1", + api_key = "ds-test", + ) + async for _ in client.stream_chat_completion( + messages = [{"role": "user", "content": "hi"}], + model = "deepseek-chat", + temperature = 0.5, + top_p = 0.9, + max_tokens = 64, + seed = 7, + ): + pass + await client.close() + + _drive(run()) + body = captured["body"] + assert body.get("seed") == 7, body + assert "random_seed" not in body, body + + +def test_openai_compat_deepseek_stop_cap_is_16(monkeypatch): + """DeepSeek docs allow up to 16 stop sequences; the previous + 4-cap silently truncated valid configs.""" + captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) + + async def run(): + client = ExternalProviderClient( + provider_type = "deepseek", + base_url = "https://api.deepseek.com/v1", + api_key = "ds-test", + ) + async for _ in client.stream_chat_completion( + messages = [{"role": "user", "content": "hi"}], + model = "deepseek-chat", + temperature = 0.5, + top_p = 0.9, + max_tokens = 64, + stop = [f"S{i}" for i in range(20)], + ): + pass + await client.close() + + _drive(run()) + body = captured["body"] + assert len(body.get("stop", [])) == 16, body def test_openai_compat_forwards_stop_array(monkeypatch): @@ -282,14 +337,18 @@ def test_openai_compat_forwards_stop_array(monkeypatch): assert "stop_sequences" not in body, body -def test_openai_compat_truncates_stop_to_four(monkeypatch): +def test_openai_compat_truncates_stop_to_default_cap(monkeypatch): + """Default OAI-compat cap is 16 (DeepSeek and Mistral both accept + that many); only OpenAI Chat has a tighter 4-entry hard limit.""" captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) - body = _drive_openai_compat(captured, stop = ["a", "b", "c", "d", "e", "f"]) - assert body.get("stop") == ["a", "b", "c", "d"], body + body = _drive_openai_compat(captured, stop = [f"s{i}" for i in range(20)]) + assert len(body.get("stop", [])) == 16, body + assert body["stop"][0] == "s0" + assert body["stop"][-1] == "s15" def test_openai_compat_stop_dedup_and_drop_empties(monkeypatch): - """Duplicates and empties shouldn't eat into the 4-entry cap.""" + """Duplicates and empties shouldn't eat into the cap.""" captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) body = _drive_openai_compat(captured, stop = ["END", "", "END", "DONE", "FIN", "END"]) assert body.get("stop") == ["END", "DONE", "FIN"], body @@ -398,10 +457,8 @@ def test_openai_responses_forwards_documented_service_tiers(monkeypatch, value): @pytest.mark.parametrize("bogus", ["scale", "standard_only", "bogus", ""]) def test_openai_responses_drops_undocumented_service_tier(monkeypatch, bogus): """`scale` and `standard_only` are not in the documented Responses - request enum. Drop them client-side so a stale frontend cannot 400 - the request. Round 4 consensus (~9/20 reviewers) flagged the - earlier permissive list as a 400 risk; this test pins the - restricted form.""" + request enum; drop them client-side so a stale frontend never + sends an upstream-rejected value.""" captured = _install_mock(monkeypatch, sse_payload = _responses_done_payload()) body = _drive_openai_responses(captured, service_tier = bogus) assert "service_tier" not in body, body @@ -526,10 +583,8 @@ def test_kimi_web_search_bypass_forwards_new_sampling_fields(monkeypatch): def test_local_openai_passthrough_forwards_new_sampling_fields(): - """Round 1 reviewers (10/20) flagged that - `_build_openai_passthrough_body` dropped frequency_penalty / seed / - parallel_tool_calls when forwarding to llama-server. Pin the - extended contract.""" + """`_build_openai_passthrough_body` forwards frequency_penalty, + seed, stop, and parallel_tool_calls to llama-server.""" from models.inference import ChatCompletionRequest from routes.inference import _build_openai_passthrough_body @@ -554,11 +609,9 @@ def test_local_openai_passthrough_forwards_new_sampling_fields(): def test_responses_to_chat_bridge_preserves_parallel_tool_calls(): - """Round 3 reviewers flagged that `_build_chat_request` (the - /v1/responses → /v1/chat/completions translator) dropped - `parallel_tool_calls`, so a Responses-API caller that set - `parallel_tool_calls=false` never saw the flag reach llama-server. - Pin the translation.""" + """`_build_chat_request` (the /v1/responses to /v1/chat/completions + translator) must forward parallel_tool_calls so a Responses-API + caller's preference reaches llama-server.""" from models.inference import ChatMessage, ResponsesRequest from routes.inference import _build_chat_request, _build_openai_passthrough_body @@ -578,9 +631,9 @@ def test_responses_to_chat_bridge_preserves_parallel_tool_calls(): def test_responses_to_chat_bridge_omits_unset_parallel_tool_calls(): - """Unset `parallel_tool_calls` (None) must not appear on the - translated body — the upstream default is `true` everywhere, so - forwarding `parallel_tool_calls=None` would over-specify.""" + """Unset parallel_tool_calls (None) must not appear on the + translated body; the upstream default is true everywhere so + forwarding None would over-specify.""" from models.inference import ChatMessage, ResponsesRequest from routes.inference import _build_chat_request, _build_openai_passthrough_body @@ -599,9 +652,9 @@ def test_responses_to_chat_bridge_omits_unset_parallel_tool_calls(): def test_chat_settings_payload_accepts_new_sampling_keys(): - """Round 1 reviewers flagged that `ChatSettingsPayload.extra="forbid"` - with the old field list 422'd every settings save that contained - any of the new keys. Pin that the new keys round-trip.""" + """ChatSettingsPayload has extra="forbid" so the new keys must be + listed explicitly; otherwise every settings save with any of them + 422s. Pin the round-trip.""" from routes.chat_history import ChatSettingsPayload parsed = ChatSettingsPayload.model_validate( diff --git a/studio/frontend/src/components/ui/stop-sequences-input.tsx b/studio/frontend/src/components/ui/stop-sequences-input.tsx index 5d65fdd36a..2e647de01e 100644 --- a/studio/frontend/src/components/ui/stop-sequences-input.tsx +++ b/studio/frontend/src/components/ui/stop-sequences-input.tsx @@ -38,13 +38,9 @@ export function StopSequencesInput({ const atCap = value.length >= maxEntries; function commitDraft() { - // Reject chips that are empty or contain ONLY whitespace - // (Anthropic 400s on those and OpenAI silently drops them), but - // preserve significant leading/trailing whitespace inside otherwise - // -meaningful stops like " END", "### ", or "\n\n" — stop matching - // is exact, so stripping would silently change the semantics. The - // backend re-validates per-provider before the request hits the - // wire. + // Reject empty / whitespace-only chips but preserve significant + // leading/trailing whitespace (stop matching is exact). Backend + // re-validates per provider. if (!draft || !draft.trim()) return; if (atCap) return; if (value.includes(draft)) { diff --git a/studio/frontend/src/features/chat/chat-settings-sheet.tsx b/studio/frontend/src/features/chat/chat-settings-sheet.tsx index d02369b4ef..271dfe22af 100644 --- a/studio/frontend/src/features/chat/chat-settings-sheet.tsx +++ b/studio/frontend/src/features/chat/chat-settings-sheet.tsx @@ -427,14 +427,10 @@ export function ChatSettingsPanel({ isExternalModel && Boolean(providerCapabilities?.serviceTier); const showParallelToolCalls = !isExternalModel || Boolean(providerCapabilities?.parallelToolCalls); - // OpenAI Chat docs cap `stop` at 4 entries; Anthropic accepts more. - // Pick the right ceiling per active connection so the chips editor's - // placeholder doesn't lie. Local backends (llama.cpp / vLLM / ollama - // / generic OpenAI-compat connections) accept many more — match the - // Anthropic cap there so we do not block users from using stop - // sequences the backend would happily accept. The wire-side - // truncation in `_stream_openai_compat` will still trim to OpenAI's - // 4-entry hard cap when a cloud OpenAI endpoint receives the body. + // OpenAI Chat caps `stop` at 4; Anthropic, DeepSeek, Mistral, and + // local llama.cpp / vLLM / ollama backends accept more. Use 16 as + // the UI ceiling for everything that is not OpenAI cloud Chat; the + // backend re-trims per provider on the wire. const stopMaxEntries = !isExternalModel || externalProviderType === "anthropic" ? 16 : 4; const serviceTierOptions = getServiceTierOptions(externalProviderType); diff --git a/studio/frontend/src/features/chat/presets/preset-policy.ts b/studio/frontend/src/features/chat/presets/preset-policy.ts index 7cd5c76048..d853ef5073 100644 --- a/studio/frontend/src/features/chat/presets/preset-policy.ts +++ b/studio/frontend/src/features/chat/presets/preset-policy.ts @@ -14,13 +14,10 @@ export interface Preset { } // Fields that belong to a preset. Sampling knobs are included so a -// user can save a preset that fixes their preferred decoding style and -// re-apply it on any model. Operational knobs (`serviceTier`, which is -// account-level and per-provider, and `parallelToolCalls`, which is -// tool-level state) are intentionally excluded so switching presets -// does not silently change request routing for the active provider. -// `seed` is also excluded — it is per-request determinism state, not a -// reusable preset value. +// user can save a preset that fixes their preferred decoding style. +// Operational knobs (serviceTier, parallelToolCalls) and per-request +// determinism state (seed) are intentionally excluded so switching +// presets does not silently change request routing. export type PresetOwnedParams = Pick< InferenceParams, | "temperature" diff --git a/studio/frontend/src/features/chat/provider-capabilities.ts b/studio/frontend/src/features/chat/provider-capabilities.ts index 1c628fb199..8b9212512c 100644 --- a/studio/frontend/src/features/chat/provider-capabilities.ts +++ b/studio/frontend/src/features/chat/provider-capabilities.ts @@ -69,15 +69,13 @@ export type ServiceTierOption = | "standard_only"; /** - * Legal `service_tier` values per provider, sourced from each upstream's - * docs. Anthropic exposes only `auto` and `standard_only`. OpenAI in - * Studio is routed through `/v1/responses` (not Chat Completions). The - * live OpenAI Responses API reference and the PR contract both list - * only `auto|default|flex|priority` for /v1/responses, so `scale` is - * deliberately excluded here even though the openai-python SDK type - * is wider — sending an undocumented Responses value is a 400 risk. - * Other providers fall through to a permissive `auto` / `default` - * pair so the picker stays usable for OpenAI-compat backends. + * Legal `service_tier` values per provider. Anthropic exposes only + * `auto` and `standard_only`. OpenAI in Studio is routed through + * `/v1/responses`, which the live docs list as + * `auto|default|flex|priority`; `scale` is excluded here even though + * the openai-python SDK type happens to include it. Other providers + * fall through to a permissive `auto` / `default` pair so the picker + * stays usable for OpenAI-compat backends. */ export function getServiceTierOptions( providerType: string | null | undefined, diff --git a/studio/frontend/src/features/chat/utils/chat-settings-storage.ts b/studio/frontend/src/features/chat/utils/chat-settings-storage.ts index e529b792d1..e6f6b693f1 100644 --- a/studio/frontend/src/features/chat/utils/chat-settings-storage.ts +++ b/studio/frontend/src/features/chat/utils/chat-settings-storage.ts @@ -45,15 +45,12 @@ const NUMERIC_INFERENCE_FIELDS = [ "maxTokens", ] as const satisfies readonly (keyof PersistedInferenceParams)[]; -// `seed` is numeric but nullable (null = "no seed field on the wire") so -// it can't go through the NUMERIC_INFERENCE_FIELDS Finite-number filter. -// Keep this set in sync with `ServiceTier` in ../types/runtime.ts and with -// `getServiceTierOptions` in ../provider-capabilities.ts. `standard_only` -// is Anthropic-only and was missing here — dropping it on save erased the -// user's Anthropic tier choice on reload. `scale` was removed from the UI -// (Codex review feedback) but accepting it on load is harmless for stale -// persisted state, so it stays in the allowlist; the resolver no longer -// surfaces it. +// `seed` is numeric but nullable (null = no seed on the wire) so it +// can't go through the NUMERIC_INFERENCE_FIELDS finite-number filter. +// Keep this set in sync with `ServiceTier` in ../types/runtime.ts and +// `getServiceTierOptions` in ../provider-capabilities.ts. `scale` is +// kept in the allowlist for forward-compat with legacy persisted data +// even though the resolver no longer surfaces it for OpenAI Responses. const VALID_SERVICE_TIERS = new Set([ "auto", "default", @@ -165,16 +162,10 @@ function sanitizeInferenceParams( } else if (typeof value.seed === "number" && Number.isInteger(value.seed)) { params.seed = value.seed; } - // stop: capped string array. Use Anthropic's 16-entry max so the - // sanitizer never silently discards entries the user actually typed - // for an Anthropic session. The backend's per-provider stream helper - // re-truncates to the wire cap (4 for OpenAI Chat, 16 for Anthropic, - // dropped entirely for OpenAI Responses) before the request hits the - // network. Capping to 4 here would defeat Anthropic's UI cap of 16 - // for users who switch providers between sessions. An EMPTY array - // must persist as an empty array (not be sanitized away) so the user - // can clear the last chip and have the change saved — otherwise the - // previously stored stops come back on reload. + // stop: cap at 16 (Anthropic's widest supported). The backend's + // per-provider stream helper re-truncates to the wire cap. An EMPTY + // array must persist (not be sanitized away) so clearing the last + // chip actually saves; otherwise the old stops come back on reload. if (Array.isArray(value.stop)) { const stops = value.stop.filter((s): s is string => typeof s === "string"); params.stop = stops.slice(0, 16);