From 0be1e094218131e23a8994076b3d17af6650c837 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 19:39:37 +0000 Subject: [PATCH] Clean local stop list + cap Responses bridge parallel tool calls (PR #5711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 17 reviewer consensus on two extensions of the round 16 cap: 1. Direct GGUF stop forwarding (3/10 + sibling findings) — every llama_cpp.py payload builder and routes/inference.py direct-GGUF call site pass `stop` through unfiltered, while the external provider helper and `_build_passthrough_payload` already strip empty / non-string entries. Add a shared `_clean_local_stop_list` helper in the route layer for the two callers, mirror the same inline filter in `llama_cpp.py`'s three payload builders. Stops `stop=["", "END"]` from a stale client 400'ing llama-server. 2. Responses bridge tool-call cap (3/10 streaming + 1/10 non-streaming) — `_responses_stream` iterated every streamed `delta.tool_calls` index and `_responses_non_streaming` translated every returned `message.tool_calls` entry, even when `parallel_tool_calls=false`. Latch the first tool-call index in the streaming bridge and drop subsequent siblings; cap to one in the non-streaming bridge. Matches the GGUF agentic-loop / Anthropic-passthrough caps. Local-passthrough OpenAI paths (verbatim SSE / verbatim JSON) are left alone because the contract is "raw upstream forwarding"; clients calling /v1/chat/completions through Studio directly should still see llama-server's native output. --- studio/backend/core/inference/llama_cpp.py | 27 ++++++++++-- studio/backend/routes/inference.py | 48 +++++++++++++++------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 0a8a86136c..0776f0fdfe 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -4287,8 +4287,17 @@ class LlamaCppBackend: else (self._effective_context_length or _DEFAULT_MAX_TOKENS_FLOOR) ) payload["t_max_predict_ms"] = _DEFAULT_T_MAX_PREDICT_MS + # Strip empty / non-string stop entries before forwarding to + # llama-server (matches `_normalize_stop_for_provider` for the + # external path). Without this a stale `stop=["", "END"]` can + # 400 the upstream. if stop: - payload["stop"] = stop + if isinstance(stop, str): + payload["stop"] = stop + elif isinstance(stop, list): + _cleaned = [s for s in stop if isinstance(s, str) and s] + if _cleaned: + payload["stop"] = _cleaned # Optional sampling extensions, gated on `is not None` so 0, # 0.0, and False all reach the wire. if frequency_penalty is not None: @@ -4524,8 +4533,14 @@ class LlamaCppBackend: else (self._effective_context_length or _DEFAULT_MAX_TOKENS_FLOOR) ) payload["t_max_predict_ms"] = _DEFAULT_T_MAX_PREDICT_MS + # Same empty-string filter as the standard payload builder. if stop: - payload["stop"] = stop + if isinstance(stop, str): + payload["stop"] = stop + elif isinstance(stop, list): + _cleaned = [s for s in stop if isinstance(s, str) and s] + if _cleaned: + payload["stop"] = _cleaned # Optional sampling extensions; gated on `is not None`. if frequency_penalty is not None: payload["frequency_penalty"] = frequency_penalty @@ -5209,8 +5224,14 @@ class LlamaCppBackend: else (self._effective_context_length or _DEFAULT_MAX_TOKENS_FLOOR) ) stream_payload["t_max_predict_ms"] = _DEFAULT_T_MAX_PREDICT_MS + # Same empty-string filter as the standard / tool payload builder. if stop: - stream_payload["stop"] = stop + if isinstance(stop, str): + stream_payload["stop"] = stop + elif isinstance(stop, list): + _cleaned = [s for s in stop if isinstance(s, str) and s] + if _cleaned: + stream_payload["stop"] = _cleaned # Match the per-iteration tool loop above so sampling behavior # stays consistent when the cap-exhausted final-answer pass runs. if frequency_penalty is not None: diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index af2925f946..c14d721cec 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -239,6 +239,22 @@ router = APIRouter() studio_router = APIRouter() +def _clean_local_stop_list(stop) -> Optional[list[str]]: + """Strip empty / non-string entries from a stop sequence input. + + Mirrors `_normalize_stop_for_provider` (external_provider.py) so + local llama-server callers cannot ship `stop=["", "END"]` and + get a 400. Returns `None` when nothing survives, so the caller + can omit the field entirely. + """ + if isinstance(stop, str): + return [stop] if stop else None + if isinstance(stop, list): + cleaned = [s for s in stop if isinstance(s, str) and s] + return cleaned or None + return None + + def _detect_safetensors_features(backend, chat_template: Optional[str]) -> dict: """Classify reasoning/tool capabilities via the GGUF classifier so flags match across backends. gpt-oss is overridden because Harmony @@ -2482,13 +2498,7 @@ 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 - ), + stop = _clean_local_stop_list(payload.stop), cancel_event = cancel_event, enable_thinking = payload.enable_thinking, reasoning_effort = payload.reasoning_effort, @@ -2671,13 +2681,7 @@ 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 - ), + stop = _clean_local_stop_list(payload.stop), cancel_event = cancel_event, enable_thinking = payload.enable_thinking, reasoning_effort = payload.reasoning_effort, @@ -3874,6 +3878,11 @@ async def _responses_non_streaming( msg = choices[0].get("message", {}) or {} text = msg.get("content", "") or "" tool_calls = msg.get("tool_calls") or [] + # Match the cap applied on GGUF / Anthropic / safetensors tool + # paths: when the caller opted out of parallel tool calls, surface + # at most one. llama.cpp may not enforce the flag. + if payload.parallel_tool_calls is False and tool_calls: + tool_calls = tool_calls[:1] usage_data = body.get("usage", {}) input_tokens = usage_data.get("prompt_tokens", 0) @@ -3995,6 +4004,12 @@ async def _responses_stream( tool_call_state: dict[int, dict] = {} # Text message lives at output_index 0; tool calls claim 1, 2, ... next_output_index = 1 + # When the caller opted out of parallel tool calls, latch the + # first index we see and drop subsequent siblings — mirrors the + # GGUF agentic-loop / Anthropic-passthrough caps; llama.cpp may + # not enforce the upstream flag (ggml-org/llama.cpp#22043). + serial_tool_calls = payload.parallel_tool_calls is False + first_serial_idx: Optional[int] = None def _snapshot_output() -> list[dict]: """Snapshot of all completed output items for response.completed.""" @@ -4111,6 +4126,11 @@ async def _responses_stream( for tc in delta.get("tool_calls") or []: idx = tc.get("index", 0) + if serial_tool_calls: + if first_serial_idx is None: + first_serial_idx = idx + if idx != first_serial_idx: + continue st = tool_call_state.get(idx) fn = tc.get("function") or {} if st is None: