Fix null handling in tool schemas for chat templates - #33
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The core change alters a shared chat-templating path used for all tool schemas with subtle Swift Optional/existential semantics whose downstream behavior cannot be verified here, and the added tests are on-device model tests that do not run in CI.
Pull request overview
This PR fixes rendering failures that occur when tool JSON schemas contain null values (e.g., nullable types like ["string", "null"] and enums such as ["person", "organization", null]). In this codebase such nulls are represented as NSNull() inside the any Sendable tool dictionaries. The core change makes _Tokenizer.applyChatTemplate recursively convert NSNull to Swift nil before delegating the tool schemas to the upstream tokenizer, so the chat-template engine can render them. The remainder of the PR adds a shared test-input helper and a matching per-model test across every model suite.
Changes:
- Add a recursive
replacingNullshelper inTokenizerLoader.swiftand apply it totoolsbefore callingupstream.applyChatTemplate. - Add a
nullableToolSearchInput(enableThinking:)helper inHelpers.swiftthat builds a multi-turn tool-search conversation with a nullablecontact_updateschema. - Add
canContinueAfterToolSearchRevealsNullableSchematests to 19 model test files, following each file's existing conventions (e.g., Gemma files guard onSHLLM.isSupportedDevice, Qwen/others rely on theloadModelearly-return).
File summaries
| File | Description |
|---|---|
| Sources/SHLLM/TokenizerLoader.swift | Core fix: recursively replace NSNull with nil in tool schemas before chat templating |
| Tests/SHLLMTests/Helpers.swift | New nullableToolSearchInput helper providing a nullable tool schema and multi-turn messages |
| Tests/SHLLMTests/Models/Qwen3-*Tests.swift, Qwen3_5-*Tests.swift, Qwen2_5-*Tests.swift | New nullable-schema tool test per Qwen suite |
| Tests/SHLLMTests/Models/Gemma4-*Tests.swift | New nullable-schema tool test with isSupportedDevice guard and enable_thinking: true |
| Tests/SHLLMTests/Models/Orchestrator-8BTests.swift, NemotronNano-30BTests.swift, Ministral-3-14BTests.swift, LFM2-8B-A1BTests.swift, GPTOSS-20BTests.swift, Devstral2Small-24BTests.swift | New nullable-schema tool test per model suite |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
No description provided.