[pratikbuilds]

Compose the new chat family from the shared samples into one story, in both scroll policies: a conversation with agent steps, approvals and generated images, a prompt input with its toolbar and attachments, the conversation map and history window, and a panel with a thread tab. A browser spec drives the story to check that sending settles, approvals keep keyboard focus after they fold, and attachments move into the sent message.

Stacked on #126: review only this commit. Part of #94.

Notes for review

• e2e/full-chat.spec.ts drives the built Storybook story (the :61000 server the axe sweep already uses) rather than an e2e/fixtures.tsx route: fixtures can't import stories/, and a fixture copy would test a duplicate of the composition. It depends on the story ids.
• After a keyboard approval decision folds the call, the story moves focus to the run's ActivityTrigger (or the batch's next waiting call); asserted under settle and stick.
• Left out: the failed-send sample (covered in Chat / Message), MessageActionMenu and PromptInputContext.

Verification

• This commit passes bun run typecheck and bun run lint on its own (the stack is checked commit by commit).
• At the top of the stack: bun run check, bun run build, bun run dep-guard, bun run contrast-test and bun run test:e2e (Playwright specs and the Storybook axe sweep in light and dark) pass.

Closes #109

[TheGreatAxios]

Automated multi-agent review: independent review + second-agent validation; only confirmed items listed. The story itself is genuinely strong; the e2e spec under it has one blocking structural error.

• blocking (verified against Ladle 5.1.1 itself — package contents + routing source) e2e/full-chat.spec.ts:9-11 — the spec drives Storybook URLs (/iframe.html?id=…&viewMode=story&globals=theme:light, waits for body.sb-show-main) at a Ladle server. Ladle ships exactly one HTML file (lib/app/index.html, no iframe.html), ignores id/viewMode/globals params, has no sb-show-main class ever, and its own comment in playwright.config.ts calls it "the Ladle stories build" — so all five tests fail at the first goto, permanently, in CI. Fix: use the sibling spec's real Ladle scheme — page.goto(\${STORIES}/?story=chat-full-chat--${story}&mode=preview&theme=light`)and wait for[data-storyloaded](the story idschat-full-chat--settle/--stickare already in correct Ladle format) — then rerunbun run build && bun run test:e2e`.
• should-fix (plan gap) stories/chat/full-chat.stories.tsx:473-486 — the Sources family never composes: the story's case "sources" branch is dead code because the only sample carrying a sources part (shortChat) is never mounted, while task 15's objective is "prove every family composes". Give one mounted answer a sources part.
• minor (process) no e2e/fixtures.tsx entry, while the plan requires one entry per scenario and every sibling spec follows that contract. Defensible here (the composition lives in the story and the axe sweep hits the same server) — but say so in the PR body rather than silently dropping it.
• minor (flake) e2e/full-chat.spec.ts:133-141 — Stop is clicked on a live label that exists for a single 120 ms replay frame, from inside a page.waitForFunction(..., { polling: "raf" }) side effect; one missed frame = permanent false condition = 30 s timeout despite a clean settle. Poll read-only (expect.poll on the trigger text) and click via locator.
• nits — onOpen={(image, opener) => …} misnames an index (the prop hands over (index, opener); rename to index, :470); approved tool outputs are hardcoded "Done" (:~308) — vary or derive them; attached files are never openable (viewerItemsOf only maps image kinds and message-tile AttachmentPreview gets no onOpen — story-coverage follow-up); MODELS/MODES/attachment refusal copy is duplicated from prompt-input.stories.tsx/attachments.stories.tsx and already diverged (2 vs 3 models; MAX_FILES vs literal 20) — hoist to samples/.

[pratikbuilds]

Thanks for the review. Here's what happened to each point.

On the blocking item

This repo runs Storybook 10, not Ladle (see package.json), so the spec's iframe.html?id=… URLs are correct. They pass in CI and locally.

Fixed

1. The Sources component never appeared in the whole-chat story (should-fix)

• The fix: the story now includes the short sample, whose answer cites two sources, so the accessibility sweep covers them.
• Test: "an answer's sources show as links under it". It failed before the fix.

2. The Stop test was flaky (minor)

• The fix: the test now uses Playwright's page clock. It pauses the clock once the story has loaded, steps the replay forward one frame at a time, then clicks Stop normally. There are no more clicks from inside the page.
• Result: with the CPU slowed 4–10x, the old test failed 14 of 60 runs, and the new one passed 110 of 110.

