Repository navigation
Test coverage plan: the unit suite has never caught a bug #128
Description
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:
| Caught by | Defects |
|---|---|
| Loading a real checkpoint | #118 (OptiQ norm shift), #120 (Gemma 4 KV-shared), the b674 e2b regression, per_layer_model_projection quantization |
| Code review | stop-sequence leak (#126), a delete-guard bypass, CLI cache-root mismatch, listed-but-not-loadable models |
| The unit suite | none |
~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:
| Shape | Representative | Covers |
|---|---|---|
| KV-shared, weights present | gemma-4-e2b-it-4bit |
the b674 regression — landed in #127 |
| KV-shared, weights absent | gemma-4-e4b-it-4bit |
#120 |
| Mixed precision + files outside the weight index | Qwen3.6-27B-OptiQ-4bit |
#118 |
| MoE with nested config | Qwen3.6-35B-A3B-4bit |
#112 |
| Dense baseline | Qwen2.5-0.5B-Instruct-4bit |
— |
Split 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 / content split, 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 test aborts on the second test with Failed 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 documented make test.
Tier 4 — Discipline
- Red-check every new test. This cycle produced a deletion-safety test that never called
delete()and passed against the bug it claimed to cover. - Turn on
--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
- CI-minute budget per PR, which sets the small/nightly split for Tier 1.
- Whether large checkpoints live in a persistent cache or are fetched per run.
Activity
test: make the local suite actually runnable (1 test → 245) (#130)
test: add OpenAI-compatibility contract tests, fix earliest-stop sele…
test: simulate opencode's request instead of running its CLI (#131)
test: exercise chunked prefill with a prompt over the 512-token boundary
feat: make --mtp work for model families that ship MTP heads separate…
test: cover load-path defect shapes with synthetic checkpoints (#143)
solderzzc commentedon Aug 15, 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.
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:
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.
test: add a coverage report that names where the suites are blind (#146)
solderzzc commentedon Aug 29, 2026
Status update — most of this plan is already done:
integration_matrix (contract, ...)job on every PR.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).
fix: wire up Gemma4MTPBench target + repair regressed mlx-swift-lm/ml…
HamedShams commentedon Oct 9, 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:swift test --skip-build --filter SwiftLMTests --disable-swift-testingswift test --skip-build --filter SwiftBuddyTests --disable-swift-testingThe 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.HamedShams commentedon Oct 9, 2026
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.solderzzc commentedon Oct 9, 2026
Follow-up on the local test setup, in #222 (merged).
What I verified on Swift 6.4 / Xcode 27 (M-series Mac):
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.<bundle>.xctest/Contents/MacOSinvalidates the bundle signature, and the next incrementalswift build --build-testsfails at CodeSign (reproduced).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.HamedShams commentedon Oct 9, 2026
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.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.test: stop copying metallib into .xctest bundles; check the Metal too…
solderzzc commentedon Oct 10, 2026
Status summary, as of main @ 562babb.
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).Tier 3, what is and is not established
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..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.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
scripts/bootstrap_local_tests.shfrom a fresh checkout and report back, that would close Tier 3 fully.Once those are decided or split into their own issues, this one can be closed.
solderzzc commentedon Oct 11, 2026
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.What that says
Caching
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.Caveats: failed and cancelled runs are excluded, and queue time is not subtracted.
solderzzc commentedon Oct 11, 2026
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.swift build --build-tests.build/arm64-apple-macosx/layout, the same one CI usesswift testMLX error: Failed to load the default metallibscripts/bootstrap_local_tests.shswift test --skip-buildafterwardsSo 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.