From e0a9b1d76aa854471740697fff7d3a7df9198a2d Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 15:54:26 +0000 Subject: [PATCH] Drop Kimi frequency_penalty and gate generic service_tier on opt-in 5/10 reviewers in the last round flagged Kimi forwarding non-default frequency_penalty as a 400 risk for K2.5 / K2.6, mirroring the existing lock on temperature and top_p. Hide the slider on the frontend and add frequency_penalty to Kimi's body_omit so even stale clients have the field stripped before the request hits the wire. service_tier on the generic OpenAI-compatible branch was forwarding whatever value the dispatcher received, so a stale frontend could send standard_only (Anthropic) or scale to providers like Mistral that do not document the field, producing 400s. Gate the forward on an explicit accepts_service_tier=True provider registry opt-in; Anthropic and OpenAI Responses already handle service_tier inside their own helpers. --- .../core/inference/external_provider.py | 5 ++++- studio/backend/core/inference/providers.py | 11 ++++++----- .../tests/test_sampling_params_routing.py | 19 +++++++++++++------ .../features/chat/provider-capabilities.ts | 5 ++++- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/studio/backend/core/inference/external_provider.py b/studio/backend/core/inference/external_provider.py index a9d8f12e5b..a8840a8ee3 100644 --- a/studio/backend/core/inference/external_provider.py +++ b/studio/backend/core/inference/external_provider.py @@ -519,7 +519,10 @@ class ExternalProviderClient: normalized_stop = _normalize_stop_for_provider(stop, provider_info) if normalized_stop: body["stop"] = normalized_stop - if service_tier is not None: + # service_tier is OpenAI Chat-only on the generic OAI-compat + # branch; opt-in via `accepts_service_tier=True` on the registry + # entry. Anthropic and Responses handle it in their own helpers. + if service_tier is not None and provider_info.get("accepts_service_tier", False): body["service_tier"] = service_tier if parallel_tool_calls is not None: body["parallel_tool_calls"] = parallel_tool_calls diff --git a/studio/backend/core/inference/providers.py b/studio/backend/core/inference/providers.py index 026d19217b..b967123ada 100644 --- a/studio/backend/core/inference/providers.py +++ b/studio/backend/core/inference/providers.py @@ -157,11 +157,12 @@ PROVIDER_REGISTRY: dict[str, dict[str, Any]] = { "auth_prefix": "Bearer ", "notes": "Moonshot API key. China: use base URL https://api.moonshot.cn/v1", "model_id_allowlist": re.compile(r"^kimi-k2\.[56]$"), - # Both k2.6 and k2.5 are reasoning-class. The API rejects custom - # sampling: "invalid temperature: only 1 is allowed for this model" - # (and the same shape for top_p). Strip both fields from the - # outbound body so the server falls back to its required defaults. - "body_omit": ("temperature", "top_p"), + # Both k2.6 and k2.5 are reasoning-class. The API rejects + # custom sampling: "invalid temperature: only 1 is allowed for + # this model" (and the same shape for top_p). frequency_penalty + # is reported by reviewers to follow the same lock; strip it + # too so non-default values from stale clients do not 400. + "body_omit": ("temperature", "top_p", "frequency_penalty"), # Kimi accepts at most 5 stop strings (each <= 32 bytes) per # https://platform.kimi.ai/docs/api/chat "stop_max": 5, diff --git a/studio/backend/tests/test_sampling_params_routing.py b/studio/backend/tests/test_sampling_params_routing.py index 0cfda6b7d1..90509ead5d 100644 --- a/studio/backend/tests/test_sampling_params_routing.py +++ b/studio/backend/tests/test_sampling_params_routing.py @@ -360,10 +360,16 @@ def test_openai_compat_empty_stop_omitted(monkeypatch): assert "stop" not in body, body -def test_openai_compat_forwards_service_tier(monkeypatch): +def test_openai_compat_drops_service_tier_by_default(monkeypatch): + """Generic OAI-compat providers (mistral, deepseek, openrouter, ...) + do not document a `service_tier` field. The dispatcher must drop + it unless the provider registry explicitly opts in with + `accepts_service_tier=True`; otherwise a stale frontend could + smuggle Anthropic/OpenAI-Responses-only values onto unrelated + providers.""" captured = _install_mock(monkeypatch, sse_payload = _oai_done_payload()) body = _drive_openai_compat(captured, service_tier = "flex") - assert body.get("service_tier") == "flex", body + assert "service_tier" not in body, body def test_openai_compat_forwards_parallel_tool_calls(monkeypatch): @@ -567,14 +573,15 @@ def test_kimi_web_search_bypass_forwards_new_sampling_fields(monkeypatch): _drive(run()) body = captured["body"] - assert body.get("frequency_penalty") == 1.25, body + # Per-provider drops: Kimi locks frequency_penalty/temperature/top_p. + assert "frequency_penalty" not in body, body + assert "temperature" not in body, body + assert "top_p" not in body, body + # Other knobs forward through the bypass. assert body.get("seed") == 7, body assert body.get("stop") == ["END"], body assert body.get("parallel_tool_calls") is False, body assert body.get("presence_penalty") == 0.5, body - # body_omit still strips temperature / top_p for Kimi. - assert "temperature" not in body, body - assert "top_p" not in body, body def test_kimi_web_search_uses_kimi_stop_cap_5(monkeypatch): diff --git a/studio/frontend/src/features/chat/provider-capabilities.ts b/studio/frontend/src/features/chat/provider-capabilities.ts index 1565516dd8..7a39583d42 100644 --- a/studio/frontend/src/features/chat/provider-capabilities.ts +++ b/studio/frontend/src/features/chat/provider-capabilities.ts @@ -454,7 +454,10 @@ const PROVIDER_CAPABILITIES: Record = { minP: false, repetitionPenalty: false, presencePenalty: true, - frequencyPenalty: true, + // K2.5/K2.6 lock sampling the same way temperature/top_p are + // locked; reviewers report non-default frequency_penalty 400s + // upstream, so hide the slider and strip the field in body_omit. + frequencyPenalty: false, seed: false, stop: true, serviceTier: false,