Chat UI: shadcn/ui layout, a truthful loading state, config-by-URL, and a slot-filled settings dialog - #15
Merged
Merged
Conversation
Nothing in this app rendered a component under test before. The UI work needs a red/green loop that does not launch a browser. Global jsdom environment (vs per-file docblocks): switching environment from "node" to "jsdom" globally did not break idb-stores.test.ts (fake-indexeddb) or any of the pure-Node tests — all 124 pre-existing tests plus the new one pass. No per-file override was needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Against a local model, prompt processing can make 'sent, nothing back yet' last 30s. One spinner cannot tell that apart from a hung request, and neither can the person watching it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adopted for the accessibility the hand-rolled Modal never had: the dialog now traps focus and closes on Escape, and the test asserts it. The shadcn CLI (v4.21, Nova preset) works against Tailwind 4 + Vite + React 19, but its `init` pulls in a whole opinionated design system (Geist font via @fontsource, tw-animate-css, `shadcn` itself as a runtime dependency, a third-party `cn` package) and writes components to src/components/ui, not the src/ui/primitives the brief calls for. Reverted that init and hand-installed the nine primitives from the canonical shadcn/Radix source instead: real @radix-ui/* packages, clsx + tailwind-merge for cn(), class-variance-authority for variants, lucide-react for icons — matching what the brief named. A `@/*` path alias was required for shadcn-style imports (tsconfig paths, plus a resolve.alias in vite.config.ts and vitest.config.ts); TypeScript here has dropped `baseUrl`, so `paths` alone does the job. theme.css carries the light/ dark CSS variables in Tailwind 4's CSS-first form, imported from styles.css, which stays the single Vite entry. A package-local biome.json (extends the root config by relative path; the `//` monorepo-root shorthand didn't resolve here) enables the CSS parser's tailwindDirectives for @theme/@apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
useSyncExternalStore over observe/getSnapshot, so a panel registered after the dialog mounts appears without a remount. getSnapshot on a keyed slot returns a ReadonlyMap; the derived T[] is cached per Map reference so the array identity only changes when the bus actually notifies, not on every render. Extracted to a package when a second app needs it, not before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dialog imports no panel and nothing from src/mesh -- a test enforces it. That is what lets index.html show two tabs and mesh.html four, with no branch anywhere deciding it. ConnectionPanel and ModelsPanel take the old SettingsDialog/ModelDialog content and are registered into settingsPanelsSlot by ChatApp for now (Task 6 moves the registration up to an app shell). Deleting Modal.tsx/ModelDialog.tsx touched five more importers: Thread.tsx and pages/mesh.tsx now source their button/input classes from the new ui/button-styles.ts, and mesh/member-key.tsx moved its own dialog onto the shadcn Dialog primitive instead of keeping a second implementation around. startupStep's non-dismissible first-run gating is preserved (now driving one dialog instead of two) and covered by a new chat-app.test.tsx, since no prior test exercised ChatApp's render tree; that render also needed a ResizeObserver polyfill in tests/setup.ts that assistant-ui's Thread requires under jsdom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
startupStep("models") kept the dialog non-dismissible but SettingsDialog
always defaulted to the first panel (Connection) -- a user who just saved
their base URL landed back on the form they finished, not on Models.
SettingsDialog takes an optional initialPanelId (a plain panel id, never a
concept like "models", so the dialog still imports no panel): resolved fresh
on every render against the currently registered panels, falling back to the
first panel when absent or when the id names a panel that isn't registered
(yet, or ever) -- a panel can register after the dialog has mounted. ChatApp
passes MODELS_PANEL_ID when startupStep === "models", using the same id
constant it registers the panel under.
Both new settings-dialog.test.tsx cases (opens on a registered id; falls back
for an unregistered one) and the extended chat-app.test.tsx "models" case
(asserting the Models tab is aria-selected) failed against the prior code
before this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ree once AppShell takes no chat concept, so the mesh page reuses it unchanged. The breakpoint itself is Playwright's to prove, not jsdom's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…psed counter Thread no longer takes a ChatController; it takes ChatState, now, and plain callbacks, so it renders and asserts standalone. Three phases replace the old ThreadPrimitive.If running split: idle (nothing), waiting (a placeholder bubble, data-testid="waiting-indicator", aria-live="polite", elapsed whole seconds, Stop), streaming (partial text, Stop stays). thread-adapter.ts derives isRunning from phase (isRunningFor) so phase is the one thing passed through to assistant-ui's runtime, not a duplicate field. Composer, previously inline in Thread's own tree, is now its own component wired through AppShell's composer slot -- the sticky, safe-area-padded footer (Task 6) finally reaches the real app instead of sitting unused. Watched against scripts/fake-llm.mjs with an opt-in FIRST_TOKEN_DELAY_MS: a 6s wait shows a ticking 0s..5s counter and a live Stop button, not a hang; streaming replaces the placeholder the instant the first token lands; Stop mid-wait aborts immediately and clears the indicator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix round 1/5 for Task 7. Actually ran `pnpm dlx shadcn@latest add "https://r.assistant-ui.com/thread" --overwrite` against React 19.3.0 and @assistant-ui/react 0.15.19 (both unforced) instead of reasoning about it from outside. It installs, but its output conflicts on every other axis: resurrects the `cn`-npm-package and `radix-ui`-meta-package choices Task 3 deliberately rejected (radix-ui lands alongside the individual @radix-ui/react-* packages, not instead of them), rewrites button.tsx/ dialog.tsx/tooltip.tsx which this task doesn't own, and pulls unrelated dependencies and 17 files for attachment/reasoning/tool-call/branch-picker surface this text-only OpenAI-compatible chat has no use for. Reverted the experiment in full (working tree confirmed clean, node_modules resynced), kept the hand-rolled Thread/Composer, and recorded the attempt and the concrete conflicts in Thread.tsx's header comment so the next reader doesn't re-litigate it from scratch. No dependency or version changed; 180/180 still green, typecheck and build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lue winning A user who typed an endpoint does not have it replaced on the next load. A static host's 200-with-an-HTML-index is rejected where it happens, not three hops later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same rule discover.ts already applies to the hub's service document: a URL the app obeys redirects every message and every key. Origin comparison, never a string prefix -- the @-userinfo and suffix cases are in the suite. Once a mesh edge is known, plain same-origin is not enough either: the edge's host serves every peer's mount side by side, so only the specific edge path is trusted, matching discover.ts's "another peer is not your hub" rule. An RFC 3986 §4.2 guard (colon in a relative reference's first segment) keeps "::::" distrusted without ever needing to throw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On load each page reads ?config=: judgeConfigUrl (Task 9) decides trust against the page URL and, on mesh.html, the discovered edge base -- mesh.html only mounts ChatApp once discovery has succeeded, so a ?config= arriving earlier is naturally held rather than fetched. Trusted URLs fetch, parse, resolveConfig and save without asking. Untrusted ones go through the new ConfirmConfigDialog; accept does the same as trusted, reject leaves the config untouched and says so. Any fetch/parse failure is a visible, non-fatal notice naming the URL, falling back to the saved config or the settings screen. Cancel is the dialog's default focused action and Escape rejects, because its job is to stop something, not to be dismissed out of the way. ExternalConfig.defaultModel, parsed since Task 8 but dropped by applyEndpoint, is now fed as the candidate default to refreshModels right after GET /models -- the same function the model picker's "Refresh" already uses. The dialog never names the WHATWG opaque origin "null"; callers substitute the raw URL. config-trust.ts also now refuses %2f/%5c in a mesh-edge candidate path, since RFC 3986 means new URL() never collapses them the way a downstream decoder might. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… hub's endpoint Fix round 1/5 for Task 10's review. mesh.tsx's discover() always saves the hub-discovered endpoint before ChatApp mounts, so resolveConfig's "saved wins" rule already made ?config= inert on the mesh page -- confirmed deliberate, not accidental: a document that could redirect the chat away from the discovered hub is exactly the attack discover.ts's "trust only the hub" rule and the edge-base same-origin-skip exist to prevent. Add a test pinning that precedence (pre-populated configStore + edgeBase set, a same-edge document with a different baseUrl, endpoint unchanged), and a comment at the resolveConfig call site explaining why, not just that. ?config= remains a standalone-page mechanism; the mesh page always prefers what it discovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
index.html shows two tabs and mesh.html four, with no branch deciding it and no mesh code reachable from the standalone entry point -- asserted by closure. ChatApp gains an optional `slots` prop so mesh.tsx's registerMeshPanels can contribute into the same dialog Connection and Models already fill, instead of the mesh page's own headerExtra render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Works without a mesh" asserted as no ServiceWorker and no request to any
other host -- not merely that the page renders. Seven assertions against a
real Chromium driving dist/index.html: a non-dismissible first-run settings
screen, the fake endpoint's models, a delayed-first-token waiting indicator
with an elapsed counter, Stop-during-wait leaving the session intact, the
two-tab (Connection/Models) settings dialog, no ServiceWorker/no stray
requests, and zero page errors or unhandled rejections.
fake-llm.mjs's CLI entry point gains --first-token-delay-ms, reconciled with
Task 7's FIRST_TOKEN_DELAY_MS env var rather than adding a second mechanism
-- both feed the same startFakeLlm({ firstTokenDelayMs }) option the new
spec calls directly.
scripts/smoke.mjs had known stale drift from Task 5's slot-filled tabbed
Settings dialog (it still targeted a dead two-dialog "Connection
settings"/"Choose a model" flow with a "Change connection" button); updated
its selectors to the current single dialog so it stays a useful, passing
regression check alongside this new acceptance spec rather than bitrotting
further.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, and a live-mesh spec Ten assertions at 1280x800 and 390x844 (responsive.spec.mjs), including no horizontal scroll and a composer that survives focus. jsdom asserts none of this and never claimed to. The first real run failed in the test harness itself (waiting on "New chat", which is legitimately hidden at a phone viewport) rather than in AppShell; fixing that surfaced a genuine bug while stress-testing assertion 5 with a long session title: ScrollArea's Viewport wraps children in an inline `display:table` div that defeats `truncate`/`min-w-0` below it, so a long title pushed its delete button out of the mobile Sheet drawer with no page-level horizontal scroll to show for it. Fixed with `min-w-0` on the row (ThreadList.tsx) and a targeted `!important` override for Radix's inline style (styles.css), and pinned by strengthening assertion 5 to check that a long-titled row's own delete button stays on screen, not just that the thread-list landmark is visible. mesh.spec.mjs promotes scripts/mesh-smoke.mjs, which had drifted onto the dead "Choose a model" dialog and a `?join=` link. The new spec joins by pasting a blob into the join form (never `?join=`), confirms the settings dialog's four tabs (Connection, Models, Sharing, Keys), mints a key for a member from the Keys panel, and waits for a genuinely streamed reply -- run and passing against the live local appliance at worktrees/llm-appliance-local, minting its own admin invitation via that appliance's own bin/invite.sh rather than a hardcoded door URL (the old script's HUB_DOOR_URL defaulted to :8080, which on this box is someone else's SSH tunnel to production, not this appliance). Skips with a clear message when no appliance is reachable. mesh-smoke.mjs is now a one-line forward so the two can't drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Audited against the code before merge: README.md written from scratch (none existed), covering index.html vs mesh.html, the ?config= document shape / resolution order / trust rule (foreign origin confirmed not refused; the mesh page's discovered endpoint always wins by design), the settings-dialog slot contract (SettingsPanel shape, register(decl, id, value) is 3-arg, getSnapshot returns a ReadonlyMap, useSlot returns an array), the three run phases and why `waiting` is its own phase, and how to run each test layer including the mesh spec's live-appliance requirement and clean skip. Doc-comment sweep against `git diff origin/main --stat`: two stale comments found and fixed. ChatApp.tsx's `headerExtra` prop still claimed to be "the mesh page's link status and links", but mesh.tsx stopped passing it once Sharing/Keys became settings-dialog tabs and the pre-chat header grew its own links -- neither shipped page passes it today. button-styles.ts claimed Thread.tsx's composer as a consumer; Thread's composer moved onto shadcn primitives and no longer imports it -- mesh.tsx's pre-chat forms are the only consumer left. `config.ts`'s "one config per page" and `discover.ts`'s trust-rule header were checked and are still accurate; left alone. Three of the four deferred findings closed rather than recorded: added tests/member-key.test.tsx (accessible name + Escape close path for the shadcn Dialog migration that only typecheck/build covered before); added a config-trust.test.ts case pinning judgeConfigUrl's opaque origin "null" and a chat-app.test.tsx case proving ChatApp substitutes the raw URL for display, never "null"; annotated styles.css's global `!important` override with a note for whoever adds ScrollArea's second consumer. The fourth (theme.css/ThreadList's XIcon/mesh-session.ts's configRef idiom) was left alone, as instructed, since none of those files were touched. Bundles (dist/, confirmed by content grep, not just chunk names): index.html loads styles-*.js (759.6 kB/231.7 kB gzip, React+assistant-ui+ app code, zero @statewalker/httpeers-* and zero libp2p/multiaddr matches) plus a 0.28 kB entry, a 0.77 kB runtime chunk and 28.1 kB/6.4 kB gzip of CSS -- about 770 kB raw / 239 kB gzip total, no mesh code. mesh.html adds mesh-*.js (690.6 kB/208.0 kB gzip, confirmed to contain libp2p) plus two QR-scan chunks used only by the join widget (151.4 kB/55.1 kB gzip, 369.4 kB/108.4 kB gzip) -- about 1.91 MB raw / 610 kB gzip total. Suite: 1365 passed / 8 skipped / 0 failed assembly-wide (was 1259/8/0), apps/llm-chat at 230/20 files (was 124; +4 tests/+1 file from this task, 226/19 unchanged from Task 13). No new failures anywhere. typecheck and lint:check clean. All three e2e specs pass: standalone (7/7), responsive (10/10, no failures to diagnose this run), mesh (ran to completion against a live appliance rather than skipping -- join, four tabs, minted key, genuinely streamed reply all observed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- SettingsDialog: reset the manually-picked tab when the dialog closes, so a later `initialPanelId` (e.g. routing a user with no default model to Models) is never permanently defeated by an earlier, unrelated tab click. - Thread `fetchImpl` through to all four listModels() call sites (ModelPicker refresh, ConnectionPanel's Test, ModelsPanel's load) so the doc comment's claim actually holds. - Add unit coverage for ConnectionPanel and ModelsPanel, including that the resolved default model is marked (spec §9), previously asserted only incidentally by an e2e spec. - Pin useNow's setInterval tick in vitest with fake timers, so deleting it can no longer ship silently past a vitest-only CI run. - Stop offering ConfirmConfigDialog for an untrusted ?config= on the mesh page: its "Use this configuration" button could never do what it said, since the hub's discovered endpoint always wins there. Ignore the document with a plain notice instead; behavior is unchanged, only the promise the dialog made is fixed. Each fix has a regression test that was confirmed to fail before the fix. apps/llm-chat: 238/238 (was 230). Assembly-wide: 1373 passed / 8 skipped. standalone.spec.mjs 7/7, responsive.spec.mjs 10/10; mesh.spec.mjs skipped -- the live appliance's traefik is bound to 127.0.0.1:8080, the port this pass was instructed never to touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChatApp's fetchImpl reached the config and model calls but not the chat stream: endpointClient called streamChat without it, so the stream always used the global fetch. The elapsed-time test's never-resolving stub was therefore a real DNS lookup of llm.test, and the test failed whenever that lookup failed before the fake clock advanced 3 s -- 2 in 40 runs locally, and the one red check on PR #15. endpointClient now takes a fetch getter, which ChatApp feeds from a ref as it does the config. Production is unaffected: both pages pass the bound global fetch. A new test pins the wiring deterministically; the 40-run reproduction now passes 120/120. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Rebuilds
apps/llm-chat's UI on shadcn/ui: a layout that works on a phone, a loading state thattells the truth while a local model thinks, configuration by fetched URL with a trust rule, and a
settings dialog filled entirely through slots — while
index.htmlstill works with no mesh atall.
Implements statewalker/umbrella#36. Spec and plan:
docs/superpowers/specs/2026-09-20-llm-chat-ui-design.mdand the matching plan (umbrella repo,statewalker/umbrella#39). Independent of the appliance PR #14 — different files, no overlap.
What changed
Modal.tsxhad no focus trap and no Escapehandling; the Dialog now gets both from Radix, and a test asserts it.
idle→waiting→streaming.isRunningis now derived fromphase, so the two cannot disagree about whether a request is in flight. Against a localllama.cpp model
waitinglegitimately lasts 30 s+, and one spinner cannot tell that apart from ahung request;
waitingshows a placeholder with an elapsed counter and Stop.md, sticky composer with safe-area padding andh-dvh. The sidebar appears in the accessibility tree exactly once.?config=<url>— the app fetches a config document. Resolution: defaults < document < whatthe user saved.
discover.ts's "TRUST ONLY THE HUB" reasoning: a URL theapp obeys is a redirect of every message and any key in it.
@statewalker/shared-slots. It imports no panel. The chatcontributes Connection and Models;
mesh.htmlcontributes Sharing and Keys.index.htmlshowstwo tabs and
mesh.htmlfour, with no branch anywhere deciding it.Measured
apps/llm-chatwent 124 → 238.tests/e2e/standalone.spec.mjs7/7,tests/e2e/responsive.spec.mjs10/10 at 1280×800and 390×844,
tests/e2e/mesh.spec.mjspassing live against a real appliance with a streamedreply from a local model.
index.html's bundle contains zero httpeers/libp2p content matches (grepped by content, notchunk name): ~770 kB raw / ~239 kB gzip.
Security notes worth reading before merging
mesh.html, a?config=cannot override the endpoint discovered from the hub. That isdeliberate: otherwise a crafted link would redirect the chat to a foreign endpoint while the user
believed they were talking to the mesh they had just joined, carrying the mesh's key header. The
spec originally advertised the opposite and was amended. A test pins it, and inverting the
precedence makes exactly that test fail.
serves every peer's mount side by side — so "same-origin" would otherwise re-admit sibling peers
and
..escapes.name the
baseUrlit would use. Nothing is applied without consent, and the fetch carries nocredentials — a crafted link is a ping, not a leak. Spec §6 was amended to say this plainly.
Decisions reversed on evidence
against React 19.3.0 and
@assistant-ui/react0.15.19, and were rejected for four concreteconflicts: they rewrite three files other tasks own, install
radix-uias a meta-packagealongside the individual packages, pull 17 files of attachment/tool-call/branch-picker surface
unused by a text-only chat, and break the controller-free
Threadinterface. Recorded inThread.tsx's header with what it costs.Known gaps
fake-timer race, not a logic bug. Worth watching in CI.
ModelPicker.tsxhas no tests (pre-existing, untouched here).styles.csscarries a global!importantoverriding Radix ScrollArea's inlinedisplay:table,which defeated
truncateand clipped a long session title's delete button out of the mobiledrawer — with no page-level scroll to show for it. One consumer today; the risk of a second is
noted beside the rule.
🤖 Generated with Claude Code