chore: upgrade mlx to the latest version - #1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the mlx submodule dependency to a newer commit version.
- Updates the mlx submodule reference from commit
a4b3bc969b0f645209430475797d4fab8c67a70bto1b591ec73673c68a3dc8f6a9d6e568b8ed07141e
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Brooooooklyn
deleted the
12-15-chore_upgrade_mlx_to_the_latest_version
branch
December 15, 2025 15:20
Brooooooklyn
added a commit
that referenced
this pull request
Apr 11, 2026
…test_version chore: upgrade mlx to the latest version
Brooooooklyn
added a commit
that referenced
this pull request
May 4, 2026
flushPendingReleases now fires BUFFER_RELEASE_BATCH as a fire-and-forget (F&F) RPC: it stages handles in UNIFORM_DATA, flips STATUS=PENDING, notifies, and returns without calling Atomics.wait. The caller never consumes the return value, so skipping the round-trip directly shortens the decode-time bridge critical path — during decode the release queue hits MAX_RELEASE_BATCH many times per token, and each flush previously paid a full wasm-worker -> gpu-worker -> wasm-worker wake-up. The single-slot cmd SAB only lets one F&F be in flight at a time, so before any cmd-SAB write we drain the prior F&F via drainFireAndForget(): Atomics.wait(STATUS=PENDING) with the same timeout/retry pattern as rpcCall, then reset STATUS=IDLE to match the post-normal-RPC invariant. When STATUS is already DONE the wait returns 'not-equal' immediately, so the drain is free on the happy path. Drain sites: rpcCall and rpcCallWithHi (after their flushes, before cmd-SAB writes), flushPendingReleases itself (top, so consecutive auto-flushes interlock), and the dispatch hot path in wgpuComputePassEncoderDispatchWorkgroups (before writing the callback ring into UNIFORM_DATA). Histogram parity is preserved by bumping bridgeStats[BUFFER_RELEASE_BATCH] at the F&F site (since we bypass rpcCall's own bump). Tests: release-batch-faf.test.ts adds 4 cases — auto-flush at MAX_RELEASE_BATCH stages the batch and leaves STATUS=PENDING, the F&F returns in <5 ms (catches any accidental Atomics.wait regression), half-full queue leaves STATUS=IDLE, and a worker-based scenario proves the drain interlock: F&F #1 stages handles 1..64, the worker simulates the gpu-worker by flipping STATUS=DONE, F&F #2 then stages 100..163 — the pre-drain snapshot still shows the #1 handles (no clobber before drain) and the post-drain state shows #2 correctly staged. Atomics.wait is main-thread-forbidden so this scenario runs inside a Worker created from release-batch-faf-worker.ts. 212/212 tests pass (up from 208). Measured impact will be verified after this commit; controller runs the perf check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
May 15, 2026
flushPendingReleases now fires BUFFER_RELEASE_BATCH as a fire-and-forget (F&F) RPC: it stages handles in UNIFORM_DATA, flips STATUS=PENDING, notifies, and returns without calling Atomics.wait. The caller never consumes the return value, so skipping the round-trip directly shortens the decode-time bridge critical path — during decode the release queue hits MAX_RELEASE_BATCH many times per token, and each flush previously paid a full wasm-worker -> gpu-worker -> wasm-worker wake-up. The single-slot cmd SAB only lets one F&F be in flight at a time, so before any cmd-SAB write we drain the prior F&F via drainFireAndForget(): Atomics.wait(STATUS=PENDING) with the same timeout/retry pattern as rpcCall, then reset STATUS=IDLE to match the post-normal-RPC invariant. When STATUS is already DONE the wait returns 'not-equal' immediately, so the drain is free on the happy path. Drain sites: rpcCall and rpcCallWithHi (after their flushes, before cmd-SAB writes), flushPendingReleases itself (top, so consecutive auto-flushes interlock), and the dispatch hot path in wgpuComputePassEncoderDispatchWorkgroups (before writing the callback ring into UNIFORM_DATA). Histogram parity is preserved by bumping bridgeStats[BUFFER_RELEASE_BATCH] at the F&F site (since we bypass rpcCall's own bump). Tests: release-batch-faf.test.ts adds 4 cases — auto-flush at MAX_RELEASE_BATCH stages the batch and leaves STATUS=PENDING, the F&F returns in <5 ms (catches any accidental Atomics.wait regression), half-full queue leaves STATUS=IDLE, and a worker-based scenario proves the drain interlock: F&F #1 stages handles 1..64, the worker simulates the gpu-worker by flipping STATUS=DONE, F&F #2 then stages 100..163 — the pre-drain snapshot still shows the #1 handles (no clobber before drain) and the post-drain state shows #2 correctly staged. Atomics.wait is main-thread-forbidden so this scenario runs inside a Worker created from release-batch-faf-worker.ts. 212/212 tests pass (up from 208). Measured impact will be verified after this commit; controller runs the perf check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 2, 2026
Codex No-shipped the lfm2 MoE compiled-flat golden harness on two findings;
this is TEST-ONLY hardening (no production source touched). It gives the MoE
oracle real teeth and backs the benign-tie diagnosis with committed evidence.
H1 — assert FULL generation in BOTH golden tests. Both goldens were captured at
80 mid-reasoning ids (no EOS), so the correct decode finishes by LENGTH at 80.
Pin num_tokens == max_new and finish_reason == "length" so an early EOS/stop
can no longer shrink the oracle window and pass a short prefix while hiding a
downstream bug.
H2 — MoE teeth. Raise GOLDEN_MIN_PREFIX_BYTES_MOE from 120 to the PROVEN 138
(the full byte-identical agreement) so the floor sits AT the divergence, not
below it, and add GOLDEN_MOE_DIVERGENT_TAIL = "So" + a post-prefix pin: when our
output diverges, the divergent continuation MUST begin with the documented
benign one-ULP flip (' So', mlx-lm rank #2). Floor catches an EARLY divergence
(a real routing/rope/eps bug diverges at token 1-3); the pin catches a CHANGED
pick exactly at the known benign tie — together: fail on unexplained tail
divergence.
H3 — vacuous-pass fix. Add resolve_moe_model(): when LFM2_MOE_MODEL_PATH is set,
MoE coverage was REQUESTED and the MoE tests must run + hard-FAIL (never skip) if
the checkpoint is missing or non-MoE. Both MoE tests now resolve through it; the
existing is_moe SKIP stays as the UNSET-fallback (dense default) path so hosts
without the 16G 8B remain green.
H4 — reproducible evidence. Commit scripts/probe_lfm2_divergence.py and rewrite
the GOLDEN_MIN_PREFIX_BYTES_MOE doc so the benign-tie claim cites the COMMITTED
probe: real top-3 at generated step 33 is ' One'=-1.50 (#1), ' So'=-1.75 (#2,
ours), ' That'=-1.75 (tied #2); 0.25 logprob gap == 0.25 raw-logit gap == 1 bf16
ULP, the tightest near-tie in all 80 steps; 33 byte-identical tokens INCLUDING
exact-tie steps 24 & 26 resolved identically to mlx-lm; compiled===eager rules
out a compile-tracing artifact. Corrects the prior wrong 'That/One tied at
37.75' wording (self-reported, not in any artifact).
H5 — latent harness bug. lfm2_eager_flat_vs_compiled_capture read the
dense-named lfm2_e2e_compiled_flat.txt unconditionally; against the 8B that
diffed MoE-eager vs a STALE dense-compiled artifact (common_prefix=0 => spurious
"REAL compile bug" panic). Make the compiled-artifact read topology-qualified
(lfm2_moe_e2e_compiled_flat.txt when src is an lfm2_moe checkpoint).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 2, 2026
…allback leaks Closes both Codex No-ships on 90ed443. [high] First-wins over multiple in-tool straddle candidates leaked. When two tool calls each carry an in-tool `</think>\n` and NO top-level terminator exists, `straddle.get_or_insert` kept the EARLIER (literal) close, so the reasoning range reached only the first span — dropping tool #1 but leaking `more secret` plus the later reasoning-started tool #2 (`function=real`). FIX: last-wins — keep the LATEST in-tool candidate, so the reasoning range reaches the final straddle and the overlap-drop removes EVERY straddling span and the inter-call reasoning. A top-level terminator still preempts any tentative (returned immediately); single-candidate straddle and the raw-newline-before-top-level cases are unchanged. [medium] No-tool path no longer stripped SUCCESSIVE missing-open spans. Removing the `tool_ranges.is_empty()` fast path made the no-tool case a single range pass, which finds only the first top-level terminator: `secret </think>\nmore secret </think>\n final` left `more secret </think>\nfinal`, whereas the deleted strip_all_reasoning fixpoint stripped to `final`. FIX: the scrubber now iterates the pass (`strip_reasoning_once`) to a fixpoint, so successive missing-open spans are all removed — matching the old fixpoint, but on the scrubber's own `missing_open_close` (which scans past literal closes, unlike the generic parse_thinking) rather than delegating. Each pass shrinks or stabilizes the text, so it terminates; the with-tool cases are single-pass fixpoints, so their behavior is unchanged. New tests: two in-tool straddle candidates / no top-level terminator (asserts both tool calls + all reasoning dropped, only "final"); successive no-tool missing-open spans (asserts "final"). All prior cases hold. Still fallback-only / unreachable by real models. tools 91 + chat_common 53 pass; clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 2, 2026
W6's decode_loop_mtp! macro never pushed the initial y token (sampled from prefill's last logits) to $gen / $hist / tracker. AR's decode_loop! pushes $y at the top of each iteration; MTP's Step A only pushed the *sampled* next_token, silently dropping y0. Smoke on qwen3.5-4b (T=0, depth=3) before fix: AR : "```python\\ndef fibonacci(n):..." MTP : "python\\ndef fibonacci(n):..." (missing leading ```) After fix, both outputs start identically with "Deterministic sampling at temperature 0 is" (42 chars matching). This is one of two real bugs surfaced by the W8 smoke benchmark. Bug #2 — MTP cache offset drift from main offset (+2 per cycle from Step A's 1 forward + verify's D+1 forwards vs MTP draft's D forwards) — still causes mid-stream divergence at char 43 and catastrophic acceptance-rate collapse. Tracked as task #12; investigation reset with the correct diagnosis. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 2, 2026
Two correctness improvements driven by smoke benchmark (b366bc3). Fix #2 — MTP K/V cache offset drift vs main offset: - Per outer cycle, main offset advanced by D+2 (1 Step A + D+1 verify forwards), MTP draft offset advanced by D. Drift = +2/cycle. MTP draft's RoPE positions diverged from the absolute sequence positions, poisoning attention and collapsing acceptance to 0. - Fix: new FFI `mlx_qwen35_mtp_compiled_begin_cycle(main_offset)` zeroes MTP K/V caches and re-anchors `g_mtp_offset_int = main_offset` at the start of every cycle. MoE twin `mlx_qwen35_moe_mtp_compiled_begin_cycle` mirrors. `MtpOps` gains a `begin_cycle` closure; macro calls it before `run_mtp_cycle_inner`. Fix #3 — accept_with_residual non-deterministic at T=0: - W4 spec said T=0 collapses to argmax(p_target) == draft_id, but the implementation always took the raw-softmax ratio path and sampled from (p_target - p_draft)+ via MLX `categorical()` on rejection — stochastic via MLX's global RNG. - Fix: `accept_with_residual` now takes `sampling_config: &SamplingConfig`. When temperature <= 1e-6, returns (argmax(p_target) == draft_id, argmax(p_target)) — zero RNG consumed. Stochastic path unchanged for T > 0. - Unit tests: 2 new (`accepts_argmax_at_t_zero`, `rejects_non_argmax_at_t_zero_emits_argmax`) + existing 7 threaded through helper builders. Smoke result after both fixes: - Bug #1 fix advanced parity prefix from 0 → 42 chars. - Bug #2 fix held that and eliminated the offset drift. - Bug #3 fix made the residual emission deterministic. - AR : "...is essential for testing speculative decoding because it eliminates stochasticity, ensuring..." - MTP : "...is useful for testing speculative decoding implementations.\n\nThis ensures that every run produces identical outputs..." Both AR and MTP now produce coherent, deterministic output, but they still diverge at char 43. Remaining gap (Bug #4): the verify FFI's logits at position 0 disagree with `forward_compiled_with_hidden`'s logits for the same prefix and same K/V state. Both paths should end up in `qwen35_decode_fn`, so the divergence implies stateful interaction between Step A and verify that needs runtime instrumentation to isolate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 2, 2026
Addresses 4 Codex adversarial-review findings on B3: (high) Paged-specific accepted-prefix tape/replay. The B3 wiring reused `mlx_qwen35_compiled_tape_arm/_replay`, which early-return when `g_compile_inited == false` — true on pure-paged turns. Result: the shared `g_gdn_*_tape_acc[]` accumulators were never armed by the verify path, and rollback restored the pre-verify snapshot instead of the accepted-prefix state. Adds paged variants `mlx_qwen35_compiled_tape_arm_paged()` / `mlx_qwen35_compiled_tape_replay_paged()` that gate on `g_dense_paged_inited` and write back into `g_dense_paged_linear_caches[]`. The paged verify graph already emits tape outputs into the shared accumulators when armed, so no graph change is required. (high) Export `g_dense_paged_linear_caches` -> `self.caches` before the MTP branch returns. Mirrors the AR compiled-paged tail at `model.rs:3953`. Prevents `mtp_compiled_reset` from clearing C++ globals under stale Rust-side checkpoints. (high) Rollback un-emitted tokens on mid-cycle stop. Adds `MtpOps::rollback_unemitted: FnMut(usize)`; the macro tracks `cycle_emitted` and invokes the closure with the un-emitted count on EOS / cancel / length / repetition cutoff. Paged path truncates the live paged adapter; dense / MoE / tests pass no-op closures (their compiled cursors are driven by `commit_mtp`, which already ran for the full cycle). (medium) Materialize `prompt_hidden` before `synchronize_and_clear_cache()` in `project_last_token_logits_with_full_hidden`. Adds shape / dtype assertions for `[1, prompt_len, hidden_dim]` bf16. Smoke (M3 Max, depth 3, T=0, 256 tokens, NVFP4 27B, gate=ON): AR=22.39 tok/s MTP=24.26 tok/s ratio=1.08x Parity OK: AR and MTP produced identical output (0 chars). Stop=length (no repetition cutoff — Codex finding #1 fixed). `mtpCycles` profiler threading deferred to B4b; the per-position acceptance numbers will surface there. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
5 tasks done
Brooooooklyn
added a commit
that referenced
this pull request
Jun 4, 2026
… fix paged prefill-tps telemetry Quantized lfm2_moe checkpoints carry `.scales` tensors, so they can never register with the compiled-PAGED C++ path; with `use_block_paged_cache` defaulting ON they fell onto the eager-PAGED loop (~12 `synchronize_mlx()`/token, blocking `y.eval()`, no async double-buffering) — ~1.84× slower than the eager-FLAT path (in-graph KVCache + `async_eval_arrays`) on the measured mxfp8 LFM2.5-8B-A1B (74 → 131 tok/s on M5 Max, closing most of the gap vs oMLX's 187). Fix #1: resolve the `use_block_paged_cache` default in `Lfm2Inner::load_from_dir`, keyed on the authoritative `.scales` tensor signal (the SAME signal that gates compiled registration, so the two can't diverge for a checkpoint whose quantization block lacks top-level bits/mode) — quantized + unset → flat; bf16 → paged (PR #66 compiled-PAGED unaffected); an explicit config.json value always wins. The server auto-routes via `has_block_paged_cache()`, so batched / cross-request reuse is unaffected (bf16 stays paged; quantized moves to the warm-slot path the server already supports). Single-stream only. Fix #4: the paged prefill-throughput telemetry divided ttft by the attention SUFFIX (`tokens.len() - cached_prefix_len`), but paged prefill reprocesses the FULL prompt through the conv layers, so ttft is full-prompt scale — this under-reported prefill tok/s by the cache-hit ratio (~37 vs thousands on warm content-addressed prefix reuse). Use the full prompt count at both paged call sites. Qwen3.5's identical expression is correct (no conv reprocessing) and is left untouched. Deferred (plan only): quantized compiled-PAGED decode (#2, for batched serving) and a cross-family paged-loop wired_limit (#3a, no-op for the flat default on a 128GB host). Tests: pure-helper policy unit test; a parse_config-does-not-resolve contract test; a compute_performance_metrics divider test; and two #[ignore] real-weights regressions (quantized default load → flat via has_block_paged_cache(); paged warm-reuse prefill-tps is full-prompt scale). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 4, 2026
…ode (~2.34×) + prefill-tps telemetry fix (#67) ## Summary Closes most of the LFM2.5-8B-A1B quantized single-stream decode gap to oMLX by fixing the default decode path and extending the compiled C++ path to quantized weights. Two commits: - **`fb234575` — quantized → flat default (~1.84×) + paged prefill-tps telemetry fix** - **`46760077` — quantized compiled flat+paged decode (~2.34× paged over eager-paged)** ### #1 — quantized single-stream defaults to FLAT decode (~1.84×) Quantized `lfm2`/`lfm2_moe` was silently defaulting to the **eager-PAGED** loop (~12 `synchronize_mlx()`/token + blocking `y.eval()` + no async double-buffering), ~1.84× slower than FLAT on the measured mxfp8 8B-A1B (74 → 131 tok/s, M5 Max). The default is now keyed on the authoritative `.scales` tensor signal: quantized → FLAT, bf16 → PAGED (unchanged). Explicit `use_block_paged_cache` in `config.json` always wins. ### #4 — paged prefill-tps telemetry fix The paged path reported a bogus ~37 `prefillTokensPerSecond` (it divided full-prompt ttft by the attention *suffix* count on warm prefix-cache hits). Now uses the full-prompt count as the numerator; guarded by `lfm2_paged_prefill_tps_is_full_prompt_scale_on_warm_reuse`. ### #2 — quantized compiled flat+paged decode (~2.34× over eager-paged) Extends the compiled C++ decode path (previously bf16-only) to quantized `lfm2`/`lfm2_moe`. A per-projection quant-info registry (`mlx_store_quant_info`, keyed on each `.scales` prefix) makes the C++ `(mode, bits, group_size)` dispatch **authoritative** instead of the companion-tensor heuristic (which mislabels mxfp4/nvfp4 as mxfp8); the heuristic is retained only as a fallback. Compiled-PAGED is ~2.34× over eager-PAGED, rescuing the pinned-paged quant path (e.g. server/batched). A packed embedding (`embed_tokens.scales`) bars the compiled path (C++ does a dense `take`). Env escape hatch: `MLX_LFM2_DISABLE_QUANT_COMPILED`. ## Correctness Byte-identical to the pure-Rust eager path across **{mxfp8, 4-bit affine} × {flat, paged}**, proven via the model-id **eviction oracle** in `lfm2_compiled_e2e.rs` (`quant_compiled_vs_eager_parity`): loading the compiled model evicts the eager-ref's process-global weights, so the eager-ref runs the *independent* `QuantizedLinear`/`QuantizedSwitchLinear` modules — a C++ dispatch mislabel would diverge early. This is stronger than a same-graph `MLX_NO_COMPILE` reference. ## Perf context This is the **quantized** path — the relevant one for oMLX's 8-bit headline. Separately verified this session: for **bf16**, our decode (~110 tok/s) is at **exact op-for-op parity with mlx-lm** and is **memory-bandwidth-bound** (MoE gather already saturates ~404 GB/s at the k=4 decode shape, ~80% of the M5 Max ceiling); the residual bf16 gap to oMLX is host/measurement, not software. The real lever for absolute decode speed is reducing bytes-per-token (quantization) — which is exactly what these changes make fast. ## Test plan - [x] `cargo clippy --all-targets -- -D warnings` — clean - [x] `cargo fmt --check` — clean - [x] 30 unit tests pass (`cargo test -p mlx-core`, incl. the compiled-registration gate tests) - [x] Byte-identical parity matrix (mxfp8/4-bit × flat/paged) via the eviction oracle (opt-in: `LFM2_COMPILED_E2E=1` + `LFM2_QUANT_MODEL_PATH`) - [x] `yarn build:native` clean; no `index.d.cts` drift ## Review status The mandated `codex:adversarial-review` runtime **hung twice** mid quant-dispatch cross-reference (a codex-runtime issue, not a code signal). A thorough Claude-subagent adversarial review cleared it **SHIP / no blocking bug** — verifying dispatch parity for every projection class (MoE experts, router gate, dense-MLP, attention q/k/v/out, conv, untied lm_head) and ruling out the truncated codex concern on all three plausible completions (packed-embedding guard, registry-authoritative quant modes, pre-existing flat bf16 invariant). **Deferred follow-ups (non-blocking):** - [Medium] Synthetic non-gated quantized parity test (parity is currently operator-verified via `LFM2_COMPILED_E2E=1`; the synthetic harness only generates bf16 weights, and the completeness `debug_assert_eq!` is compiled out in release). - [Low] `mlx_store_weight` transposes packed 2D quant `.weight` into `g_weight_transposes` that's never read (pre-existing waste, surfaced not introduced). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes default decode routing and global compiled weight registration for quantized LFM2, where incorrect quant dispatch or gating would affect correctness and performance; mitigated by expanded unit tests and documented escape hatches. > > **Overview** > **LFM2 load and decode routing** now treat quantized checkpoints differently: when `use_block_paged_cache` is unset, presence of `.scales` tensors defaults to **flat** decode (instead of paged), with resolution moved from `parse_config` to `load_from_dir` so it matches the registration gate. Explicit `config.json` values still win. > > **Quantized models can use the compiled C++ path** (flat and paged): registration publishes per-projection quant info via `mlx_store_quant_info`, `should_register_compiled` and `paged_compiled_decode_setup` use `non_quant_floats_bf16` plus `MLX_LFM2_DISABLE_QUANT_COMPILED`, and packed `embed_tokens` blocks compiled registration because the C++ path does a dense embedding lookup. > > **Paged chat performance metrics** use the full prompt token count for prefill throughput (conv layers re-run the full prompt), fixing inflated TTFT/prefill-tps on warm prefix-cache hits. > > Most other diff hunks are **comment and docstring cleanup** (phase/W6/PR ticket references removed); behavior in convert, MTP, Qwen3, and banded-attention modules is unchanged aside from wording. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a4a760d. 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 (1M context) <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 17, 2026
…n3.6 classes) (#72) ## Summary Collapses the **O(T) per-step gated-delta (GDN) recurrence** on the CUDA (non-Metal) prefill path into **O(T/BT) chunk-serial batched matmuls** (cuBLAS / tensor cores) — a device-agnostic, pure-`MxArray` port of the in-tree Metal chunked kernel (`crates/mlx-sys/src/metal/gated_delta_chunked.metal.inc`). - **Zero build changes** (no nvcc/NVRTC) — matmuls route through cuBLAS. - **Default-on** for the CUDA ops path; `MLX_GDN_KERNEL=perstep` reverts for same-binary A/B. - **No Metal impact** — `use_kernel=true` never reaches this path; the Mac/Metal production path is byte-identical. This attacks the *"GDN per-step recurrence is the prefill floor"* bottleneck that [PR #71](#71 benchmark flagged as the **#1 dense lever**. ## Measured (GB10 / DGX Spark, Qwen3.6, warm, prefill TTFT vs per-step) | model | 1577-tok speedup | parity | |--------------|------------------|--------| | dense-Q4 | **1.62×** | byte-identical / late-drift | | dense-NVFP4 | 1.33× | identical | | MoE-Q4 | **1.75×** | coherent | | MoE-NVFP4 | 1.40× | coherent | Win **grows with prompt length** (dense-Q4: 1.06×@200 → ~1.58×@1577+; the chunked inverse is fixed-cost while per-step is O(T)). chunked prefill tok/s climbs to ~242 vs per-step's flat ~150. MoE wins more (GDN is a larger prefill fraction); NVFP4 less (dequant dominates its prefill). ## Numerical-stability fixes (each with a Mac regression test) 1. **Triangular inverse overflow.** `M = (I+A)⁻¹` by repeated squaring overflows f32 at `BT=64` (`N³² ≈ 4e57` before nilpotency zeroes it at `N⁶⁴`), producing garbage. Replaced with **row-iterative forward substitution** (FLA / vLLM `solve_tril`): `M[i,:] = eᵢ − A[i,:]·M` — no powers of A, stable for any `‖A‖`, serial depth independent of T. Test: `chunked_ops_stable_with_correlated_unit_norm_keys`. 2. **Gate underflow (MoE-only garbage).** `g_log = g.log()` round-trips through the exp-space gate; strong decay (which MoE has, dense doesn't) underflows `g` to 0 → `log(0) = -inf` → chunked `gcum_i − gcum_j = inf − inf = NaN`. Now compute `g_log = -exp(a_log)·softplus(a + dt_bias)` **directly in log-space** (matches the native `g_log` the fused Metal gating returns). Test: `compute_g_log_finite_under_strong_decay`. ## Validation - 6/6 `gated_delta` Rust unit tests, `cargo clippy -p mlx-core --all-targets`, `cargo fmt` — green. - Correctness validated on the DGX across **all four Qwen3.6 classes** (dense/MoE × Q4/NVFP4) — per-step vs chunked greedy A/B, coherent output everywhere (garbage only before the two fixes above). - Algorithm derivation in `docs/gdn-chunked-ops-spec.md`. ## Follow-ups (not blocking) - FLA 16-block row-iterative + block-merge inverse to cut short-prompt inverse depth 63→~16 (long-prompt asymptote is carry-bound, won't move). - Runtime non-finite guard → fall back to per-step (overflow is currently silent; the `Err` fallback doesn't catch `Inf`/`NaN`). - Possibly raise `CHUNK_THRESHOLD`→256 (the 200-tok win is only 1.06×). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches core Qwen3.5 inference recurrence and output numerics on CUDA, though Metal is gated off, per-step fallback exists, and behavior is covered by parity/stability tests. > > **Overview** > Adds a **default-on CUDA prefill fast path** for Qwen gated-delta (GDN): long, unmasked sequences on the non-Metal ops branch now run **`gated_delta_chunked_ops`**, a pure `MxArray` chunk-parallel port (BT=64) that replaces the O(T) per-token loop with O(T/BT) chunk carries and batched matmuls. Metal/`use_kernel=true` routing is unchanged; decode and masked calls still use per-step ops. **`MLX_GDN_KERNEL=perstep`** (and **`ForceChunkedOps`** / `chunked_ops` aliases) support same-binary A/B. > > Two **numerical fixes** ship with the chunked path: **`compute_g_log`** computes the decay gate in log-space (avoids `log(0)` → NaN on strong MoE decay), and **`invert_i_plus_strict_lower`** builds `(I+A)⁻¹` via forward substitution instead of f32 power squaring that overflows at BT=64. Chunked ops errors fall back to per-step with a stderr warning. > > Adds **`docs/gdn-chunked-ops-spec.md`** plus unit tests for env parsing, chunked vs per-step parity across chunk boundaries, correlated-key inverse stability, and strong-decay gating. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit efe9c8f. 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 (1M context) <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jun 25, 2026
…n_embedder bf16 (#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 #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>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
flushPendingReleases now fires BUFFER_RELEASE_BATCH as a fire-and-forget (F&F) RPC: it stages handles in UNIFORM_DATA, flips STATUS=PENDING, notifies, and returns without calling Atomics.wait. The caller never consumes the return value, so skipping the round-trip directly shortens the decode-time bridge critical path — during decode the release queue hits MAX_RELEASE_BATCH many times per token, and each flush previously paid a full wasm-worker -> gpu-worker -> wasm-worker wake-up. The single-slot cmd SAB only lets one F&F be in flight at a time, so before any cmd-SAB write we drain the prior F&F via drainFireAndForget(): Atomics.wait(STATUS=PENDING) with the same timeout/retry pattern as rpcCall, then reset STATUS=IDLE to match the post-normal-RPC invariant. When STATUS is already DONE the wait returns 'not-equal' immediately, so the drain is free on the happy path. Drain sites: rpcCall and rpcCallWithHi (after their flushes, before cmd-SAB writes), flushPendingReleases itself (top, so consecutive auto-flushes interlock), and the dispatch hot path in wgpuComputePassEncoderDispatchWorkgroups (before writing the callback ring into UNIFORM_DATA). Histogram parity is preserved by bumping bridgeStats[BUFFER_RELEASE_BATCH] at the F&F site (since we bypass rpcCall's own bump). Tests: release-batch-faf.test.ts adds 4 cases — auto-flush at MAX_RELEASE_BATCH stages the batch and leaves STATUS=PENDING, the F&F returns in <5 ms (catches any accidental Atomics.wait regression), half-full queue leaves STATUS=IDLE, and a worker-based scenario proves the drain interlock: F&F #1 stages handles 1..64, the worker simulates the gpu-worker by flipping STATUS=DONE, F&F #2 then stages 100..163 — the pre-drain snapshot still shows the #1 handles (no clobber before drain) and the post-drain state shows #2 correctly staged. Atomics.wait is main-thread-forbidden so this scenario runs inside a Worker created from release-batch-faf-worker.ts. 212/212 tests pass (up from 208). Measured impact will be verified after this commit; controller runs the perf check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
…e findings DESIGN (from a 4-philosophy design workshop → synthesized editorial + tooltip-dock spec): give /jspace real identity and craft on par with the course. - Instrument-Serif "J-Space" wordmark over a mono eyebrow with the course's glowing pink dot; warm subtitle; header divider. Two earned CSS primitives — .jspace-panel (soft translucent surface over the fixed page gradient) and .jspace-eyebrow. - Every region becomes a panel or a recessed well; controls read as a crafted bar; consent + skeleton get serif heads; strict one-accent-per-job (pink = you/action/ selection, qwen indigo = Jacobian telemetry only, book-* = pin data). - The one spatial move: dock the per-cell readout in a sticky column beside the argmax grid on lg, turning the former dead right-half into an instrument+readout pair (source order stays grid-then-detail, so F2/F3 selection wiring is unaffected). - Warm canvas inks (canvas-theme.ts → --text/--text-dim family) + the matching RankChart gridline; editorial column max-w-[82rem]. FIXES (all 6 re-gate findings, each independently verified against real control flow): - #2 layersFor returns 1..24 for BOTH modes (spec §6 comparability); the old comment claiming the pack only fits 11 layers was factually false — it carries all 23 J's. runSelfTest keeps JACOBIAN_LAYERS (its baked reference is 11-layer). - #6 heatmap flex item gets min-w-0 flex-1 so the 128-token 8750px canvas scrolls inside its bounded scroller instead of overflowing the page (verified live). - #5 wire the spec's second RankChart: rank-vs-position at the selected layer. - #3 cancel the pending permalink selection on any explicit divergence (select / prompt edit / mode change), so a stale restore never resurfaces after Run. - #4 reset ArgmaxGridCanvas lastHoverKey on selection change, so keyboard nav then an intra-cell pointer move re-syncs hover with the tooltip/charts/Pin target. - #1 self-test structural gate also asserts per-cell (layer,position) identity. Verified: tsc 0 · lint clean on touched files · 54/54 jspace tests (10 files) · vp run build green · Chrome live-verified across cold/selected/skeleton states, the 128-token no-overflow case, and zero app console errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
…mts clobbering starters
Four verified Task-2 defects from the raw vet data, plus one consequential fix:
1. Robust pin resolution in bake-gallery.mts: derivePins pins the first token of
` ${concept}`, a sub-word fragment for some concepts (' Africa' rank 6). Now, after
the readout, re-pin each concept to the exact top-K token whose decoded text
(leading space trimmed, case-folded) equals the concept word, at its best rank
across displayed layers; fall back to derivePins only when no exact match exists.
giza Africa 9871(rank6) -> 71090(rank2). Re-runs readouts when a pin moves.
2. Regrade giza-continent STRONG -> WEAK in gallery.ts (same faint-but-genuine class
as eiffel-capital). Counts: 5 STRONG + 3 WEAK; jspace-gallery.test.ts -> 5/3.
3. Grade-consistency assertion in bake-gallery.mts: STRONG => primary concept rank <=3,
any grade => <= TOP_K, else throw loud with slug + measured rank.
4. Remove bake.mts's mirror write to demo/jspace/starters/; the lesson bake writes only
to demo/learn/widgets/jlens/baked/ (that output is byte-unchanged). Starters are now
owned exclusively by bake-gallery.mts.
Consequential: runSelfTest re-derived french-season pins via derivePins and compared to
the starter frame, whose pins fix #1 changed -> structural mismatch would refuse the live
fitted-Jacobian badge (vitest misses it: it uses the untouched lesson frame). Source the
self-test's live pins from the baked frame it validates instead; verified offline ok:true,
agreement 1.0, delta 0. Removed the now-unused derivePins import from JSpaceApp.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
…t ranks/bands, guards Adversarial-review reconciliation for the Tier-1 gallery (all 8 verified against the committed frames + control flow before fixing): P1 #1 DROP eiffel-capital: 'Paris' IS the ℓ24 greedy output, a SPOKEN answer — violates the 'unspoken word' invariant. Gallery now 7 tiles (5 STRONG+2 WEAK). New bake assertion: no pinned concept may equal the ℓ24 argmax. P1 #2 self-test: reject invalid live rank <=0 so a collapsed rank path can't pass the fuzzy low-rank delta on the french-season oracle. P2 #3 grammar caption 'both lenses raise incorrect to rank 1-2' was false for logit (pinned 79034 is rank ~20 there) → rewrite to the honest J-vs-logit framing. P2 #4 openStarter race: monotonic runIntent token; run/mode-change continuations bail after their awaits instead of painting a live frame over the tile. P2 #5 rank captions were 0-based vs the 1-based API — fix inner-sum 2->3 (logit 6->7), precedence 0->1, few-shot 2->3, int-cast 1->2 (en+zh). P2 #6 band drift: grammar peak ℓ18(rank4)->ℓ17(rank2), giza {16,17}->{17,18} to match the rank-2 plateau; tighten the bake to require band.peak on a min-rank layer. P2 #7 cold-URL guard drops the mode check so opening a tile keeps the URL clean. P2 #8 LandingPage persists storeLocale(useLocale()) so a direct /zh visitor's 'Open J-Space' inherits zh (the standalone /jspace reads only localStorage). Frames re-baked (7 byte-identical, eiffel-capital.json removed); +2 committed-data regression tests encode the unspoken-word and band-peak invariants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 22, 2026
Round-3 adversarial review of the starterSlug permalink fix surfaced three real defects (all verified against control flow before fixing): #1 (HIGH) A crafted `#s=` slug could CRASH /jspace. STARTERS is a plain object, so `STARTERS['toString']` / `['__proto__']` / `['constructor']` resolve INHERITED Object.prototype members (truthy → the `?? default` fallback never fires); the render then read `.logit`/`.cells` off a non-frame and reviveRun threw during render. The starterSlug fix opened this by flowing untrusted `s=` into the lookup. Fix: `resolveStarterSlug` (own-property-safe via Object.hasOwn) clamps untrusted slugs to the default tile, applied at BOTH the restore clamp and the render lookup (defense in depth). #2 (MED) The encoder's cold gate was `.trim() === ''` while the app's authoritative `isColdPrompt` is exactly-empty. A whitespace prompt therefore carried a hidden `s=<tile>` on the wire while the app rendered it as custom content — a later "clear" could expose the sender's stale tile. Fix: the encoder now imports and uses the SAME `isColdPrompt`, so codec and app can never diverge. #3 (MED) Returning to the full default left a dirty hash: cold → type a prompt → clear it wrote `#p=&mode=l&s=french-season` because the guard only SKIPPED the write when already clean. Fix: when isColdDefault, actively STRIP the hash (replaceState to pathname+search) and reset the ref, so the URL truly returns to bare. Tests: +resolveStarterSlug clamp (toString/__proto__/constructor/unknown → default), flipped the whitespace codec test (whitespace ≠ cold), +2 app regressions (#s=__proto__ renders default not crash; cold→custom→cold strips the hash). tsc clean; 50 jspace tests pass; build + SSG prerender green; lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 25, 2026
…udit Audit found via 7-surface workflow, each finding independently re-verified (source + live-measured) before fixing. Contrast (root causes, hit every page): - #1 --text-muted #6a6168 (~2.9–3.2:1, failed WCAG AA) → #9a908a; now 5.6–6.2:1 on breadcrumbs, chapter numbers, problem statements, placeholders (1260 text-muted-foreground uses + a few raw color: uses). - #2 `@custom-variant dark` gated every `dark:` utility on a `.dark` ancestor that nothing added → intended dark callout/button colors were inert (light base rendered: amber/violet chips 2.3–3.6:1). Add `.dark` at boot (index.html shell + seo-head, captured by the prerender serialization). Callouts now render amber-300/violet-300/emerald-400 at 8.9–12:1. Responsive: - #3 no cross-chapter nav below 1024px (sidebar is `hidden lg:block`, no drawer/pager) → ChapterPager prev/next at every chapter foot, order from the `localizedChapters` registry, locale-prefixed `<a href>` + SPA-intercept, first/last handled, sub-chapters threaded (en+zh strings added). - #4 /chapters hub header collided at 375px (Back over free-chat 8px, 中文 wrapped) → labels collapse to icon+sr-only below sm (mirrors LessonLayout), groups shrink-0, switcher whitespace-nowrap. - #8 landing locale toggle overlapped the hero badge at 375px (68×23px) → corner toggle hidden below sm; mobile in-flow toggle rides above the badge; hero top-anchored so the non-scrolling overlay never clips it. Controls / a11y: - #5 four widgets (TeacherForcingAnimation/GenerationLoop/LmHeadWalkthrough/ WeightTyingVisual) auto-animated ignoring prefers-reduced-motion, unlike 48 siblings → seed `playing` from matchMedia (verbatim sibling idiom) + gated aria-live on the step caption. - #6 dead enabled "Voice input" mic button (no handler) → removed + dropped the unused Mic import. - #7 shared <Button> stayed 32–40px on touch (the coarse-pointer 44px rule only matched chat classes) → min-height:44px for [data-slot=button] (+ icon min-width) under pointer:coarse. Verified: build green (vite+prerender), `.dark` in all prerendered pages, desktop unchanged, mobile pager/headers/landing confirmed live at 375px. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…nload identity, session safety Six confirmed defects from the final whole-branch review (each control-flow verified + probed before fixing); one finding refuted, one documented residual. - #6 metrics (regression from R11): the trace-only token UNION projected GROSS traces.prompt_tokens into the `input` column while turns store NET (prompt-cacheRead), double-counting cached tokens. Clamp the trace side to MAX(prompt-cached,0) in tokensByDay + turnTotals; fixture totals corrected. - #4 downloads: gate the install-skip on the completion marker's repo+revision (new readCompletion helper), not mere presence — a different revision of the same slug no longer reports a false `done`; mismatch falls through to the owned-swap. - #7 trace ingest: move the vanished-file row reconciliation BEFORE the ingest loop so a renamed trace file's globally-unique rows are freed before the new name is ingested (were permanently dropped); watermark reconcile stays after. - #8 session ingest: reject a non-object top-level entry (a `header\nnull\n` file) in isValidSessionTopology before dereferencing .type, routing it to the existing quarantine path instead of a warn-only catch that retained stale rows. - #3 session rename: refuse (409) renaming a session whose file was modified within a liveness window — pi has no cross-process lock, so appending to a file a live agent is extending would race that turn. Documented best-effort. - #5 cold cache: publish the index entry before the directory fsync so a post-rename dir-fsync failure keeps in-process accounting consistent with the on-disk file (rebuild_index already self-heals on restart). - #2 cold-tier fingerprint→materialize window: documented as an accepted v1 residual (off-by-default, adversary-owns-files; fd-coupled hashing out of scope). - #1 (train KV poisoning): refuted — training never uses the paged/cold path. Gate: typecheck exit0 · cold_cache 33/0 · dashboard 8 suites/175 · lint 0-err.
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…sion discovery, cold-cache durability Seven confirmed defects from the final whole-branch review (each control-flow verified + probed); one documented as an accepted fail-open residual. - #8 (ui): resume commands interpolated the session path into a shell string raw — a real injection ($()/backticks/&& fired in a probe) and a break on any path with a space. Added a shared POSIX shQuote used at both resume sites. - #5 (models): walkDirStats followed symlinks with statSync and recursed into symlinked directory targets unbounded — two links to '.' hit ~2^31 stats and froze the synchronous /api/models. Now recurses only real dirs (Dirent lstat semantics), with a dev:ino visited-set and an entry budget. - #4 (ingest): pi with an explicit --session-dir writes sessions flat as <dir>/*.jsonl, but the scanner only accepted <root>/--cwd--/*.jsonl, so the documented shared --session-dir flow produced an empty dashboard. Now scans both layouts (cwd read from each file's header). - #6 (download): an unowned (marker-less) install was refused only inside publish, after a full multi-GB download+hash. Preflight ownership up front (after staging recovery, before any hub work) so a doomed job moves no bytes; reworded errors to drop the unexposed overwrite advice. - #7 (metrics): per-session metrics omitted delegated (subagent) turns — session-scoped trace-only union with MAX(prompt-cached,0) net clamp + trace_id dedup, mirroring the global overview fix, so counts/tokens match the shown child badge + throughput. - #2 (cold cache): the capture loop kept blitting Metal + persisting descendants after a queue-drop; now stops on the first non-Ok(true), keeping the enqueued set a contiguous restorable prefix. - #3 (cold cache): mlx agent -p exited with accepted blocks still queued (no drain). Added a bounded native drain barrier (WriteJob::Barrier + ColdCacheManager::drain -> #[napi] cold_cache_drain) invoked on clean shutdown. - #1 (docs): documented that LRU eviction is chain-unaware and can strand prefix chains under quota pressure (fail-open; common case unaffected) — chain-aware eviction is a follow-up. Gate: cold_cache 35/0 · typecheck exit0 · dashboard 187 tests/9 files · lint 0-err · UI build 0. (build:native's metallib floor gate fails on a known toolchain/env hazard, independent of this branch — no shader/build files touched.)
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…ounded model walk, no-follow install preflight, genuine-child session metrics Fix-delta review of the review-#14 fixes surfaced 5 edge-case tails (each a refinement of an R14 fix, none a reverted/broken fix). All verified via control-flow analysis before fixing. Native (crates/mlx-paged-attn/src/cold_cache.rs): - #5 honest drain durability: the writer discarded persist_block errors and acked the barrier unconditionally, so drain() reported success even when a covered block failed to persist. Barrier now carries SyncSender<bool>; the writer accumulates a per-window failure flag and acks !failed; drain returns true only on an Ok(true) ack. - #4 whole-drain bounded by timeout: drain's blocking send admitted the barrier before recv_timeout, so a full queue behind a stuck fsync could exceed the timeout or hang exit. Admission is now deadline-aware try_send (recovering the barrier job on Full) plus a remaining-time recv_timeout — total wall time is bounded by the timeout. Dashboard: - #1 (models.ts walkDirStats): readdirSync eagerly materialized the whole Dirent[] before the entry budget applied. Switched to incremental opendirSync/readSync so the budget bounds memory and time on a pathological high-fan-out directory; closeSync in finally. - #2 (download.ts processJob): existsSync(finalDir) follows symlinks, so a dangling finalDir symlink bypassed the ownership preflight and triggered a full wasted download. Added a no-follow occupied() (lstatSync) used at the preflight and both publish-time ownership checks. - #3 (api.ts handleSessionMetrics): the session_id = ? OR root_session_id = ? union resurrected an abandoned root turn (dropped from turns after a branch) as a fake delegated turn, inflating count/token totals. Narrowed to genuine children (root_session_id = ? AND session_id != ?); verified the child-trace discriminator against the metrics-trace writer. Also de-flaked download.test.ts's mid-job-error case: staging cleanup runs in the job finally, after the error event the test unblocks on, so it now waits for eventual staging reclaim instead of racing that teardown. Gate: cold_cache 37/0; typecheck 0; dashboard 191/191 (×2, race gone); lint 0; clippy clean. NAPI signature unchanged — no build:native needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
… iterative FD-bounded model walk Fix-delta review of the review-#15 fixes confirmed #3/#4/#5 sound and surfaced two Important edge-case defects in the two dashboard #15 fixes. Both verified via control-flow analysis before fixing. - A (models.ts isDownloaderOwned): the ownership marker was read with readFileSync, which FOLLOWS symlinks, so a live finalDir symlink into an external directory carrying a valid .mlx-download-complete.json was reported downloader-owned — slipping a foreign install past the no-follow occupancy preflight (a wasted overwrite, or a false "done" through the link). R15 #2 hardened only the occupancy check. Now isDownloaderOwned lstat-gates on a REAL directory before reading the marker, so ANY symlink/non-directory (dangling OR live-into-a-marked-dir) is foreign at all three ownership call sites. - B (models.ts walkDirStats): R15 #1's incremental opendirSync recursed into a child while the parent Dir handle was still open, holding one fd per level; a deep real-directory tree under a low RLIMIT_NOFILE hit EMFILE on a child opendirSync, which was swallowed with truncated=false — a silent false-complete undercount. Rewrote the walk as an explicit worklist of directory paths: each handle is closeSync'd before any child is opened (at most one fd ever live), and opendirSync/readSync failures now set truncated=true (a lower bound) instead of a silent return. Budget, no-follow symlink semantics, dev:ino cycle guard, and the exactly-maxEntries boundary are preserved. Gate: typecheck 0; dashboard 198/198 (×2); lint 0. Dashboard-only — native (#4/#5) and api.ts (#3) unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…nload identity, session safety Six confirmed defects from the final whole-branch review (each control-flow verified + probed before fixing); one finding refuted, one documented residual. - #6 metrics (regression from R11): the trace-only token UNION projected GROSS traces.prompt_tokens into the `input` column while turns store NET (prompt-cacheRead), double-counting cached tokens. Clamp the trace side to MAX(prompt-cached,0) in tokensByDay + turnTotals; fixture totals corrected. - #4 downloads: gate the install-skip on the completion marker's repo+revision (new readCompletion helper), not mere presence — a different revision of the same slug no longer reports a false `done`; mismatch falls through to the owned-swap. - #7 trace ingest: move the vanished-file row reconciliation BEFORE the ingest loop so a renamed trace file's globally-unique rows are freed before the new name is ingested (were permanently dropped); watermark reconcile stays after. - #8 session ingest: reject a non-object top-level entry (a `header\nnull\n` file) in isValidSessionTopology before dereferencing .type, routing it to the existing quarantine path instead of a warn-only catch that retained stale rows. - #3 session rename: refuse (409) renaming a session whose file was modified within a liveness window — pi has no cross-process lock, so appending to a file a live agent is extending would race that turn. Documented best-effort. - #5 cold cache: publish the index entry before the directory fsync so a post-rename dir-fsync failure keeps in-process accounting consistent with the on-disk file (rebuild_index already self-heals on restart). - #2 cold-tier fingerprint→materialize window: documented as an accepted v1 residual (off-by-default, adversary-owns-files; fd-coupled hashing out of scope). - #1 (train KV poisoning): refuted — training never uses the paged/cold path. Gate: typecheck exit0 · cold_cache 33/0 · dashboard 8 suites/175 · lint 0-err.
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…sion discovery, cold-cache durability Seven confirmed defects from the final whole-branch review (each control-flow verified + probed); one documented as an accepted fail-open residual. - #8 (ui): resume commands interpolated the session path into a shell string raw — a real injection ($()/backticks/&& fired in a probe) and a break on any path with a space. Added a shared POSIX shQuote used at both resume sites. - #5 (models): walkDirStats followed symlinks with statSync and recursed into symlinked directory targets unbounded — two links to '.' hit ~2^31 stats and froze the synchronous /api/models. Now recurses only real dirs (Dirent lstat semantics), with a dev:ino visited-set and an entry budget. - #4 (ingest): pi with an explicit --session-dir writes sessions flat as <dir>/*.jsonl, but the scanner only accepted <root>/--cwd--/*.jsonl, so the documented shared --session-dir flow produced an empty dashboard. Now scans both layouts (cwd read from each file's header). - #6 (download): an unowned (marker-less) install was refused only inside publish, after a full multi-GB download+hash. Preflight ownership up front (after staging recovery, before any hub work) so a doomed job moves no bytes; reworded errors to drop the unexposed overwrite advice. - #7 (metrics): per-session metrics omitted delegated (subagent) turns — session-scoped trace-only union with MAX(prompt-cached,0) net clamp + trace_id dedup, mirroring the global overview fix, so counts/tokens match the shown child badge + throughput. - #2 (cold cache): the capture loop kept blitting Metal + persisting descendants after a queue-drop; now stops on the first non-Ok(true), keeping the enqueued set a contiguous restorable prefix. - #3 (cold cache): mlx agent -p exited with accepted blocks still queued (no drain). Added a bounded native drain barrier (WriteJob::Barrier + ColdCacheManager::drain -> #[napi] cold_cache_drain) invoked on clean shutdown. - #1 (docs): documented that LRU eviction is chain-unaware and can strand prefix chains under quota pressure (fail-open; common case unaffected) — chain-aware eviction is a follow-up. Gate: cold_cache 35/0 · typecheck exit0 · dashboard 187 tests/9 files · lint 0-err · UI build 0. (build:native's metallib floor gate fails on a known toolchain/env hazard, independent of this branch — no shader/build files touched.)
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…ounded model walk, no-follow install preflight, genuine-child session metrics Fix-delta review of the review-#14 fixes surfaced 5 edge-case tails (each a refinement of an R14 fix, none a reverted/broken fix). All verified via control-flow analysis before fixing. Native (crates/mlx-paged-attn/src/cold_cache.rs): - #5 honest drain durability: the writer discarded persist_block errors and acked the barrier unconditionally, so drain() reported success even when a covered block failed to persist. Barrier now carries SyncSender<bool>; the writer accumulates a per-window failure flag and acks !failed; drain returns true only on an Ok(true) ack. - #4 whole-drain bounded by timeout: drain's blocking send admitted the barrier before recv_timeout, so a full queue behind a stuck fsync could exceed the timeout or hang exit. Admission is now deadline-aware try_send (recovering the barrier job on Full) plus a remaining-time recv_timeout — total wall time is bounded by the timeout. Dashboard: - #1 (models.ts walkDirStats): readdirSync eagerly materialized the whole Dirent[] before the entry budget applied. Switched to incremental opendirSync/readSync so the budget bounds memory and time on a pathological high-fan-out directory; closeSync in finally. - #2 (download.ts processJob): existsSync(finalDir) follows symlinks, so a dangling finalDir symlink bypassed the ownership preflight and triggered a full wasted download. Added a no-follow occupied() (lstatSync) used at the preflight and both publish-time ownership checks. - #3 (api.ts handleSessionMetrics): the session_id = ? OR root_session_id = ? union resurrected an abandoned root turn (dropped from turns after a branch) as a fake delegated turn, inflating count/token totals. Narrowed to genuine children (root_session_id = ? AND session_id != ?); verified the child-trace discriminator against the metrics-trace writer. Also de-flaked download.test.ts's mid-job-error case: staging cleanup runs in the job finally, after the error event the test unblocks on, so it now waits for eventual staging reclaim instead of racing that teardown. Gate: cold_cache 37/0; typecheck 0; dashboard 191/191 (×2, race gone); lint 0; clippy clean. NAPI signature unchanged — no build:native needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
… iterative FD-bounded model walk Fix-delta review of the review-#15 fixes confirmed #3/#4/#5 sound and surfaced two Important edge-case defects in the two dashboard #15 fixes. Both verified via control-flow analysis before fixing. - A (models.ts isDownloaderOwned): the ownership marker was read with readFileSync, which FOLLOWS symlinks, so a live finalDir symlink into an external directory carrying a valid .mlx-download-complete.json was reported downloader-owned — slipping a foreign install past the no-follow occupancy preflight (a wasted overwrite, or a false "done" through the link). R15 #2 hardened only the occupancy check. Now isDownloaderOwned lstat-gates on a REAL directory before reading the marker, so ANY symlink/non-directory (dangling OR live-into-a-marked-dir) is foreign at all three ownership call sites. - B (models.ts walkDirStats): R15 #1's incremental opendirSync recursed into a child while the parent Dir handle was still open, holding one fd per level; a deep real-directory tree under a low RLIMIT_NOFILE hit EMFILE on a child opendirSync, which was swallowed with truncated=false — a silent false-complete undercount. Rewrote the walk as an explicit worklist of directory paths: each handle is closeSync'd before any child is opened (at most one fd ever live), and opendirSync/readSync failures now set truncated=true (a lower bound) instead of a silent return. Budget, no-follow symlink semantics, dev:ino cycle guard, and the exactly-maxEntries boundary are preserved. Gate: typecheck 0; dashboard 198/198 (×2); lint 0. Dashboard-only — native (#4/#5) and api.ts (#3) unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…nload identity, session safety Six confirmed defects from the final whole-branch review (each control-flow verified + probed before fixing); one finding refuted, one documented residual. - #6 metrics (regression from R11): the trace-only token UNION projected GROSS traces.prompt_tokens into the `input` column while turns store NET (prompt-cacheRead), double-counting cached tokens. Clamp the trace side to MAX(prompt-cached,0) in tokensByDay + turnTotals; fixture totals corrected. - #4 downloads: gate the install-skip on the completion marker's repo+revision (new readCompletion helper), not mere presence — a different revision of the same slug no longer reports a false `done`; mismatch falls through to the owned-swap. - #7 trace ingest: move the vanished-file row reconciliation BEFORE the ingest loop so a renamed trace file's globally-unique rows are freed before the new name is ingested (were permanently dropped); watermark reconcile stays after. - #8 session ingest: reject a non-object top-level entry (a `header\nnull\n` file) in isValidSessionTopology before dereferencing .type, routing it to the existing quarantine path instead of a warn-only catch that retained stale rows. - #3 session rename: refuse (409) renaming a session whose file was modified within a liveness window — pi has no cross-process lock, so appending to a file a live agent is extending would race that turn. Documented best-effort. - #5 cold cache: publish the index entry before the directory fsync so a post-rename dir-fsync failure keeps in-process accounting consistent with the on-disk file (rebuild_index already self-heals on restart). - #2 cold-tier fingerprint→materialize window: documented as an accepted v1 residual (off-by-default, adversary-owns-files; fd-coupled hashing out of scope). - #1 (train KV poisoning): refuted — training never uses the paged/cold path. Gate: typecheck exit0 · cold_cache 33/0 · dashboard 8 suites/175 · lint 0-err.
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…sion discovery, cold-cache durability Seven confirmed defects from the final whole-branch review (each control-flow verified + probed); one documented as an accepted fail-open residual. - #8 (ui): resume commands interpolated the session path into a shell string raw — a real injection ($()/backticks/&& fired in a probe) and a break on any path with a space. Added a shared POSIX shQuote used at both resume sites. - #5 (models): walkDirStats followed symlinks with statSync and recursed into symlinked directory targets unbounded — two links to '.' hit ~2^31 stats and froze the synchronous /api/models. Now recurses only real dirs (Dirent lstat semantics), with a dev:ino visited-set and an entry budget. - #4 (ingest): pi with an explicit --session-dir writes sessions flat as <dir>/*.jsonl, but the scanner only accepted <root>/--cwd--/*.jsonl, so the documented shared --session-dir flow produced an empty dashboard. Now scans both layouts (cwd read from each file's header). - #6 (download): an unowned (marker-less) install was refused only inside publish, after a full multi-GB download+hash. Preflight ownership up front (after staging recovery, before any hub work) so a doomed job moves no bytes; reworded errors to drop the unexposed overwrite advice. - #7 (metrics): per-session metrics omitted delegated (subagent) turns — session-scoped trace-only union with MAX(prompt-cached,0) net clamp + trace_id dedup, mirroring the global overview fix, so counts/tokens match the shown child badge + throughput. - #2 (cold cache): the capture loop kept blitting Metal + persisting descendants after a queue-drop; now stops on the first non-Ok(true), keeping the enqueued set a contiguous restorable prefix. - #3 (cold cache): mlx agent -p exited with accepted blocks still queued (no drain). Added a bounded native drain barrier (WriteJob::Barrier + ColdCacheManager::drain -> #[napi] cold_cache_drain) invoked on clean shutdown. - #1 (docs): documented that LRU eviction is chain-unaware and can strand prefix chains under quota pressure (fail-open; common case unaffected) — chain-aware eviction is a follow-up. Gate: cold_cache 35/0 · typecheck exit0 · dashboard 187 tests/9 files · lint 0-err · UI build 0. (build:native's metallib floor gate fails on a known toolchain/env hazard, independent of this branch — no shader/build files touched.)
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
…ounded model walk, no-follow install preflight, genuine-child session metrics Fix-delta review of the review-#14 fixes surfaced 5 edge-case tails (each a refinement of an R14 fix, none a reverted/broken fix). All verified via control-flow analysis before fixing. Native (crates/mlx-paged-attn/src/cold_cache.rs): - #5 honest drain durability: the writer discarded persist_block errors and acked the barrier unconditionally, so drain() reported success even when a covered block failed to persist. Barrier now carries SyncSender<bool>; the writer accumulates a per-window failure flag and acks !failed; drain returns true only on an Ok(true) ack. - #4 whole-drain bounded by timeout: drain's blocking send admitted the barrier before recv_timeout, so a full queue behind a stuck fsync could exceed the timeout or hang exit. Admission is now deadline-aware try_send (recovering the barrier job on Full) plus a remaining-time recv_timeout — total wall time is bounded by the timeout. Dashboard: - #1 (models.ts walkDirStats): readdirSync eagerly materialized the whole Dirent[] before the entry budget applied. Switched to incremental opendirSync/readSync so the budget bounds memory and time on a pathological high-fan-out directory; closeSync in finally. - #2 (download.ts processJob): existsSync(finalDir) follows symlinks, so a dangling finalDir symlink bypassed the ownership preflight and triggered a full wasted download. Added a no-follow occupied() (lstatSync) used at the preflight and both publish-time ownership checks. - #3 (api.ts handleSessionMetrics): the session_id = ? OR root_session_id = ? union resurrected an abandoned root turn (dropped from turns after a branch) as a fake delegated turn, inflating count/token totals. Narrowed to genuine children (root_session_id = ? AND session_id != ?); verified the child-trace discriminator against the metrics-trace writer. Also de-flaked download.test.ts's mid-job-error case: staging cleanup runs in the job finally, after the error event the test unblocks on, so it now waits for eventual staging reclaim instead of racing that teardown. Gate: cold_cache 37/0; typecheck 0; dashboard 191/191 (×2, race gone); lint 0; clippy clean. NAPI signature unchanged — no build:native needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
Brooooooklyn
added a commit
that referenced
this pull request
Jul 28, 2026
… iterative FD-bounded model walk Fix-delta review of the review-#15 fixes confirmed #3/#4/#5 sound and surfaced two Important edge-case defects in the two dashboard #15 fixes. Both verified via control-flow analysis before fixing. - A (models.ts isDownloaderOwned): the ownership marker was read with readFileSync, which FOLLOWS symlinks, so a live finalDir symlink into an external directory carrying a valid .mlx-download-complete.json was reported downloader-owned — slipping a foreign install past the no-follow occupancy preflight (a wasted overwrite, or a false "done" through the link). R15 #2 hardened only the occupancy check. Now isDownloaderOwned lstat-gates on a REAL directory before reading the marker, so ANY symlink/non-directory (dangling OR live-into-a-marked-dir) is foreign at all three ownership call sites. - B (models.ts walkDirStats): R15 #1's incremental opendirSync recursed into a child while the parent Dir handle was still open, holding one fd per level; a deep real-directory tree under a low RLIMIT_NOFILE hit EMFILE on a child opendirSync, which was swallowed with truncated=false — a silent false-complete undercount. Rewrote the walk as an explicit worklist of directory paths: each handle is closeSync'd before any child is opened (at most one fd ever live), and opendirSync/readSync failures now set truncated=true (a lower bound) instead of a silent return. Budget, no-follow symlink semantics, dev:ino cycle guard, and the exactly-maxEntries boundary are preserved. Gate: typecheck 0; dashboard 198/198 (×2); lint 0. Dashboard-only — native (#4/#5) and api.ts (#3) unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKtaz9j1ih1YtSPBf5TtQm
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.

No description provided.