feat(dashboard): add first-run model onboarding - #149
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1de34879-b3aa-4582-9d27-157fc4da1030) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd42c572de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_62c418d4-6b0d-4ad3-b491-305c97544707) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d52d7466
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6d070438-33b7-4992-9b33-9f7768ad30fd) |
`lfm2_paged_budget_forced_warm_continue_parity` reddened `main` on three consecutive runs (#148, #149, #150) with one signature: the warm paged continue and the flat carried-state oracle agreed for 23 bytes and then split at the first content word after `<think>` ("keep" vs "what"), `first_diff_byte=Some(23)` identical on every run, while the same commit passes on a developer host. The failing run's own log rules out the carry path and the fixture: `turn1` and `FLAT turn1` are both `"</think> Ok"`, and `cached_tokens` (23) and `num_tokens` (32) match on both sides. What split is the first *free-form* decision inside the thinking span — exactly the kind of near-tie the accepted ~1-ULP paged-vs-flat kernel-stack class flips (`models/lfm2/attention.rs`, plus this file's own "deliberately NOT asserted" note for long generations). Byte parity was being asserted over a coin flip. Fix: run turn 2 budget-forced like turn 1, so both arms commit the same forced `</think>` first and the compared tokens are answer-region ones. Byte parity stays fatal; nothing is weakened. Also asserted now, because they were missing: the arms' turn-1 outputs are equal (turn 1 is cold, so a mismatch is a fixture lottery and the message says so instead of sending a reader into the continue path), the reuse lengths are equal, and the committed token counts are equal (the qwen3 sibling already asserts that one). Verified: gate passes locally with both arms byte-identical (`[PASS] … warm cached=23, tokens=32`); the count invariant was confirmed to fire by temporarily shortening the flat oracle's turn 2 — "the arms committed different token counts (warm=32 flat=31)".
…ns (#152) ## The problem, corrected `main` failed `e2e lfm2` on three consecutive runs (#148, #149, #150), and it is **not** a random flake — it is host-dependent determinism: ``` [warm-cont] turn1: num_tokens=2 finish=length raw_text="</think> Ok" [warm-cont] FLAT turn1: num_tokens=2 finish=length raw_text="</think> Ok" ← arms identical before the boundary [warm-cont] turn2 WARM: num_tokens=32 cached_tokens=23 finish=length [warm-cont] turn2 FLAT oracle: num_tokens=32 cached_tokens=23 finish=length ← same reuse, same length WARM raw_text="<think> Okay, let's see. The user said to keep going from where I left off, but …" FLAT raw_text="<think> Okay, let's see what the user is asking here. They said, \"Now …" ``` `first_diff_byte=Some(23)` is identical on all three red runs, and the same commit passes locally. So the carry path is fine, the arms are comparable, and what split is the first **free-form** decision inside the thinking span — the near-tie that this file already documents as flippable by the accepted ~1-ULP paged-vs-flat kernel-stack class (`models/lfm2/attention.rs`, and the *"Deliberately NOT asserted: byte-equality of the answer texts"* note on the memory-probe test). Byte parity was being asserted over a coin flip. I first tried downgrading that assertion to a diagnostic; an adversarial review rightly rejected it — *"a stale or incorrect conv tensor can preserve turn-1 output, reuse 23 tokens, and generate 32 incorrect tokens"*, so invariants cannot replace the bite. This is the reviewer's own recommended alternative instead. ## The change (one file) - **Turn 2 is now budget-forced, like turn 1.** Both arms therefore commit the same forced `</think>` first and the compared tokens are answer-region ones, not a sentence-opening word choice. The boundary under test (turn-1/turn-2 carry, `ContinuedLivePrefix`, 23-token prefix) is untouched. - **Byte parity stays fatal** — the assertion is unchanged in strength. - **Three invariants added**, which the fixture was missing: the arms' turn-1 outputs must be equal (turn 1 is cold, so a mismatch is a fixture lottery, and the message says so instead of sending a reader into the continue path), the reuse lengths must be equal, and the committed token counts must be equal (the qwen3 sibling already asserts that one). ## Verification | Check | Result | | --- | --- | | Targeted test locally | `[PASS] … (warm cached=23, tokens=32)`, both arms byte-identical | | Whole gate file locally | 9 passed, 0 failed | | New count invariant actually fires | confirmed by temporarily shortening the flat oracle's turn 2 → `the arms committed different token counts (warm=32 flat=31)` | | `cargo fmt --all` | touched only this file | | **On the affected (CI) host** | CI dispatched on this branch: [run 34939712245](https://github.com/mlx-node/mlx-node/actions/runs/34939712245) — result will be posted here | ## What is still not proven The compared region is now answer-region, but a near-tie there is still possible in principle, so this is a **fixture pin validated on the affected host**, not a proof that the split was harmless rounding. The durable instrument is the one `models/qwen3/model.rs` uses for its mixed-step gate: logits at turn-2's first forward within `test_support::bf16_scaled_tolerance`, plus `top - second > 2 × tolerance`. That needs a logits readout a test can reach — `ChatResult` carries none and `Lfm2Inner` is crate-private — so it is a follow-up, not something to fake with a looser text comparison here. This clears the `model-test` gate that has been skipping `Publish` on `main`.
New users with an empty model library now open a guided model chooser before entering the workspace. Three portrait cards present the available models, and a persistent bottom panel keeps the download action and progress visible while the page scrolls.
The page reuses the existing model download, cancellation, retry, and completion handling. Completing setup opens Coding Agents; skipping is remembered per model library, with links to return from Overview and the Coding Agents empty state. Exit controls wait for the library lookup to succeed so an early click cannot lose the dismissal and send the user back to onboarding. The shared catalog also now names Qwen AgentWorld 35B A3B correctly.
Validation
git diff --checkpassed.Note
Medium Risk
New default routing and first-launch redirects affect every empty-library user and interact with downloads and localStorage dismissal, though behavior is heavily covered by tests and reuses existing download APIs.
Overview
Adds a first-run onboarding flow at
/welcomefor users with an empty model library who have not skipped setup. Opening/shows a boot state, then redirects to the welcome experience (no sidebar) where they pick a catalog model, download via the existing pipeline, and can continue to Coding Agents or dismiss per library path inlocalStorage.The Models page gains an
onboardingmode that renders the new Onboarding UI and surfaces per-repo download errors; catalog install buttons get async “starting” / custom settling labels. Coding Agents and Overview now point “install / choose first model” CTAs at/welcomeinstead of/models. App routing registers/welcomeoutside the main shell layout.Renames the catalog display label Qwen-AgentWorld-35B → Qwen-AgentWorld-35B-A3B. Test harness
deferredcan gate delayed API failures; a large onboarding test suite covers redirects, dismissal, delayed library load, and download edge cases.Reviewed by Cursor Bugbot for commit ed93ed3. Bugbot is set up for automated code reviews on this repo. Configure here.