From b4bdb702f9f77c2dbf96dafede0d32ce0400855e Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 19:39:10 -0700 Subject: [PATCH] fix(diffs): start the restore ceiling at view attach, not component mount Two P1s from adversarial review (Opus), both introduced by the previous commit. The ceiling was armed in the mount-time layout effect, but the deadline can only be extended at attach while `now < ceiling`. A large or remote diff whose first postRender lands after the ceiling had already elapsed got no window at all, so the restore never ran once -- scroll, selection and setDeletedTextSelectionActive were silently dropped. This repo's own spec allows 20s for the first diff line to appear, so renders past a mount-anchored 15s ceiling are expected. Arm the ceiling at first attach instead, and clamp every deadline re-arm to it. The self-heal also cleared `loadedIndices` unconditionally. requestSectionReload already clears it when it proceeds and refuses while the row is dirty -- which is exactly the state that triggers the skip -- so the only observable effect was to strand the index unscheduled, making the next virtualizer scroll-in refetch a row the user is typing in, over git, on SSH. --- .../use-combined-diff-section-loader.ts | 7 ++++--- .../pierre-diff/use-pierre-diff-native-view.ts | 18 +++++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-loader.ts b/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-loader.ts index 4439ae951dc..4ff92515578 100644 --- a/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-loader.ts +++ b/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-loader.ts @@ -161,9 +161,10 @@ export function useCombinedDiffSectionLoader({ storedContent.modifiedContent === liveDraft && storedContent.originalContent === liveSection?.originalContent if (liveDraft !== draftAtFetchStart && !payloadMatchesLive) { - // Why: this index is already marked loaded, so drop that mark and re-drive the fetch — - // otherwise a rejected payload pins the section stale with no path back on its own. - loadedIndicesRef.current.delete(index) + // Why: re-drive the fetch so a rejected payload does not pin the section stale. Do not + // clear `loadedIndices` here — requestSectionReload clears it itself when it proceeds, + // and refuses while the row is dirty. Clearing unconditionally would strand the index + // unscheduled, making the next virtualizer scroll-in refetch a row being typed in. requestSectionReloadRef.current(index) return } diff --git a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-native-view.ts b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-native-view.ts index 08469b2788c..99a311df55d 100644 --- a/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-native-view.ts +++ b/src/renderer/src/components/editor/pierre-diff/use-pierre-diff-native-view.ts @@ -109,13 +109,13 @@ export function usePierreDiffNativeView( }) }, [editorRef]) useLayoutEffect(() => { + // Why: no window is granted here. The ceiling starts at the first view attach, not at + // component mount — a large or remote diff can attach many seconds later, and a window + // measured from mount would already be spent before the view ever exists. const now = Date.now() - // Arm once: the ceiling bounds this restore, not each mount. Re-arming per mount (or per - // activeGroupId change) would let an unmount/mount cycle extend it forever. - if (ceiling.current === 0) { - ceiling.current = now + RESTORE_CEILING_MS + if (ceiling.current !== 0 && now < ceiling.current) { + deadline.current = Math.min(now + RESTORE_DEADLINE_MS, ceiling.current) } - deadline.current = now + RESTORE_DEADLINE_MS schedule() }, [activeGroupId, schedule]) useLayoutEffect(() => { @@ -193,9 +193,13 @@ export function usePierreDiffNativeView( } { view.current = { host, instance } - // Extend while a restore is still pending, but never past the ceiling, and never re-arm - // the ceiling here: FileDiff emits 'mount' on every remount cycle. + // Start the ceiling at the first attach, then extend while a restore is still pending but + // never past it. FileDiff emits 'mount' on every remount cycle, so the ceiling is armed + // once and never re-armed here. const now = Date.now() + if (ceiling.current === 0) { + ceiling.current = now + RESTORE_CEILING_MS + } if (pending.current && now < ceiling.current) { deadline.current = Math.min(now + RESTORE_DEADLINE_MS, ceiling.current) }