Skip to content

feat(dashboard): add first-run model onboarding - #149

Merged
Brooooooklyn merged 3 commits into
mainfrom
codex/app-onboarding
Sep 14, 2026
Merged

Brooooooklyn merged 3 commits into
mainfrom
codex/app-onboarding

Conversation

@Brooooooklyn

@Brooooooklyn Brooooooklyn commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

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

  • 97 targeted tests passed across onboarding (13 tests), page behavior, coding-agent setup, and reconnection. Regression coverage includes delayed library loading, failed requests followed by retry, all three header exit actions, and persisted dismissal after remounting.
  • TypeScript build, production UI build, targeted lint/format checks, and git diff --check passed.
  • Verified the built UI in an isolated Electron window using the production protocol, preload, and dashboard runtime: first launch, model selection, skipping, persistence after reload, and returning to setup.
  • Checked 1180×800, 720×480, and 390×844 layouts; the Download button stays visible while scrolling. Download lifecycle tests use fixtures; no full model download was performed during the UI smoke check.

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 /welcome for 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 in localStorage.

The Models page gains an onboarding mode 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 /welcome instead of /models. App routing registers /welcome outside the main shell layout.

Renames the catalog display label Qwen-AgentWorld-35BQwen-AgentWorld-35B-A3B. Test harness deferred can 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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T14:52:58.654984Z ed93ed3 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 24066249-0da9-4805-8d7d-5c5081dc3303

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Sep 14, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread packages/dashboard/ui/src/pages/onboarding.tsx Outdated
@cursor

cursor Bot commented Sep 14, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread packages/dashboard/ui/src/pages/models.tsx Outdated
@cursor

cursor Bot commented Sep 14, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@Brooooooklyn
Brooooooklyn merged commit 766ea92 into main Sep 14, 2026
8 checks passed
@Brooooooklyn
Brooooooklyn deleted the codex/app-onboarding branch September 14, 2026 15:49
Brooooooklyn added a commit that referenced this pull request Sep 15, 2026
`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)".
Brooooooklyn added a commit that referenced this pull request Sep 15, 2026
…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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant