Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates SHLLM’s tokenizer integration and adds coverage/support scaffolding for the Qwen3.5 4B (MLX 4-bit) model variant.
Changes:
- Update dependencies (mlx-swift-lm → 0.0.14, replace swift-transformers with swift-tokenizers) and introduce a
TokenizerLoaderadapter to bridge upstreamTokenizersintoMLXLMCommon.Tokenizer. - Add Qwen3.5-4B model URL helper + a new end-to-end test suite for streaming/awaiting, tool-calls, and basic VLM image extraction.
- Adjust truncation/tokenizer test utilities and truncation decoding to use the updated tokenizer API (
decode(tokenIds:)).
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/SHLLMTests/TruncatingUserInputProcessorTests.swift | Updates the test tokenizer to conform to MLXLMCommon.Tokenizer and aligns chat templating types. |
| Tests/SHLLMTests/Models/Qwen3_5-4BTests.swift | Adds a full test suite for Qwen3.5-4B (text + tools + image extraction). |
| Sources/SHLLM/TruncatingUserInputProcessor.swift | Switches decode call site to decode(tokenIds:) to match the new tokenizer protocol. |
| Sources/SHLLM/TokenizersLoader.swift | Adds adapter TokenizerLoader and _Tokenizer wrapper around Tokenizers.AutoTokenizer. |
| Sources/SHLLM/SHLLM.swift | Re-exports ToolSpec from MLXLMCommon instead of Tokenizers. |
| Sources/SHLLM/ModelCache.swift | Loads model context using loadModel(from:using:) with the new TokenizerLoader. |
| Sources/SHLLM/LLM.swift | Adds qwen3_5__4B directory accessor; reorganizes the SmolLM extension placement. |
| Package.swift | Bumps mlx-swift-lm version and switches dependency to swift-tokenizers; updates target products. |
| Package.resolved | Updates resolved dependency graph accordingly. |
| bin/download.sh | Adds the Qwen3.5-4B-MLX-4bit model ID to the download list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func applyChatTemplate( | ||
| messages: [[String: any Sendable]], | ||
| tools: [[String: any Sendable]]?, | ||
| additionalContext: [String: any Sendable]? | ||
| ) throws -> [Int] { | ||
| do { | ||
| return try upstream.applyChatTemplate( | ||
| messages: messages, | ||
| tools: tools, | ||
| additionalContext: additionalContext | ||
| ) | ||
| } catch let error as Tokenizers.TokenizerError | ||
| where error == .missingChatTemplate | ||
| { | ||
| throw MLXLMCommon.TokenizerError.missingChatTemplate | ||
| } | ||
| } |
There was a problem hiding this comment.
applyChatTemplate only translates .missingChatTemplate into MLXLMCommon.TokenizerError, but any other Tokenizers.TokenizerError (or other upstream errors) will currently escape as a Tokenizers-module error. That leaks the Tokenizers dependency through SHLLM’s public error surface; consider catching/wrapping all Tokenizers.TokenizerError cases into MLXLMCommon.TokenizerError (or a SHLLM-defined error) to keep the API consistent.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot Are you working on a fix for this?
There was a problem hiding this comment.
Fixed in 177f0fe. applyChatTemplate now catches all Tokenizers.TokenizerError cases: .missingChatTemplate maps to MLXLMCommon.TokenizerError.missingChatTemplate, and all other cases are wrapped as NSError to prevent the Tokenizers type from leaking through SHLLM's public error surface.
| import Foundation | ||
| import MLXLMCommon | ||
| import Tokenizers | ||
|
|
||
| struct TokenizerLoader: MLXLMCommon.TokenizerLoader { | ||
| func load(from directory: URL) async throws -> any MLXLMCommon.Tokenizer { | ||
| let upstream = try await AutoTokenizer.from(directory: directory) | ||
| return _Tokenizer(upstream) | ||
| } | ||
| } |
There was a problem hiding this comment.
File name TokenizersLoader.swift doesn’t match the primary type it defines (TokenizerLoader). Renaming the file to TokenizerLoader.swift would make it easier to locate via search and keep file/type naming consistent with the rest of Sources/SHLLM (where filenames generally match their main type).
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Agent-Logs-Url: https://github.com/shareup/shllm/sessions/4e85f0d2-349b-4dd8-8452-5231b169b0f0 Co-authored-by: atdrendel <202402+atdrendel@users.noreply.github.com>
Agent-Logs-Url: https://github.com/shareup/shllm/sessions/4ddd1be8-a557-4d98-a213-73bbc4f6a933 Co-authored-by: atdrendel <202402+atdrendel@users.noreply.github.com>
atdrendel
left a comment
atdrendel
left a comment
There was a problem hiding this comment.
Works well in testing.
No description provided.