Fix combined diff saves before React commits native edits

src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx

@@ -1,11 +1,11 @@
 import React, { useCallback, useRef, useState } from 'react'
+import { useCombinedDiffSectionsState } from './use-combined-diff-sections-state'
 import { useAppStore } from '@/store'
 import { createProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks'
 import { useWorkspaceFileBrowserActionPredicate } from '@/lib/file-preview'
 import { selectWorktreeDiffCommentsOrEmpty } from '@/store/worktree-diff-comments-selector'
 import type { OpenFile } from '@/store/slices/editor'
 import '@/lib/monaco-setup'
-import type { DiffSection } from '../diff-section-types'
 import {
   EMPTY_GIT_BRANCH_ENTRIES,
   EMPTY_GIT_STATUS_ENTRIES,
@@ -66,7 +66,7 @@ export default function CombinedDiffViewer({
   const activeGroupId = useAppStore((s) => s.activeGroupIdByWorktree[file.worktreeId])
   const canOpenWorkspaceFileBrowserForPath = useWorkspaceFileBrowserActionPredicate(file.worktreeId)
 
-  const [sections, setSections] = useState<DiffSection[]>([])
+  const { sections, sectionsRef, setSections } = useCombinedDiffSectionsState()
   const [sectionHeights, setSectionHeights] = useState<Record<number, number>>({})
   const [generation, setGeneration] = useState(0)
   // Why: a browser scroll clamp must re-pin the restore without being recorded as user intent.
@@ -74,7 +74,7 @@ export default function CombinedDiffViewer({
   const [programmaticScrollMarks] = useState(createProgrammaticScrollMarks)
   const scrollContainerRef = useRef<HTMLDivElement>(null)
 
-  const registry = useCombinedDiffSectionLoadRegistry(sections)
+  const registry = useCombinedDiffSectionLoadRegistry(sectionsRef)
   const entrySet = useCombinedDiffEntrySet({
     file,
     gitStatusEntries,
@@ -170,7 +170,7 @@ export default function CombinedDiffViewer({
         registry.loadSchedulerRef.current.request(index)
       }
     },
-    [registry.loadSchedulerRef, registry.sectionsRef]
+    [registry.loadSchedulerRef, registry.sectionsRef, setSections]
   )
 
   const treeNavigation = useCombinedDiffTreeNavigation({

src/renderer/src/components/editor/combined-diff/load-sections/combined-diff-section-load-registry.ts

@@ -33,12 +33,11 @@ export type CombinedDiffSectionLoadRegistry = {
 }
 
 export function useCombinedDiffSectionLoadRegistry(
-  sections: DiffSection[]
+  sectionsRef: React.RefObject<DiffSection[]>
 ): CombinedDiffSectionLoadRegistry {
   const loadedIndicesRef = useRef<Set<number>>(new Set())
   const loadingIndicesRef = useRef<Set<number>>(new Set())
   const deferredLoadRequestsRef = useRef<Set<number>>(new Set())
-  const sectionsRef = useRef<DiffSection[]>([])
   const generationRef = useRef(0)
   // Why: per-section reload token, so a sibling's reload can't discard this section's in-flight load.
   const sectionLoadTokensRef = useRef<Map<number, number>>(new Map())
@@ -51,7 +50,6 @@ export function useCombinedDiffSectionLoadRegistry(
   loadSchedulerRef.current ??= createCombinedDiffLoadScheduler({
     loadSection: (index) => loadSectionRef.current(index)
   })
-  sectionsRef.current = sections
 
   useEffect(() => {
     // Why: React StrictMode replays effect cleanup in dev; reset revives the scheduler for the replayed mount.

src/renderer/src/components/editor/combined-diff/remember-view/use-combined-diff-view-restore.test.tsx

@@ -1,6 +1,7 @@
 // @vitest-environment happy-dom
 
 import { afterEach, describe, expect, it } from 'vitest'
+import { useRef } from 'react'
 import { cleanup, renderHook } from '@testing-library/react'
 import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types'
 import type { GitStatusEntry } from '../../../../../../shared/git-status-types'
@@ -39,7 +40,7 @@ function buildAllModeEntrySet(
 function restoreSections(entrySet: CombinedDiffEntrySet, viewStateKey: string): DiffSection[] {
   let sections: DiffSection[] = []
   renderHook(() => {
-    const registry = useCombinedDiffSectionLoadRegistry([])
+    const registry = useCombinedDiffSectionLoadRegistry(useRef([]))
     return useCombinedDiffViewRestore({
       entrySet,
       gitStatusEntries: [],

src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-section-save.test.tsx

@@ -1,7 +1,8 @@
 // @vitest-environment happy-dom
 import { act, renderHook } from '@testing-library/react'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { useRef, useState } from 'react'
+import { useState } from 'react'
+import { useCombinedDiffSectionsState } from '../use-combined-diff-sections-state'
 import type { OpenFile } from '@/store/slices/editor'
 import type { DiffSection } from '../../diff-section-types'
 import { useCombinedDiffSectionSave } from './use-combined-diff-section-save'
@@ -45,10 +46,8 @@ function section(key = 'file.ts'): DiffSection {
 }
 function setup(initial = [section()]) {
   return renderHook(() => {
-    const [sections, setSections] = useState(initial)
+    const { sections, setSections, sectionsRef } = useCombinedDiffSectionsState(initial)
     const [heights, setSectionHeights] = useState<Record<number, number>>({ 0: 100, 1: 200 })
-    const sectionsRef = useRef(sections)
-    sectionsRef.current = sections
     const save = useCombinedDiffSectionSave({ file, sectionsRef, setSections, setSectionHeights })
     return { sections, setSections, heights, save }
   })
@@ -68,6 +67,19 @@ beforeEach(() => {
 })
 
 describe('combined diff section saves', () => {
+  it('saves native editor edits before React commits a render', async () => {
+    writeFile.mockResolvedValue(undefined)
+    const view = setup([{ ...section(), dirty: false, modifiedContent: 'disk' }])
+    await act(async () => {
+      view.result.current.setSections((prev) => [
+        { ...prev[0], modifiedContent: 'typed immediately before save', dirty: true }
+      ])
+      await view.result.current.save.current(0)
+    })
+    expect(writeFile.mock.calls[0]?.[2]).toBe('typed immediately before save')
+    expect(view.result.current.sections[0].dirty).toBe(false)
+  })
+
   it('keeps edits made during a delayed remote write and advances only the saved baseline', async () => {
     const pending = deferred()
     writeFile.mockReturnValueOnce(pending.promise)

src/renderer/src/components/editor/combined-diff/use-combined-diff-sections-state.ts

@@ -0,0 +1,14 @@
+import { useCallback, useRef, useState, type SetStateAction } from 'react'
+import type { DiffSection } from '../diff-section-types'
+
+export function useCombinedDiffSectionsState(initial: DiffSection[] = []) {
+  const [sections, renderSections] = useState(initial)
+  const sectionsRef = useRef(sections)
+  const setSections = useCallback((update: SetStateAction<DiffSection[]>) => {
+    const next = typeof update === 'function' ? update(sectionsRef.current) : update
+    // Native editor events can reach Save before React commits their queued render.
+    sectionsRef.current = next
+    renderSections(next)
+  }, [])
+  return { sections, sectionsRef, setSections }
+}

src/renderer/src/components/editor/pierre-diff/use-pierre-diff-native-view.ts

@@ -1,4 +1,4 @@
-import { useCallback, useLayoutEffect, useRef } from 'react'
+import { useCallback, useLayoutEffect, useRef, useState } from 'react'
 import type { FileDiffMetadata, PostRenderPhase } from '@pierre/diffs'
 import type { Editor } from '@pierre/diffs/edit'
 import type { PierreDiffInstance } from './PierreDiffSurface'
@@ -24,7 +24,8 @@ export function usePierreDiffNativeView(
   useLayoutEffect(() => {
     latest.current = { fileDiff, editable, activeGroupId }
   }, [fileDiff, editable, activeGroupId])
-  const pending = useRef(key ? getPierreNativeView(key) : undefined)
+  const [restoreSeed] = useState(() => (key ? getPierreNativeView(key) : undefined))
+  const pending = useRef(restoreSeed)
   const frame = useRef<number | null>(null)
   const attempts = useRef(0)
   const lastSnapshot = useRef<PierreNativeViewState | undefined>(undefined)

tests/e2e/diff-context-copy.spec.ts

@@ -1,4 +1,5 @@
 import { rmSync, writeFileSync } from 'node:fs'
+import { diffTextSelectionPoints } from './diff-text-selection'
 import { test, expect } from './helpers/orca-app'
 import { waitForSessionReady } from './helpers/store'
 import { addAndActivateRepo } from './helpers/isolated-repo-activation'
@@ -31,24 +32,27 @@ test('copies backwards selections with file and line context from each diff side
   let copied = ''
   try {
     for (const side of ['deletions', 'additions']) {
-      const coordinates = await orcaPage.locator('diffs-container').evaluate((host, side) => {
-        const rows = [
-          ...host.shadowRoot!.querySelectorAll(`[data-${side}] [data-content] [data-line]`)
-        ]
-        const rect = (row: Element) => {
-          const range = document.createRange()
-          range.selectNodeContents(row)
-          const bounds = range.getBoundingClientRect()
-          return { left: bounds.left, right: bounds.right, y: bounds.top + bounds.height / 2 }
-        }
-        return { start: rect(rows[0]), end: rect(rows.at(-1)) }
-      }, side)
-      await orcaPage.mouse.move(coordinates.end.right, coordinates.end.y)
-      await orcaPage.mouse.down()
-      await orcaPage.mouse.move(coordinates.start.left, coordinates.start.y, { steps: 8 })
-      await orcaPage.mouse.up()
-      await orcaPage.keyboard.press('ControlOrMeta+Alt+c')
       const contents = side === 'deletions' ? original : modified
+      const coordinates = await diffTextSelectionPoints(
+        orcaPage.locator(`diffs-container [data-code][data-${side}]`),
+        contents.trimEnd()
+      )
+      await orcaPage.mouse.move(coordinates.end.x, coordinates.end.y)
+      await orcaPage.mouse.down()
+      await orcaPage.mouse.move(coordinates.start.x, coordinates.start.y, { steps: 8 })
+      await orcaPage.mouse.up()
+      await expect
+        .poll(() =>
+          orcaPage
+            .locator('diffs-container')
+            .evaluate((host) =>
+              (host.shadowRoot as ShadowRoot & { getSelection(): Selection })
+                .getSelection()
+                .toString()
+            )
+        )
+        .toBe(contents.trimEnd())
+      await orcaPage.keyboard.press('ControlOrMeta+Alt+c')
       copied = `File: ${fixture.relativePath}\nLines: 1-3\n\n\`\`\`ts\n${contents}\`\`\``
       await expect
         .poll(() => electronApp.evaluate(({ clipboard }) => clipboard.readText()))

tests/e2e/diff-edit-state-restoration.spec.ts

@@ -75,9 +75,9 @@ for (const surface of ['file', 'combined']) {
       await entry.click()
     } else {
       const header = orcaPage.locator('[data-combined-diff-section-row] .sticky').first()
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
       await expect(orcaPage.locator('diffs-container')).toHaveCount(0)
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
     }
     await expect(line).toHaveText(`${modified.trimEnd()}X`, { timeout: 20_000 })
     await expect
@@ -103,10 +103,10 @@ for (const surface of ['file', 'combined']) {
       await entry.click()
     } else {
       const header = orcaPage.locator('[data-combined-diff-section-row] .sticky').first()
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
       await expect(orcaPage.locator('diffs-container')).toHaveCount(0)
       writeFileSync(fixture.absolutePath, 'external replacement\n')
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
     }
     await expect(host.locator('[data-content]').last()).toContainText('external replacement', {
       timeout: 20_000

tests/e2e/diff-native-state-restoration.spec.ts

@@ -1,5 +1,6 @@
 import { rmSync, writeFileSync } from 'node:fs'
 import path from 'node:path'
+import { diffTextSelectionPoints } from './diff-text-selection'
 import { test, expect } from './helpers/orca-app'
 import { waitForSessionReady } from './helpers/store'
 import { addAndActivateRepo } from './helpers/isolated-repo-activation'
@@ -49,31 +50,10 @@ for (const mode of ['original-file', 'readonly-combined']) {
     const side = mode === 'original-file' ? 'deletions' : 'additions'
     const code = host.locator(`[data-code][data-${side}]`)
     await expect(code).toBeVisible({ timeout: 20_000 })
-    await code.evaluate((node) => {
-      node.scrollLeft = 1000
-    })
-    const word = await code
-      .locator('[data-line]')
-      .first()
-      .evaluate((row) => {
-        const walker = document.createTreeWalker(row, NodeFilter.SHOW_TEXT)
-        let node: Node | null
-        while ((node = walker.nextNode())) {
-          const start = node.textContent?.indexOf('SELECT_ME') ?? -1
-          if (start < 0) {
-            continue
-          }
-          const range = document.createRange()
-          range.setStart(node, start)
-          range.setEnd(node, start + 9)
-          const rect = range.getBoundingClientRect()
-          return { x: rect.x, right: rect.right, y: rect.y + rect.height / 2 }
-        }
-        throw new Error('Missing target text')
-      })
-    await orcaPage.mouse.move(word.right, word.y)
+    const points = await diffTextSelectionPoints(code, 'SELECT_ME')
+    await orcaPage.mouse.move(points.end.x, points.end.y)
     await orcaPage.mouse.down()
-    await orcaPage.mouse.move(word.x, word.y, { steps: 6 })
+    await orcaPage.mouse.move(points.start.x, points.start.y, { steps: 6 })
     await orcaPage.mouse.up()
     const selectedText = () =>
       host.evaluate((host) =>
@@ -94,9 +74,9 @@ for (const mode of ['original-file', 'readonly-combined']) {
         .filter({ hasText: path.basename(fixture.relativePath) })
         .locator('.sticky')
         .first()
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
       await expect(host).toHaveCount(0)
-      await header.click()
+      await header.click({ position: { x: 4, y: 8 } })
     }
     await expect.poll(() => code.evaluate((node) => node.scrollLeft)).toBeCloseTo(scrollLeft, 0)
     await expect.poll(selectedText).toBe('SELECT_ME')

tests/e2e/diff-text-selection.ts

@@ -0,0 +1,58 @@
+import type { Locator } from '@playwright/test'
+
+export async function diffTextSelectionPoints(code: Locator, text: string) {
+  return code.evaluate(async (code, text) => {
+    const content = code.querySelector('[data-content]')!
+    const nodes: Text[] = []
+    for (const row of content.querySelectorAll('[data-line]')) {
+      if (nodes.length > 0 && !nodes.at(-1)!.data.endsWith('\n')) {
+        // Pierre renders line breaks as separate rows rather than text nodes.
+        nodes.push(document.createTextNode('\n'))
+      }
+      const walker = document.createTreeWalker(row, NodeFilter.SHOW_TEXT)
+      while (walker.nextNode()) {
+        nodes.push(walker.currentNode as Text)
+      }
+    }
+    const contents = nodes.map((node) => node.data).join('')
+    const start = contents.indexOf(text)
+    if (start === -1) {
+      throw new Error('Selection text is absent from the rendered diff')
+    }
+    const position = (offset: number) => {
+      for (const node of nodes) {
+        if (offset < node.length) {
+          return { node, offset }
+        }
+        offset -= node.length
+      }
+      throw new Error('Selection endpoint is outside the rendered diff')
+    }
+    const glyph = (offset: number) => {
+      const point = position(offset)
+      const range = document.createRange()
+      range.setStart(point.node, point.offset)
+      range.setEnd(point.node, point.offset + 1)
+      return range
+    }
+    const first = glyph(start)
+    const last = glyph(start + text.length - 1)
+    const viewport = code.getBoundingClientRect()
+    const left = Math.min(first.getBoundingClientRect().left, last.getBoundingClientRect().left)
+    const right = Math.max(first.getBoundingClientRect().right, last.getBoundingClientRect().right)
+    code.scrollLeft += left - viewport.left - Math.max(24, (viewport.width - (right - left)) / 2)
+    await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()))
+    const point = (range: Range, end: boolean) => {
+      const rect = range.getBoundingClientRect()
+      const x = end ? rect.right - 0.5 : rect.left + 0.5
+      const y = rect.top + rect.height / 2
+      const root = code.getRootNode() as ShadowRoot
+      const hit = root.elementFromPoint(x, y)
+      if (!hit || !content.contains(hit) || document.elementFromPoint(x, y) !== root.host) {
+        throw new Error('Selection endpoint is clipped or covered by another diff pane')
+      }
+      return { x, y }
+    }
+    return { start: point(first, false), end: point(last, true) }
+  }, text)
+}