From 089712bc2cfa916b58dff4100f1bc681bdbe91f6 Mon Sep 17 00:00:00 2001 From: Hamed Shams Date: Sat, 10 Oct 2026 00:55:01 +0300 Subject: [PATCH] fix: accept `stop` as a string, and answer a request that does not decode with 400 OpenAI's `stop` is a string, an array, or null; both request types decoded only the array form, so a bare string failed to decode. StopSequences now takes either form, and null or absent still means no client stops. That decode failure was answered as 500 server_error, which the OpenAI SDKs retry as a transient error. Only a DecodingError from the request-body decode now maps to 400 invalid_request_error with `param` taken from the coding path; everything else on these routes still returns 500. Closes #221. --- Sources/SwiftLM/Server.swift | 82 +++++++++++++- .../SwiftLMTests/RequestValidationTests.swift | 104 ++++++++++++++++++ tests/test-contract.sh | 24 ++++ 3 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 tests/SwiftLMTests/RequestValidationTests.swift diff --git a/Sources/SwiftLM/Server.swift b/Sources/SwiftLM/Server.swift index 52da5c3..d1cd50b 100644 --- a/Sources/SwiftLM/Server.swift +++ b/Sources/SwiftLM/Server.swift @@ -2338,7 +2338,12 @@ func handleChatCompletion( dflashTargetModel: (any DFlashTargetModel)? = nil, mtpAssistant: (any DualModelMTP)? = nil ) async throws -> Response { - let chatReq = try JSONDecoder().decode(ChatCompletionRequest.self, from: bodyData) + let chatReq: ChatCompletionRequest + do { + chatReq = try JSONDecoder().decode(ChatCompletionRequest.self, from: bodyData) + } catch let error as DecodingError { + return invalidRequestResponse(error) + } let isStream = chatReq.stream ?? false let jsonMode = chatReq.responseFormat?.type == "json_object" let emitPrefillProgress = prefillProgressEnabled(in: request) @@ -2350,7 +2355,7 @@ func handleChatCompletion( let topK = chatReq.topK ?? config.topK ?? 50 let minP = chatReq.minP.map(Float.init) ?? config.minP ?? 0.0 let repeatPenalty = chatReq.repetitionPenalty.map(Float.init) ?? config.repeatPenalty - let stopSequences = (chatReq.stop ?? []) + ["", "<|im_end|>", "<|eot_id|>", "", "<|tool_response|>"] + let stopSequences = (chatReq.stop?.values ?? []) + ["", "<|im_end|>", "<|eot_id|>", "", "<|tool_response|>"] let includeUsage = chatReq.streamOptions?.includeUsage ?? false // Log extra sampling params if provided (accepted for API compat, not all are used) @@ -3532,7 +3537,12 @@ func handleTextCompletion( semaphore: AsyncSemaphore, stats: ServerStats ) async throws -> Response { - let compReq = try JSONDecoder().decode(TextCompletionRequest.self, from: bodyData) + let compReq: TextCompletionRequest + do { + compReq = try JSONDecoder().decode(TextCompletionRequest.self, from: bodyData) + } catch let error as DecodingError { + return invalidRequestResponse(error) + } let isStream = compReq.stream ?? false let emitPrefillProgress = prefillProgressEnabled(in: request) @@ -3542,7 +3552,7 @@ func handleTextCompletion( let topK = compReq.topK ?? config.topK ?? 50 let minP = compReq.minP.map(Float.init) ?? config.minP ?? 0.0 let repeatPenalty = compReq.repetitionPenalty.map(Float.init) ?? config.repeatPenalty - let stopSequences = compReq.stop ?? [] + let stopSequences = compReq.stop?.values ?? [] var generateParams = GenerateParameters( maxTokens: tokenLimit, @@ -4090,6 +4100,51 @@ func rejectLongPrompt(promptTokens: Int, limit: Int?, slot: GenerationSlot, stat return Response(status: .badRequest, headers: jsonHeaders(), body: .init(byteBuffer: ByteBuffer(string: body))) } +/// The 400 for a request body that does not decode. OpenAI answers these with +/// `invalid_request_error` and names the offending `param`; the OpenAI SDKs treat a 400 +/// as final but retry a 5xx, so answering a decode failure with 500 `server_error` cost +/// every malformed request three attempts and told the client the server was at fault. +func invalidRequestResponse(_ error: DecodingError) -> Response { + Response( + status: .badRequest, + headers: jsonHeaders(), + body: .init(byteBuffer: ByteBuffer(string: invalidRequestJSON(error))) + ) +} + +func invalidRequestJSON(_ error: DecodingError) -> String { + func dotted(_ path: [CodingKey]) -> String { + path.map { $0.intValue.map(String.init) ?? $0.stringValue }.joined(separator: ".") + } + let message: String + let path: [CodingKey] + switch error { + case .keyNotFound(let key, let context): + path = context.codingPath + [key] + message = "Missing required parameter: '\(dotted(path))'." + case .typeMismatch(_, let context): + path = context.codingPath + message = "Invalid type for '\(dotted(path))': \(context.debugDescription)" + case .valueNotFound(_, let context): + path = context.codingPath + message = "Missing value for '\(dotted(path))'." + case .dataCorrupted(let context): + path = context.codingPath + message = context.debugDescription + @unknown default: + path = [] + message = String(describing: error) + } + var payload: [String: Any] = ["message": message, "type": "invalid_request_error"] + if !path.isEmpty { payload["param"] = dotted(path) } + guard let data = try? JSONSerialization.data(withJSONObject: ["error": payload]), + let json = String(data: data, encoding: .utf8) + else { + return "{\"error\":{\"message\":\"invalid request\",\"type\":\"invalid_request_error\"}}" + } + return json +} + /// Build an OpenAI-style `{"error":{...}}` body for a server error. The message /// is JSON-encoded, so quotes, backslashes and newlines in it stay valid JSON. func errorJSON(_ error: Error) -> String { @@ -4443,7 +4498,7 @@ struct ChatCompletionRequest: Decodable { let frequencyPenalty: Double? let presencePenalty: Double? let tools: [ToolDef]? - let stop: [String]? + let stop: StopSequences? let seed: Int? let streamOptions: StreamOptions? let responseFormat: ResponseFormat? @@ -4474,6 +4529,21 @@ struct ChatCompletionRequest: Decodable { } } +/// OpenAI's `stop` is "string / array / null": a single sequence may be sent bare, and +/// several clients do. Decoding only the array form answered those requests with a 500. +struct StopSequences: Decodable { + let values: [String] + + init(from decoder: Swift.Decoder) throws { + let container = try decoder.singleValueContainer() + if let single = try? container.decode(String.self) { + values = [single] + } else { + values = try container.decode([String].self) + } + } +} + struct TextCompletionRequest: Decodable { let model: String? let prompt: String @@ -4484,7 +4554,7 @@ struct TextCompletionRequest: Decodable { let topK: Int? let minP: Double? let repetitionPenalty: Double? - let stop: [String]? + let stop: StopSequences? let seed: Int? enum CodingKeys: String, CodingKey { diff --git a/tests/SwiftLMTests/RequestValidationTests.swift b/tests/SwiftLMTests/RequestValidationTests.swift new file mode 100644 index 0000000..26acf11 --- /dev/null +++ b/tests/SwiftLMTests/RequestValidationTests.swift @@ -0,0 +1,104 @@ +import XCTest +import Foundation +@testable import SwiftLM + +// MARK: - Request shapes the OpenAI API allows that the server used to reject +// +// `stop` is documented as "string / array / null". Both request types decoded only the +// array form, so `"stop": "\n"` — what several SDKs and llama.cpp-style clients send — +// failed to decode. And a decode failure was answered with a 500 `server_error`, which +// the OpenAI SDKs retry (they treat 5xx as transient), instead of the 400 +// `invalid_request_error` that tells a client its request is the problem. +final class RequestValidationTests: XCTestCase { + + private func decodeChat(_ json: String) throws -> ChatCompletionRequest { + try JSONDecoder().decode(ChatCompletionRequest.self, from: try XCTUnwrap(json.data(using: .utf8))) + } + + private func decodeText(_ json: String) throws -> TextCompletionRequest { + try JSONDecoder().decode(TextCompletionRequest.self, from: try XCTUnwrap(json.data(using: .utf8))) + } + + private let messages = #"[{"role":"user","content":"hi"}]"# + + // MARK: stop: string | [string] | null + + func testChatStopAcceptsAString() throws { + let req = try decodeChat(#"{"messages":\#(messages),"stop":"\n"}"#) + XCTAssertEqual(req.stop?.values, ["\n"]) + } + + func testChatStopStillAcceptsAnArray() throws { + let req = try decodeChat(#"{"messages":\#(messages),"stop":["a","b"]}"#) + XCTAssertEqual(req.stop?.values, ["a", "b"]) + } + + func testChatStopNullAndAbsentAreNil() throws { + XCTAssertNil(try decodeChat(#"{"messages":\#(messages),"stop":null}"#).stop) + XCTAssertNil(try decodeChat(#"{"messages":\#(messages)}"#).stop) + } + + func testTextCompletionStopAcceptsAString() throws { + let req = try decodeText(#"{"prompt":"x","stop":"<|end|>"}"#) + XCTAssertEqual(req.stop?.values, ["<|end|>"]) + } + + func testTextCompletionStopStillAcceptsAnArray() throws { + let req = try decodeText(#"{"prompt":"x","stop":["<|end|>","---"]}"#) + XCTAssertEqual(req.stop?.values, ["<|end|>", "---"]) + } + + /// Anything else is still a decode error — a number is not a stop sequence. + func testStopRejectsOtherTypes() { + XCTAssertThrowsError(try decodeChat(#"{"messages":\#(messages),"stop":42}"#)) { error in + XCTAssertTrue(error is DecodingError) + } + } + + // MARK: Decode failures become 400 invalid_request_error + + private func errorObject(_ json: String) throws -> [String: Any] { + let data = try XCTUnwrap(json.data(using: .utf8)) + let top = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + return try XCTUnwrap(top["error"] as? [String: Any]) + } + + func testTypeMismatchNamesTheParameter() throws { + let error = try captureDecodingError { try decodeChat(#"{"messages":\#(messages),"stop":42}"#) } + let payload = try errorObject(invalidRequestJSON(error)) + XCTAssertEqual(payload["type"] as? String, "invalid_request_error") + XCTAssertEqual(payload["param"] as? String, "stop") + XCTAssertTrue((payload["message"] as? String ?? "").contains("stop")) + } + + func testMissingKeyNamesTheParameter() throws { + let error = try captureDecodingError { try decodeChat(#"{"model":"x"}"#) } + let payload = try errorObject(invalidRequestJSON(error)) + XCTAssertEqual(payload["type"] as? String, "invalid_request_error") + XCTAssertEqual(payload["param"] as? String, "messages") + } + + func testNestedKeyPathIsDotted() throws { + let error = try captureDecodingError { try decodeChat(#"{"messages":[{"content":"hi"}]}"#) } + let payload = try errorObject(invalidRequestJSON(error)) + XCTAssertEqual(payload["param"] as? String, "messages.0.role") + } + + func testMalformedJSONHasNoParameter() throws { + let error = try captureDecodingError { try decodeChat("{not json") } + let payload = try errorObject(invalidRequestJSON(error)) + XCTAssertEqual(payload["type"] as? String, "invalid_request_error") + XCTAssertNil(payload["param"]) + XCTAssertFalse((payload["message"] as? String ?? "").isEmpty) + } + + private func captureDecodingError(_ body: () throws -> Any) throws -> DecodingError { + do { + _ = try body() + } catch let error as DecodingError { + return error + } + XCTFail("expected a DecodingError") + throw XCTSkip("no DecodingError thrown") + } +} diff --git a/tests/test-contract.sh b/tests/test-contract.sh index 8b8cc55..f7dc65b 100755 --- a/tests/test-contract.sh +++ b/tests/test-contract.sh @@ -241,6 +241,30 @@ else fail "chunk-prefilled prompt returned no content: $(echo "$LONG_RESP" | head -c 120)" fi +# ── 9. Request shapes the OpenAI API allows ────────────────────────────────── +# `stop` may be a bare string; both request types decoded only the array form, and the +# decode failure surfaced as a 500 `server_error`, which the OpenAI SDKs retry. +log "Test 9: stop as a plain string" +BODY=$(chat '{"model":"x","messages":[{"role":"user","content":"Count: one two three four five"}],"max_tokens":80,"temperature":0,"stop":"three"}' || true) +CONTENT=$(echo "$BODY" | python3 -c 'import json,sys; print(json.load(sys.stdin)["choices"][0]["message"].get("content") or "")' 2>/dev/null || echo "__no_body__") +if [ "$CONTENT" = "__no_body__" ]; then + fail "stop as a string was rejected: $(echo "$BODY" | head -c 120)" +elif echo "$CONTENT" | grep -q "three"; then + fail "stop string 'three' leaked into content: $(echo "$CONTENT" | head -c 80)" +else + pass "a bare stop string is accepted and honoured" +fi + +log "Test 9b: a request that does not decode is a 400 invalid_request_error" +CODE=$(curl -s -o /tmp/SwiftLM-contract-400.json -w '%{http_code}' "$URL/v1/chat/completions" \ + -H 'Content-Type: application/json' -d '{"model":"x","messages":[{"role":"user","content":"hi"}],"stop":42}') +ERR_TYPE=$(python3 -c 'import json; e=json.load(open("/tmp/SwiftLM-contract-400.json"))["error"]; print(e.get("type"), e.get("param"))' 2>/dev/null || echo "unparseable") +if [ "$CODE" = "400" ] && [ "$ERR_TYPE" = "invalid_request_error stop" ]; then + pass "400 invalid_request_error naming param 'stop'" +else + fail "got HTTP $CODE with error '$ERR_TYPE' (expected 400 invalid_request_error stop)" +fi + log "═══════════════════════════════════════" log "Results: $PASS passed, $FAIL failed, $SKIP skipped, $TOTAL total" log "═══════════════════════════════════════"