Merge pull request #10782 from Kilo-Org/dune-earwig

fix(vscode): preserve diff scroll during agent edits
2026-09-21 05:52:35 +08:00 · 2026-06-01 11:53:33 +02:00
parent e4b808e267 695d1d0dd3
commit d1c2fa3990
8 changed files with 243 additions and 5 deletions
@@ -0,0 +1,5 @@
+---
+"kilo-code": patch
+---
+
+Preserve the Changes review scroll position while agents update files.
@@ -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)
+})
@@ -50,6 +50,7 @@ async function disableAnimations(page: Page) {
 // Permission dock config-preloaded has non-deterministic toggle rendering.
 const SKIP = new Set<string>([
  "agentmanager--worktree-item-busy",
+  "agentmanager--full-screen-diff-agent-edit-scroll",
  "composite-webview--permission-dock-config-preloaded",
 ])

@@ -50,6 +50,7 @@ async function disableAnimations(page: Page) {
 // Permission dock config-preloaded has non-deterministic toggle rendering.
 const SKIP = new Set<string>([
  "agentmanager--worktree-item-busy",
+  "agentmanager--full-screen-diff-agent-edit-scroll",
  "agentmanager--pr-badge-checks-pending",
  "composite-webview--permission-dock-config-preloaded",
 ])
@@ -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<DiffPanelProps> = (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<DiffPanelProps> = (props) => {
      <Show when={props.diffs.length > 0}>
        <div class="am-diff-content" data-component="session-review" ref={scroller}>
          <Accordion multiple value={open()} onChange={setOpen}>
-            <For each={sorted()}>
+            <For each={rows()}>
              {(diff) => {
                const isAdded = () => diff.status === "added"
                const isDeleted = () => diff.status === "deleted"
@@ -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<FullScreenDiffViewProps> = (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<FullScreenDiffViewProps> = (props) =>
          <Show when={props.diffs.length > 0}>
            <div class="am-review-diff-content" data-component="session-review">
              <Accordion multiple value={open()} onChange={setOpen}>
-                <For each={sorted()}>
+                <For each={rows()}>
                  {(diff) => {
                    const isAdded = () => diff.status === "added"
                    const isDeleted = () => diff.status === "deleted"
@@ -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 <For> 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<string, { diff: WorktreeFileDiff; set: (diff: WorktreeFileDiff) => void }>()
+  let current: string | undefined
+
+  return createMemo(() => {
+    const nextKey = key()
+    if (current !== nextKey) {
+      current = nextKey
+      cache.clear()
+    }
+
+    const files = new Set<string>()
+    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.
@@ -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 (
+      <StoryProviders noPadding>
+        <div style={{ height: "700px", display: "flex", "flex-direction": "column" }}>
+          <div style={{ display: "flex", gap: "8px", padding: "4px", "align-items": "center" }}>
+            <Button size="small" onClick={update}>
+              Apply agent edit
+            </Button>
+            <Button size="small" onClick={change}>
+              Switch review context
+            </Button>
+            <span data-testid="agent-edit-version">{version()}</span>
+            <span data-testid="review-context">{key()}</span>
+          </div>
+          <div style={{ display: "flex", "min-height": "0", flex: "1" }}>
+            <FullScreenDiffView
+              diffs={diffs()}
+              loading={false}
+              sessionKey={key()}
+              diffStyle="unified"
+              onDiffStyleChange={() => {}}
+              comments={[]}
+              onCommentsChange={() => {}}
+              onClose={() => {}}
+            />
+          </div>
+        </div>
+      </StoryProviders>
+    )
+  },
+}
+
 // ---------------------------------------------------------------------------
 // WorktreeItem — shared mock helpers
 // ---------------------------------------------------------------------------