* studio/openai: align chat completions docstring with stream=false default The schema and regression test for ChatCompletionRequest.stream were already corrected to default `false` (matching OpenAI's spec), but the route docstring still claimed streaming was the default -- misleading for anyone reading the source while debugging the original report. Updates the docstring to reflect the actual behavior, adds an explicit note pointing to #5047, and tags the existing regression test with the issue reference and the .NET / System.Text.Json client class so the intent survives future cleanup. Closes #5047 * studio/openai: address review — move #5047 ref out of OpenAPI doc, add route-level test Two follow-ups on review feedback: - Drop "(see #5047)" from the openai_chat_completions docstring so the internal issue number doesn't leak into the FastAPI-generated OpenAPI / Swagger schema. The reference now lives in a code comment next to the actual `if payload.stream:` branch, where it's most useful for the next person debugging the same class of report. - Add test_post_without_stream_field_decodes_to_stream_false_over_http: a TestClient-based wire-level guard that POSTs a body without `stream` (the exact shape naive curl / .NET / System.Text.Json clients send) and asserts both that the request deserialises into stream=False *and* that the response Content-Type is application/json, never text/event-stream. The existing constructor-level test would silently miss regressions introduced by middleware or alias rewrites that mutate the body before the pydantic model is built. Refs #5047 * studio/openai: route-level test mounts real router instead of synthetic echo app * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Roland Tannous <115670425+rolandtannous@users.noreply.github.com> Co-authored-by: Roland Tannous <rolandtannous@gravityq.ai> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
20e66adae3
commit
867b1e1873
2 changed files with 55 additions and 3 deletions
|
|
@ -1959,8 +1959,11 @@ async def openai_chat_completions(
|
|||
Supports multimodal messages: ``content`` may be a plain string or a
|
||||
list of content parts (``text`` / ``image_url``).
|
||||
|
||||
Streaming (default): returns SSE chunks matching OpenAI's format.
|
||||
Non-streaming: returns a single ChatCompletion JSON object.
|
||||
Non-streaming (default): returns a single ChatCompletion JSON object.
|
||||
Streaming: returns SSE chunks matching OpenAI's format.
|
||||
|
||||
``stream`` defaults to ``false`` to match OpenAI's spec; clients opt
|
||||
into SSE by sending ``stream: true``.
|
||||
|
||||
Automatically routes to the correct backend:
|
||||
- GGUF models → llama-server via LlamaCppBackend
|
||||
|
|
@ -2184,6 +2187,9 @@ async def openai_chat_completions(
|
|||
|
||||
cancel_event = threading.Event()
|
||||
completion_id = f"chatcmpl-{uuid.uuid4().hex[:12]}"
|
||||
# `stream` defaults to False on ChatCompletionRequest (OpenAI spec
|
||||
# parity). Naive curl / .NET / System.Text.Json clients omitting
|
||||
# the field used to get SSE here and choke on deserialization (#5047).
|
||||
if payload.stream:
|
||||
return await _openai_passthrough_stream(
|
||||
request,
|
||||
|
|
|
|||
|
|
@ -299,11 +299,57 @@ class TestChatCompletionRequestToolFields:
|
|||
def test_stream_defaults_false_matching_openai_spec(self):
|
||||
# OpenAI's /v1/chat/completions spec defaults `stream` to false.
|
||||
# Studio previously defaulted to true, which broke naive curl
|
||||
# clients that omit `stream` (they expect a JSON blob, got SSE).
|
||||
# clients (and .NET / System.Text.Json SDKs per #5047) that omit
|
||||
# `stream` -- they expect a JSON blob, got SSE.
|
||||
# Pin the corrected default so it can't silently regress.
|
||||
req = self._make()
|
||||
assert req.stream is False
|
||||
|
||||
def test_post_without_stream_field_decodes_to_stream_false_over_http(
|
||||
self, monkeypatch
|
||||
):
|
||||
# Wire-level guard for the same default: a POST body that omits
|
||||
# `stream` entirely (the exact shape naive curl / .NET clients
|
||||
# send) must deserialise into stream=False *and* the response
|
||||
# must be `application/json`, never `text/event-stream`.
|
||||
# Mounts the real `routes.inference.router` so this catches
|
||||
# regressions in middleware/aliasing on the actual endpoint
|
||||
# (e.g. someone adding a request layer that injects stream=True
|
||||
# before pydantic builds the model). Backends are bypassed by
|
||||
# routing through `provider_type` and stubbing the external
|
||||
# provider proxy.
|
||||
from fastapi import FastAPI
|
||||
from fastapi.responses import JSONResponse
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
import routes.inference as inference_route
|
||||
from auth.authentication import get_current_subject
|
||||
|
||||
captured = {}
|
||||
|
||||
async def _fake_proxy(payload, request):
|
||||
captured["stream"] = payload.stream
|
||||
return JSONResponse({"choices": [], "object": "chat.completion"})
|
||||
|
||||
monkeypatch.setattr(inference_route, "_proxy_to_external_provider", _fake_proxy)
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(inference_route.router)
|
||||
app.dependency_overrides[get_current_subject] = lambda: "test-user"
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/chat/completions",
|
||||
json = {
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"provider_type": "openai",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers["content-type"].startswith("application/json")
|
||||
assert "text/event-stream" not in resp.headers["content-type"]
|
||||
assert captured["stream"] is False
|
||||
|
||||
def test_multiturn_tool_loop_messages(self):
|
||||
req = ChatCompletionRequest(
|
||||
messages = [
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue