Skip to content

Commit ff0c28d

Browse files
fix: strip NSNull from chat templates before Jinja render (#168)
JSON nulls in tool schemas (common in opencode zod/effect output) decode to NSNull via AnyCodable and crash swift-jinja Value.init(any:) before the template is ever rendered — mislabeled as a broken chat template. The #142 startup probe used tools: nil so it never hit this path, and opencode also rejected the prefill_progress heartbeat for missing choices. - sanitizeForJinja: recursively drop NSNull / unwrap optionals in TransformersTokenizerBridge.applyChatTemplate (messages, tools, additionalContext) - extend startup probe with a minimal tools array (warn-only; does not abort startup) - include choices: [] in ssePrefillChunk so strict OpenAI chunk validators accept the named heartbeat event - bump swift-jinja 2.3.5 -> 2.5.1 for accurate conversion errors - unit tests for the sanitizer + updated SSE expectations - test-opencode.sh Test 3: tools + prefill-progress combined (#75 gap) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c70957c commit ff0c28d

5 files changed

Lines changed: 262 additions & 8 deletions

File tree

‎Package.resolved‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Sources/SwiftLM/Server.swift‎

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,15 @@ private struct TransformersTokenizerBridge: MLXLMCommon.Tokenizer, Sendable {
182182
additionalContext: [String: any Sendable]?
183183
) throws -> [Int] {
184184
do {
185+
// Issue #168: JSON `null` in tool schemas decodes to NSNull via AnyCodable.
186+
// swift-jinja's Value.init(any:) has no NSNull case, so a single null anywhere
187+
// in a tool spec aborted the whole render with a misleading "Optional<Any>"
188+
// conversion error that was then mislabeled as a broken template. Strip nulls
189+
// (and unwrap nested optionals) before handing anything to the template engine.
185190
return try upstream.applyChatTemplate(
186-
messages: messages, tools: tools, additionalContext: additionalContext)
191+
messages: messages.map { $0.mapValuesDeep(sanitizeForJinja) },
192+
tools: tools?.map { $0.mapValuesDeep(sanitizeForJinja) },
193+
additionalContext: additionalContext.map { $0.mapValuesDeep(sanitizeForJinja) })
187194
} catch Tokenizers.TokenizerError.missingChatTemplate {
188195
throw MLXLMCommon.TokenizerError.missingChatTemplate
189196
} catch {
@@ -196,6 +203,42 @@ private struct TransformersTokenizerBridge: MLXLMCommon.Tokenizer, Sendable {
196203
}
197204
}
198205

206+
/// Returns `nil` when the value must be dropped (JSON `null` / NSNull), otherwise a
207+
/// structure with every nested null removed. See `TransformersTokenizerBridge.applyChatTemplate`.
208+
func sanitizeForJinja(_ value: any Sendable) -> any Sendable? {
209+
if value is NSNull { return nil }
210+
let mirror = Mirror(reflecting: value)
211+
if mirror.displayStyle == .optional {
212+
guard let child = mirror.children.first else { return nil }
213+
return sanitizeForJinja(child.value as any Sendable)
214+
}
215+
if let dict = value as? [String: Any] {
216+
var out: [String: any Sendable] = [:]
217+
for (key, val) in dict {
218+
if let cleaned = sanitizeForJinja(val as any Sendable) {
219+
out[key] = cleaned
220+
}
221+
}
222+
return out
223+
}
224+
if let arr = value as? [Any] {
225+
return arr.compactMap { sanitizeForJinja($0 as any Sendable) }
226+
}
227+
return value
228+
}
229+
230+
extension Dictionary where Key == String, Value == any Sendable {
231+
func mapValuesDeep(_ transform: (any Sendable) -> any Sendable?) -> [String: any Sendable] {
232+
var out: [String: any Sendable] = [:]
233+
for (key, val) in self {
234+
if let cleaned = transform(val) {
235+
out[key] = cleaned
236+
}
237+
}
238+
return out
239+
}
240+
}
241+
199242
// ── CLI ──────────────────────────────────────────────────────────────────────
200243

201244
final class ProgressTracker {
@@ -1115,6 +1158,37 @@ struct MLXServer: AsyncParsableCommand {
11151158
// supply. Neither is a reason to refuse to start.
11161159
}
11171160

1161+
// Issue #168: render once more with a minimal non-empty tools array. The probe
1162+
// above uses `tools: nil`, so a checkpoint whose template (or tool payload)
1163+
// breaks only under the tools branch loaded clean and then failed every real
1164+
// agentic request. Warn rather than abort: a tools-broken model still serves
1165+
// plain chat and /v1/completions.
1166+
do {
1167+
let probeTokenizer = await container.tokenizer
1168+
let probeTool: [String: any Sendable] = [
1169+
"type": "function",
1170+
"function": [
1171+
"name": "probe",
1172+
"description": "startup chat-template tools probe",
1173+
"parameters": [
1174+
"type": "object",
1175+
"properties": [
1176+
"query": ["type": "string", "default": NSNull() as any Sendable]
1177+
],
1178+
] as [String: any Sendable],
1179+
] as [String: any Sendable],
1180+
]
1181+
_ = try probeTokenizer.applyChatTemplate(
1182+
messages: [["role": "user", "content": "ping"]],
1183+
tools: [probeTool],
1184+
additionalContext: ["add_generation_prompt": true]
1185+
)
1186+
} catch let error as MalformedChatTemplate {
1187+
print("[SwiftLM] ⚠️ chat-template tools probe failed (plain chat may still work): \(error.description)")
1188+
} catch {
1189+
// Same lenient pass as above: no template, or context the probe lacks.
1190+
}
1191+
11181192
print("[SwiftLM] Model loaded. Starting HTTP server on \(host):\(port)")
11191193

11201194
// ── Capture CLI defaults into a shared config ──
@@ -3161,23 +3235,26 @@ func sseChunk(modelId: String, reasoningContent: String?, content: String?, fini
31613235

31623236
/// Prefill-progress heartbeat chunk — emitted every 2s while the server is processing the prompt
31633237
/// when explicitly enabled via `X-SwiftLM-Prefill-Progress: true`.
3164-
/// It is sent as a named SSE event (`event: prefill_progress`) to avoid breaking strict
3165-
/// OpenAI-compatible clients (e.g. OpenCode), which reject unknown `data:` objects.
3238+
/// It is sent as a named SSE event (`event: prefill_progress`).
31663239
/// Format mirrors llama-server's slot_update event:
31673240
/// n_past : tokens evaluated so far (real value from chunked prefill, or 0 for single-chunk)
31683241
/// n_prompt_tokens : total prompt token count
31693242
/// fraction : n_past / n_prompt_tokens (0.0–1.0), useful for progress bars
31703243
/// elapsed_seconds : wall-clock time since the request started
31713244
/// Note: `model` is intentionally omitted — clients can correlate from preceding stream chunks.
31723245
/// Note: `on` is accepted as a truthy header value for parity with common reverse proxy conventions.
3246+
/// Issue #168: `choices: []` is present so strict OpenAI chunk validators (opencode's
3247+
/// ChatCompletionChunk union) accept the payload even when they parse every `data:` line
3248+
/// regardless of `event:`. The named event alone was not enough.
31733249
func ssePrefillChunk(nPast: Int = 0, promptTokens: Int, elapsedSeconds: Int) -> String {
31743250
let fraction = promptTokens > 0 ? Double(nPast) / Double(promptTokens) : 0.0
31753251
let chunk: [String: Any] = [
31763252
"status": "processing",
31773253
"n_past": nPast,
31783254
"n_prompt_tokens": promptTokens,
31793255
"fraction": fraction,
3180-
"elapsed_seconds": elapsedSeconds
3256+
"elapsed_seconds": elapsedSeconds,
3257+
"choices": [Any]()
31813258
]
31823259
let data = try! JSONSerialization.data(withJSONObject: chunk)
31833260
return "event: prefill_progress\r\ndata: \(String(data: data, encoding: .utf8)!)\r\n\r\n"
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import XCTest
2+
import Foundation
3+
@testable import SwiftLM
4+
5+
/// Issue #168: JSON `null` (NSNull) in tool schemas used to abort Jinja.Value conversion
6+
/// with a misleading "Optional<Any>" error, mislabeled as a broken chat template.
7+
final class JinjaSanitizerTests: XCTestCase {
8+
9+
func testDropsTopLevelNSNull() {
10+
XCTAssertNil(sanitizeForJinja(NSNull()))
11+
}
12+
13+
func testDropsNestedObjectNulls() throws {
14+
let tool: [String: any Sendable] = [
15+
"type": "function",
16+
"function": [
17+
"name": "probe",
18+
"parameters": [
19+
"type": "object",
20+
"properties": [
21+
"query": [
22+
"type": "string",
23+
"default": NSNull() as any Sendable,
24+
] as [String: any Sendable],
25+
] as [String: any Sendable],
26+
] as [String: any Sendable],
27+
] as [String: any Sendable],
28+
]
29+
30+
let cleaned = try XCTUnwrap(sanitizeForJinja(tool) as? [String: any Sendable])
31+
let fn = try XCTUnwrap(cleaned["function"] as? [String: any Sendable])
32+
let params = try XCTUnwrap(fn["parameters"] as? [String: any Sendable])
33+
let props = try XCTUnwrap(params["properties"] as? [String: any Sendable])
34+
let query = try XCTUnwrap(props["query"] as? [String: any Sendable])
35+
36+
XCTAssertNil(query["default"], "null default must be stripped")
37+
XCTAssertEqual(query["type"] as? String, "string")
38+
XCTAssertEqual(params["type"] as? String, "object")
39+
}
40+
41+
func testDropsNullArrayElements() throws {
42+
let value: [String: any Sendable] = [
43+
"enum": [1, NSNull(), 2] as [any Sendable]
44+
]
45+
let cleaned = try XCTUnwrap(sanitizeForJinja(value) as? [String: any Sendable])
46+
let enumValues = try XCTUnwrap(cleaned["enum"] as? [Any])
47+
XCTAssertEqual(enumValues.count, 2)
48+
XCTAssertFalse(enumValues.contains { $0 is NSNull })
49+
}
50+
51+
func testPreservesScalarsAndStructure() throws {
52+
let value: [String: any Sendable] = [
53+
"type": "string",
54+
"minimum": 0,
55+
"required": ["command"] as [any Sendable],
56+
"flag": true,
57+
]
58+
let cleaned = try XCTUnwrap(sanitizeForJinja(value) as? [String: any Sendable])
59+
XCTAssertEqual(cleaned["type"] as? String, "string")
60+
XCTAssertEqual(cleaned["minimum"] as? Int, 0)
61+
XCTAssertEqual(cleaned["flag"] as? Bool, true)
62+
XCTAssertEqual((cleaned["required"] as? [Any])?.count, 1)
63+
}
64+
65+
func testUnwrapsNestedOptional() throws {
66+
let wrapped: any Sendable = Optional<String>.some("hi")
67+
let cleaned = sanitizeForJinja(wrapped)
68+
XCTAssertEqual(cleaned as? String, "hi")
69+
70+
let empty: any Sendable = Optional<String>.none
71+
XCTAssertNil(sanitizeForJinja(empty))
72+
}
73+
74+
func testMapValuesDeepOnToolDict() throws {
75+
let dict: [String: any Sendable] = [
76+
"keep": "x",
77+
"drop": NSNull(),
78+
]
79+
let cleaned = dict.mapValuesDeep(sanitizeForJinja)
80+
XCTAssertEqual(cleaned["keep"] as? String, "x")
81+
XCTAssertNil(cleaned["drop"])
82+
}
83+
}

‎tests/SwiftLMTests/ServerSSETests.swift‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ final class ServerSSETests: XCTestCase {
4646
XCTAssertEqual(json["n_prompt_tokens"] as? Int, 128)
4747
XCTAssertEqual(json["elapsed_seconds"] as? Int, 4)
4848
XCTAssertNil(json["object"])
49-
XCTAssertNil(json["choices"])
49+
// Issue #168: empty choices keeps strict OpenAI chunk validators (opencode) happy
50+
// even when they validate every data: line regardless of event name.
51+
XCTAssertEqual((json["choices"] as? [Any])?.count, 0)
5052
}
5153

5254
// MARK: - 1b: Zero-token boundary (no divide-by-zero crash)
@@ -93,7 +95,24 @@ final class ServerSSETests: XCTestCase {
9395
XCTAssertNil(json["id"], "prefill chunk must not carry an id field")
9496
XCTAssertNil(json["object"], "prefill chunk must not carry an object field")
9597
XCTAssertNil(json["model"], "prefill chunk must not carry a model field")
96-
XCTAssertNil(json["choices"], "prefill chunk must not carry a choices field")
98+
// Issue #168: choices must be present but empty — opencode's chunk union
99+
// requires `choices` (or `error`); omitting it fails type validation.
100+
XCTAssertEqual((json["choices"] as? [Any])?.count, 0,
101+
"prefill chunk must carry an empty choices array")
102+
}
103+
104+
// MARK: - Issue #168: empty choices is what strict validators require
105+
106+
func testPrefillChunk_ChoicesIsEmptyArrayForStrictValidators() throws {
107+
let chunk = ssePrefillChunk(nPast: 1, promptTokens: 4, elapsedSeconds: 1)
108+
let prefix = "event: prefill_progress\r\ndata: "
109+
let suffix = "\r\n\r\n"
110+
let payload = String(chunk.dropFirst(prefix.count).dropLast(suffix.count))
111+
let data = try XCTUnwrap(payload.data(using: .utf8))
112+
let json = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any])
113+
114+
let choices = try XCTUnwrap(json["choices"] as? [Any], "choices must be present")
115+
XCTAssertTrue(choices.isEmpty)
97116
}
98117

99118
// MARK: - 1e: PrefillState.finish() is idempotent (Issue #2 guard)

‎tests/test-opencode.sh‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,81 @@ else
222222
fail "opencode-shaped request failed: $AGENT_OUT"
223223
fi
224224

225+
# ── Test 3: tools + heartbeat combined (Issue #168) ────────────────
226+
# Test 1 covers heartbeat without tools; Test 2 covers tools without the heartbeat
227+
# header. Issue #168 reported both gaps at once: a tools-bearing request with
228+
# X-SwiftLM-Prefill-Progress enabled failed on the null-bearing tool schema (Jinja
229+
# NSNull conversion) *and* would have hit opencode's strict validation of the
230+
# prefill_progress payload. Exercise the intersection here.
231+
log "Test 3: tools + prefill-progress heartbeat (Issue #168)"
232+
233+
cat << 'PYEOF' > /tmp/opencode_tools_heartbeat_test.py
234+
import json, os, sys
235+
import openai
236+
237+
client = openai.OpenAI(base_url=os.environ["OPENAI_BASE_URL"], api_key="sk-test", max_retries=0)
238+
239+
# A tool schema shaped like opencode's zod/effect output — includes JSON nulls
240+
# (`default: null`) that previously crashed swift-jinja Value.init(any:) with
241+
# "Cannot convert value of type Optional<Any> to Jinja Value" (#168).
242+
TOOLS = [
243+
{"type": "function", "function": {
244+
"name": "bash",
245+
"description": "Execute a shell command",
246+
"parameters": {"type": "object",
247+
"properties": {
248+
"command": {"type": "string"},
249+
"timeout": {"type": "integer", "default": None},
250+
},
251+
"required": ["command"]}}},
252+
]
253+
MESSAGES = [
254+
{"role": "system", "content": "You are a coding agent."},
255+
{"role": "user", "content": "Say hi."},
256+
]
257+
258+
try:
259+
stream = client.chat.completions.create(
260+
model=os.environ["MODEL"], messages=MESSAGES, tools=TOOLS,
261+
stream=True, max_tokens=64, temperature=0,
262+
stream_options={"include_usage": True},
263+
# Enables the named `event: prefill_progress` heartbeat payloads.
264+
extra_headers={"X-SwiftLM-Prefill-Progress": "true"},
265+
)
266+
except Exception as e:
267+
print(f"Error: request rejected: {e}")
268+
sys.exit(1)
269+
270+
chunks = 0
271+
finish = None
272+
try:
273+
for chunk in stream:
274+
chunks += 1
275+
for choice in chunk.choices:
276+
if choice.finish_reason:
277+
finish = choice.finish_reason
278+
except Exception as e:
279+
print(f"Error: SSE stream failed to parse (heartbeat or tools payload rejected): {e}")
280+
sys.exit(1)
281+
282+
if chunks == 0:
283+
print("Error: stream produced no chunks")
284+
sys.exit(1)
285+
286+
print(f"Success: {chunks} chunks, finish_reason={finish}")
287+
PYEOF
288+
289+
set +e
290+
HB_OUT=$("$VENV_DIR/bin/python" /tmp/opencode_tools_heartbeat_test.py 2>&1)
291+
HB_EXIT=$?
292+
set -e
293+
294+
if [ $HB_EXIT -eq 0 ]; then
295+
pass "tools + heartbeat stream accepted — $HB_OUT"
296+
else
297+
fail "tools + heartbeat stream rejected: $HB_OUT"
298+
fi
299+
225300
# ── Results ──────────────────────────────────────────────────────────
226301
echo ""
227302
log "═══════════════════════════════════════"

0 commit comments

Comments
 (0)