diff --git a/.changeset/steady-review-scroll.md b/.changeset/steady-review-scroll.md new file mode 100644 index 00000000000..aba17d76729 --- /dev/null +++ b/.changeset/steady-review-scroll.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Preserve the Changes review scroll position while agents update files. diff --git a/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts b/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts new file mode 100644 index 00000000000..82246023dce --- /dev/null +++ b/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts @@ -0,0 +1,102 @@ +import { expect, test, type Page } from "@playwright/test" + +const GLOBALS = "colorScheme:dark;theme:kilo-vscode;vscodeTheme:dark-modern" +const STORY_ID = "agentmanager--full-screen-diff-agent-edit-scroll" + +function storyUrl() { + return `/iframe.html?id=${STORY_ID}&viewMode=story&globals=${GLOBALS}` +} + +async function disableAnimations(page: Page) { + await page.addStyleTag({ + content: ` + *, *::before, *::after { + animation-duration: 0s !important; + animation-delay: 0s !important; + transition-duration: 0s !important; + transition-delay: 0s !important; + } + `, + }) +} + +async function openStory(page: Page) { + await page.setViewportSize({ width: 800, height: 720 }) + await page.addInitScript(() => { + const win = window as Window & { nativeIntersectionObserver?: typeof IntersectionObserver } + win.nativeIntersectionObserver = window.IntersectionObserver + Object.defineProperty(window, "IntersectionObserver", { configurable: true, value: undefined, writable: true }) + }) + await page.goto(storyUrl(), { waitUntil: "load" }) + await disableAnimations(page) + await page.waitForSelector("#storybook-root *", { state: "attached" }) + + const first = page.locator('[data-file-path="src/agent-edit.ts"] [data-component="diff"]') + await expect.poll(async () => first.evaluate((el) => el.getBoundingClientRect().height)).toBeGreaterThan(3_000) + return first +} + +test("preserves diff scroll position while an agent edit refreshes a file", async ({ page }) => { + const first = await openStory(page) + const scroller = page.locator(".am-review-diff") + const target = page.locator('[data-file-path="src/target.ts"]') + + // The initial tall diff rendered eagerly. Restore the real observer before + // moving it offscreen so an unfixed row remount takes the deferred path. + await page.evaluate(() => { + const win = window as Window & { nativeIntersectionObserver?: typeof IntersectionObserver } + Object.defineProperty(window, "IntersectionObserver", { + configurable: true, + value: win.nativeIntersectionObserver, + writable: true, + }) + }) + + await scroller.evaluate((el) => { + const target = el.querySelector('[data-file-path="src/target.ts"]') + if (!(target instanceof HTMLElement)) throw new Error("Target diff row not found") + el.scrollTop += target.getBoundingClientRect().top - el.getBoundingClientRect().top - 24 + }) + + const before = await scroller.evaluate((el) => el.scrollTop) + const top = await target.evaluate((el) => el.getBoundingClientRect().top) + expect(before).toBeGreaterThan(3_000) + + await page.getByRole("button", { name: "Apply agent edit" }).click() + await expect(page.getByTestId("agent-edit-version")).toHaveText("after") + await expect.poll(async () => first.evaluate((el) => el.getBoundingClientRect().height)).toBeGreaterThan(3_000) + await page.evaluate(() => new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve)))) + + const after = await scroller.evaluate((el) => el.scrollTop) + const next = await target.evaluate((el) => el.getBoundingClientRect().top) + expect(after).toBeCloseTo(before, 0) + expect(next).toBeCloseTo(top, 0) +}) + +test("remounts diff rows when the review context changes", async ({ page }) => { + const first = await openStory(page) + await page.evaluate(() => { + class IdleObserver { + readonly root = null + readonly rootMargin = "0px" + readonly thresholds = [] + + disconnect() {} + observe() {} + takeRecords() { + return [] + } + unobserve() {} + } + + Object.defineProperty(window, "IntersectionObserver", { + configurable: true, + value: IdleObserver, + writable: true, + }) + }) + + await page.getByRole("button", { name: "Switch review context" }).click() + await expect(page.getByTestId("review-context")).toHaveText("changed-context") + await expect.poll(async () => first.evaluate((el) => el.getBoundingClientRect().height)).toBe(1_200) +}) diff --git a/packages/kilo-vscode/tests/visual-regression.spec.mts b/packages/kilo-vscode/tests/visual-regression.spec.mts index 6877dfe7b6c..0ac3d831970 100644 --- a/packages/kilo-vscode/tests/visual-regression.spec.mts +++ b/packages/kilo-vscode/tests/visual-regression.spec.mts @@ -50,6 +50,7 @@ async function disableAnimations(page: Page) { // Permission dock config-preloaded has non-deterministic toggle rendering. const SKIP = new Set([ "agentmanager--worktree-item-busy", + "agentmanager--full-screen-diff-agent-edit-scroll", "composite-webview--permission-dock-config-preloaded", ]) diff --git a/packages/kilo-vscode/tests/visual-regression.spec.ts b/packages/kilo-vscode/tests/visual-regression.spec.ts index 75940ae1de5..b1b7fe412c6 100644 --- a/packages/kilo-vscode/tests/visual-regression.spec.ts +++ b/packages/kilo-vscode/tests/visual-regression.spec.ts @@ -50,6 +50,7 @@ async function disableAnimations(page: Page) { // Permission dock config-preloaded has non-deterministic toggle rendering. const SKIP = new Set([ "agentmanager--worktree-item-busy", + "agentmanager--full-screen-diff-agent-edit-scroll", "agentmanager--pr-badge-checks-pending", "composite-webview--permission-dock-config-preloaded", ]) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx index acef4a9c2f1..ffba465c16a 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx @@ -41,7 +41,7 @@ import { import { DiffEndMarker } from "./DiffEndMarker" import { treeOrder } from "./file-tree-utils" import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView" -import { diffToken } from "./diff-state" +import { createDiffRows, diffToken } from "./diff-state" // --- Data model --- @@ -120,6 +120,7 @@ export const DiffPanel: Component = (props) => { // Reorder diffs to match the file-tree's depth-first visual order so // scrolling through the accordion matches the tree grouping. const sorted = createMemo(() => treeOrder(props.diffs)) + const rows = createDiffRows(sorted, () => props.sessionKey) const eager = createMemo(() => eagerDiffFiles(sorted())) const comments = () => props.comments @@ -484,7 +485,7 @@ export const DiffPanel: Component = (props) => { 0}>
- + {(diff) => { const isAdded = () => diff.status === "added" const isDeleted = () => diff.status === "deleted" diff --git a/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx b/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx index d609816367e..8920a04850c 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/FullScreenDiffView.tsx @@ -48,7 +48,7 @@ import { } from "./diff-open-policy" import { DiffEndMarker } from "./DiffEndMarker" import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView" -import { diffToken } from "./diff-state" +import { createDiffRows, diffToken } from "./diff-state" type DiffStyle = "unified" | "split" @@ -136,6 +136,7 @@ export const FullScreenDiffView: Component = (props) => // Reorder diffs to match the file-tree's depth-first visual order so // scrolling through the diff panel matches the tree on the left. const sorted = createMemo(() => treeOrder(props.diffs)) + const rows = createDiffRows(sorted, () => props.sessionKey) const eager = createMemo(() => eagerDiffFiles(sorted())) const comments = () => props.comments @@ -581,7 +582,7 @@ export const FullScreenDiffView: Component = (props) => 0}>
- + {(diff) => { const isAdded = () => diff.status === "added" const isDeleted = () => diff.status === "deleted" diff --git a/packages/kilo-vscode/webview-ui/agent-manager/diff-state.ts b/packages/kilo-vscode/webview-ui/agent-manager/diff-state.ts index e54e156e0ff..4d977e13ce0 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/diff-state.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/diff-state.ts @@ -1,3 +1,4 @@ +import { createMemo, createSignal } from "solid-js" import type { WorktreeFileDiff } from "../src/types/messages" export function sameDiffMeta(left: WorktreeFileDiff, right: WorktreeFileDiff) { @@ -18,6 +19,45 @@ export function diffToken(diff: WorktreeFileDiff) { return diff.stamp ?? parts.join(":") } +// Keep each rendered row mounted while live detail refreshes replace its data. +// Otherwise Solid's keyed remounts the row and deferred rendering swaps a +// previously rendered diff above the viewport for a short placeholder. +export function createDiffRows(source: () => WorktreeFileDiff[], key: () => string | undefined) { + const cache = new Map void }>() + let current: string | undefined + + return createMemo(() => { + const nextKey = key() + if (current !== nextKey) { + current = nextKey + cache.clear() + } + + const files = new Set() + const diffs = source().map((next) => { + files.add(next.file) + const cached = cache.get(next.file) + if (cached) { + cached.set(next) + return cached.diff + } + + const [value, setValue] = createSignal(next) + const diff = new Proxy(next, { + get: (_, prop) => Reflect.get(value(), prop), + }) + cache.set(next.file, { diff, set: setValue }) + return diff + }) + + for (const file of cache.keys()) { + if (files.has(file)) continue + cache.delete(file) + } + return diffs + }) +} + export interface MergeResult { diffs: WorktreeFileDiff[] /** Files whose metadata changed while we preserved cached content. diff --git a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx index 1c1fa21f2c3..36eca47f4af 100644 --- a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx +++ b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx @@ -15,7 +15,7 @@ import { IconButton } from "@kilocode/kilo-ui/icon-button" import { Icon } from "@kilocode/kilo-ui/icon" import { TooltipKeybind } from "@kilocode/kilo-ui/tooltip" import { ContextMenu } from "@kilocode/kilo-ui/context-menu" -import type { JSX } from "solid-js" +import { createSignal, type JSX } from "solid-js" import type { WorktreeFileDiff, WorktreeState, WorktreeGitStats, PRStatus } from "../types/messages" import "../../agent-manager/agent-manager.css" import "../../agent-manager/agent-manager-review.css" @@ -63,6 +63,48 @@ const foldedDiffs: WorktreeFileDiff[] = [ }, ] +const ROWS = 140 +function edited(seed: string): WorktreeFileDiff { + const before = Array.from({ length: ROWS }, (_, i) => `const row${i} = "${seed}-old-${i}"\n`).join("") + const after = Array.from({ length: ROWS }, (_, i) => `const row${i} = "${seed}-new-${i}"\n`).join("") + const patch = [ + "diff --git a/src/agent-edit.ts b/src/agent-edit.ts", + "--- a/src/agent-edit.ts", + "+++ b/src/agent-edit.ts", + `@@ -1,${ROWS} +1,${ROWS} @@`, + ...before + .trimEnd() + .split("\n") + .map((line) => `-${line}`), + ...after + .trimEnd() + .split("\n") + .map((line) => `+${line}`), + "", + ].join("\n") + + return { + file: "src/agent-edit.ts", + status: "modified", + additions: ROWS, + deletions: ROWS, + before, + after, + patch, + } +} + +const tail: WorktreeFileDiff = { + file: "src/target.ts", + status: "modified", + additions: 1, + deletions: 1, + before: "const target = 'before'\n", + after: "const target = 'after'\n", + patch: + "diff --git a/src/target.ts b/src/target.ts\n--- a/src/target.ts\n+++ b/src/target.ts\n@@ -1 +1 @@\n-const target = 'before'\n+const target = 'after'\n", +} + // --------------------------------------------------------------------------- // Meta // --------------------------------------------------------------------------- @@ -235,6 +277,51 @@ export const FullScreenDiffWithCollapsedContext: Story = { ), } +export const FullScreenDiffAgentEditScroll: Story = { + name: "FullScreenDiffView - preserve scroll during agent edit", + render: () => { + const [diffs, setDiffs] = createSignal([edited("before"), tail]) + const [version, setVersion] = createSignal("before") + const [key, setKey] = createSignal("agent-edit-scroll") + const update = () => { + setDiffs([edited("after"), tail]) + setVersion("after") + } + const change = () => { + setDiffs([edited("context"), tail]) + setKey("changed-context") + } + return ( + +
+
+ + + {version()} + {key()} +
+
+ {}} + comments={[]} + onCommentsChange={() => {}} + onClose={() => {}} + /> +
+
+
+ ) + }, +} + // --------------------------------------------------------------------------- // WorktreeItem — shared mock helpers // ---------------------------------------------------------------------------