Merge pull request #10846 from Kilo-Org/rune-airship

fix(vscode): preserve inline review drafts across diff refreshes
This commit is contained in:
Marius
2026-06-03 12:59:04 +02:00
committed by GitHub
7 changed files with 276 additions and 34 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---
Preserve unfinished inline review comments while diffs refresh.
@@ -9,7 +9,12 @@ import {
} from "../../webview-ui/agent-manager/review-comments"
import { markdownCommentBlocks } from "../../webview-ui/agent-manager/markdown-comment-ranges"
import {
buildFileAnnotations,
clearReviewComposer,
createReviewComposer,
reviewAnnotationSpeechKey,
reviewComposerDraft,
reviewComposerEdit,
reviewDraftSpeechKey,
reviewEditSpeechKey,
} from "../../webview-ui/agent-manager/review-annotations"
@@ -288,6 +293,93 @@ describe("review annotation speech keys", () => {
})
})
// ── buildFileAnnotations composer metadata ─────────────────────────────────
describe("buildFileAnnotations composer metadata", () => {
it("preserves unfinished draft text when an annotation is rebuilt", () => {
const draft = { file: "a.ts", side: "additions" as const, line: 2 }
const first = buildFileAnnotations("a.ts", [], null, draft, null, null)
if (!first.draftMeta) throw new Error("expected draft metadata")
first.draftMeta.text = "unfinished draft"
const next = buildFileAnnotations("a.ts", [], null, draft, first.draftMeta, first.editMeta)
expect(next.draftMeta).toBe(first.draftMeta)
expect(next.draftMeta?.text).toBe("unfinished draft")
})
it("creates fresh draft metadata when the anchor changes", () => {
const draft = { file: "a.ts", side: "additions" as const, line: 2 }
const first = buildFileAnnotations("a.ts", [], null, draft, null, null)
const next = buildFileAnnotations("a.ts", [], null, { ...draft, line: 3 }, first.draftMeta, first.editMeta)
expect(next.draftMeta).not.toBe(first.draftMeta)
})
it("preserves unfinished edits when an annotation is rebuilt", () => {
const current = comment({ file: "a.ts", line: 2 })
const first = buildFileAnnotations("a.ts", [current], current.id, null, null, null)
if (!first.editMeta) throw new Error("expected edit metadata")
first.editMeta.text = "unfinished edit"
const next = buildFileAnnotations("a.ts", [current], current.id, null, first.draftMeta, first.editMeta)
expect(next.editMeta).toBe(first.editMeta)
expect(next.editMeta?.text).toBe("unfinished edit")
})
it("creates fresh edit metadata when the edited comment changes", () => {
const firstComment = comment({ file: "a.ts", line: 2 })
const secondComment = comment({ file: "a.ts", line: 3 })
const first = buildFileAnnotations("a.ts", [firstComment], firstComment.id, null, null, null)
const next = buildFileAnnotations("a.ts", [secondComment], secondComment.id, null, first.draftMeta, first.editMeta)
expect(next.editMeta).not.toBe(first.editMeta)
})
it("drops unfinished edit metadata after edit mode ends", () => {
const current = comment({ file: "a.ts", line: 2 })
const first = buildFileAnnotations("a.ts", [current], current.id, null, null, null)
const next = buildFileAnnotations("a.ts", [current], null, null, first.draftMeta, first.editMeta)
expect(next.editMeta).toBeNull()
})
it("hands draft and edit composers between review surfaces", () => {
const current = comment({ file: "a.ts", line: 2 })
const composer = createReviewComposer()
const draft = { file: "a.ts", side: "additions" as const, line: 3 }
const first = buildFileAnnotations("a.ts", [current], current.id, draft, null, null)
if (!first.draftMeta || !first.editMeta) throw new Error("expected composer metadata")
first.draftMeta.text = "unfinished draft"
first.editMeta.text = "unfinished edit"
composer.draft = first.draftMeta
composer.edit = first.editMeta
expect(reviewComposerDraft(composer)).toEqual(draft)
expect(reviewComposerEdit(composer)).toBe(current.id)
expect(composer.draft.text).toBe("unfinished draft")
expect(composer.edit.text).toBe("unfinished edit")
})
it("clears handed-off composers when the review context changes", () => {
const composer = createReviewComposer()
composer.draft = { type: "draft", comment: null, file: "a.ts", side: "additions", line: 2 }
composer.edit = {
type: "comment",
comment: comment({ file: "a.ts", line: 2 }),
file: "a.ts",
side: "additions",
line: 2,
}
clearReviewComposer(composer)
expect(reviewComposerDraft(composer)).toBeNull()
expect(reviewComposerEdit(composer)).toBeNull()
})
})
// ── getDirectory / getFilename ──────────────────────────────────────────────
describe("getDirectory", () => {
@@ -107,6 +107,7 @@ import { FullScreenDiffView } from "./FullScreenDiffView"
import { ApplyDialog } from "./ApplyDialog"
import { groupApplyConflicts } from "./apply-conflicts"
import type { ReviewComment } from "./review-comments"
import { clearReviewComposer, createReviewComposer } from "./review-annotations"
import { CurrentTabsMenu, createCurrentTabItems, focusCurrentTab } from "./CurrentTabsMenu"
import { BranchSelect } from "../src/components/shared/BranchSelect"
import { WorktreeItem } from "./WorktreeItem"
@@ -246,6 +247,7 @@ const AgentManagerContent: Component = () => {
const [reviewOpenByContext, setReviewOpenByContext] = createSignal<Record<string, boolean>>({})
const [reviewCommentsByContext, setReviewCommentsByContext] = createSignal<Record<string, ReviewComment[]>>({})
const reviewComposer = createReviewComposer()
const [reviewActive, setReviewActive] = createSignal(false)
const [reviewDiffStyle, setReviewDiffStyle] = createSignal<"unified" | "split">("unified")
const markdown = createMarkdownRender(vscode)
@@ -285,6 +287,7 @@ const AgentManagerContent: Component = () => {
setPendingDelete(null)
}
createEffect(on(selection, () => cancelPendingDelete(), { defer: true }))
createEffect(on(selection, () => clearReviewComposer(reviewComposer), { defer: true }))
onCleanup(() => clearTimeout(pendingDeleteTimer))
// Per-context tab memory: maps sidebar selection key -> last active session/pending ID
@@ -3063,6 +3066,7 @@ const AgentManagerContent: Component = () => {
onMarkdownRenderChange={markdown.update}
comments={reviewComments()}
onCommentsChange={setReviewCommentsForSelection}
composer={reviewComposer}
onClose={() => setSidePanel(null)}
onExpand={selection() !== null ? openReviewTab : undefined}
onRequestDiff={requestDiffFile}
@@ -3092,6 +3096,7 @@ const AgentManagerContent: Component = () => {
sessionKey={diffSessionKey()}
comments={reviewComments()}
onCommentsChange={setReviewCommentsForSelection}
composer={reviewComposer}
onSendAll={closeReviewTab}
diffStyle={reviewDiffStyle()}
onDiffStyleChange={setSharedDiffStyle}
@@ -24,10 +24,16 @@ import { getDirectory, getFilename, lineCount, sanitizeReviewComments, type Revi
import {
buildFileAnnotations,
buildReviewAnnotation,
clearReviewComposer,
createReviewComposer,
reviewComposerDraft,
reviewComposerEdit,
reviewDraftSpeechKey,
reviewEditSpeechKey,
type AnnotationLabels,
type AnnotationMeta,
type ReviewComposer,
type ReviewDraft,
} from "./review-annotations"
import { createReviewAnnotationSpeechRenderer } from "./review-annotation-speech"
import {
@@ -57,6 +63,7 @@ interface DiffPanelProps {
onMarkdownRenderChange?: (render: boolean) => void
comments: ReviewComment[]
onCommentsChange: (comments: ReviewComment[]) => void
composer?: ReviewComposer
onSendAll?: () => void
onClose: () => void
onExpand?: () => void
@@ -90,11 +97,11 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
edit: t("common.edit"),
delete: t("common.delete"),
})
const localComposer = createReviewComposer()
const composer = () => props.composer ?? localComposer
const [open, setOpen] = createSignal<string[]>([])
const [draft, setDraft] = createSignal<{ file: string; side: AnnotationSide; line: number; endLine?: number } | null>(
null,
)
const [editing, setEditing] = createSignal<string | null>(null)
const [draft, setDraft] = createSignal<ReviewDraft | null>(reviewComposerDraft(composer()))
const [editing, setEditing] = createSignal<string | null>(reviewComposerEdit(composer()))
const speechKeys = createMemo(() => {
const keys = new Set<string>()
const current = draft()
@@ -127,9 +134,10 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
const setComments = (next: ReviewComment[]) => props.onCommentsChange(next)
const updateComments = (updater: (prev: ReviewComment[]) => ReviewComment[]) => setComments(updater(comments()))
// Stable draft metadata ref avoids recreating the object on every signal read
// so pierre's annotation cache doesn't invalidate and destroy the textarea
let draftMeta: AnnotationMeta | null = null
// Stable composer metadata refs avoid recreating the object on every signal read
// so pierre's annotation cache doesn't invalidate and destroy the textarea.
let draftMeta: AnnotationMeta | null = composer().draft
let editMeta: AnnotationMeta | null = composer().edit
// Ref to the scrollable container — used to preserve scroll position when
// annotation changes cause pierre to fully re-render diffs
@@ -171,6 +179,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
preserveScroll(() => {
setDraft(null)
draftMeta = null
composer().draft = null
})
focusRoot()
}
@@ -214,7 +223,13 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
() => props.sessionKey,
() => {
requested.clear()
setDraft(null)
draftMeta = null
setEditing(null)
editMeta = null
clearReviewComposer(composer())
},
{ defer: true },
),
)
@@ -250,6 +265,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
updateComments((prev) => [...prev, { id, file, side, line, comment: text, selectedText }])
setDraft(null)
draftMeta = null
composer().draft = null
})
focusRoot()
}
@@ -258,6 +274,8 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
preserveScroll(() => {
updateComments((prev) => prev.map((c) => (c.id === id ? { ...c, comment: text } : c)))
setEditing(null)
editMeta = null
composer().edit = null
})
focusRoot()
}
@@ -265,12 +283,20 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
const deleteComment = (id: string) => {
preserveScroll(() => {
updateComments((prev) => prev.filter((c) => c.id !== id))
if (editing() === id) setEditing(null)
if (editing() === id) {
setEditing(null)
editMeta = null
composer().edit = null
}
})
focusRoot()
}
const setEditState = (id: string | null) => {
if (editing() !== id) {
editMeta = null
composer().edit = null
}
preserveScroll(() => setEditing(id))
if (id === null) focusRoot()
}
@@ -287,6 +313,8 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
const edit = editing()
if (edit && !valid.some((comment) => comment.id === edit)) {
setEditing(null)
editMeta = null
composer().edit = null
}
const currentDraft = draft()
@@ -295,6 +323,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
if (!diff) {
setDraft(null)
draftMeta = null
composer().draft = null
return
}
const content = currentDraft.side === "deletions" ? diff.before : diff.after
@@ -302,11 +331,13 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
if (currentDraft.line < 1 || currentDraft.line > max) {
setDraft(null)
draftMeta = null
composer().draft = null
return
}
if (currentDraft.endLine !== undefined && currentDraft.endLine > max) {
setDraft(null)
draftMeta = null
composer().draft = null
}
},
),
@@ -325,8 +356,11 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
})
const annotationsForFile = (file: string): DiffLineAnnotation<AnnotationMeta>[] => {
const result = buildFileAnnotations(file, commentsByFile().get(file) ?? [], editing(), draft(), draftMeta)
const result = buildFileAnnotations(file, commentsByFile().get(file) ?? [], editing(), draft(), draftMeta, editMeta)
draftMeta = result.draftMeta
editMeta = result.editMeta
composer().draft = draft() ? draftMeta : null
composer().edit = editing() ? editMeta : null
return result.annotations
}
@@ -356,7 +390,10 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
if (draft()) return
const side: AnnotationSide = range.side === "deletions" ? "deletions" : "additions"
preserveScroll(() => {
setDraft({ file, side, line: range.start, endLine: range.end })
const next = { file, side, line: range.start, endLine: range.end }
draftMeta = { type: "draft", comment: null, ...next }
composer().draft = draftMeta
setDraft(next)
})
}
@@ -32,10 +32,16 @@ import { getDirectory, getFilename, lineCount, sanitizeReviewComments, type Revi
import {
buildFileAnnotations,
buildReviewAnnotation,
clearReviewComposer,
createReviewComposer,
reviewComposerDraft,
reviewComposerEdit,
reviewDraftSpeechKey,
reviewEditSpeechKey,
type AnnotationLabels,
type AnnotationMeta,
type ReviewComposer,
type ReviewDraft,
} from "./review-annotations"
import { createReviewAnnotationSpeechRenderer } from "./review-annotation-speech"
import {
@@ -60,6 +66,7 @@ interface FullScreenDiffViewProps {
sessionKey?: string
comments: ReviewComment[]
onCommentsChange: (comments: ReviewComment[]) => void
composer?: ReviewComposer
onSendAll?: () => void
diffStyle: DiffStyle
onDiffStyleChange: (style: DiffStyle) => void
@@ -100,11 +107,11 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
edit: t("common.edit"),
delete: t("common.delete"),
})
const localComposer = createReviewComposer()
const composer = () => props.composer ?? localComposer
const [open, setOpen] = createSignal<string[]>([])
const [draft, setDraft] = createSignal<{ file: string; side: AnnotationSide; line: number; endLine?: number } | null>(
null,
)
const [editing, setEditing] = createSignal<string | null>(null)
const [draft, setDraft] = createSignal<ReviewDraft | null>(reviewComposerDraft(composer()))
const [editing, setEditing] = createSignal<string | null>(reviewComposerEdit(composer()))
const speechKeys = createMemo(() => {
const keys = new Set<string>()
const current = draft()
@@ -123,7 +130,8 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
const [activeFile, setActiveFile] = createSignal<string | null>(null)
const [treeWidth, setTreeWidth] = createSignal(240)
let nextId = 0
let draftMeta: AnnotationMeta | null = null
let draftMeta: AnnotationMeta | null = composer().draft
let editMeta: AnnotationMeta | null = composer().edit
// Tracks the session key for which initial open state has already run. When the
// key changes (different worktree) we expand reviewable files. Within the same key,
// only pruning happens so the user's manual collapse state is preserved.
@@ -174,6 +182,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
preserveScroll(() => {
setDraft(null)
draftMeta = null
composer().draft = null
})
focusRoot()
}
@@ -224,7 +233,13 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
() => props.sessionKey,
() => {
requested.clear()
setDraft(null)
draftMeta = null
setEditing(null)
editMeta = null
clearReviewComposer(composer())
},
{ defer: true },
),
)
@@ -260,6 +275,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
updateComments((prev) => [...prev, { id, file, side, line, comment: text, selectedText }])
setDraft(null)
draftMeta = null
composer().draft = null
})
focusRoot()
}
@@ -268,6 +284,8 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
preserveScroll(() => {
updateComments((prev) => prev.map((c) => (c.id === id ? { ...c, comment: text } : c)))
setEditing(null)
editMeta = null
composer().edit = null
})
focusRoot()
}
@@ -275,12 +293,20 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
const deleteComment = (id: string) => {
preserveScroll(() => {
updateComments((prev) => prev.filter((c) => c.id !== id))
if (editing() === id) setEditing(null)
if (editing() === id) {
setEditing(null)
editMeta = null
composer().edit = null
}
})
focusRoot()
}
const setEditState = (id: string | null) => {
if (editing() !== id) {
editMeta = null
composer().edit = null
}
preserveScroll(() => setEditing(id))
if (id === null) focusRoot()
}
@@ -302,6 +328,8 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
const edit = editing()
if (edit && !valid.some((comment) => comment.id === edit)) {
setEditing(null)
editMeta = null
composer().edit = null
}
const currentDraft = draft()
@@ -310,6 +338,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
if (!diff) {
setDraft(null)
draftMeta = null
composer().draft = null
return
}
const content = currentDraft.side === "deletions" ? diff.before : diff.after
@@ -317,11 +346,13 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
if (currentDraft.line < 1 || currentDraft.line > max) {
setDraft(null)
draftMeta = null
composer().draft = null
return
}
if (currentDraft.endLine !== undefined && currentDraft.endLine > max) {
setDraft(null)
draftMeta = null
composer().draft = null
}
},
),
@@ -340,8 +371,11 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
})
const annotationsForFile = (file: string): DiffLineAnnotation<AnnotationMeta>[] => {
const result = buildFileAnnotations(file, commentsByFile().get(file) ?? [], editing(), draft(), draftMeta)
const result = buildFileAnnotations(file, commentsByFile().get(file) ?? [], editing(), draft(), draftMeta, editMeta)
draftMeta = result.draftMeta
editMeta = result.editMeta
composer().draft = draft() ? draftMeta : null
composer().edit = editing() ? editMeta : null
return result.annotations
}
@@ -365,7 +399,10 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (props) =>
if (draft()) return
const side: AnnotationSide = range.side === "deletions" ? "deletions" : "additions"
preserveScroll(() => {
setDraft({ file, side, line: range.start, endLine: range.end })
const next = { file, side, line: range.start, endLine: range.end }
draftMeta = { type: "draft", comment: null, ...next }
composer().draft = draftMeta
setDraft(next)
})
}
@@ -27,6 +27,7 @@ function insertReviewSpeechText(textarea: HTMLTextAreaElement, value: string): v
textarea.value = result.text
textarea.setSelectionRange(result.pos, result.pos)
textarea.dispatchEvent(new Event("input", { bubbles: true }))
textarea.focus()
}
@@ -24,6 +24,35 @@ export interface AnnotationMeta {
line: number
endLine?: number
editing?: boolean
text?: string
}
export type ReviewDraft = Pick<AnnotationMeta, "file" | "side" | "line" | "endLine">
export interface ReviewComposer {
draft: AnnotationMeta | null
edit: AnnotationMeta | null
}
export function createReviewComposer(): ReviewComposer {
return { draft: null, edit: null }
}
export function clearReviewComposer(composer: ReviewComposer): void {
composer.draft = null
composer.edit = null
}
export function reviewComposerDraft(composer: ReviewComposer): ReviewDraft | null {
const draft = composer.draft
if (!draft || draft.type !== "draft") return null
return { file: draft.file, side: draft.side, line: draft.line, endLine: draft.endLine }
}
export function reviewComposerEdit(composer: ReviewComposer): string | null {
const edit = composer.edit
if (!edit || edit.type !== "comment") return null
return edit.comment?.id ?? null
}
type SpeechDraft = Pick<AnnotationMeta, "file" | "side" | "line" | "endLine">
@@ -71,6 +100,14 @@ function focusWhenConnected(el: HTMLTextAreaElement): void {
requestAnimationFrame(tick)
}
// Keep composer text off the disposable annotation DOM without making each keystroke reactive.
function trackText(meta: AnnotationMeta, textarea: HTMLTextAreaElement, fallback = ""): void {
textarea.value = meta.text ?? fallback
textarea.addEventListener("input", () => {
meta.text = textarea.value
})
}
function makeIcon(pathData: string): SVGSVGElement {
const ns = "http://www.w3.org/2000/svg"
const svg = document.createElementNS(ns, "svg")
@@ -100,21 +137,48 @@ export function buildFileAnnotations(
file: string,
fileComments: ReviewComment[],
edit: string | null,
draft: { file: string; side: AnnotationSide; line: number; endLine?: number } | null,
draft: ReviewDraft | null,
draftMeta: AnnotationMeta | null,
): { annotations: DiffLineAnnotation<AnnotationMeta>[]; draftMeta: AnnotationMeta | null } {
const result: DiffLineAnnotation<AnnotationMeta>[] = fileComments.map((c) => ({
side: c.side,
lineNumber: c.line,
metadata: {
type: "comment" as const,
comment: c,
file: c.file,
side: c.side,
line: c.line,
editing: c.id === edit,
},
}))
editMeta: AnnotationMeta | null,
): {
annotations: DiffLineAnnotation<AnnotationMeta>[]
draftMeta: AnnotationMeta | null
editMeta: AnnotationMeta | null
} {
if (!edit) editMeta = null
const result: DiffLineAnnotation<AnnotationMeta>[] = fileComments.map((c) => {
if (c.id !== edit) {
return {
side: c.side,
lineNumber: c.line,
metadata: {
type: "comment" as const,
comment: c,
file: c.file,
side: c.side,
line: c.line,
},
}
}
if (
!editMeta ||
editMeta.comment?.id !== c.id ||
editMeta.file !== c.file ||
editMeta.side !== c.side ||
editMeta.line !== c.line
) {
editMeta = {
type: "comment",
comment: c,
file: c.file,
side: c.side,
line: c.line,
editing: true,
}
}
editMeta.comment = c
return { side: c.side, lineNumber: c.line, metadata: editMeta }
})
if (draft && draft.file === file) {
if (
@@ -135,7 +199,7 @@ export function buildFileAnnotations(
}
result.push({ side: draft.side, lineNumber: draft.line, metadata: draftMeta })
}
return { annotations: result, draftMeta }
return { annotations: result, draftMeta, editMeta }
}
export function buildReviewAnnotation(
@@ -158,6 +222,7 @@ export function buildReviewAnnotation(
textarea.className = "am-annotation-textarea"
textarea.rows = 3
textarea.placeholder = handlers.labels.placeholder
trackText(meta, textarea)
const actions = document.createElement("div")
actions.className = "am-annotation-actions"
@@ -226,7 +291,7 @@ export function buildReviewAnnotation(
const textarea = document.createElement("textarea")
textarea.className = "am-annotation-textarea"
textarea.rows = 3
textarea.value = comment.comment
trackText(meta, textarea, comment.comment)
const actions = document.createElement("div")
actions.className = "am-annotation-actions"