fix(tests): resolve checkpoints under the current store roots - #1722
Merged
Merged
Conversation
`repo_model_dir` looked only under `models/<name>`, and every caller treats a missing directory as "skip this test". The store was later consolidated into `models/mlx/`, with checkpoints over 120GB in `models/mlx-big/`, so the lookup stopped resolving and the tests went silently green instead of failing. On this M1 Ultra tree none of the 30 distinct names used across `tests/` resolved under `models/<name>`, so every real-model integration test in the repository was a no-op. `tests/mamba2_hybrid_finite.rs` was one of them: the NaN guard written in #1718 ran in 0.00s and asserted nothing. With the roots fixed it runs 250 forwards per checkpoint in 10.64s. `MLXCEL_REQUIRE_MODELS=1` turns a name that resolves nowhere into a panic naming every root tried, so a local gate run cannot pass by skipping everything. CI has no checkpoints and leaves it unset, keeping the skip there. Fifteen of the 30 names now resolve. The other 15 are renames from the same consolidation (`llama-3.1-8b-4bit` is `meta-llama-3.1-8b-instruct-4bit`, `qwen2.5-7b-4bit` is `qwen2.5-7b-instruct-4bit`, `gemma3-1b-4bit` is `gemma-3-1b-it-4bit`) or genuinely gone, and mapping them needs per-name judgment: `jamba-v0.1-4bit` has no successor in the store and the nearest name is a different model. Left as a follow-up rather than guessed, since a wrong mapping points a parity test at the wrong checkpoint, which is worse than skipping.
inureyes
added a commit
that referenced
this pull request
Sep 9, 2026
…1726) #1722 made a name that resolves nowhere loud instead of silent. Running with `MLXCEL_REQUIRE_MODELS=1` afterwards, 20 of the 46 distinct checkpoint names under `tests/` resolved to nothing on M5 Max, so those tests had been skipping while reporting `ok`. Fifteen are fixed here, in 55 places. Ten came from the rename pass and are looked up through the catalog's `aliases` column, so each replacement is backed by a recorded former name rather than by a guess: `llama-3.1-8b-4bit` to `meta-llama-3.1-8b-instruct-4bit`, `qwen2.5-7b-4bit` to `qwen2.5-7b-instruct-4bit`, and so on. Five more differed only in case or in a suffix the directory carries, `GLM-4.5V` against `glm-4.5v-4bit` among them, and one had a stale `mlx/` prefix from the old two-level layout. `jamba-v0.1-4bit` is deliberately not remapped, and its alias is removed from the catalog. The directory that carried that name held `AI21-Jamba-Reasoning-3B-4bit`, a 3B 28-layer model rather than Jamba v0.1, so renaming it was correct and recording the old name as an alias was not: an alias means one set of weights had another name, not that a directory was misnamed after a different model. Following it would have pointed a parity test at the wrong architecture. `docs/model-catalog.md` now says that. The remaining five are absent checkpoints rather than wrong names and cannot be fixed by editing a string: `glm5-4bit` was an aborted download deleted on 2026-09-09, `llama-3.2-1b-4bit-lora` is an adapter directory, and `mistral-7b-instruct-v0.3-4bit`, `nvidia-nemotron-h-8b-base-8bit` and the jamba entry are not in this store. Gate: 10539 tests pass with no failures, up from 10534, the difference being tests that now run.
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.
What was wrong
tests/common/mod.rs::repo_model_dirresolved a checkpoint only undermodels/<name>. Every one of its 164 call sites treats a missing directory as "skip this test", so pointing at the wrong root does not fail: it turns the test into a no-op that still reportsok.The store was consolidated into
models/mlx/(with checkpoints over 120GB inmodels/mlx-big/) and the lookup was never moved with it. Measured on the M1 Ultra tree: of the 30 distinct model names used acrosstests/, zero resolve undermodels/<name>. Every real-model integration test in the repository is currently skipping while the suite stays green.tests/mamba2_hybrid_finite.rsis the case that surfaced this. It is the NaN guard written in #1718, the one whose whole point was that the safety claim ingranitemoehybrid.rsandfalcon_h1.rsrested on a test that did not exist. It exists now, and it was skipping:finished in 0.00s, two tests reportedok, 500 forwards never run.What changed
repo_model_dirsearchesmodels/,models/mlx/,models/mlx-big/, and the twomlxcel-internalequivalents, first hit wins. On a miss it returns the same non-existent path as before, so the callers' skip behavior is unchanged.MLXCEL_REQUIRE_MODELS=1turns a miss into a panic that names every root tried. That is the part that stops this recurring: a local gate run can no longer pass by skipping everything, and the next store move fails loudly instead of quietly. CI has no checkpoints and leaves the variable unset.Validation
Before:
cargo test --test mamba2_hybrid_finitefinished in 0.00s, 2 passed. After: 10.64s, 2 passed, 250 forwards per checkpoint actually run. That pair is the evidence the change does something.The flag was falsified with a throwaway probe calling
repo_model_dir("this-checkpoint-does-not-exist-4bit"): unset it returnsmodels/this-checkpoint-does-not-exist-4bitwithexists=falseand the test passes; set to 1 it fails withresolves nowhere. Tried: <five paths>.cargo clippy -p mlxcel --all-targetsandcargo fmt --all -- --checkare clean. Workspace clippy is red on main for two unrelated lints inmlxcel-core, fixed in #1720.Known remainder
15 of the 30 names now resolve. The other 15 are renames from the same consolidation (
llama-3.1-8b-4bitis nowmeta-llama-3.1-8b-instruct-4bit,qwen2.5-7b-4bitisqwen2.5-7b-instruct-4bit,gemma3-1b-4bitisgemma-3-1b-it-4bit) or genuinely gone. Mapping them needs per-name judgment rather than a sed:jamba-v0.1-4bithas no successor in the store, and the nearest name,ai21-jamba-reasoning-3b-4bit, is a different model. Pointing a parity test at the wrong checkpoint is worse than skipping it, so those are left for a follow-up pass withMLXCEL_REQUIRE_MODELS=1as the checklist.