-
Notifications
You must be signed in to change notification settings - Fork 2
refactor(session): rewrite session view per W1 visual lock for slice 11b.1 #589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 48 commits
1cf9f6a
bb381ed
a70e960
53a4cea
722d9fa
3911332
e421774
55d77ae
421841a
b5a995c
9594e0f
548fa26
de6a321
0fbee29
20ccbb1
e335a1a
892ef1a
5401c26
610e2db
cad41ef
c3518eb
eef95b8
1c01839
78e9506
36100de
d44e0e4
1b43dac
e82bf8f
c87cb47
e4b21bd
ade42fe
d81e0bf
9a1ec65
116d542
cb90f6b
e177559
77f8e8c
4457e73
add81e2
c5a030f
a4b44e2
6b62f8b
d8addf2
e2e5def
90e3903
186d5f8
55893a9
b48a860
8ac986b
8ef83fa
83d1dc7
9943552
b2ee6cb
47a4760
9e071d2
969f6a9
f7646cb
61e7a0f
aed3994
f49fd6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| import { test, expect } from "../fixtures" | ||
|
|
||
| /** | ||
| * Slice 11b.1 — issue #440 §5.2 E1 + E9 + E10. | ||
| * | ||
| * Smoke E2E for the W1 user-bubble + agent-round surfaces now that | ||
| * `session-turn.tsx` mounts the W1 leaves on the default user path | ||
| * (Phase 2b integration commit). | ||
| * | ||
| * The verifications are structural / behavioral — visual rhythm, | ||
| * shimmer cadence, and reduce-motion are covered by the dev:desktop | ||
| * checklist (D-items) and by the unit tests on the leaves themselves. | ||
| */ | ||
|
|
||
| const USER_BUBBLE = '[data-component="session-turn-user-bubble"]' | ||
| const BUBBLE_TEXT = `${USER_BUBBLE} [data-slot="bubble-text"]` | ||
| const BUBBLE_COPY = `${USER_BUBBLE} [data-action="copy"]` | ||
| const BUBBLE_RESET = `${USER_BUBBLE} [data-action="reset"]` | ||
|
|
||
| const AGENT_ROUND = '[data-component="session-turn-agent-round"]' | ||
| const AGENT_WORKING_TIME = `${AGENT_ROUND} [data-slot="agent-working-time"]` | ||
| const AGENT_PROSE = `${AGENT_ROUND} [data-slot="agent-prose"]` | ||
| const AGENT_COPY = `${AGENT_ROUND} [data-action="copy"]` | ||
| const AGENT_FORK = `${AGENT_ROUND} [data-action="fork"]` | ||
|
|
||
| test("E1 — user message + agent stream mounts W1 leaves on the default path", async ({ page, llm, project }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
|
|
||
| const reply = "W1 agent reply text" | ||
| await llm.text(reply) | ||
| await project.prompt("E1 verify W1 surface") | ||
|
|
||
| // User bubble is the W1 leaf, not the legacy `<UserMessageDisplay>`. | ||
| await expect(page.locator(USER_BUBBLE)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(BUBBLE_TEXT)).toContainText("E1 verify W1 surface") | ||
|
|
||
| // Agent round is the W1 leaf, agent-toolbar is mounted (visibility | ||
| // is CSS-only; the DOM presence + aria-hidden gate is the testable | ||
| // surface here). | ||
| await expect(page.locator(AGENT_ROUND)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(AGENT_PROSE)).toContainText(reply, { timeout: 30_000 }) | ||
|
|
||
| // Working-time tick exists once the round has started; the freeze | ||
| // value after the round completes is whatever the assistant message | ||
| // `time.completed - time.created` resolved to. | ||
| await expect(page.locator(AGENT_WORKING_TIME)).toHaveCount(1, { timeout: 30_000 }) | ||
| const tick = await page.locator(AGENT_WORKING_TIME).textContent() | ||
| expect(tick).toMatch(/\d+s/) | ||
| }) | ||
|
|
||
| test("E9 — bubble [Copy] writes the user text to the clipboard", async ({ page, llm, project, browserName }) => { | ||
| test.setTimeout(120_000) | ||
| // Clipboard API is gated to the active document and requires the | ||
| // Playwright permission grant before navigation in chromium. | ||
| test.skip(browserName !== "chromium", "navigator.clipboard.writeText only granted in chromium fixture") | ||
| await page.context().grantPermissions(["clipboard-read", "clipboard-write"]) | ||
|
|
||
| await project.open() | ||
| await llm.text("agent ack") | ||
| await project.prompt("E9 copy this user text") | ||
| await expect(page.locator(USER_BUBBLE)).toHaveCount(1, { timeout: 30_000 }) | ||
|
|
||
| await page.locator(BUBBLE_COPY).click() | ||
| await expect(page.locator(`${BUBBLE_COPY}[data-copied]`)).toHaveCount(1, { timeout: 5_000 }) | ||
|
|
||
| const value = await page.evaluate(() => navigator.clipboard.readText()) | ||
| expect(value).toContain("E9 copy this user text") | ||
| }) | ||
|
|
||
| test("E10 — agent toolbar [Copy] writes the concatenated prose to the clipboard", async ({ | ||
| page, | ||
| llm, | ||
| project, | ||
| browserName, | ||
| }) => { | ||
| test.setTimeout(120_000) | ||
| test.skip(browserName !== "chromium", "navigator.clipboard.writeText only granted in chromium fixture") | ||
| await page.context().grantPermissions(["clipboard-read", "clipboard-write"]) | ||
|
|
||
| const prose = "Agent prose paragraph one\n\nparagraph two" | ||
|
|
||
| await project.open() | ||
| await llm.text(prose) | ||
| await project.prompt("E10 user kickoff") | ||
| await expect(page.locator(AGENT_PROSE)).toContainText("paragraph two", { timeout: 30_000 }) | ||
|
|
||
| // The agent-toolbar copy button stays visibility-hidden while the | ||
| // round is running. Wait for the streaming agent message to fully | ||
| // settle by polling the SDK before asserting the copy fires. | ||
| const sessionID = await page.evaluate(() => { | ||
| const url = new URL(window.location.href) | ||
| return url.pathname.split("/session/")[1] ?? "" | ||
| }) | ||
| if (!sessionID) throw new Error("expected a session id in the URL") | ||
| await expect | ||
| .poll( | ||
| async () => | ||
| await project.sdk.session | ||
| .messages({ sessionID, limit: 50 }) | ||
| .then((r) => (r.data ?? []).filter((m) => m.info.role === "assistant" && m.info.time.completed !== undefined).length), | ||
| { timeout: 30_000, intervals: [200, 500, 1_000] }, | ||
| ) | ||
| .toBeGreaterThan(0) | ||
|
|
||
| // Force the toolbar visible via hover so the click lands. The leaf | ||
| // keeps `pointer-events:none` while running. | ||
| await page.locator(AGENT_ROUND).hover() | ||
| await page.locator(AGENT_COPY).click() | ||
| await expect(page.locator(`${AGENT_COPY}[data-copied]`)).toHaveCount(1, { timeout: 5_000 }) | ||
|
|
||
| const value = await page.evaluate(() => navigator.clipboard.readText()) | ||
| expect(value).toContain("Agent prose paragraph one") | ||
| }) | ||
|
|
||
| test("E7 — bubble [Reset] adopts the legacy revert path (revertDock, no dialog)", async ({ | ||
| page, | ||
| llm, | ||
| project, | ||
| }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
|
|
||
| await llm.text("agent ack") | ||
| await project.prompt("E7 reset target") | ||
| await expect(page.locator(USER_BUBBLE)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(AGENT_PROSE)).toContainText("agent ack", { timeout: 30_000 }) | ||
|
|
||
| await page.locator(USER_BUBBLE).hover() | ||
| await page.locator(BUBBLE_RESET).click() | ||
|
|
||
| // The W1 [Reset] mounts the existing revert dock above the composer. | ||
| // The bubble for the resetted message is removed from the timeline. | ||
| await expect(page.locator(BUBBLE_TEXT)).toHaveCount(0, { timeout: 30_000 }) | ||
| }) | ||
|
|
||
| test("E8 — agent toolbar [Fork] is mounted next to [Copy] for the W1 surface", async ({ | ||
| page, | ||
| llm, | ||
| project, | ||
| }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
|
|
||
| await llm.text("ack pre-fork") | ||
| await project.prompt("E8 fork source") | ||
| await expect(page.locator(AGENT_ROUND)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(AGENT_PROSE)).toContainText("ack pre-fork", { timeout: 30_000 }) | ||
|
|
||
| // Fork stays mounted in the DOM (visibility is CSS) so a presence | ||
| // assertion is enough — the navigation behaviour is covered by | ||
| // session-w1-fork.spec when the host wires sdk.session.fork. | ||
| await page.locator(AGENT_ROUND).hover() | ||
| await expect(page.locator(AGENT_FORK)).toHaveCount(1) | ||
| }) |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||||||||
| import { test, expect } from "../fixtures" | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Slice 11b.1 — issue #440 §5.2 E14. | ||||||||||||
| * | ||||||||||||
| * Tool error path: failed tool calls render the per-row "failed" | ||||||||||||
| * gloss inside the trow-body. The summary copy switches to the | ||||||||||||
| * "with-failed" variant when the grouped case has at least one | ||||||||||||
| * failure. No color shift on the parent (DESIGN.md L466). | ||||||||||||
| * | ||||||||||||
| * The single-tool / grouped paths share the same `reduceTrowBlock` | ||||||||||||
| * + `trowSummaryI18nKey` reducer; the unit reducer test | ||||||||||||
| * (`session-turn-trow-block.reducer.test.ts`) already pins the pure | ||||||||||||
| * matrix. This spec validates the E2E wiring: the leaf reads the | ||||||||||||
| * SDK ToolPart `state.status === "error"` correctly and surfaces | ||||||||||||
| * the trow-block data-failed attribute that the CSS keys off. | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| const TROW_BLOCK = '[data-component="session-turn-trow-block"]' | ||||||||||||
| const TROW_BODY = `${TROW_BLOCK} [data-slot="trow-body"]` | ||||||||||||
| const AGENT_PROSE = '[data-component="session-turn-agent-round"] [data-slot="agent-prose"]' | ||||||||||||
|
|
||||||||||||
| test("E14 — failed tool surfaces the data-failed marker on the trow", async ({ page, llm, project }) => { | ||||||||||||
| test.setTimeout(120_000) | ||||||||||||
| await project.open() | ||||||||||||
|
|
||||||||||||
| // Mock a bash tool that fails. The TestLLMServer will mark the | ||||||||||||
| // tool state.status as `error` because the tool call has no | ||||||||||||
| // settled output and the assistant returns a fail envelope. | ||||||||||||
| await llm.tool("bash", { command: "false", description: "always fails" }) | ||||||||||||
| await llm.text("tool failed") | ||||||||||||
|
|
||||||||||||
| await project.prompt("E14 tool failure") | ||||||||||||
| await expect(page.locator(AGENT_PROSE)).toContainText("tool failed", { timeout: 60_000 }) | ||||||||||||
|
|
||||||||||||
| const trow = page.locator(TROW_BLOCK).first() | ||||||||||||
| await expect(trow).toHaveCount(1, { timeout: 60_000 }) | ||||||||||||
|
|
||||||||||||
|
Comment on lines
+36
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result:
Citations:
Assert exact count on the unscoped locator, not on The current assertion Proposed fix- const trow = page.locator(TROW_BLOCK).first()
- await expect(trow).toHaveCount(1, { timeout: 60_000 })
+ await expect(page.locator(TROW_BLOCK)).toHaveCount(1, { timeout: 60_000 })
+ const trow = page.locator(TROW_BLOCK).first()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| // Expand to make sure the per-row body is rendered (i.e. renderTool | ||||||||||||
| // ran and the rich body was mounted). | ||||||||||||
| await page.locator(`${TROW_BLOCK} summary`).first().click() | ||||||||||||
| await expect(page.locator(TROW_BODY)).toBeVisible({ timeout: 10_000 }) | ||||||||||||
| }) | ||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import { test, expect } from "../fixtures" | ||
|
|
||
| /** | ||
| * Slice 11b.1 — issue #440 §5.2 E2 + E5 + E15 + E16. | ||
| * | ||
| * Covers the secondary W1 surfaces that hang off the bubble + agent | ||
| * round: attachment rail above the bubble (E2), interrupted system | ||
| * event line (E5), multi-turn gap rhythm (E15), and the command | ||
| * palette /fork backup that still ships in 11b.1 alongside the new | ||
| * toolbar Fork (E16). | ||
| */ | ||
|
|
||
| const USER_BUBBLE = '[data-component="session-turn-user-bubble"]' | ||
| const BUBBLE_ATTACHMENT_ROW = `${USER_BUBBLE} [data-slot="bubble-attachment-row"]` | ||
| const BUBBLE_ATTACHMENT_CHIP = '[data-component="attachment-chip"]' | ||
| const AGENT_ROUND = '[data-component="session-turn-agent-round"]' | ||
| const AGENT_PROSE = `${AGENT_ROUND} [data-slot="agent-prose"]` | ||
| const SYSTEM_EVENT = '[data-component="session-turn-event"]' | ||
| const SYSTEM_EVENT_INTERRUPTED = `${SYSTEM_EVENT}[data-kind="interrupted"]` | ||
|
|
||
| test("E2 — attachment row sits above the bubble for file / image parts", async ({ page, llm, project }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
| const sdk = project.sdk | ||
|
|
||
| await llm.text("ack attachment") | ||
|
|
||
| // Use the SDK directly to seed a user message with an attached file | ||
| // part — the browser file picker is out of scope for an E2E smoke | ||
| // test, the rendering contract is what we're verifying here. | ||
| const session = await sdk.session.create({ directory: sdk.directory, title: "E2 attachment" }) | ||
| if (!session.data?.id) throw new Error("session.create failed") | ||
| const sessionID = session.data.id | ||
| project.trackSession(sessionID) | ||
|
|
||
| await sdk.session.promptAsync({ | ||
| sessionID, | ||
| parts: [ | ||
| { type: "text", text: "E2 with attachment" }, | ||
| { | ||
| type: "file", | ||
| mime: "image/png", | ||
| filename: "tiny.png", | ||
| url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=", | ||
| }, | ||
| ], | ||
| }) | ||
|
|
||
| await project.gotoSession(sessionID) | ||
| await expect(page.locator(USER_BUBBLE)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(BUBBLE_ATTACHMENT_ROW)).toHaveCount(1, { timeout: 30_000 }) | ||
| await expect(page.locator(BUBBLE_ATTACHMENT_CHIP).first()).toBeVisible({ timeout: 30_000 }) | ||
| }) | ||
|
|
||
| test("E5 — Ctrl+C interrupt renders the muted system-event caption", async ({ page, llm, project }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
|
|
||
| // Queue a hung response so we can interrupt mid-stream. | ||
| await llm.hang() | ||
|
|
||
| await project.prompt("E5 interrupt me") | ||
| await expect(page.locator(AGENT_ROUND)).toHaveCount(1, { timeout: 30_000 }) | ||
|
|
||
| // Hit the existing escape / interrupt keybinding. The legacy | ||
| // interrupt divider used a `MessageDivider`; the W1 surface drops | ||
| // that and routes the same signal through `SystemEvent` inside the | ||
| // agent round. | ||
| await page.keyboard.press("Escape") | ||
| await expect(page.locator(SYSTEM_EVENT_INTERRUPTED)).toHaveCount(1, { timeout: 30_000 }) | ||
| }) | ||
|
|
||
| test("E15 — multi-turn rounds each get their own working-time + agent round", async ({ | ||
| page, | ||
| llm, | ||
| project, | ||
| }) => { | ||
| test.setTimeout(180_000) | ||
| await project.open() | ||
|
|
||
| await llm.text("first round done") | ||
| await project.prompt("E15 first turn") | ||
| await expect(page.locator(AGENT_PROSE)).toContainText("first round done", { timeout: 60_000 }) | ||
|
|
||
| await llm.text("second round done") | ||
| await project.prompt("E15 second turn") | ||
|
|
||
| await expect(page.locator(USER_BUBBLE)).toHaveCount(2, { timeout: 60_000 }) | ||
| await expect(page.locator(AGENT_ROUND)).toHaveCount(2, { timeout: 60_000 }) | ||
| // Each round has its own working-time tick — locator scoped to the | ||
| // agent-round leaf prevents bleed between rounds. | ||
| await expect(page.locator(`${AGENT_ROUND} [data-slot="agent-working-time"]`)).toHaveCount(2) | ||
| }) | ||
|
|
||
| test("E16 — command palette /fork backup is still mounted alongside the W1 toolbar Fork", async ({ | ||
| page, | ||
| llm, | ||
| project, | ||
| }) => { | ||
| test.setTimeout(120_000) | ||
| await project.open() | ||
|
|
||
| await llm.text("pre-fork ack") | ||
| await project.prompt("E16 dialog backup") | ||
| await expect(page.locator(AGENT_PROSE)).toContainText("pre-fork ack", { timeout: 30_000 }) | ||
|
|
||
| // Open the palette and type /fork — the entry should still be | ||
| // discoverable until the post-11b.1 retirement PR removes it. | ||
| await page.keyboard.press("Meta+K") | ||
| await page.keyboard.type("/fork") | ||
| await expect(page.locator('[data-slash-id="session.fork"]').first()).toBeVisible({ timeout: 10_000 }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { test, expect } from "../fixtures" | ||
| import { promptSelector, scrollViewportSelector, sessionTurnListSelector } from "../selectors" | ||
|
|
||
| /** | ||
| * Slice 11b.1 — issue #440 §5.2 E6. | ||
| * | ||
| * Verifies the W1 floating jump-to-bottom button — the new | ||
| * `JumpToBottom` leaf mounted by `message-timeline.tsx` (Phase 2b | ||
| * commit). The button must: | ||
| * | ||
| * - be invisible while the user is pinned to the bottom; | ||
| * - appear immediately when the user scrolls up (no "has new | ||
| * content since unpin" gate, per design doc §3.4); | ||
| * - scroll the viewport to the bottom on click and re-hide. | ||
| */ | ||
|
|
||
| const JUMP_BUTTON = '[data-component="session-turn-jump"]' | ||
|
|
||
| test("E6 — ↓ button mounts and re-pins the timeline to the bottom on click", async ({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a lowercase descriptive test name. Rename the title to match the suite convention (lowercase, descriptive phrasing). As per coding guidelines: Use lowercase, descriptive test names (e.g., 'sidebar can be toggled'). 🤖 Prompt for AI Agents |
||
| page, | ||
| llm, | ||
| project, | ||
| }) => { | ||
| test.setTimeout(180_000) | ||
| await project.open() | ||
|
|
||
| // Seed enough turns to force overflow. | ||
| for (let i = 0; i < 6; i++) { | ||
| await llm.text(`turn ${i} agent reply with enough text to push the column past the viewport height`) | ||
| } | ||
| for (let i = 0; i < 6; i++) { | ||
| await project.prompt( | ||
| `turn ${i} user message with enough content to take up vertical room ${"a".repeat(120)}`, | ||
| ) | ||
| } | ||
|
|
||
| await expect(page.locator(promptSelector)).toBeVisible({ timeout: 60_000 }) | ||
|
|
||
| // Scroll the timeline viewport up so the user is no longer pinned. | ||
| await page.evaluate( | ||
| ({ scrollViewportSelector, turnListSelector }) => { | ||
| const list = document.querySelector(turnListSelector) | ||
| const viewport = list?.closest(scrollViewportSelector) | ||
| if (viewport instanceof HTMLElement) viewport.scrollTop = 0 | ||
| }, | ||
| { scrollViewportSelector, turnListSelector: sessionTurnListSelector }, | ||
| ) | ||
|
|
||
| await expect(page.locator(JUMP_BUTTON)).toBeVisible({ timeout: 10_000 }) | ||
|
|
||
| await page.locator(JUMP_BUTTON).click() | ||
|
|
||
| // After click the viewport should be back near the bottom and the | ||
| // button should hide. We don't pin a literal `distance === 0` — | ||
| // the resume scroll path uses an animated scroll, and re-pin is | ||
| // committed once the user is within the bottom threshold. | ||
| await expect | ||
| .poll( | ||
| async () => | ||
| await page.evaluate( | ||
| ({ scrollViewportSelector, turnListSelector }) => { | ||
| const list = document.querySelector(turnListSelector) | ||
| const viewport = list?.closest(scrollViewportSelector) | ||
| if (!(viewport instanceof HTMLElement)) return -1 | ||
| return viewport.scrollHeight - viewport.clientHeight - viewport.scrollTop | ||
| }, | ||
| { scrollViewportSelector, turnListSelector: sessionTurnListSelector }, | ||
| ), | ||
| { timeout: 15_000 }, | ||
| ) | ||
| .toBeLessThan(40) | ||
|
|
||
| await expect(page.locator(JUMP_BUTTON)).toBeHidden({ timeout: 10_000 }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a lowercase descriptive test name.
Rename the test title to follow the E2E naming convention.
As per coding guidelines: Use lowercase, descriptive test names (e.g., 'sidebar can be toggled').
🤖 Prompt for AI Agents