3. Nits

• The image argument is now named index.
• An approved call's output now comes from its name instead of always being "Done".

Not changed, and why

• No e2e fixture entry: the whole composition lives in the story, and the accessibility sweep runs against that same page.
• Moving the duplicated models and modes into samples/: that would edit earlier PRs' stories.
• Making attached files openable in the story: a follow-up.

Also in this PR

A refactor(stories) commit stops the story writing a ref during render.

Commits: test(stories): compose sources in the full chat; steady the Stop test, refactor(stories): stop a closed thread's answer without a render ref

[TheGreatAxios]

Review · Request changes

Adds a story of the whole chat. Stop and unmount do clear the replay timer in the story. The e2e does not prove that.

Findings

• e2e/full-chat.spec.ts:164 — Stop can pass while the next frame is still scheduled. playTurn emits the running frame and queues the next one 120ms later (play-turn.ts:183). stopAnswer applies stopped() synchronously (full-chat.stories.tsx:1194-1205), so "1 failed", data-status="sent", and the tool's error are already true before page.clock.resume(). Resume unpauses and does not advance that 120ms, and data-status="sent" is also the natural last frame. Dropping stop.current?.() and keeping the state update still goes green if the expects finish inside the step. After Stop, runFor several REPLAY_STEP_MS and assert the tool is still error.

• e2e/full-chat.spec.ts:191 — The "4 replies" expect does not wait for the streamed answer. send appends the user reply and playTurn emits the empty streaming frame in the same turn (full-chat.stories.tsx:988-996, play-turn.ts:185). The parent count is threadReplies.length, so it becomes 4 before any timer. A playTurn that emits once and returns would still pass. Wait for the reply text before asserting the count.

[pratikbuilds]

Thanks for the review. Both tests are tightened. The story itself is unchanged, because its behaviour was already right.

1. The Stop test could pass with the next frame still scheduled (test gap)

• The fix: right after Stop, while the page clock is still paused, the test runs the clock forward by a full replay's worth of frames. Then it resumes the clock and asserts. Any frame that Stop failed to cancel now lands before the asserts.

2. The "4 replies" check didn't wait for the answer (test gap)

• The fix: before checking the count, the test waits for the thread answer's full text in the tab panel. That also shows the final frame replaces the streaming one rather than adding a fifth reply.

Tests for both: I ran both specs against a broken copy of the story. In that copy, Stop doesn't cancel the replay and the thread answer never streams past its first frame.

• The old specs passed it 10 of 10 times.
• The new specs fail it 10 of 10 times, and pass the real story 20 of 20.

Commit: test(stories): prove stop and thread replies in the full chat e2e

[pratikbuilds]

Round 3 fixes

A fresh review pass over the stack turned up the points below; each is fixed in a new commit at the end of this branch, and the stack is rebased onto it.

• 971d70d: The composer's role="status" notice used empty:hidden, taking the live region out of the accessibility tree until text arrived, so the refusal was likely not announced. It now stays mounted.
• 07082a1: restamp dated the latest messages after the story's "now" ("in 30s"). Every message is now dated before now, in order.
• 4cead3f: Sent file tiles were keyed by file name; they are keyed by position. (At the top of the stack MessageParts takes over and keys the same way.)

bun run check and dep-guard pass on every branch of the stack, and the full e2e suite passes at the top of the stack (163/163, including the stories axe sweep). Each behaviour fix comes with a test that fails without it; the forwardRef tests can only guard, since the suite runs on React 19.

Superseded by the completed stacked review; blocking fixes were addressed and the rebased stack is being landed.

[corbits-triage]

Triage: Fail

@pratikbuilds, please address the following:

• required checks are failing: policy-check

Stacked on #126: only this pull request's own changes were checked.

Notes:

• added code matches a code pattern: stories/chat/full-chat.stories.tsx:1135 Invented fallback `?? []`; handle the missing value instead., stories/chat/full-chat.stories.tsx:895 Nested ternary; use if statements or a lookup.
• added code matches a code pattern: stories/chat/full-chat.stories.tsx:250 Invented fallback (an empty string, || [] or || 0); handle the missing value instead.

Triage will request changes.

[corbits-triage]

Triage requests changes:

• CI green: required checks are failing