fix(editor): keep an invalidated diff visible until its reload lands (#13777)

Mark invalidated diff content stale instead of dropping it, mirroring the file-content treatment from #13634. The diff read-generation fence (diffReadGenerationRef) already rejects a superseded read's write-back, so retaining the bytes and swapping them on arrival is safe and keeps every RPC saving -- it just stops the loading placeholder flashing on each reveal.

The git-status reload effect now treats a stale entry like a missing one so the lazy-load effect stays the single fetcher for it.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-08-11 01:43:44 -07:00
committed by GitHub
co-authored by Orca
parent b3965f3205
commit 12f58f6bee
3 changed files with 56 additions and 11 deletions
@@ -29,4 +29,7 @@ export type FileContent = {
isStale?: boolean
}
export type DiffContent = GitDiffResult
export type DiffContent = GitDiffResult & {
/** Superseded by an external change; still rendered until the lazy reload lands. */
isStale?: boolean
}
@@ -187,8 +187,11 @@ export function useEditorPanelContentState({
const next = { ...prev }
let changed = false
for (const fileId of uniqueIds) {
if (fileId in next) {
delete next[fileId]
const existing = next[fileId]
// Why: keep the last-known diff rendered and swap it when the lazy
// reload lands — dropping it flashes "Loading diff…" on every reveal.
if (existing && existing.isStale !== true) {
next[fileId] = { ...existing, isStale: true }
changed = true
}
}
@@ -525,9 +528,13 @@ export function useEditorPanelContentState({
!hasLiveRead(fileReadGenerationRef.current, outstandingFileReadsRef.current, fileId)
)
}
const needsDiffRead = (fileId: string): boolean =>
!diffContents[fileId] &&
!hasLiveRead(diffReadGenerationRef.current, outstandingDiffReadsRef.current, fileId)
const needsDiffRead = (fileId: string): boolean => {
const cached = diffContents[fileId]
return (
(!cached || cached.isStale === true) &&
!hasLiveRead(diffReadGenerationRef.current, outstandingDiffReadsRef.current, fileId)
)
}
useEffect(() => {
if (!isVisible) {
@@ -643,9 +650,10 @@ export function useEditorPanelContentState({
invalidateDiffContent([current.id])
return
}
// Why: the lazy-load effect already fetches on first open; forcing here
// races a duplicate git-diff RPC for the same tab.
if (!diffContentsRef.current[current.id]) {
// Why: the lazy-load effect already fetches on first open and on a retained
// stale entry; forcing here races a duplicate git-diff RPC for the same tab.
const cachedDiff = diffContentsRef.current[current.id]
if (!cachedDiff || cachedDiff.isStale === true) {
return
}
void loadDiffContent(current, { force: true })
@@ -387,6 +387,40 @@ describe('useEditorPanelContentState visibility', () => {
expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2)
})
it('keeps an invalidated diff on screen until the reveal read lands', async () => {
const file = makeFile('diff-no-flash', { mode: 'diff', diffSource: 'unstaged' })
const freshDiff = createDeferred<DiffContent>()
mocks.getRuntimeGitDiff
.mockResolvedValueOnce(textDiff('old diff'))
.mockReturnValueOnce(freshDiff.promise)
await act(async () => root.render(<Probe activeFile={file} />))
await vi.waitFor(() =>
expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('old diff')
)
await act(async () => root.render(<Probe activeFile={file} isVisible={false} />))
dispatchExternalChange(file)
// Why: the viewers render "Loading diff…" purely on a missing entry, so a
// retained (stale-marked) entry is what keeps the pane painted.
expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('old diff')
expect(snapshots.get('main')?.diffContents[file.id]?.isStale).toBe(true)
expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce()
await act(async () => root.render(<Probe activeFile={file} />))
await vi.waitFor(() => expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2))
expect(snapshots.get('main')?.diffContents[file.id]?.modifiedContent).toBe('old diff')
await act(async () => {
freshDiff.resolve(textDiff('fresh diff'))
await freshDiff.promise
})
await vi.waitFor(() =>
expect(snapshots.get('main')?.diffContents[file.id]).toEqual(textDiff('fresh diff'))
)
expect(mocks.getRuntimeGitDiff).toHaveBeenCalledTimes(2)
})
it('invalidates a hidden Git-status diff without reading until reveal', async () => {
const file = makeFile('status', { mode: 'diff', diffSource: 'unstaged' })
const status: GitStatusEntry[] = [
@@ -401,7 +435,7 @@ describe('useEditorPanelContentState visibility', () => {
await act(async () =>
root.render(<Probe activeFile={file} gitStatusEntries={status} isVisible={false} />)
)
expect(snapshots.get('main')?.diffContents[file.id]).toBeUndefined()
expect(snapshots.get('main')?.diffContents[file.id]?.isStale).toBe(true)
expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce()
await act(async () => root.render(<Probe activeFile={file} gitStatusEntries={status} />))
@@ -430,7 +464,7 @@ describe('useEditorPanelContentState visibility', () => {
dispatchExternalChange(file)
await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2))
expect(mocks.getRuntimeGitDiff).toHaveBeenCalledOnce()
expect(snapshots.get('main')?.diffContents[file.id]).toBeUndefined()
expect(snapshots.get('main')?.diffContents[file.id]?.isStale).toBe(true)
await act(async () =>
root.render(<Probe activeFile={file} editorViewMode={changesMode} isChangesMode />)