Merge pull request #13200 from Kilo-Org/fix-inline-diff-viewer-scroll-jump

fix(vscode): stabilize Agent Manager inline diff scrolling
This commit is contained in:
Marius
2026-08-18 11:58:53 +02:00
committed by GitHub
11 changed files with 181 additions and 9 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---
Keep the Agent Manager inline diff position stable while scrolling upward through large reviews
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c1115e3b2056f1b00c6b9a657f61c3540e00b14ec0ff4fb04f072157bb5059b3
size 17935
@@ -24,6 +24,7 @@ export function Diff<T>(props: SSRDiffProps<T>) {
"selectedLines",
"commentedLines",
"virtualized",
"sizeKey",
])
const workerPool = useWorkerPool(props.diffStyle)
+33 -1
View File
@@ -23,6 +23,23 @@ const MIN_PLACEHOLDER_HEIGHT = 160
const MAX_PLACEHOLDER_HEIGHT = 1200
type Job = { run: () => void; cancelled: boolean }
const sizes = new WeakMap<object, Map<number, number>>()
const WIDTH_LIMIT = 8
function remember(key: object | undefined, width: number, height: number) {
if (!key || width <= 0 || height <= 0) return
const widths = sizes.get(key) ?? new Map<number, number>()
widths.delete(width)
widths.set(width, height)
if (widths.size > WIDTH_LIMIT) widths.delete(widths.keys().next().value!)
sizes.set(key, widths)
}
function reserved(key: object | undefined, width: number) {
if (!key || width <= 0) return
return sizes.get(key)?.get(width)
}
// A review can contain many expanded diff components. Creating one
// IntersectionObserver per diff showed up in profiles, so all deferred diffs
// share a single observer and only register their element + render callback.
@@ -173,6 +190,7 @@ export function Diff<T>(props: DiffProps<T>) {
"commentedLines",
"onRendered",
"virtualized",
"sizeKey",
])
const mobile = createMediaQuery("(max-width: 640px)")
@@ -247,7 +265,7 @@ export function Diff<T>(props: DiffProps<T>) {
createEffect(() => {
if (visible()) return
container.style.minHeight = `${estimate()}px`
container.style.minHeight = `${reserved(local.sizeKey, container.clientWidth) ?? estimate()}px`
})
createEffect(() => {
@@ -266,6 +284,17 @@ export function Diff<T>(props: DiffProps<T>) {
return root
}
createEffect(() => {
if (typeof ResizeObserver === "undefined") return
const resize = new ResizeObserver(() => {
const root = getRoot()
if (!visible() || !current() || !root?.querySelector("[data-line]")) return
remember(local.sizeKey, container.clientWidth, container.offsetHeight)
})
resize.observe(container)
onCleanup(() => resize.disconnect())
})
const applyScheme = () => {
const host = container.querySelector("diffs-container")
if (!(host instanceof HTMLElement)) return
@@ -370,6 +399,7 @@ export function Diff<T>(props: DiffProps<T>) {
if (token !== renderToken) return
// Clear the height pin now that Pierre has rendered new content.
container.style.minHeight = ""
remember(local.sizeKey, container.clientWidth, container.offsetHeight)
setSelectedLines(lastSelection)
local.onRendered?.()
})
@@ -411,6 +441,7 @@ export function Diff<T>(props: DiffProps<T>) {
if (typeof MutationObserver === "undefined") {
container.style.minHeight = ""
if (!root || !isReady(root)) return
remember(local.sizeKey, container.clientWidth, container.offsetHeight)
setSelectedLines(lastSelection)
local.onRendered?.()
return
@@ -777,6 +808,7 @@ export function Diff<T>(props: DiffProps<T>) {
if (!instance) return
instance.setLineAnnotations(annotations ?? [])
instance.rerender()
notifyRendered()
},
{ defer: true },
),
+3
View File
@@ -58,6 +58,9 @@ type DiffShared<T> = FileDiffOptions<T> & {
// files so eager rendering does not expand full before/after content.
// Defaults to virtualized.
virtualized?: boolean
// Stable rendered-content identity used to preserve deferred height when a
// surrounding row virtualizer unmounts and later re-creates this diff.
sizeKey?: object
class?: string
classList?: ComponentProps<"div">["classList"]
}
@@ -2,11 +2,16 @@ 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"
const INLINE_STORY_ID = "agentmanager--diff-panel-scroll-up"
function storyUrl() {
return `/iframe.html?id=${STORY_ID}&viewMode=story&globals=${GLOBALS}`
}
function inlineStoryUrl() {
return `/iframe.html?id=${INLINE_STORY_ID}&viewMode=story&globals=${GLOBALS}`
}
async function disableAnimations(page: Page) {
await page.addStyleTag({
content: `
@@ -159,3 +164,60 @@ test("resets virtual measurements and scroll when the review context changes", a
await expect.poll(async () => first.evaluate((el) => el.getBoundingClientRect().height)).toBe(1_200)
await expect.poll(async () => scroller.evaluate((el) => el.scrollTop)).toBe(0)
})
test("keeps the inline diff position stable while scrolling upward", async ({ page }) => {
await page.setViewportSize({ width: 900, height: 760 })
await page.goto(inlineStoryUrl(), { waitUntil: "load" })
await disableAnimations(page)
await page.waitForSelector(".am-diff-content diffs-container", { state: "attached" })
const result = await page.locator(".am-diff-content").evaluate(async (el) => {
const frame = () => new Promise((resolve) => requestAnimationFrame(resolve))
const settle = async (count: number) => {
for (let i = 0; i < count; i++) await frame()
}
const seen = new Set(
Array.from(el.querySelectorAll("[data-file-path]"), (row) => row.getAttribute("data-file-path")),
)
let remounts = 0
const observer = new MutationObserver((records) => {
for (const record of records) {
for (const node of record.addedNodes) {
if (!(node instanceof HTMLElement)) continue
const rows = node.matches("[data-file-path]") ? [node] : Array.from(node.querySelectorAll("[data-file-path]"))
for (const row of rows) {
const file = row.getAttribute("data-file-path")
if (seen.has(file)) remounts++
seen.add(file)
}
}
}
})
observer.observe(el, { childList: true, subtree: true })
// Materialize every row once, then start from the settled bottom. The bug
// appears when upward scrolling re-creates rows above the viewport.
while (el.scrollTop < el.scrollHeight - el.clientHeight - 1) {
el.scrollTop = Math.min(el.scrollHeight - el.clientHeight, el.scrollTop + 120)
await frame()
}
await settle(30)
let correction = 0
let range = 0
while (el.scrollTop > 0) {
const height = el.scrollHeight
const intended = Math.max(0, el.scrollTop - 80)
el.scrollTop = intended
await settle(2)
correction = Math.max(correction, Math.abs(el.scrollTop - intended))
range = Math.max(range, Math.abs(el.scrollHeight - height))
}
observer.disconnect()
return { correction, range, remounts }
})
expect(result.remounts).toBeGreaterThan(0)
expect(result.correction).toBeLessThanOrEqual(1)
expect(result.range).toBeLessThanOrEqual(1)
})
@@ -1,5 +1,5 @@
import { describe, expect, it } from "bun:test"
import { mergeWorktreeDiffs } from "../../webview-ui/diff-viewer/diff-state"
import { diffSizeKey, mergeWorktreeDiffs } from "../../webview-ui/diff-viewer/diff-state"
import {
EXTREME_DIFF_CHANGED_LINES,
allOpenFiles,
@@ -28,6 +28,18 @@ function diff(overrides: Partial<WorktreeFileDiff>): WorktreeFileDiff {
}
}
describe("diffSizeKey", () => {
it("changes with rendered content, style, and review context", () => {
const base = diff({ summarized: false, patch: "@@ -1 +1 @@\n-old\n+new\n" })
const key = diffSizeKey("review-a", base, "unified")
expect(diffSizeKey("review-a", base, "unified")).toBe(key)
expect(diffSizeKey("review-b", base, "unified")).not.toBe(key)
expect(diffSizeKey("review-a", base, "split")).not.toBe(key)
expect(diffSizeKey("review-a", { ...base, patch: "@@ -1 +1 @@\n-old\n+newer\n" }, "unified")).not.toBe(key)
})
})
describe("agent manager diff state", () => {
it("preserves loaded detail and patch when summary metadata is unchanged", () => {
const prev = [diff({ summarized: false, before: "old\n", after: "new\n", patch: "@@ -1 +1 @@\n-old\n+new\n" })]
@@ -60,7 +60,7 @@ import { VirtualDiffList } from "../diff-viewer/VirtualDiffList"
import { treeOrder } from "../diff-viewer/file-tree-utils"
import { isMarkdownFile, MarkdownDiffView } from "../diff-viewer/MarkdownDiffView"
import { ImageDiffView } from "../diff-viewer/ImageDiffView"
import { createDiffRows } from "../diff-viewer/diff-state"
import { createDiffRows, diffSizeKey } from "../diff-viewer/diff-state"
import { createDiffRequests } from "../diff-viewer/diff-requests"
// --- Data model ---
@@ -728,6 +728,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
after={{ name: diff.file, contents: diff.after }}
patch={diff.patch}
diffStyle={props.diffStyle ?? "unified"}
sizeKey={diffSizeKey(props.sessionKey, diff, props.diffStyle ?? "unified")}
virtualized={shouldVirtualizeDiff(diff)}
annotations={annotationsForFile(diff.file)}
renderAnnotation={buildAnnotation}
@@ -61,7 +61,7 @@ import { DiffEndMarker } from "./DiffEndMarker"
import { VirtualDiffList } from "./VirtualDiffList"
import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView"
import { ImageDiffView } from "./ImageDiffView"
import { createDiffRows } from "./diff-state"
import { createDiffRows, diffSizeKey } from "./diff-state"
import { createDiffRequests } from "./diff-requests"
type DiffStyle = "unified" | "split"
@@ -802,6 +802,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
after={{ name: diff.file, contents: diff.after }}
patch={diff.patch}
diffStyle={props.diffStyle}
sizeKey={diffSizeKey(props.sessionKey, diff, props.diffStyle)}
virtualized={shouldVirtualizeDiff(diff)}
annotations={annotationsForFile(diff.file)}
renderAnnotation={buildAnnotation}
@@ -1,6 +1,18 @@
import { createMemo, createSignal } from "solid-js"
import type { WorktreeFileDiff } from "../src/types/messages"
const sizeKeys = new WeakMap<
WorktreeFileDiff,
{
context: string | undefined
style: string
patch: string | undefined
before: string
after: string
key: object
}
>()
export function sameDiffMeta(left: WorktreeFileDiff, right: WorktreeFileDiff) {
return (
left.file === right.file &&
@@ -20,6 +32,23 @@ export function diffToken(diff: WorktreeFileDiff) {
return diff.stamp ?? parts.join(":")
}
export function diffSizeKey(context: string | undefined, diff: WorktreeFileDiff, style: string) {
const cached = sizeKeys.get(diff)
if (
cached &&
cached.context === context &&
cached.style === style &&
cached.patch === diff.patch &&
cached.before === diff.before &&
cached.after === diff.after
)
return cached.key
const key = {}
sizeKeys.set(diff, { context, style, patch: diff.patch, before: diff.before, after: diff.after, key })
return key
}
// 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.
@@ -87,13 +87,13 @@ const foldedDiffs: WorktreeFileDiff[] = [
]
const ROWS = 140
function edited(seed: string): WorktreeFileDiff {
function edited(seed: string, file = "src/agent-edit.ts"): 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",
`diff --git a/${file} b/${file}`,
`--- a/${file}`,
`+++ b/${file}`,
`@@ -1,${ROWS} +1,${ROWS} @@`,
...before
.trimEnd()
@@ -107,7 +107,7 @@ function edited(seed: string): WorktreeFileDiff {
].join("\n")
return {
file: "src/agent-edit.ts",
file,
status: "modified",
additions: ROWS,
deletions: ROWS,
@@ -322,6 +322,29 @@ export const DiffPanelWithDiffs: Story = {
),
}
export const DiffPanelScrollUp: Story = {
name: "DiffPanel - scroll upward through large diffs",
render: () => {
const diffs = Array.from({ length: 5 }, (_, i) => edited(`review-${i}`, `src/review-${i}.ts`))
return (
<StoryProviders noPadding>
<div style={{ height: "700px", display: "flex", "flex-direction": "column" }}>
<DiffPanel
diffs={diffs}
loading={false}
sessionKey="inline-scroll-up"
diffStyle="unified"
onDiffStyleChange={() => {}}
comments={[]}
onCommentsChange={() => {}}
onClose={() => {}}
/>
</div>
</StoryProviders>
)
},
}
const buttonFixtureStyle: JSX.CSSProperties = {
display: "inline-flex",
"align-items": "center",