fix(app): merge adjacent patches inside used groups (#45477)

This commit is contained in:
Luke Parker
2026-08-27 14:36:27 +10:00
committed by GitHub
Unverified
parent fcc6568fcb
commit e2d6c4bd4a
4 changed files with 198 additions and 16 deletions
@@ -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(
@@ -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()
})
}
@@ -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 (
<section class="mx-auto flex w-full max-w-[860px] flex-col gap-4 p-6">
<div class="flex flex-wrap gap-3">
<button type="button" onClick={() => setState("phase", "running")}>
Start follow-up patch
</button>
<button type="button" onClick={() => setState("phase", "completed")}>
Finish follow-up patch
</button>
</div>
<CurrentSessionProviders document={storyDocument(tools())}>
<CurrentContextToolGroup
tools={tools()}
busy={state.phase === "running"}
open={state.open}
onOpenChange={(open) => setState("open", open)}
/>
</CurrentSessionProviders>
</section>
)
},
}
+31 -15
View File
@@ -502,6 +502,15 @@ export function CurrentContextToolGroup(props: {
const items = createMemo(() =>
props.tools.reduce<SessionMessageAssistantTool[][]>((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: {
<Show
when={tool().name === "skill" && group().length > 1 && skills().length === group().length}
fallback={
<ToolDisplay
id={tool().id}
tool={tool().name}
input={currentToolInput(tool())}
metadata={currentToolMetadata(tool())}
output={currentToolOutput(tool())}
error={currentToolError(tool())}
status={tool().state.status}
defaultOpen={false}
deferContent
virtualizeDiff={false}
onContentRendered={props.onSizeChange}
/>
<Show
when={tool().name === "patch" && tool().state.status !== "error"}
fallback={
<ToolDisplay
id={tool().id}
tool={tool().name}
input={currentToolInput(tool())}
metadata={currentToolMetadata(tool())}
output={currentToolOutput(tool())}
error={currentToolError(tool())}
status={tool().state.status}
defaultOpen={false}
deferContent
virtualizeDiff={false}
onContentRendered={props.onSizeChange}
/>
}
>
<CurrentFileToolGroup tools={group()} onSizeChange={props.onSizeChange} />
</Show>
}
>
<div
@@ -662,8 +678,8 @@ export function CurrentContextToolGroup(props: {
export function CurrentFileToolGroup(props: {
tools: SessionMessageAssistantTool[]
fileOpen: (path: string) => 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 }[]) => {