chore(deps): bump vite-plus to v0.2.2#11
Conversation
…n_embedder bf16 (mlx-node#76) ## What Enable `mlx convert` to quantize **`gemma4_unified`** (Gemma 4 12B — encoder-free text+vision+audio) checkpoints. The runtime loader already supports `gemma4_unified` (PR mlx-node#74); only the **converter** didn't recognize it, and the default quant predicate would have corrupted one vision weight. Motivating model: `google/gemma-4-12B-it-qat-q4_0-unquantized` (dense 12B, bf16, QAT-trained for llama.cpp `q4_0`). The best quality+speed target is **4-bit affine group_size 32** — near-bf16 quality at 4-bit memory/speed. ``` mlx convert <ckpt> -o <out> --quantize --q-mode affine --q-bits 4 --q-group-size 32 ``` ## Changes 1. **Recognize `gemma4_unified`** — `Gemma4Recipe::model_types` / `recipe_for` / `CONVERTIBLE_MODEL_TYPES` route it to the existing shared `Gemma4Recipe` (it is dense — no MoE split, sym8-supported, no MTP — so it behaves identically to `gemma4` on every `recipe_for`-keyed path). 2. **Keep `vision_embedder.*` bf16** — `should_quantize()` excludes any key containing `vision_embedder`. The encoder-free unified vision patch projection is installed **dense-bf16-only** by the loader (`apply_unified_vision_embedder_weights`, no `.scales` branch), so quantizing `patch_dense.weight` would corrupt the vision path. `embed_vision`/`embed_audio` projections **still quantize** (they have affine loader branches). `vision_embedder` is unified-only → zero collateral on other families. 3. **Sanitize keeps `vision_embedder.*` bare** — sibling of `embed_vision.*`, not mis-prefixed under `language_model.model.`. 4. **CLI passes `gemma4_unified` through unchanged** — *not* collapsed to `'gemma4'`. The driver's `model_type` is the CLI value, and the gemma-QAT (wNa8o8) prequantized importer gate is exact `Some("gemma4")`; collapsing would (a) dead-code the native `recipe_for("gemma4_unified")` arm and (b) misroute a gemma-QAT unified checkpoint into the E2B-only importer. ## End-to-end validation Converted the real 23GB checkpoint → **8.4GB** `{bits:4, group_size:32, mode:affine}`: - auto-detected `gemma4_unified` (passthrough confirmed) - `vision_embedder.*` → **bf16, no `.scales`** ✓ · text / `embed_vision` / `embed_audio` → quantized (U32 + scales) ✓ - model **loads and generates coherent text** (e.g. *"The capital of France is Paris."*) ## Tests - Rust: `should_quantize` excludes `vision_embedder` (with positive controls that `embed_vision` + text layers still quantize); `gemma4_unified` resolves to a recipe + is in `CONVERTIBLE_MODEL_TYPES`; sanitize keeps `vision_embedder.*` bare; extended `recipe_registry_reproduces_inline_flags` (sym8 allowlist). - TS: `convert-cmd.test.ts` drives `run()` and asserts the `modelType` handed to native is `gemma4_unified` (and `gemma4`/`gemma4_text` still map to `gemma4`). - Gates: `convert::` 106 pass, fmt + clippy clean, CLI test 4/4, typecheck clean. ## Review Three `/codex:adversarial-review` cycles → final verdict **approve, no material findings**. The CLI-collapse misroute (cycle #1) was fixed; a flagged sanitize-test SIGABRT (cycle #2) was confirmed to be a review-harness/metallib environment artifact (the pre-existing `gemma4_recipe_sanitize_transforms` test aborts identically; CI runs one process per binary). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes conversion routing and quantization rules for a new model type (wrong passthrough could misroute QAT checkpoints); CI runner change is low risk but affects all macOS jobs. > > **Overview** > Adds first-class **`gemma4_unified`** support in `mlx convert` so encoder-free Gemma 4 unified checkpoints can be quantized without breaking vision or misrouting QAT imports. > > The native converter registers **`gemma4_unified`** on the shared **`Gemma4Recipe`**, excludes **`vision_embedder.*`** from quantization (bf16-only loader path), and keeps those weights at **bare keys** during sanitize. The CLI **auto-detects** `gemma4_unified` (including architecture-only configs) and **passes the string through** instead of collapsing to `gemma4`, avoiding the E2B prequantized importer gate. > > CI moves macOS jobs to **`macos-26`** and runs ignored cargo model e2e tests with **`--test-threads=1`** to avoid Metal GPU watchdog timeouts on shared runners. Docs and TS/Rust tests cover detection, quant exclusions, and sanitize routing. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fb4e8e0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…lx-node#77) ## What Loads a **quantized tied embedding / lm_head** for gemma4 as a **packed** table instead of dequantizing the whole thing to dense bf16. Before: a quantized `embed_tokens` went through `Embedding::load_quantized`, which dequantizes the entire table into a dense bf16 buffer and transposes it into `embed_weight_t` for the tied lm_head matmul (~2 GB bf16 for the 12B `262144x3840` vocab). After: both tied and untied embeddings go through `load_quantized_packed`. `forward()` dequantizes only the gathered rows, and the tied lm_head projects through `Embedding::as_linear` (`mlx_quantized_matmul`, `transpose=true`) **without ever materializing the dense table**. The five logits sites in `model.rs` detect the packed backend via `is_packed_quantized()` and take the `as_linear` branch, so `embed_weight_t` stays `None` on the packed path. The embedding quant mode is resolved from the per-layer config (`affine` or `mxfp8`) and threaded to the packed backend; biases-presence is driven off the actual `embed_tokens.biases` tensor key (affine has biases; mxfp8 does not). Any other quant mode at this key is rejected loud rather than silently mis-dequantizing. ## Why / measured (gemma4 12B QAT-q4_0, M5 Max) Quantizing the tied head from bf16 → 8-bit affine (gs32): | | bf16 head | 8-bit affine head | |---|---|---| | decode tok/s | ~46 | ~50 (equal within noise) | | peak RSS | 8.9 GB | 8.1 GB (**−0.8 GB**) | | on-disk | 8.3 GB | 7.5 GB (**−0.8 GB**) | | greedy output | — | near-lossless (answers unchanged) | Validated coherent + near-lossless on **both MLX 0.31.2 and 0.32.0**. We also bench'd mxfp8 for this head — it's perf-tied with affine and only ~90 MB smaller, so 8-bit affine is the chosen default; the loader stays mode-generic so an mxfp8 tied head also loads. ## Scope **Load-side support.** It activates only when a checkpoint actually carries a quantized `embed_tokens` (`.scales` present + per-layer config). Dense bf16 tied models are unaffected (`is_packed_quantized()` is false → existing `embed_weight_t` path, unchanged). Auto-emitting a quantized tied head from `mlx convert` for gemma4 is a follow-up. ## Validation - `cargo clippy --all-targets -- -D warnings` + `cargo fmt --check`: clean - Packed-embedding unit tests green, incl. `packed_affine_as_linear_matches_dense_matmul` (guards the exact packed-`as_linear` ≡ dense-matmul equivalence the tied lm_head relies on) and `packed_mxfp8_forward_matches_dequant_full_then_gather` - `/codex:adversarial-review`: **approve, no material findings** (verified transpose orientation, softcap-after-logits at all 5 sites, dense-tied + untied routing intact, no uncovered MTP/draft/warmup reader of `embed_weight_t`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core inference logits routing for quantized tied checkpoints; load-time guards reduce mis-dequant risk, but correctness depends on packed matmul matching the prior dense path. > > **Overview** > **Gemma4** no longer fully dequantizes a quantized `embed_tokens` into dense bf16 for a tied lm_head. Load now always uses **`load_quantized_packed`** (tied and untied), with **`resolve_packed_embed_params`** aligning affine vs mxfp8 to on-disk scales/biases and rejecting mis-resolved mxfp4/nvfp4-as-mxfp8 layouts. > > At **five logits sites** in `model.rs`, when there is no separate `lm_head` and embeddings are packed, logits come from **`embed_tokens.as_linear`** (`mlx_quantized_matmul`) instead of **`embed_weight_t`** or a dense transpose of the full table—avoiding a large resident bf16 vocab matrix on the hot path. > > Dense bf16 embeddings and an explicit `lm_head` are unchanged; the new behavior applies only when `.scales` are present and the packed backend is active. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9b34999. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…n_proj_a/b (mlx-node#78) ## Summary Converting `qwen-agentworld-35b-a3b` (a `qwen3_5_moe` `*ForConditionalGeneration`, GatedDeltaNet hybrid) to **unsloth + mxfp8** with an imatrix surfaced two real bugs in `apply_awq_prescaling` that silently degrade AWQ for **VLM-wrapped** and **GDN** checkpoints. Both are fixed here with a unit test, and validated end-to-end on the real model. ## Bug 1 — imatrix AWQ was a silent no-op on VLM checkpoints imatrix importance is always keyed canonical `model.layers.N.*` (produced by `gguf_name_to_hf` from `blk.N.*`). But sanitized VLM checkpoints carry `language_model.model.layers.N.*`, and `apply_awq_prescaling` auto-detected that prefix and used it for **both** the weight ops **and** the importance lookups → every `importance.get` missed → `modified 0`, with no warning (the missing-key warning only fires on a *partial* match). **Fix:** `imatrix_lookup_key()` strips the `language_model.` wrapper before the importance lookup; weight ops keep the detected prefix. `apply_awq_prescaling` now returns the modified count, and the caller emits a `warn!` when it is 0 despite an imatrix being supplied (so this class of silent no-op is visible). ## Bug 2 — AWQ Group D distorted GDN `in_proj_a` / `in_proj_b` Group D divides the shared `input_layernorm` by per-channel `s` but scaled only `in_proj_qkv` + `in_proj_z`. All four `in_proj_*` projections read the **same** `input_layernorm` output: - `decoder_layer.rs:203,207` — `normed = input_layernorm.forward(x)` → `gdn.forward(&normed, …)` - `gated_delta_net.rs:264/270-271` — that `normed` feeds **both** `in_proj_qkvz` **and** `in_proj_ba` (= b ++ a) So leaving `in_proj_a`/`in_proj_b` unscaled divided their inputs by `s` (here ~0.18–5.5×/channel) with un-compensated weights → corrupted GDN decay (`a`) and beta (`b`) gates. The reparametrization is output-preserving only if *every* consumer of the divided norm is column-scaled by `s`. **Fix:** column-scale `in_proj_a`/`in_proj_b` by the same `s` (their 8-bit-affine bit-width is orthogonal and unchanged). Also corrects the stale doc comment that claimed a/b "have no preceding norm" — only `o_proj`/`out_proj` lack one. ## Test `awq_prescaling_matches_vlm_prefixed_weights`: VLM-prefixed GDN (Group D) + full-attention (Group C) layers with a canonically-keyed imatrix must report `modified == 9`. ``` modified == 0 before fix 1 (prefix mismatch → no-op) modified == 7 before fix 2 (a/b skipped) modified == 9 after both fixes ``` ## Validation - `cargo test -p mlx-core --lib convert::` → 104 passed; clippy + fmt clean. - Real convert of `qwen-agentworld-35b-a3b` with `--q-recipe unsloth --q-mxfp --q-bits 4 --imatrix-path …` (67GB bf16 → ~20GB, 4 shards) → coherent generation. - On-artifact AWQ fingerprint `conv/(orig+1)`: per-channel **non-uniform** on `input_layernorm` (Group C full-attn + Group D GDN), ≈1.0 on `post_attention_layernorm` (Group A correctly inert — no dense FFN in MoE). > Note: `--q-recipe unsloth --q-mode mxfp8` is rejected by the CLI (recipes allow > only `affine`|`nvfp4`); the recipe+micro-scaling path is `--q-mxfp`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes quantization weight math on convert for VLM and GDN models; incorrect scaling would affect model quality, but scope is limited to the AWQ path with an imatrix and is covered by a targeted regression test. > > **Overview** > Fixes **AWQ pre-scaling** during convert so imatrix-based importance actually applies on **VLM-wrapped** checkpoints and stays **mathematically consistent** on **GatedDeltaNet** layers. > > **VLM / imatrix key mismatch:** Weight keys stay on the detected prefix (e.g. `language_model.model.layers.*`) while imatrix lookups now strip `language_model.` via `imatrix_lookup_key`, matching canonical `model.layers.*` entries from GGUF imatrix files. Previously every lookup missed and AWQ did nothing with no signal. `apply_awq_prescaling` now returns a **modified tensor count**; the convert path **warns** when an imatrix was supplied but zero weights were touched. > > **GDN Group D:** When AWQ divides shared `input_layernorm` and column-scales `in_proj_qkv` / `in_proj_z`, it also column-scales **`in_proj_a` and `in_proj_b`** with the same scale (importance still derived from qkv+z). Those projections share the normed input; skipping them left gates wrongly scaled. Docs for the unsloth recipe are updated to reflect this. > > Adds regression test **`awq_prescaling_matches_vlm_prefixed_weights`** expecting `modified == 9`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 43468d0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…lx-node#79) ## Problem Main CI intermittently fails on a single Rust unit test (most recently on the PR mlx-node#78 merge commit, run `28209793148`): ``` FAILED: nn::losses_test::tests::test_cross_entropy_qwen3_vocab crates/mlx-core/src/nn/losses_test.rs:630 "Loss should be > 10.0, got 9.881153" (1868 passed, 1 failed) ``` It is **unrelated to whatever PR happens to be merging** — the test was last touched in mlx-node#12 and only `convert.rs` changed in mlx-node#78. It is a pre-existing flaky test. ## Root cause The test builds random logits/targets with **no seed** and asserts a tight band on the mean-reduced loss: ```rust logits = random_normal([2, 151936], 0,1, None) // None is the DTYPE arg, not a seed targets = randint([2], 0, 151936) loss = mean_over_batch( logsumexp(logits) − logits[target] ) // ≈ log(151936) ≈ 11.93 assert!(10.0 < loss < 15.0) ``` With `batch_size = 2`, the loss is `≈ 12.43 − N(0, 0.71)` — a high random target logit occasionally drags the mean below `10.0` (the observed `9.88` is a ~3.5σ draw). ## Fix Bump `batch_size` 2 → 64. The mean-reduction then concentrates the loss at `log(V)`, shrinking the spread from ±0.71 to ±0.13. Vocab is unchanged, so the large-vocab **chunking path is still exercised**. ### Verification (2,000,000-draw simulation of the exact loss model) | batch | loss | min | P(loss<10) | P(loss>15) | |------:|------|----:|-----------:|-----------:| | 2 | 12.43 ± 0.71 | 9.01 | **0.030%** | 0.015% | | 64 | 12.43 ± 0.13 | 11.82 | **0** | 0 | At batch=64 the loss stays in `[11.8, 13.0]` across 2M draws — ~15σ from the `10.0` bound. (`logsumexp` constant measured = 12.4313, matching `ln(V)+0.5`.) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Test-only change with no production or loss-implementation edits; slightly more work per test run from a larger batch. > > **Overview** > Stabilizes **`test_cross_entropy_qwen3_vocab`** in `losses_test.rs` by raising **`batch_size` from 2 to 64** while keeping the Qwen3-sized vocab (151936) and the same `(10.0, 15.0)` loss band. > > The test still uses unseeded random logits/targets; with a tiny batch, a lucky target logit could drag the mean cross-entropy below 10.0 and fail CI. A larger batch tightens the mean-reduced loss around `log(vocab)` so the assertion is reliable without changing what the test exercises (large-vocab / chunked cross-entropy path). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0225f73. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…mbed, compressed-position decode + delta lifetime) (mlx-node#80) ## Summary Fixes **Qwen3.5-VL image inference** for both the `qwen3_5` (dense) and `qwen3_5_moe` families. Before this branch, an image turn ran end-to-end but produced garbage descriptions — the model could not read text and hallucinated unrelated subjects (e.g. described a bank-reconciliation table as "dark grey stone / white fabric"). Text inference and `mlx convert` were unaffected; only the visual-feature path was wrong. Found via `ornith-1.0-35b` (a Qwen3.5-VL-MoE-35B-A3B post-train). Root-caused by numeric bisection against the reference `mlx-vlm` implementation. ## Root causes & fixes (all verified vs `mlx-vlm`) 1. **Interleaved multimodal RoPE for image tokens** (`fb95c6d8`) Qwen3.5-VL uses the **interleaved** (stride-3 per-freq) M-RoPE selector, not the PaddleOCR **sectioned/contiguous** one. Text tokens (`t==h==w`) are identical either way — which is exactly why text always worked while images (`t≠h≠w`) got the wrong rotation axis on ~17% of frequencies, every attention layer, destroying the 2D positional structure. 2. **Patch embedding: sum Conv3d temporal slices + load `patch_embed.proj.bias`** (`6a889bdb`) The patch embed previously used only the first temporal slice and hard-coded the conv bias to `None`. 3. **Decode rotates at the compressed M-RoPE position** (`b5c4420e`) Image prefill compresses ~754 placeholder tokens into ~29 M-RoPE positions, so `rope_deltas = max_position + 1 − seq_len` (≈ −726). Decode must rotate the query at `physical_slot + rope_deltas` while K/V still writes at the physical slot. Threaded a `rope_position_offset: i32` through `forward_paged` / `decoder_layer` / `paged_forward` (dense + MoE share one `forward_paged`); the negative delta is carried on `VisionMerge.rope_deltas` and stored in `cached_rope_deltas` at VLM prefill. The physical position is cast `u32 → i32` **before** the negative add to avoid underflow. 4. **rope-delta lifetime gate** (`f7ae00ca`) Found by adversarial review of (3). The cross-turn delta was reset only when `cached_prefix_len == 0`, but the paged-turn planner also yields `cached_prefix_len > 0` with `continued_live_prefix == false` on a **non-live prefix-cache hit**. Because the model instance is shared across all sessions, a stale negative delta from a prior image turn could then leak into an unrelated **text-only** request that merely shares a cached text prefix — rotating that text at `physical + stale_delta` → garbage. Factored the decision into `rope_delta_for_paged_turn(cached_rope_deltas, continued_live_prefix)` (keep the delta iff it is a live image continuation, else clear) and wired it through all six paged reset gates (dense + MoE × sync / stream / engine). This is safe because image requests prefill with `skip_lookup` and never publish a hashable text stream that collides with their expanded-placeholder blocks, so every non-live hit restores only pure-text prefix blocks (delta 0); only a live continuation re-attends the image's compressed-position K/V. Added three model-free lifecycle regression tests. 5. **e2e correctness gate** (`ab044b88`) `qwen3_5_moe_vl_reads_document_text` and `qwen3_5_vl_image_chat.rs` (dense) — `#[ignore]`, env-gated (`MLX_TEST_QWEN35MOE_VL_MODEL_PATH` + `MLX_TEST_VLM_IMAGE_PATH`), asserting the model reads ≥2 `DOC_KEYWORDS` from `examples/ocr.png`. These are the ground-truth gates for VL image inference. ### Note on the addmm commit (`12e89b3a` + `8e6d69d9`) `12e89b3a` replaced the fused `mlx_array_addmm` primitive with an explicit `matmul + add`, originally attributed to a bug in `mlx::core::addmm`. **That premise was wrong** — PyPI MLX's `addmm` applies the `C` term correctly (maxdiff 0) and the FFI wrapper passes its arguments correctly. The real cause was a corrupt local metallib that miscompiled the fused GEMM kernels (the same bad build also miscompiled the NAX gemm). The explicit form is kept as robustness against this project's documented non-deterministic metallib corruption — it is correctness-equivalent and vision/bias-only so the perf cost is negligible — and the `nn::linear` C-application tests double as a build canary. `8e6d69d9` corrects the misleading comment so it no longer claims an mlx source bug. ## Validation - **e2e READS-DOC** on the mxfp8-converted `ornith-1.0-35b` checkpoint: the model reads the document, matching all 7 keywords (`reconciliation, bank, council, trunch, october, 2019, balance`). - `nn::linear` addmm/bias tests, `paged_forward` rope-offset + delta-lifetime tests, and the broader rope/m-rope/delta unit sweep (70 tests) pass. - `cargo clippy -p mlx-core --all-targets -D warnings` clean; `cargo fmt` clean. - Adversarial review of the branch: **approve, no material findings**. > The full `cargo test -p mlx-core --lib` (debug) shows pre-existing Metal-f32 > failures (banded-attn / attention-vjp) plus a few cross-family numerical tests > that fail only against the corrupted local **debug** metallib; all of them pass > in release against a correctly-built metallib (and reproduce on `main`), so the > branch introduces no new failures. ## Test plan ```bash # unit (release, correct metallib) cargo test --release -p mlx-core --lib -- nn::linear paged_forward rope delta # e2e image-correctness (env-gated, sequential — large models) MLX_TEST_QWEN35MOE_VL_MODEL_PATH=<converted-qwen3.5-vl-moe> \ MLX_TEST_VLM_IMAGE_PATH=$PWD/examples/ocr.png \ cargo test --release -p mlx-core --test qwen3_5_moe_vl_image_chat -- \ --ignored --nocapture qwen3_5_moe_vl_reads_document_text ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **High Risk** > Changes core attention RoPE, vision weights, and shared paged inference state across dense/MoE VL; incorrect delta lifetime or offset math would corrupt multi-turn or cached-prefix text, though extensive unit and env-gated e2e tests mitigate this. > > **Overview** > Fixes **Qwen3.5-VL image inference** (dense and MoE) by aligning vision, RoPE, and paged decode with `mlx-vlm`. > > **Interleaved M-RoPE** — Adds `apply_multimodal_rotary_pos_emb_interleaved` and switches Qwen3.5 attention from the PaddleOCR sectioned apply so image tokens get stride-3 per-frequency axis selection; text-only paths stay unchanged via invariance tests. > > **Vision tower** — `addmm` is implemented as explicit `matmul` + scaled add (avoids local metallib fused-GEMM corruption that dropped biases). Patch embed loads optional conv bias and collapses Conv3d weights by summing temporal slices instead of taking slice 0. > > **Compressed M-RoPE decode** — `VisionMerge` carries `rope_deltas`; `get_rope_index` uses the global max over t/h/w axes. Paged prefill/decode/MTP thread `cached_rope_deltas` through `paged_rope_offset` and `rope_position_offset` so rotation uses compressed positions while KV stays at physical slots. `rope_delta_for_paged_turn` keeps the delta only on `continued_live_prefix` so stale image deltas do not leak into unrelated text prefix-cache hits. > > **Tests** — Unit tests for interleaved RoPE, rope offset/delta lifecycle, patch embed, linear bias; env-gated e2e `reads_document_text` gates for dense and MoE VL. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8e6d69d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tition cutoff (mlx-node#81) ## Why Running `mlx launch claude` against a local MLX model surfaced two server-side defects: 1. **Claude Code's auto-mode safety classifier always sends `stop_sequences`.** The `/v1/messages` mapper rejected any request carrying `stop_sequences` with a `400`, so the classifier failed with *"temporarily unavailable, cannot determine safety of Agent"* and the agent stalled. 2. **The native anti-repetition cutoff truncated legitimate output.** A wide ASCII box border (`────`), a separator (`====`/`----`), a table rule, or repeated indentation tripped the default cutoff (`maxConsecutiveTokens=16`, `maxNgramRepeats=3`), returning `finish_reason:"repetition"` → mapped to `end_turn` — so answers were silently cut off mid-diagram and looked like a clean finish. ## What - **Honor `stop_sequences` end-to-end in `/v1/messages`** (streaming + non-streaming): detect, truncate exactly, and report `stop_reason:"stop_sequence"` + `stop_sequence`. Removes the 400. - **Loosen the Qwen anti-repetition cutoff** in the launch presets (`256 / 8 / 64`) so wide boxes/tables/separators survive while a true runaway loop still stops. TS-only in `packages/server` — **no native/Rust change**, `ChatConfig`/`packages/core` untouched. ## How - `StopSequenceBuffer` (`packages/server/src/stop-sequence-buffer.ts`) — cross-delta detection mirroring `ToolCallTagBuffer`: partial-suffix holdback so a stop split across two streamed deltas is caught; earliest-match-wins, longest-on-tie (including a longer stop that begins at an earlier index); whitespace-only and empty stops filtered to a no-op. - Request/response mappers thread `stopSequences` and can emit `stop_reason:"stop_sequence"` (precedence over `max_tokens`/`tool_use`/`end_turn`). - Streaming done-path uses **suppress-then-natural-done**: it keeps consuming to the native `done` chunk (commit gate + history commit still fire), and finalizes the stop scan over one continuous buffer — streamed deltas, the held partial, tag residue, and the recovered terminal text — **before** emitting the safe prefix, deciding tool-call emission, or computing `stop_reason`. A matched stop suppresses any `tool_use` block so a response never carries both a tool call and `stop_reason:"stop_sequence"`. Empty/absent `stop_sequences` is a byte-identical no-op. - Presets (`packages/server/src/presets.ts`): all four `QWEN_SAMPLING_DEFAULTS` variants get `maxConsecutiveTokens:256, maxNgramRepeats:8, ngramSize:64`. Gemma4/LFM2 unchanged. ## Testing - `__test__/server/` — **24 files, 688 tests pass** (incl. new buffer, mapper, and handler coverage; the original 400-asserting tests replaced with honor-behavior tests). - TDD throughout (failing test first), then 4 adversarial review rounds (Codex); every confirmed finding fixed and re-verified by execution. The one remaining review note (an overlap edge that would only bite if the native terminal chunk were *incremental*) was verified non-occurring — `backend.rs` `finish()` sends the full cumulative `result.text` (`clean_text`), which the overlap logic is built for. - `vp lint --type-aware --type-check` and `yarn typecheck` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Large changes to Anthropic message streaming/non-streaming output shaping and stop/tool precedence; behavior is heavily tested but regressions could affect wire format or truncation edge cases. > > **Overview** > **`/v1/messages` now honors Anthropic `stop_sequences` instead of returning 400.** The request mapper normalizes stop strings (filters empty/whitespace-only) and returns them as `stopSequences` beside `ChatConfig`, which has no native stop field. A new **`StopSequenceBuffer`** detects matches across streamed deltas (partial holdback, earliest/longest tie-breaks) and drives truncation plus **`stop_reason: stop_sequence`** and **`stop_sequence`** on responses and SSE **`message_delta`**. Streaming and non-streaming paths wire the buffer through tool-tag buffering, terminal recovery, and flush-at-end; a matched stop **suppresses `tool_use`** blocks so stop wins over tools. Absent stops remain a pass-through no-op. > > **Qwen launch presets** add loosened anti-repetition sampling (**`maxConsecutiveTokens: 256`**, **`maxNgramRepeats: 8`**, **`ngramSize: 64`**) on all four **`QWEN_SAMPLING_DEFAULTS`** variants; Gemma4/LFM2 unchanged. Tests cover buffer, mappers, handler edge cases, and preset merge survival. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ee1e456. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…-node#82) ## Problem Qwen models generating large markdown tables (a `|----------------------------------|` rule, or many byte-identical rows) get **silently truncated**. The native `check_repetition_cutoff` runs every decode step and stops generation on N identical tokens in a row (Tier 1) or a repeating n-gram block (Tier 2). It fires on legitimate repeated structure — table rules, ASCII boxes, separators, repeated rows — and the `"repetition"` finish reason is then mapped to a clean `end_turn`, so the cut looks like a normal answer. PR mlx-node#81 only loosened the **TS Qwen launch preset** (256/8/64). Every other path — non-launch servers, Gemma4/LFM2, per-request configs that omit the fields, and the napi `GenerationConfig::default()` — still fell back to the tight native defaults (16/3/64), so as few as ~6 repeated tokens still truncated tables. ## How vLLM handles it (the model for this change) vLLM ships **no** repetition-stop heuristic by default. It shapes repetition only with logit-level penalties (`repetition_penalty` 1.0, `frequency_penalty`/`presence_penalty` 0.0 — all no-ops by default) and stops generation only on real conditions (EOS, stop tokens, stop strings, `max_tokens`). Its opt-in loop detector (`repetition_detection`, recent main) defaults to `None`, and when enabled surfaces its own distinct `FINISHED_REPETITION` finish reason rather than disguising the cut. ## Change: fully align with vLLM — cutoff OFF by default, opt-in - **`fix(sampling)`** — Introduce three named constants (all `0`) in `sampling.rs` and route **every** former 16/3/64 default-fallback site through them (`params.rs`, qwen3 / qwen3.5 / qwen3.5-MoE / gemma4 / qianfan_ocr model files, and the napi `GenerationConfig::default()`). With the knobs unset the detectors resolve to disabled (`0` already trips the existing `check_consecutive` / `check_ngram` guards). The detection **algorithm is unchanged**; GRPO training (`grpo/engine.rs`) and paddleocr keep their explicit values. New unit tests lock default-off and prove a positive value still re-enables detection (opt-in). - **`fix(server)`** — Remove the now-unnecessary loosened cutoff (256/8/64) from all four Qwen launch presets; repetition is now shaped by the sampling penalties and bounded by `maxOutputTokens`, with per-request opt-in still winning via `ChatSession.mergeConfig`. Presets test flipped to assert the fields are absent while keeping the opt-in/override coverage. - **`fix(types)`** — Sync the build-generated `packages/core/index.d.cts` cutoff doc comments to match the hand-synced `crates/mlx-core/index.d.cts` (both now say `default: 0 = disabled`), and rename a stale test `describe` block. ### Behavior change / migration A degenerate small model now runs to `max_tokens` (default `max_new_tokens` = 2048) instead of stopping at ~16 tokens. This is intended and vLLM-aligned — `max_tokens` is the backstop. Operators who want the old early repetition-stop must now set `maxConsecutiveTokens` / `maxNgramRepeats` / `ngramSize` explicitly (per-request or via registered `samplingDefaults`). ## Testing - Rust: `cargo test -p mlx-core --lib repetition` → 28 passing (TDD RED→GREEN; algorithm tests unchanged). Full lib suite 1878 pass; the 7 failures are pre-existing flaky f32/Metal numerical tests (banded-attn, attention-vjp, packed-affine embedding, sparse-moe gather, chunked-prefill parity) — verified to fail identically on base `73f4a89e`, no path to this change. - TS: `vp test __test__/server/presets.test.ts` → 7/7. ## Out of scope (noted, not fixed here) An adversarial review flagged that `/v1/responses` never clamps `max_output_tokens` against the registry's per-model `outputTokenLimit` (unlike `/v1/messages`). Independently verified as **pre-existing** (byte-identical at base `73f4a89e`; this branch doesn't touch `responses.ts`/`messages.ts`) and **not** a regression of this change (the cutoff only ever caught exact-repeat loops, never bounded long coherent output). Worth a separate server-hardening PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes default generation stopping behavior across all chat paths: legitimate long repeats no longer truncate early, but runaway loops rely on `max_new_tokens` unless operators opt back in via config. > > **Overview** > **Native repetition-stop heuristics are off by default**, matching vLLM: unset `maxConsecutiveTokens`, `maxNgramRepeats`, and `ngramSize` now resolve to **0** (disabled) instead of 16/3/64. Shared `DEFAULT_*` constants in `sampling.rs` replace scattered literal fallbacks across chat param extraction and Qwen/Gemma/Qianfan generation paths; the detection logic is unchanged and **positive per-request values still opt in**. > > **Qwen launch presets** drop the previous 256/8/64 “loosened” cutoff pins—repetition is left to sampling penalties and `maxOutputTokens`, with `ChatSession.mergeConfig` still letting clients override. **API docs** (`ChatConfig` / `GenerationConfig`) and **tests** (presets, `params.rs` unit tests, LFM2 session comment) are updated for default-off behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5686106. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces support for client-supplied stop_sequences in Anthropic messages, disables repetition cutoff parameters by default to align with vLLM, and adds support for the gemma4_unified model type. It also addresses a compilation hazard by replacing fused addmm with explicit matrix multiplication and addition, and resolves Qwen3.5-VL vision issues by implementing interleaved multimodal rotary position embeddings and proper M-RoPE delta tracking. The review feedback highlights opportunities to optimize memory allocation in persistence.rs by avoiding unnecessary vector copies, and to improve robustness in language.rs by adding defensive checks against division-by-zero and out-of-bounds panics.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let weight_last = *weight.shape()?.to_vec().last().ok_or_else(|| { | ||
| Error::from_reason(format!("gemma4 {key} load: weight is 0-rank")) | ||
| })?; | ||
| let scales_last = *scales.shape()?.to_vec().last().ok_or_else(|| { | ||
| Error::from_reason(format!("gemma4 {key} load: scales is 0-rank")) | ||
| })?; |
There was a problem hiding this comment.
Calling .to_vec() on the slice returned by shape()? allocates a new Vec and copies all elements unnecessarily. You can call .last() directly on the slice to avoid this allocation.
let weight_last = *weight.shape()?.last().ok_or_else(|| {\n Error::from_reason(format!(\"gemma4 {key} load: weight is 0-rank\"))\n })?;\n let scales_last = *scales.shape()?.last().ok_or_else(|| {\n Error::from_reason(format!(\"gemma4 {key} load: scales is 0-rank\"))\n })?;| let cos_shape = cos.shape()?; // 1 FFI call — cache and reuse below | ||
| let batch_size = cos_shape[1]; | ||
| let seq_len = cos_shape[2]; | ||
| let head_dim = cos_shape[3]; | ||
| let half = head_dim / 2; | ||
| let half_usize = half as usize; |
There was a problem hiding this comment.
If head_dim is less than 2, half_usize will be 0, leading to a division-by-zero panic during the modulo operation j % half_usize. Additionally, if mrope_section has fewer than 3 elements, accessing mrope_section[1] or mrope_section[2] will cause an out-of-bounds panic. Adding defensive checks for these cases ensures robustness.
| let cos_shape = cos.shape()?; // 1 FFI call — cache and reuse below | |
| let batch_size = cos_shape[1]; | |
| let seq_len = cos_shape[2]; | |
| let head_dim = cos_shape[3]; | |
| let half = head_dim / 2; | |
| let half_usize = half as usize; | |
| let cos_shape = cos.shape()?; // 1 FFI call — cache and reuse below\n let batch_size = cos_shape[1];\n let seq_len = cos_shape[2];\n let head_dim = cos_shape[3];\n let half = head_dim / 2;\n let half_usize = half as usize;\n\n if half_usize == 0 {\n return Err(Error::from_reason(format!(\n \"head_dim must be at least 2, got {}\",\n head_dim\n )));\n }\n\n if mrope_section.len() < 3 {\n return Err(Error::from_reason(format!(\n \"mrope_section must have at least 3 elements, got {}\",\n mrope_section.len()\n )));\n } |
Summary
Bump
vite-plusand related packages to the pkg.pr.new prerelease build for v0.2.2 (registry-bridge commit build) to smoke-test the prerelease.vite-plus+vite(alias to@voidzero-dev/vite-plus-core) andvitestpinned to the commit build across deps / overrides / catalogsminimumReleaseAgeenabled with thevite-plus/@voidzero-dev/*/ oxc / oxlint stack excluded.npmrc(or.yarnrc.yml) points the package manager at the registry bridge (prerelease scaffolding)Test plan