From e2d6c4bd4a2547797f0be993434eb20393d827c0 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:36:27 +1000 Subject: [PATCH] fix(app): merge adjacent patches inside used groups (#45477) --- ...ession-timeline-reducer-projection.spec.ts | 56 +++++++++++++ .../component-tests/patch-group.spec.ts | 34 ++++++++ .../src/tools/tool-group.stories.tsx | 78 ++++++++++++++++++- .../session-ui/src/tools/tool-renderer.tsx | 46 +++++++---- 4 files changed, 198 insertions(+), 16 deletions(-) create mode 100644 packages/session-ui/component-tests/patch-group.spec.ts diff --git a/packages/app/e2e/regression/session-timeline-reducer-projection.spec.ts b/packages/app/e2e/regression/session-timeline-reducer-projection.spec.ts index e90c218dc51..189ff9ea2b5 100644 --- a/packages/app/e2e/regression/session-timeline-reducer-projection.spec.ts +++ b/packages/app/e2e/regression/session-timeline-reducer-projection.spec.ts @@ -1,4 +1,5 @@ import { expect, test } from "@playwright/test" +import { createTwoFilesPatch } from "diff" import { assistantMessage, completedAssistantInfo, @@ -78,6 +79,61 @@ test("leaves tools expanded by settings outside the collapsed stack", async ({ p await expect(page.locator('[data-timeline-spacing="tool"]')).toHaveCSS("padding-top", "8px") }) +test("combines follow-up patches into one three-file stack inside Used", async ({ page }) => { + const file = (path: string, before: number, after: number) => ({ + file: path, + status: "modified", + additions: 1, + deletions: 1, + patch: createTwoFilesPatch( + path, + path, + `export const value = ${before}\n`, + `export const value = ${after}\n`, + "", + "", + { context: Infinity }, + ), + }) + const timeline = await setupTimeline(page, { + messages: [ + userMessage(), + assistantMessage([ + shell("patch_shell", "completed"), + toolPart( + "patch_first", + "patch", + "completed", + {}, + { + metadata: { files: [file("src/a.ts", 0, 1), file("src/b.ts", 0, 1)] }, + }, + ), + ]), + ], + }) + const group = page.locator('[data-component="collapsed-tool-group"]') + await group.getByRole("button", { name: "Used Shell, Patch", exact: true }).click() + await expect(group.getByText("2 files", { exact: true })).toBeVisible() + await timeline.send( + partUpdated( + toolPart( + "patch_next", + "patch", + "completed", + {}, + { + metadata: { files: [file("src/a.ts", 1, 2), file("src/c.ts", 0, 1)] }, + }, + ), + ), + ) + await expect(group.locator('[data-component="tag"]')).toHaveText("3") + await expect(group.locator('[data-component="apply-patch-tool"]')).toHaveCount(1) + await expect(group.getByText("3 files", { exact: true })).toBeVisible() + await expect(group.locator('[data-slot="apply-patch-filename"]')).toHaveText(["a.ts", "b.ts", "c.ts"]) +}) + test("keeps failed search calls and their error cards inside the collapsed stack", async ({ page }) => { const parts = [ toolPart( diff --git a/packages/session-ui/component-tests/patch-group.spec.ts b/packages/session-ui/component-tests/patch-group.spec.ts new file mode 100644 index 00000000000..9abc2667d6a --- /dev/null +++ b/packages/session-ui/component-tests/patch-group.spec.ts @@ -0,0 +1,34 @@ +import { expect, story } from "../../storybook/playwright/story" + +story("merges follow-up patches into one stack with a distinct file count", async ({ mount }, info) => { + const root = await mount("current-tool-group--patch-follow-ups") + const group = root.locator('[data-component="collapsed-tool-group"]') + const patches = group.locator('[data-component="apply-patch-tool"]') + await expect(patches).toHaveCount(1) + await expect(patches.getByText("2 files", { exact: true })).toBeVisible() + const first = patches.locator('[data-scope="apply-patch"] button').filter({ hasText: "a.ts" }) + await first.click() + await expect(first).toHaveAttribute("aria-expanded", "true") + await root.getByRole("button", { name: "Start follow-up patch" }).click() + await expect(group.locator('[data-component="tag"]')).toHaveText("3") + await expect(patches).toHaveCount(1) + await expect(patches.getByText("2 files", { exact: true })).toBeVisible() + await root.getByRole("button", { name: "Finish follow-up patch" }).click() + await expect(patches).toHaveCount(1) + await expect(patches.getByText("3 files", { exact: true })).toBeVisible() + await expect(patches.locator('[data-slot="apply-patch-filename"]')).toHaveText(["a.ts", "b.ts", "c.ts"]) + await expect(first).toHaveAttribute("aria-expanded", "true") + await expect(patches.locator('[data-component="file"]')).toBeVisible() + await group.screenshot({ path: info.outputPath("merged.png") }) +}) + +for (const separator of ["shell", "error"]) { + story(`does not merge patches across an intervening ${separator}`, async ({ mount }) => { + const root = await mount("current-tool-group--patch-follow-ups", { args: { separator } }) + await root.getByRole("button", { name: "Finish follow-up patch" }).click() + const group = root.locator('[data-component="collapsed-tool-group"]') + await expect(group.locator('[data-component="apply-patch-tool"]')).toHaveCount(2) + await expect(group.locator('[data-slot="apply-patch-filename"]')).toHaveText(["a.ts", "b.ts", "a.ts", "c.ts"]) + if (separator === "error") await expect(group.locator('[data-kind="tool-error-card"]')).toBeVisible() + }) +} diff --git a/packages/session-ui/src/tools/tool-group.stories.tsx b/packages/session-ui/src/tools/tool-group.stories.tsx index 711aeaa175c..6424aecac31 100644 --- a/packages/session-ui/src/tools/tool-group.stories.tsx +++ b/packages/session-ui/src/tools/tool-group.stories.tsx @@ -1,4 +1,6 @@ -import { createSignal } from "solid-js" +import { createMemo, createSignal } from "solid-js" +import { createStore } from "solid-js/store" +import { createTwoFilesPatch } from "diff" import { CurrentSessionProviders } from "../storybook/current-session-story" import { storyDocument, storyTool } from "../storybook/current-session-scenarios" import { CurrentContextToolGroup } from "./tool-renderer" @@ -33,3 +35,77 @@ export const MixedTools = { ) }, } + +export const PatchFollowUps = { + args: { separator: "none" }, + argTypes: { separator: { control: "select", options: ["none", "shell", "error"] } }, + render: (args: { separator: string }) => { + const [state, setState] = createStore({ phase: "initial", open: true }) + const file = (path: string, before: number, after: number) => ({ + file: path, + status: "modified", + additions: 1, + deletions: 1, + patch: createTwoFilesPatch( + path, + path, + `export const value = ${before}\n`, + `export const value = ${after}\n`, + "", + "", + { context: Infinity }, + ), + }) + const tools = createMemo(() => [ + storyTool("patch_shell", "shell", "completed", { command: "printf checked" }, { output: "checked" }), + storyTool( + "patch_first", + "patch", + "completed", + {}, + { + metadata: { files: [file("src/a.ts", 0, 1), file("src/b.ts", 0, 1)] }, + }, + ), + ...(state.phase === "initial" + ? [] + : [ + ...(args.separator === "shell" + ? [storyTool("patch_separator", "shell", "completed", { command: "printf checked" })] + : []), + ...(args.separator === "error" + ? [storyTool("patch_error", "patch", "error", {}, { error: "Patch failed" })] + : []), + storyTool( + "patch_next", + "patch", + state.phase === "running" ? "running" : "completed", + {}, + { + metadata: state.phase === "running" ? {} : { files: [file("src/a.ts", 1, 2), file("src/c.ts", 0, 1)] }, + }, + ), + ]), + ]) + return ( +
+
+ + +
+ + setState("open", open)} + /> + +
+ ) + }, +} diff --git a/packages/session-ui/src/tools/tool-renderer.tsx b/packages/session-ui/src/tools/tool-renderer.tsx index db08b99ad69..a84d2a4ce2d 100644 --- a/packages/session-ui/src/tools/tool-renderer.tsx +++ b/packages/session-ui/src/tools/tool-renderer.tsx @@ -502,6 +502,15 @@ export function CurrentContextToolGroup(props: { const items = createMemo(() => props.tools.reduce((groups, tool) => { const previous = groups.at(-1) + if ( + tool.name === "patch" && + tool.state.status !== "error" && + previous?.[0]?.name === "patch" && + previous[0].state.status !== "error" + ) { + previous.push(tool) + return groups + } if ( tool.name === "skill" && tool.state.status !== "error" && @@ -567,19 +576,26 @@ export function CurrentContextToolGroup(props: { 1 && skills().length === group().length} fallback={ - + + } + > + + } >
boolean | undefined - onFileOpenChange: (path: string, open: boolean) => void + fileOpen?: (path: string) => boolean | undefined + onFileOpenChange?: (path: string, open: boolean) => void onSizeChange?: () => void }) { const files = createMemo((previous: { key: string; value: unknown }[]) => {