diff --git a/artifacts/grouped-file-diffs/after.png b/artifacts/grouped-file-diffs/after.png new file mode 100644 index 00000000000..e63bcf8f438 Binary files /dev/null and b/artifacts/grouped-file-diffs/after.png differ diff --git a/artifacts/grouped-file-diffs/before.png b/artifacts/grouped-file-diffs/before.png new file mode 100644 index 00000000000..d51b2404dba Binary files /dev/null and b/artifacts/grouped-file-diffs/before.png differ diff --git a/artifacts/grouped-file-diffs/comparison.png b/artifacts/grouped-file-diffs/comparison.png new file mode 100644 index 00000000000..684230f1b2b Binary files /dev/null and b/artifacts/grouped-file-diffs/comparison.png differ diff --git a/packages/app/e2e/regression/patch-sticky-header.spec.ts b/packages/app/e2e/regression/patch-sticky-header.spec.ts index 5546289fca5..a34b0b1c68e 100644 --- a/packages/app/e2e/regression/patch-sticky-header.spec.ts +++ b/packages/app/e2e/regression/patch-sticky-header.spec.ts @@ -34,7 +34,7 @@ const scenarios = [ title: false, }, { - name: "grouped edit with title", + name: "grouped edit", placement: "grouped", tools: [ toolPart( @@ -46,14 +46,14 @@ const scenarios = [ ), ], files: ["a"], - title: true, + title: false, }, { name: "running edit input fallback", placement: "grouped", tools: [toolPart("prt_sticky_edit", "edit", "running", { path: "src/a.ts", oldString: before, newString: after })], files: ["a"], - title: true, + title: false, }, { name: "grouped write", diff --git a/packages/session-ui/component-tests/session-tool-projection.spec.ts b/packages/session-ui/component-tests/session-tool-projection.spec.ts index baa08791418..72679ec88b7 100644 --- a/packages/session-ui/component-tests/session-tool-projection.spec.ts +++ b/packages/session-ui/component-tests/session-tool-projection.spec.ts @@ -4,38 +4,25 @@ import { expect, story } from "../../storybook/playwright/story" story("renders every admitted tool family and hides timeline-only exclusions", async ({ mount }) => { const timeline = await mount("current-session-research-agents--agent-research", { args: { scenario: "workflow" } }) const first = timeline.locator( - '[data-timeline-part-ids="tool_family_read,tool_family_glob,tool_family_grep,tool_family_list,tool_family_webfetch,tool_family_websearch,tool_family_subagent,tool_family_shell,tool_family_edit,tool_family_write,tool_family_patch"]', + '[data-timeline-part-ids="tool_family_read,tool_family_glob,tool_family_grep,tool_family_list,tool_family_webfetch,tool_family_websearch,tool_family_subagent,tool_family_shell,tool_family_edit,tool_family_write,tool_family_write_extra,tool_family_patch"]', ) const second = timeline.locator('[data-timeline-part-ids="tool_family_skill,tool_family_custom"]') await expect(first).toBeVisible() await expect(second).toBeVisible() await first.getByRole("button").click() await second.getByRole("button").click() - for (const id of [ - "webfetch", - "websearch", - "subagent", - "shell", - "edit", - "write", - "patch", - "question", - "skill", - "custom", - ]) { + for (const id of ["webfetch", "websearch", "subagent", "shell", "question", "skill", "custom"]) { await expect(timeline.locator(`[data-timeline-part-id="tool_family_${id}"]`), id).toBeVisible() } - for (const name of ["edit", "write", "patch"]) { - const tool = timeline.locator(`[data-timeline-part-id="tool_family_${name}"]`) - await expect(tool.getByText("1 file", { exact: true })).toBeVisible() - await expect(tool.getByRole("button")).toHaveCount(1) - await expect(tool.locator('[data-scope="apply-patch"] button')).toHaveAttribute("aria-expanded", "false") - await expect(tool.locator('[data-slot="collapsible-trigger"]')).toHaveAttribute("data-locked", "") - await expect(tool.locator('[data-slot="message-part-title-filename"]')).toHaveCount(0) - await expect(tool.locator('[data-slot="message-part-actions"]')).toHaveCount(0) - await expect(tool.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("font-size", "13px") - await expect(tool.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("line-height", "16px") - } + const files = timeline.locator( + '[data-timeline-part-ids="tool_family_edit,tool_family_write,tool_family_write_extra,tool_family_patch"]', + ) + await expect(files).toBeVisible() + await expect(files.locator('[data-scope="apply-patch"]')).toHaveCount(1) + await expect(files.locator('[data-slot="apply-patch-filename"]')).toHaveText(["a.ts", "new.ts", "extra.ts"]) + await expect(files.locator('[data-slot="basic-tool-tool-title"]')).toHaveCount(0) + await expect(files.locator('[data-scope="apply-patch"] button')).toHaveCount(3) + await expect(files.locator('[data-scope="apply-patch"] button[aria-expanded="false"]')).toHaveCount(3) await expect(timeline.locator('[data-timeline-part-id="tool_family_todo"]')).toHaveCount(0) }) diff --git a/packages/session-ui/src/timeline/research-and-agents.stories.tsx b/packages/session-ui/src/timeline/research-and-agents.stories.tsx index 4692b1827df..08837fc4ec4 100644 --- a/packages/session-ui/src/timeline/research-and-agents.stories.tsx +++ b/packages/session-ui/src/timeline/research-and-agents.stories.tsx @@ -250,6 +250,10 @@ const CompleteAgentWorkflow = { path: "src/new.ts", content: "export const stable = true", }), + storyTool("tool_family_write_extra", "write", "completed", { + path: "src/extra.ts", + content: "export const extra = true", + }), storyTool( "tool_family_patch", "patch", diff --git a/packages/session-ui/src/timeline/session-timeline-row.tsx b/packages/session-ui/src/timeline/session-timeline-row.tsx index 948fa029b82..f006f0067e3 100644 --- a/packages/session-ui/src/timeline/session-timeline-row.tsx +++ b/packages/session-ui/src/timeline/session-timeline-row.tsx @@ -64,19 +64,24 @@ export function createSessionTimelineRowRenderer(input: { }) { const i18n = useI18n() const data = useData() - // Cached timelines retain subgroup identities alongside their disclosure choices. + // Cached timelines retain file-change subgroup identities alongside their disclosure choices. const patchGroupKeys = input.disclosure.patchGroupKeys ?? new Map() const patchPartKeys = new WeakMap() const patchOwners = createMemo(() => { const owners = new Map() const rows = input.projection.rows() - // Track status changes before a group is first opened: a failed patch can + // Track status changes before a group is first opened: a failed file change can // split an existing group without changing the projection's row identities. rows.forEach((row) => { if (row._tag !== "AssistantPart" || row.group.type !== "context") return row.group.refs.forEach((ref) => { const content = Timeline.resolveContent(input.projection.messageByID().get(ref.messageID), ref.partID) - if (content?.type !== "tool" || content.name !== "patch" || content.state.status === "error") return + if ( + content?.type !== "tool" || + !["edit", "write", "patch"].includes(content.name) || + content.state.status === "error" + ) + return const part = `${ref.messageID}:${ref.partID}` const key = patchGroupKeys.get(part) if (key && !owners.has(key)) owners.set(key, part) diff --git a/packages/session-ui/src/tools/tool-renderer.tsx b/packages/session-ui/src/tools/tool-renderer.tsx index 9ccea209e0f..c2ae0a2ba6b 100644 --- a/packages/session-ui/src/tools/tool-renderer.tsx +++ b/packages/session-ui/src/tools/tool-renderer.tsx @@ -16,7 +16,7 @@ import { type JSX, } from "solid-js" import stripAnsi from "strip-ansi" -import { createTwoFilesPatch } from "diff" +import { createTwoFilesPatch, diffLines } from "diff" import { Dynamic } from "solid-js/web" import { type SessionSummary, useData } from "../context" import { useFileComponent } from "@opencode/ui/context/file" @@ -566,13 +566,7 @@ export function CurrentContextToolGroup(props: { return groups } const previous = groups.at(-1) - if ( - tool.name === "patch" && - tool.state.status !== "error" && - Array.isArray(previous) && - previous?.[0]?.name === "patch" && - previous[0].state.status !== "error" - ) { + if (isFileChangeTool(tool) && Array.isArray(previous) && previous[0] && isFileChangeTool(previous[0])) { previous.push(tool) return groups } @@ -595,7 +589,7 @@ export function CurrentContextToolGroup(props: { const patchKeys = createMemo(() => { const keys = new Map() items().forEach((item) => { - if (!Array.isArray(item) || item[0]?.name !== "patch" || item[0].state.status === "error") return + if (!Array.isArray(item) || !item[0] || !isFileChangeTool(item[0])) return const key = props.patchGroupKey?.(item) ?? item[0].id item.forEach((tool) => keys.set(tool, key)) }) @@ -715,7 +709,7 @@ export function CurrentContextToolGroup(props: { when={tool().name === "skill" && group().length > 1 && skills().length === group().length} fallback={ 0) return files.map((value, index) => ({ key: `${tool.id}:${index}`, toolID: tool.id, value })) - if (tool.name !== "write") return [] const input = currentToolInput(tool) - if (typeof input.path !== "string" || typeof input.content !== "string" || !input.content) return [] + if (typeof input.path !== "string") return [] + if (tool.name === "edit" && typeof input.oldString === "string" && typeof input.newString === "string") { + const changes = diffLines(input.oldString, input.newString) + const additions = changes + .filter((change) => change.added) + .reduce((total, change) => total + (change.count ?? 0), 0) + const deletions = changes + .filter((change) => change.removed) + .reduce((total, change) => total + (change.count ?? 0), 0) + if (additions === 0 && deletions === 0) return [] + return [ + { + key: `${tool.id}:0`, + toolID: tool.id, + value: { + file: input.path, + patch: createTwoFilesPatch(input.path, input.path, input.oldString, input.newString), + additions, + deletions, + status: "modified", + }, + }, + ] + } + if (tool.name !== "write" || typeof input.content !== "string" || !input.content) return [] return [ { key: `${tool.id}:0`, @@ -912,6 +929,10 @@ export function CurrentFileToolGroup(props: { ) } +function isFileChangeTool(tool: SessionMessageAssistantTool) { + return tool.state.status !== "error" && (tool.name === "edit" || tool.name === "write" || tool.name === "patch") +} + function samePatchFile(a: unknown, b: unknown) { if (a === b) return true if (!record(a) || !record(b)) return false