Repository navigation
Sync upstream ml-explore/mlx-swift-lm (5e46681) via real merge - #74
Merged
Merged
Conversation
* pick up mlx-swift 0.32.2
…ter (ml-explore#535) * Name each attached image in the prompt, refuse names that mimic picture markers - Names were thrown away before the prompt was built; a name now goes directly before its own picture, matching Foundation Models, across eleven model families - In MLXLMCommon: an image takes an optional name, and one shared function assembles a message's parts, so this works with no adapter - Name refused when its bracketed form is a reserved picture token, such as [IMG] on Mistral - Two smaller parity fixes: instructions images dropped and logged, and photo orientation applied - Source break: case .url(let u) = image becomes case .url(let u) = image.source * Refuse a picture name that holds a marker character - The name check asked the tokenizer whether a name held a reserved picture token, but a tokenizer flags only some of its reserved tokens as special; on GLM-OCR and FastVLM the picture token itself is not flagged, so such a name was accepted and the model then counted one more picture than it was given - A name that holds any of < > | [ ] is now refused on every model, which needs nothing from the tokenizer and so covers a marker the tokenizer cannot see at all - The tokenizer check still runs, and runs first, so the error can name the exact token; it remains the only check that catches a plain name such as IMG whose bracketed form is reserved - Names accepted before and refused now: chart [q3] (final), a <b> tag & an entity, <|, |> and <image> - New test drives the real tokenizer of all ten families and refuses each family's own picture token; without this change it fails on GLM-OCR and FastVLM * Leave a marker-carrying picture name out of the prompt - Only the Foundation Models path checked a picture name, so a caller using MLXLMCommon directly could put a reserved picture token into the prompt; on Mistral3 that doubled the picture tokens for one picture and the process then aborted instead of raising an error - A name that holds any of < > | [ ] is now left out of the prompt wherever it is assembled, and the reason is logged; the picture itself is always kept, so the count the model checks still matches - The Foundation Models path is unchanged and still raises a typed error, because its check runs before the prompt is assembled - One list of characters now serves both checks, so the two cannot drift apart * Raise an error when image tokens and features disagree - Replaces the fatalError in the LFM2VL and Mistral3 merge step with a thrown VLMError.processing. - Propagates the throw through the embedding helpers, which already run inside a throwing prepare. - Adds a test for each model that feeds one image token against a four-patch grid. Prompt text can hold a model's image placeholder, such as [IMG] on Mistral3. The tokenizer reads it as an image token, so user input can cause the mismatch. A crash of the host process is worse than an error the caller can catch. * Log dropped picture names through MLXLogger - Replaces the Apple-only logger in the message content builder with MLXLogger from mlx-swift. - Writes the marker character into the log message instead of marking it public. The Apple logger does not exist on Linux, so this file could not build there. The built-in MLXLogger handlers print only the message, so the marker must be part of the message text. * Log MLXFoundationModels through MLXLogger - Replaces the Apple logger in three adapter files with MLXLogger from mlx-swift. - Removes the privacy arguments, which MLXLogger does not take. This gives the package one logger. MLXLogger logs through OSLog under the subsystem mlx-swift, and it marks every message public. Two messages that the Apple logger redacted now log in public. They are the name of a rejected tool call and the text of a token-stream protocol error. * Move the special-token label check into MLXLMCommon - Moves the check that encodes a label's bracketed text and looks for special tokens out of the Foundation Models validator and into MLXLMCommon. - Gives the bracketed label text one source, which the message generators and the check both use. - Leaves the validator's behavior unchanged, so it still throws for such a label. The vision processors need the same check in the next change, and MLXVLM does not depend on MLXFoundationModels. * Leave special-token image labels out of VLM prompts - Adds a screen that removes each chat image label whose bracketed text the model's tokenizer reads as a special token, and logs the removal. - Runs the screen in every vision processor, before its message generator writes the prompt. - Keeps each image, so later labels still sit beside their own images. - Adds unit tests with a stub tokenizer, and an integration test that prepares a labeled image through the Mistral3 processor. The label IMG renders as [IMG], which is Mistral's image token. The Mistral3 processor splits the prompt on that text, so one image became two image blocks. A tokenizer check also catches [INST] and the other special tokens, which a fixed list would miss. * Screen image labels when a configured generator writes the prompt - Gives the wrapper that applies a configured message generator the model's tokenizer, through a package initializer. - Runs the special-token label screen in that wrapper, before the configured generator writes the prompt. - Adds a test that routes an IMG label through the wrapper with the Mistral3 generator. When a model configuration sets its own message generator, the wrapper wrote the prompt before the processor ran, so the processor's screen never saw the labels. The public initializer is unchanged. * Keep the attachment label out of the default log - Moves the label in the dropped tool-output attachment warning out of the message text and into the logger's metadata. - Splits that warning into two plain sentences. The default MLXLogger handler writes the message to the system log as readable text, and it ignores metadata. A label can hold user data, such as the name of a photo. An app that installs its own handler still receives the label. * Keep the message content helpers inside the package - Changes the message content layout enum and the content-parts function from public to package access. Only this package's vision message generators and its unit tests call them. A public version could not change later without breaking apps, and outside code could use it to write labels without the special-token check, which is package access too. * Unslop inline comments - Deletes comments that repeat the code, record measurements or review history, name a reviewer, or point at nearby tests. - Deletes comments that contradicted the code, such as the test counts in the vision integration tests, the model lists on the content layout, and the claim that the perform closure does not capture the request. - Rewrites each kept comment in short plain sentences, with the condition stated beside each claim. - Restores the original wording of five older comments that earlier commits on this branch reworded without need. - Changes only comment lines and blank lines. * Make UserInput.Audio a struct with a source, like UserInput.Image - Turns Audio from an enum into a struct that holds its old cases in a nested Source enum. - Adds a static url and a static array function with the old labels, so the code that creates an audio value still compiles. - Makes asMLXArray switch on the source, and keeps its case bodies. - Adds tests that create an audio value in each way that callers use today, and check its source. PR ml-explore#535 made Image a struct with a source. Audio now has the same structure. Code that matches on an audio value no longer compiles, and it must match on its source instead. * Make UserInput.Video a struct with a source, like UserInput.Image - Turns Video from an enum into a struct that holds its old cases in a nested Source enum. - Leaves the avAsset case and its static function behind the AVFoundation check. - Adds a static url and a static frames function with the old labels, so the code that creates a video value still compiles. - Makes the deprecated asAVAsset and MediaProcessing.asProcessedSequence switch on the source, and keeps their case bodies. - Adds tests that create a video value in each way that callers use today, and check its source. - Adds a source check to the image test for a function reference, so that it matches the audio and video tests. Image, Audio and Video now have the same structure. Code that matches on a video value no longer compiles, and it must match on its source instead. * Document the break for all three media types in the upgrade notes - Replaces the upgrade entry for UserInput.Image with one entry for Image, Video and Audio. - Lists the old cases of each type, and shows that the code that creates a media value still compiles. - Keeps the description of the image label. - Shows how to fix a switch or a case pattern that matches a media value.
…l-explore#653) - Converts each JSON boolean in a tool's parameter schema to a Swift Bool before the tool spec reaches the chat template. - Leaves each number in the schema unchanged, even a 0 or a 1. - Adds tests that check the booleans become Swift Bool values and the numbers stay numbers. JSONSerialization boxes each JSON boolean in an NSNumber. swift-jinja's Value(any:) matches Int before Bool, so it converts a boxed false to 0. When a tokenizer passed the schema to swift-jinja unchanged, the prompt showed "additionalProperties": 0. The conversion checks the type ID of each NSNumber. An `as? Bool` cast also accepts a boxed 0 or 1, so that cast would turn "minItems": 1 into true.
* adjustments for running tests on NAX/TF32 hardware * Apply suggestion from @nwh Co-authored-by: Nick Henderson <[email protected]>
* update documentation and skills - make sure we are using current best practices e.g. for #huggingFaceTokenizerLoader() - where found, update skills to current API --------- Co-authored-by: Nick Henderson <[email protected]>
…re#635) NVIDIA-Nemotron-3-Nano-4B is a dense Nemotron-H: its hybrid_override_pattern has no E layers and its config.json has no moe_intermediate_size, moe_shared_expert_intermediate_size, n_routed_experts or num_experts_per_tok. The configuration decoded all four with decode(), so loading it failed with keyNotFound before any weight was read. They are only used to build MoE layers, so decode them with decodeIfPresent and default to 0.
…e model (ml-explore#631) Fix memory leak from unregistered fused GDN projections in Qwen3.5 Because `prepare()` does not register the fused projection as a child of `Qwen35GatedDeltaNet`, traces treated its arrays as tape constants rather than state. Due to ml-explore/mlx#3932, these constants remained resident after function erasure, leaking ~2.3 GB per reload on 27B. Traces running GDN layers now explicitly declare the fused module as compile state. Verified with `testCompiledDecodeReleasesFusedProjectionWithTheModel`.
…ft-lm Real merge (--no-ff), no cherry-picks. Brings in upstream UserInput Audio/Video/Image structs with Source enums, contentParts(for:layout:), removingSpecialTokenLabels, MatmulPrecision.splitTolerance, the WHT barrier fix (ml-explore#650), fused GDN projection compile state (ml-explore#631), MTP weights dropped from converted Qwen3.5 VLM checkpoints (ml-explore#642), NemotronH optional MoE keys (ml-explore#635), Metal sync before cached buffer memory (ml-explore#667), reranker changes (ml-explore#621/ml-explore#651) and the mlx-swift 0.32.3 requirement (ml-explore#657). Conflict resolutions (fork behavior kept on purpose): - Package.swift: keep the local path dependency on ../mlx-swift. - UserInput.Audio: upstream's struct + Source enum, plus the fork-only .data(Data, format:) source; UserInput+Audio.swift handles it. - Qwen2VL / Gemma4 message generators: upstream contentParts(for:layout:) plus the fork-only audio placeholders; Gemma4 keeps its plain-string early return for non-user messages without media. - Qwen35 (VLM) sanitize: keep the MTPConfig.retainMTPWeights guard. - TurboQuantKernels: upstream's WHT barrier fix. - ContinuationAssertions: noise-floor tolerances combined with MatmulPrecision.splitTolerance. - KVCacheTests: 3-way merged after wrapping the fork side in @suite(.serialized) like upstream; upstream's eval(arrays) fixes kept. The fork-only `extension MLXTestingSuite { ... }` block stays at file scope after the wrapped struct. - MediaProcessing.extractAudioSamples (fork-only): switch on `audio.source` now that UserInput.Audio is a struct. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…e smoke testVarianceNormalizedKVCache32KIngestAndDecodePeakMetalMemorySmoke timed the first decode step against a 0.25 s bound. That step is the first place the raw-tail attention (fp16 gemv / gemv_t batched matmul) runs, because the 32K ingest ends on a tile boundary. Since mlx v0.32 those gemv kernels are JIT-compiled from source, so on a cold Metal shader cache (a fresh CI VM) the first call paid ~0.17 s per kernel library: 0.30 s (Release) and 0.38 s (Debug) on PR #74, against 0.05-0.09 s before the mlx-swift sync. Steady-state decode is unchanged (~2-4 ms on both stacks), and this test, VarianceNormalizedKVCache.swift and KVCache.swift are byte-identical across the upstream merge; lm main on the new mlx-swift shows the same slow first call. Add an untimed warm-up on a small throwaway cache with the same dtype/head layout so the timed region measures per-layer decode. The 0.25 s bound is unchanged. Cold-cache emulation (forced shader-cache miss): 0.125-0.167 s without the warm-up, ~0.004 s with it. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
… fix branch downstream_integration.yml still pointed at fix/mlx-swift-lm-throwing-newcache, a TEMPORARY ref for SwiftLM#167 that was merged long ago. The branch is 92 commits behind SwiftLM main, so the umbrella build tested an outdated SwiftLM. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
This was referenced Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Syncs the fork with upstream
ml-explore/mlx-swift-lm(5e46681) as a real merge (--no-ff, no cherry-picks). Replaces the bot branchsync/upstream-latest, which cannot be merged cleanly. Depends on the already-merged mlx-swift sync (SharpAI/mlx-swift#20, mlx v0.32.2 / mlx-cebc88f1).What comes in from upstream
UserInput.Audio/Video/Imagebecome structs withSourceenums;contentParts(for:layout:)andremovingSpecialTokenLabels(using:)for labeled images.MatmulPrecision.splitTolerancefor the continuation tests (IEEE vs TF32/NAX).Conflicts (9) and what I kept from the fork
Package.swift: keep the local../mlx-swiftpath dependency.UserInput.Audio: upstream's struct +Sourceenum, plus the fork-only.data(Data, format:)source (Server.swiftin SwiftLM builds audio from base64 data).UserInput+Audio.swiftand the fork-onlyMediaProcessing.extractAudioSamplesswitch onsource.contentParts(for:layout:)plus the fork-only audio placeholders. Gemma4 keeps its plain-string early return for non-user messages without media.sanitize: keep theMTPConfig.retainMTPWeightsguard.TurboQuantKernels.swift: upstream's WHT barrier fix.ContinuationAssertions: the fork's measured noise-floor tolerance combined withMatmulPrecision.splitTolerance.KVCacheTests: 3-way merged after wrapping the fork side in@Suite(.serialized) struct KVCacheTestslike upstream. The fork-onlyextension MLXTestingSuiteblock stays at file scope. Upstream independently added the sameeval(arrays)fixes to the six tests that save lazily loaded arrays back to the same file (needed with mlx v0.32).Checked for silent reverts
For all 87 files upstream changed, I reverse-applied upstream's delta to the merged tree: 79 files contain it completely; the other 8 are the hand-resolved files above, and their remaining differences from upstream are the fork behavior listed here.
Tests
xcodebuild test -scheme mlx-swift-lm-Package -destination 'platform=macOS' -skipPackagePluginValidation -skipMacroValidation -parallel-testing-enabled NO, againstSharpAI/mlx-swiftmain(34bc52f):Qwen35FusedGDNProjection,MatmulPrecision,UserInput{Audio,Image,Video},MessageContentParts,LabeledImageGenerator,SpecialTokenLabelScreen,NemotronH,Rerankertests).Fork()skips.pre-commit, the integration tests that need downloaded models.Checklist
pre-commit run --all-filesto format my code / installed pre-commit prior to committing changesAI usage
accurately describes the code changes.
🤖 Generated with Claude Code