Clean local stop list + cap Responses bridge parallel tool calls (PR #5711)
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.
This commit is contained in:
Daniel Han
parent
5218a01d14
commit
0be1e09421
2 changed files with 58 additions and 17 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue