Repository navigation
Test coverage plan: the unit suite has never caught a bug #128
Description
Activity
- added 6 commits that reference this issue
on Aug 8, 2026 Tier 4 done — coverage measured rather than estimated. Tooling in #146 (
scripts/coverage-report.sh).Baseline, both suites. Reading either alone misleads: from the umbrella package the model architectures look like 0%, which is an artifact of their tests living in the submodule.
suite area lines coverage SwiftLM package server 5,602 24.4% inference core 3,113 57.7% SwiftBuddy app 18,701 5.3% mlx-swift-lm model architectures (LLM) 17,126 24.1% model architectures (VLM) 13,304 0.3% MLXLMCommon 11,076 41.3% The example this issue named is confirmed, and it is the worst single case.
MLXVLM/Models/Gemma4.swift— where SharpAI/mlx-swift-lm#45 landed — has no tests, and at 1,771 lines it is the largest untested file in the model layer.The pattern around it matters more than the one file. The entire VLM half of the model layer is unexecuted by unit tests: Qwen3VL, Qwen35, LFM2VL, GlmOcr, Qwen25VL, Gemma3, Pixtral, FastVLM all at zero. CI runs vision and omni end-to-end, so these paths do get exercised — but a shape or dtype error inside one surfaces as a wrong answer about an image rather than a failing assertion. That is the same failure shape this issue was opened about: real defects reaching users through code the suite never runs.
Where that leaves the tiers:
Tier Status 1 — Architecture shapes done — #118, #120, b674, #112 each covered by a synthetic fixture, each red-green verified against the code that shipped the bug (#143, #144, #145) 2 — API contract tests done — 10 assertions incl. the chunked-prefill case 3 — Local suite runnable done 4 — Discipline coverage measurable as of #146; red-checking every new test has been practice throughout On the open question this issue raised — "CI-minute budget per PR, which sets the small/nightly split for Tier 1" — that constraint dissolved rather than being answered. The fixtures are 1.5 MB total and the job runs in ~20 s with no downloads, against a 10 GB per-repository cache budget that a single real checkpoint (gemma-4-e2b, 3.6 GB) would have dominated. No split is needed.
What I would do next, if anything: the VLM unit gap is now the largest measured hole, and it is addressable the same way Tier 1 was — a synthetic VLM fixture exercising the vision tower's shape handling would cost kilobytes. Whether that is worth it depends on whether VLM defects are actually reaching users; the ones this cycle produced were all LLM-side.
- added a commit that references this issue
on Aug 15, 2026 Status update — most of this plan is already done:
- Tier 1 (architecture smoke matrix) — done. 5 synthetic checkpoint fixtures (dense, kv-shared-absent, kv-shared-present, moe-nested, stray-shard) cover the shapes in the table above.
- Tier 2 (API contract tests) — done, running in CI as the
integration_matrix (contract, ...)job on every PR. - Tier 4's example (
MLXVLM/Models/Gemma4.swifthad zero tests) — fixed, along with 5 other VLM architectures (Qwen3VL, Qwen35, LFM2VL, GlmOcr, Qwen25VL).
Still open: Tier 3 (make the local suite runnable without CI's help). A bare
swift testin a fresh checkout still aborts withFailed to load the default metallib— CI has a dedicated install step, but locally you have to manuallycmake+make mlx-metalliband copy the.metallibinto the built.xctestbundle'sContents/MacOS/(not just.build/.../debug/). I hit this myself repeatedly working on other PRs this session. Planning to add a bootstrap script/target to fix this next.Leaving this open, scoped down to Tier 3 + the two open questions (CI-minute budget, checkpoint caching strategy).
- added a commit that references this issue
on Aug 29, 2026 Fresh-clone data point for Tier 3, since the last status here predates Xcode 27. M1 Max 64 GB, macOS 27.0, Xcode 27.0 (Swift 6.4), main @ 1b6befd.
build.sh. Fails on a fresh machine at the metallib step:
cannot execute tool 'metal' due to missing Metal Toolchain. Xcode 26+ ships the Metal toolchain as a separate download, and the script's own hint (xcodebuild -downloadComponent MetalToolchain) is the right fix. After that,./build.shtakes about 5 minutes end to end and the binary loads a model straight from~/.cache/huggingface/hubwithout re-downloading.Unit suites.
swift build --build-teststakes about 4.5 minutes, then both suites run as inci.yml:suite result wall time swift test --skip-build --filter SwiftLMTests --disable-swift-testing276 passed, 0 failed 6 s swift test --skip-build --filter SwiftBuddyTests --disable-swift-testing127 run, 10 skipped, 0 failed 15 s The metallib step no longer seems necessary. SwiftPM compiles the Cmlx kernels itself and puts
default.metallib(2.8 MB) intomlx-swift_Cmlx.bundle/Contents/Resources/inside each.xctest. To check that this is what the tests actually use, I removed themlx.metallib/default.metallibcopies fromSwiftLMTests.xctest/Contents/MacOSand re-ran the full suite: still 276/276. So with the Metal toolchain installed, a bareswift build --build-tests && swift test --skip-buildis enough here. I did not test whatswift builddoes without the Metal toolchain, becausebuild.shhad already failed earlier in the cmake step on that machine state.Two script details, in case the helpers stay around.
scripts/install-test-metallib.shsearches.build/*/release/mlx.metallib, but Swift 6.4 writes.build/out/Products/Release/mlx.metallib(.build/releaseis now a symlink), so it never finds the onebuild.shjust built and falls through to pip-installing themlxwheel.scripts/bootstrap_local_tests.shalways takes the pip route. If the bundled metallib is enough, both could reduce to the Metal toolchain check, or the search pattern could gain theout/Products/Releasepath.One more data point on the same setup, found right after posting: the manual step from the Aug 29 comment (copy
mlx.metallibintoSwiftLMTests.xctest/Contents/MacOS/) is now actively harmful on Xcode 27. The suite runs once, but the next incrementalswift build --build-testsfails at theCodeSign SwiftLMTests.xcteststep and leaves the bundle unsigned, soswift test --skip-buildthen dies withcode signature … not valid for use in process: Trying to load an unsigned library. Deleting the two copied files and rebuilding fixes it (codesign --verifypasses again, 280 tests green). So with the Metal toolchain installed, the bundleddefault.metallibis both sufficient and the only copy that should be there.Follow-up on the local test setup, in #222 (merged).
What I verified on Swift 6.4 / Xcode 27 (M-series Mac):
- SwiftPM compiles the Cmlx kernels and embeds
default.metallibinmlx-swift_Cmlx.bundleinside each.xctest, soscripts/install-test-metallib.sh/bootstrap_local_tests.shdo not need to copy or pip-install a metallib there. - Copying a metallib into
<bundle>.xctest/Contents/MacOSinvalidates the bundle signature, and the next incrementalswift build --build-testsfails at CodeSign (reproduced). - I could not reproduce the "unsigned library" abort from a bare
swift test.
What I did not verify: Swift 6.2 / Xcode 26 (the CI runner) uses the older
.build/arm64-apple-macosx/layout and does not embed the metallib, so the copy is still needed there. My first version of the PR removed it from CI and broke the run; that was corrected before merge.So the scripts now detect the Swift 6.4
.build/outlayout and only run a newscripts/check-metal-toolchain.sh(a clear error pointing toxcodebuild -downloadComponent MetalToolchainwhen the Metal Toolchain is missing); older toolchains keep the previous behaviour. The old-layout path is covered by CI only.- SwiftPM compiles the Cmlx kernels and embeds
Tested and confirmed on my machine (the same M1 Max / Xcode 27) with #222:
scripts/install-test-metallib.shexits with the "nothing to install" msg,scripts/bootstrap_local_tests.shbuilds and hands off to the toolchain check,swift test --skip-buildis green (280 on my #220 branch, i.e. main’s 276 plus its 4 new tests), and the bundle’sContents/MacOSholds only the binary.I could not reproduce the "unsigned library" abort from a bare
swift test.One clarification: it never came from a bare
swift testhere either, only after the manual copy, when the next incremental build failed at CodeSign and left the bundle unsigned. So nothing left to reproduce.- added a commit that references this issue
on Oct 10, 2026 Status summary, as of main @ 562babb.
Tier Status 1 — Architecture smoke matrix Done (5 synthetic checkpoint fixtures, per the Aug 29 update). 2 — API contract tests Done, running as integration_matrix (contract, ...)on every PR.tests/test-contract.shhas since grown with #220 (seeded sampling is reproducible, a negativeseeddoes not kill the server) and #223 (stopas a plain string, a non-decoding body answers 400).3 — Make the local suite runnable Done on Swift 6.4 / Xcode 27 (#222). Not verified on Swift 6.2 (see below). 4 — Discipline Coverage tooling landed (#146). Gemma4.swift and 5 other VLM architectures now have tests (Aug 29 update). The other VLM architectures listed in the baseline table (Gemma3, Pixtral, FastVLM) were not part of that update and have not been re-measured. Tier 3, what is and is not established
- On Swift 6.4, SwiftPM embeds
default.metallibin each.xctest, so no copy is needed. This was verified independently on two machines (thanks @HamedShams for the fresh-clone run). Copying a metallib intoContents/MacOSbreaks the next incremental build at CodeSign, soscripts/install-test-metallib.shandscripts/bootstrap_local_tests.shnow skip it on that layout and runscripts/check-metal-toolchain.shinstead, which points toxcodebuild -downloadComponent MetalToolchainwhen the Metal Toolchain is missing. - On Swift 6.2 / Xcode 26 (the CI runner,
.build/arm64-apple-macosx/layout) the metallib is not embedded, so the scripts andci.ymlkeep the old copy there. That path is covered by CI only; we have no Swift 6.2 toolchain locally. - The original symptom (a bare
swift testaborting on the second test) was not reproduced on Swift 6.4. The "unsigned library" abort only appears after the manual copy.
Still open
- Whether Swift 6.2 / Xcode 26 users still hit the original abort. If someone on that toolchain can run
scripts/bootstrap_local_tests.shfrom a fresh checkout and report back, that would close Tier 3 fully. - The two open questions from the issue: the CI-minute budget per PR (which sets the small/nightly split for Tier 1), and whether large checkpoints live in a persistent cache or are fetched per run.
Once those are decided or split into their own issues, this one can be closed.
Reacted by Hamed Shams- On Swift 6.4, SwiftPM embeds
Data for the two open questions (CI-minute budget, checkpoint caching), from the last 24 successful
ci.ymlruns.Job durations (minutes, median; range in brackets)
build_and_unit_test: 27.3 [14.3–33.8]. Every other jobneedsit.dflash-speculative-decoding: 16.3 [13.2–21.6];speculative-decoding: 15.6 [10.1–23.1];speculative-decoding-eval: 15.4 [9.8–25.1]integration_matrixentries: 1.1–4.2 each (omni, opencode, audio, server ≈ 4; vision, hybrid-cache, contract, fixtures, graph ≈ 1–2).ssd-draft-memory-guard: 2.1.- Sum of medians ≈ 101 job-minutes; a fully green run takes about 50 minutes wall-clock (max 67), roughly the 27-minute build plus the longest parallel job.
What that says
- The Tier 1 / Tier 2 matrix is the cheap part: about 29 minutes in total across 11 entries. The cost is the build and the three speculative/dflash jobs.
- The repository is public, so these are not billed minutes; the constraint is wait time.
- Suggested split: keep the current matrix on every PR. If we add the 19–20 GB shapes (Qwen3.6-27B-OptiQ, 35B-A3B), run them nightly or only on submodule-bump PRs, as the issue proposed, rather than on every PR.
Caching
- Models are cached per job via
actions/cacheon~/.cache/huggingface, keyed on the test script's hash. There are currently 9 entries, about 21.5 GB (largest: opencode and audio at 4.1 GB each).ci.ymlassumes a 10 GB per-repository cache budget; I did not check why the usage reported here is above that, so treat both numbers as unverified. - That suggests large checkpoints should not go through the Actions cache. Fetching them per nightly run (pinned to a revision, as the LFM2.5 entry already does) is simpler than a persistent cache.
Caveats: failed and cancelled runs are excluded, and queue time is not subtracted.
Tier 3 on Swift 6.2: verified.
Setup: a fresh clone of
main(7e219b4) with submodules, built with Xcode 26.3 (Apple Swift 6.2.4,swiftlang-6.2.4.1.4) on macOS 27, selected throughDEVELOPER_DIRonly, so the default toolchain was untouched.Step Result swift build --build-testssucceeds (234 s), .build/arm64-apple-macosx/layout, the same one CI usesbare swift testaborts on the second test: MLX error: Failed to load the default metallibscripts/bootstrap_local_tests.shsucceeds swift test --skip-buildafterwards435 tests, 9 skipped, 0 failures So on the pre-6.4 layout the original symptom from this issue reproduces, and the bootstrap script resolves it. Together with the Swift 6.4 results above, both toolchains are now covered by a manual run and not only by CI. Tier 3 can be considered closed.
Also: #226 (
--thinking-budget) and the contract tests added since are in; #228 adds contract coverage forfinish_reasonand stop-sequence token accounting across the streaming and non-streaming paths (it found and fixes a non-streaming path that kept generating past a stop sequence).What remains open on this issue is the two decisions: the CI-minute budget per PR, and whether large checkpoints live in a persistent cache or are fetched per run.
Reacted by Hamed Shams
Tracking issue for test-coverage work, derived from the nine defects found during the #108/#110/#112 fix cycle rather than from a generic coverage target.
The observation
Sorting those nine defects by what actually caught them:
per_layer_model_projectionquantization~250 unit tests exist and every one of them was written after its bug was already known. They lock in regressions, which is worth having, but they have never surfaced a defect. The defects lived in two places the suite does not reach: weight-and-config-shape assumptions, which only a real checkpoint exercises, and control flow, which only a reader caught.
Tier 1 — Architecture smoke matrix
Load + generate one token per checkpoint shape, not per model name. Shapes, and the defect each covers:
gemma-4-e2b-it-4bitgemma-4-e4b-it-4bitQwen3.6-27B-OptiQ-4bitQwen3.6-35B-A3B-4bitQwen2.5-0.5B-Instruct-4bitSplit by size so the cost is bounded: small shapes on every PR, the 19–20 GB ones nightly or only on submodule-bump PRs.
Tier 2 — API contract tests
One tiny model, asserting server behaviour end to end: stop sequences (including one split across token chunks), streaming deltas, the
reasoning_content/contentsplit, the two-round tool-call flow, JSON mode. This is the class review caught and the suite cannot see, and #126 is currently open with no scaffolding to verify a fix against.Tier 3 — Make the local suite runnable
A bare
swift testaborts on the second test withFailed to load the default metallib— locally about 1 of 250 tests runs. CI is unaffected because it installs the metallib explicitly (ci.yml, "Install MLX Metal library"), so this is a contributor trap: a local run looks green while testing almost nothing. Needs a test bootstrap step or a documentedmake test.Tier 4 — Discipline
delete()and passed against the bug it claimed to cover.--enable-code-coverageto find blind spots objectively. Known example:MLXVLM/Models/Gemma4.swifthas zero tests despite being where fix: Gemma 4 review follow-ups — assistant-case parity and KV-shared tests mlx-swift-lm#45's changes landed.Open questions