diff --git a/src/renderer/src/components/editor/rich-markdown-review-annotations.ts b/src/renderer/src/components/editor/rich-markdown-review-annotations.ts index 619d128bb65..90bd170a5c1 100644 --- a/src/renderer/src/components/editor/rich-markdown-review-annotations.ts +++ b/src/renderer/src/components/editor/rich-markdown-review-annotations.ts @@ -139,7 +139,7 @@ export function getRichMarkdownAnnotationHighlightRangesForComment( comment: DiffComment, markdownSourceLineOffset: number, // Why optional: callers looping over comments pass one shared build. - prebuiltBlocks?: RichMarkdownCommentBlock[] + prebuiltBlocks?: readonly RichMarkdownCommentBlock[] ): RichMarkdownAnnotationHighlightRange[] { const blocks = prebuiltBlocks ?? buildRichMarkdownCommentBlocks(editor) const selectedText = comment.selectedText?.trim() @@ -192,13 +192,15 @@ export function getRichMarkdownCommentAnchorTop( block: RichMarkdownCommentBlock, containerRect: DOMRect, containerScrollTop: number, - markdownSourceLineOffset: number + markdownSourceLineOffset: number, + prebuiltBlocks?: readonly RichMarkdownCommentBlock[] ): number | null { try { const ranges = getRichMarkdownAnnotationHighlightRangesForComment( editor, comment, - markdownSourceLineOffset + markdownSourceLineOffset, + prebuiltBlocks ) // Why: range notes should sort by the start of the selected text. Anchoring // to the end puts overlapping ranges with the same final line in creation diff --git a/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts b/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts index c54996d0ffa..4fdc12a51b9 100644 --- a/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts +++ b/src/renderer/src/components/editor/rich-markdown-review-note-positioning.ts @@ -1,9 +1,7 @@ import type { Editor } from '@tiptap/react' import type { DiffComment } from '../../../../shared/diff-comment-types' -import { - buildRichMarkdownCommentBlocks, - getRichMarkdownCommentAnchorTop -} from './rich-markdown-review-annotations' +import { getRichMarkdownCommentAnchorTop } from './rich-markdown-review-annotations' +import { getRichMarkdownReviewRailBlocks } from './rich-markdown-review-rail-blocks' import { stackRichMarkdownReviewNotePositions, type RichMarkdownReviewNotePosition @@ -23,7 +21,7 @@ export function measureRichMarkdownReviewNotePositions({ markdownSourceLineOffset }: MeasureRichMarkdownReviewNotePositionsOptions): RichMarkdownReviewNotePosition[] { const containerRect = container.getBoundingClientRect() - const blocks = buildRichMarkdownCommentBlocks(editor) + const blocks = getRichMarkdownReviewRailBlocks(editor) const nextPositions = markdownComments .map((comment): RichMarkdownReviewNotePosition | null => { const bodyLineNumber = Math.max(1, comment.lineNumber - markdownSourceLineOffset) @@ -39,7 +37,8 @@ export function measureRichMarkdownReviewNotePositions({ block, containerRect, container.scrollTop, - markdownSourceLineOffset + markdownSourceLineOffset, + blocks ) return top === null ? null : { comment, top } }) diff --git a/src/renderer/src/components/editor/rich-markdown-review-rail-benchmark.test.ts b/src/renderer/src/components/editor/rich-markdown-review-rail-benchmark.test.ts new file mode 100644 index 00000000000..7090769f71f --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-review-rail-benchmark.test.ts @@ -0,0 +1,154 @@ +// @vitest-environment happy-dom +// Run: ORCA_REVIEW_RAIL_BENCH=1 pnpm test src/renderer/src/components/editor/rich-markdown-review-rail-benchmark.test.ts +import { expect, it, vi } from 'vitest' +import { Editor as TiptapEditor } from '@tiptap/core' +import { createRichMarkdownExtensions } from './rich-markdown-extensions' +import { createRichMarkdownEditorCodec } from './rich-markdown-source-transport' +import { measureRichMarkdownReviewNotePositions } from './rich-markdown-review-note-positioning' + +// Baseline measurement body from the parent revision, before sharing source blocks. +import type { Editor } from '@tiptap/react' +import type { DiffComment } from '../../../../shared/diff-comment-types' +import { + buildRichMarkdownCommentBlocks, + getRichMarkdownCommentAnchorTop +} from './rich-markdown-review-annotations' +import { + stackRichMarkdownReviewNotePositions, + type RichMarkdownReviewNotePosition +} from './rich-markdown-review-note-layout' + +type MeasureRichMarkdownReviewNotePositionsOptions = { + container: HTMLDivElement + editor: Editor + markdownComments: DiffComment[] + markdownSourceLineOffset: number +} + +function measureBaseline({ + container, + editor, + markdownComments, + markdownSourceLineOffset +}: MeasureRichMarkdownReviewNotePositionsOptions): RichMarkdownReviewNotePosition[] { + const containerRect = container.getBoundingClientRect() + const blocks = buildRichMarkdownCommentBlocks(editor) + const nextPositions = markdownComments + .map((comment): RichMarkdownReviewNotePosition | null => { + const bodyLineNumber = Math.max(1, comment.lineNumber - markdownSourceLineOffset) + const block = blocks.find( + (candidate) => candidate.startLine <= bodyLineNumber && bodyLineNumber <= candidate.endLine + ) + if (!block) { + return null + } + const top = getRichMarkdownCommentAnchorTop( + editor, + comment, + block, + containerRect, + container.scrollTop, + markdownSourceLineOffset + ) + return top === null ? null : { comment, top } + }) + .filter((position): position is RichMarkdownReviewNotePosition => position !== null) + return stackRichMarkdownReviewNotePositions( + nextPositions, + measureReviewNoteHeights(container, nextPositions) + ) +} + +function measureReviewNoteHeights( + container: HTMLDivElement, + positions: RichMarkdownReviewNotePosition[] +): Map { + const measuredHeights = new Map() + for (const pos of positions) { + const el = container.querySelector(`[data-rich-markdown-review-note-id="${pos.comment.id}"]`) + if (el) { + measuredHeights.set(pos.comment.id, el.getBoundingClientRect().height) + } + } + return measuredHeights +} + +it.skipIf(process.env.ORCA_REVIEW_RAIL_BENCH !== '1')( + 'benchmarks full review rail measurements', + () => { + for (const blockCount of [250, 1000]) { + const editor = new TiptapEditor({ + element: document.createElement('div'), + extensions: createRichMarkdownExtensions({ codec: createRichMarkdownEditorCodec() }), + content: { + type: 'doc', + content: Array.from({ length: blockCount }, (_, index) => ({ + type: 'paragraph', + content: [{ type: 'text', text: `Paragraph ${index} with reviewable content.` }] + })) + } + }) + try { + vi.spyOn(editor.view, 'coordsAtPos').mockReturnValue({ + top: 100, + bottom: 120, + left: 0, + right: 10 + }) + const container = document.createElement('div') + const markdownComments: DiffComment[] = Array.from({ length: 5 }, (_, index) => ({ + id: `note-${index}`, + worktreeId: 'workspace', + filePath: 'notes.md', + source: 'markdown', + lineNumber: 1 + index * 40, + body: 'Review', + createdAt: index, + side: 'modified' + })) + const args = { editor, container, markdownComments, markdownSourceLineOffset: 0 } + const serialize = vi.spyOn(editor.markdown!, 'serialize') + const baseline = measureBaseline(args) + const baselineCalls = serialize.mock.calls.length + serialize.mockClear() + expect(measureRichMarkdownReviewNotePositions(args)).toEqual(baseline) + const coldCalls = serialize.mock.calls.length + serialize.mockClear() + expect(measureRichMarkdownReviewNotePositions(args)).toEqual(baseline) + const warmCalls = serialize.mock.calls.length + serialize.mockRestore() + const time = (run: () => unknown) => { + const start = performance.now() + for (let index = 0; index < 20; index++) { + run() + } + return (performance.now() - start) / 20 + } + const before: number[] = [] + const after: number[] = [] + for (let round = 0; round < 5; round++) { + before.push(time(() => measureBaseline(args))) + after.push(time(() => measureRichMarkdownReviewNotePositions(args))) + } + const median = (values: number[]) => values.sort((a, b) => a - b)[2]! + const result = { + blockCount, + comments: markdownComments.length, + beforeMs: median(before), + afterMs: median(after), + baselineCalls, + coldCalls, + warmCalls + } + process.stdout.write(`${JSON.stringify(result)}\n`) + expect(baselineCalls).toBe((2 * blockCount - 1) * 6) + expect(coldCalls).toBe(2 * blockCount - 1) + expect(warmCalls).toBe(0) + } finally { + editor.destroy() + vi.restoreAllMocks() + } + } + }, + 120_000 +) diff --git a/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.test.ts b/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.test.ts new file mode 100644 index 00000000000..18c00a47fb5 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.test.ts @@ -0,0 +1,139 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { Editor, type JSONContent } from '@tiptap/core' +import type { DiffComment } from '../../../../shared/diff-comment-types' +import { createRichMarkdownExtensions } from './rich-markdown-extensions' +import { createRichMarkdownEditorCodec } from './rich-markdown-source-transport' +import { buildRichMarkdownCommentBlocks } from './rich-markdown-review-annotations' +import { getRichMarkdownReviewRailBlocks } from './rich-markdown-review-rail-blocks' +import { measureRichMarkdownReviewNotePositions } from './rich-markdown-review-note-positioning' + +const editors: Editor[] = [] + +function createEditor( + content: string | JSONContent = '# Heading\n\nFirst paragraph\n\n- One\n- Two' +) { + const editor = new Editor({ + element: document.createElement('div'), + extensions: createRichMarkdownExtensions({ codec: createRichMarkdownEditorCodec() }), + content, + ...(typeof content === 'string' ? { contentType: 'markdown' as const } : {}) + }) + editors.push(editor) + // Settle the trailing-node plugin before measuring selection-only transactions. + editor.view.dispatch(editor.state.tr.setMeta('addToHistory', false)) + return editor +} + +afterEach(() => { + for (const editor of editors.splice(0)) { + editor.destroy() + } + vi.restoreAllMocks() +}) + +describe('review rail source block reuse', () => { + it.each([ + '```ts\nconst value = 1\n\nvalue++\n```\n\nAfter code', + '| First | Second |\n| --- | --- |\n| one | two |\n\nAfter table', + '
Toggle\n\nInside\n\n
\n\nAfter toggle' + ])('preserves multiline block boundaries for %s', (source) => { + const editor = createEditor(source) + expect(getRichMarkdownReviewRailBlocks(editor)).toEqual(buildRichMarkdownCommentBlocks(editor)) + }) + + it('preserves block lines and reuses them after selection-only transactions', () => { + const editor = createEditor() + const expected = buildRichMarkdownCommentBlocks(editor) + const serialize = vi.spyOn(editor.markdown!, 'serialize') + const blocks = getRichMarkdownReviewRailBlocks(editor) + expect(blocks).toEqual(expected) + expect(serialize).toHaveBeenCalledTimes(2 * editor.state.doc.childCount - 1) + serialize.mockClear() + editor.commands.setTextSelection(3) + expect(getRichMarkdownReviewRailBlocks(editor)).toBe(blocks) + expect(serialize).not.toHaveBeenCalled() + }) + + it('rebuilds after edits and undo while keeping editors isolated', () => { + const editor = createEditor() + const original = getRichMarkdownReviewRailBlocks(editor) + editor.commands.insertContentAt(1, 'changed\ntext') + const changed = getRichMarkdownReviewRailBlocks(editor) + expect(changed).not.toBe(original) + expect(changed).toEqual(buildRichMarkdownCommentBlocks(editor)) + editor.commands.undo() + expect(getRichMarkdownReviewRailBlocks(editor)).toEqual(original) + expect(getRichMarkdownReviewRailBlocks(createEditor())).not.toBe(original) + }) + + it('invalidates an in-place serializer replacement and a manager replacement', () => { + const editor = createEditor() + const original = getRichMarkdownReviewRailBlocks(editor) + const serialize = editor.markdown!.serialize.bind(editor.markdown) + vi.spyOn(editor.markdown!, 'serialize').mockImplementation( + (content) => `${serialize(content)}\nextra line` + ) + const changed = getRichMarkdownReviewRailBlocks(editor) + expect(changed).not.toEqual(original) + expect(changed).toEqual(buildRichMarkdownCommentBlocks(editor)) + editor.markdown = createEditor().markdown + expect(getRichMarkdownReviewRailBlocks(editor)).toEqual(original) + }) + + it('does not reuse fallback lines after a missing serializer becomes available', () => { + const editor = createEditor() + const markdown = editor.markdown + editor.markdown = undefined + const fallback = getRichMarkdownReviewRailBlocks(editor) + expect(fallback).toEqual(buildRichMarkdownCommentBlocks(editor)) + editor.markdown = markdown + expect(getRichMarkdownReviewRailBlocks(editor)).not.toEqual(fallback) + expect(getRichMarkdownReviewRailBlocks(editor)).toEqual(buildRichMarkdownCommentBlocks(editor)) + }) + + it('avoids serialization for every comment and repeated scroll while refreshing geometry', () => { + const editor = createEditor() + let sourceTop = 100 + const coords = vi.spyOn(editor.view, 'coordsAtPos').mockImplementation(() => ({ + top: sourceTop, + bottom: sourceTop + 20, + left: 0, + right: 10 + })) + const container = document.createElement('div') + const comment: DiffComment = { + id: 'note', + worktreeId: 'workspace', + filePath: 'notes.md', + source: 'markdown', + lineNumber: 1, + body: 'Review', + createdAt: 1, + side: 'modified' + } + const markdownComments = Array.from({ length: 5 }, (_, index) => ({ + ...comment, + id: `note-${index}`, + selectedText: index === 0 ? 'Heading' : undefined + })) + const serialize = vi.spyOn(editor.markdown!, 'serialize') + const measure = () => + measureRichMarkdownReviewNotePositions({ + editor, + container, + markdownComments, + markdownSourceLineOffset: 0 + }) + expect(measure()[0]?.top).toBe(100) + expect(serialize).toHaveBeenCalledTimes(2 * editor.state.doc.childCount - 1) + serialize.mockClear() + sourceTop = 200 + container.scrollTop = 30 + for (let index = 0; index < 60; index++) { + expect(measure()[0]?.top).toBe(230) + } + expect(serialize).not.toHaveBeenCalled() + expect(coords).toHaveBeenCalledTimes(61 * markdownComments.length) + }) +}) diff --git a/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.ts b/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.ts new file mode 100644 index 00000000000..35dd1bb9eb5 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-review-rail-blocks.ts @@ -0,0 +1,31 @@ +import type { Editor } from '@tiptap/core' +import { + buildRichMarkdownCommentBlocks, + type RichMarkdownCommentBlock +} from './rich-markdown-review-annotations' + +type ReviewRailBlocks = { + doc: Editor['state']['doc'] + markdown: Editor['markdown'] + serialize: NonNullable['serialize'] | undefined + blocks: readonly RichMarkdownCommentBlock[] +} + +// Keep only the current document per editor; scrolling changes geometry, not source lines. +const blocksByEditor = new WeakMap() + +export function getRichMarkdownReviewRailBlocks( + editor: Editor +): readonly RichMarkdownCommentBlock[] { + const doc = editor.state.doc + const markdown = editor.markdown + const serialize = markdown?.serialize + const cached = blocksByEditor.get(editor) + if (cached?.doc === doc && cached.markdown === markdown && cached.serialize === serialize) { + return cached.blocks + } + + const blocks = buildRichMarkdownCommentBlocks(editor) + blocksByEditor.set(editor, { doc, markdown, serialize, blocks }) + return blocks +}