backnotprop / backnotprop/plannotator

upstream(@pierre/diffs): renderDiff keeps highlighted cache when content changes, stale paints on restore

Open
#1,208 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
TypeScript
Stars
8.7k
Forks
649
PR merge metrics
PR metrics pending

Description

**TLDR:** `@pierre/diffs@1.3.2` `DiffHunksRenderer.renderDiff` refuses to swap its render cache for new diff content while the cache is highlighted. On the worker-pool path, a repaint after `updateItem` with a new `fileDiff` (fresh `cacheKey`) keeps painting the stale cached content and only queues an async worker highlight of the new content. If that worker task never lands (for example, it gets invalidated by a theme sync calling `invalidateRenderTasks`), the stale content stays on screen permanently. We hit this in edit mode: after Discard, the pristine restore write landed in item state but the diff kept displaying the edited buffer (#1193 QA finding). We work around it in `writeRestore` by calling `clearRenderCache()` plus `rerender()` on the live instance (PR linked below). This issue tracks reporting or patching it upstream.

## Where

`dist/renderers/DiffHunksRenderer.js`, `renderDiff`, in the worker-pool branch (approximately lines 431-441 of the 1.3.2 dist):

```js
const newContent = !areDiffTargetsEqual(diff, this.renderCache.diff);
...
if (!highlightPending && (forcePlainText || this.renderCache.result == null || !this.renderCache.highlighted && (newContent || newRenderRange))) {
this.renderCache.diff = diff;
...
}
if (!forcePlainText && hasContent && (!this.renderCache.highlighted || forceHighlight)) this.workerManager.highlightDiffAST(this, diff);
```

The swap condition only honors `newContent` when `!this.renderCache.highlighted`. When the cache IS highlighted and has a result, new content does not replace `renderCache.result` synchronously; the renderer paints the old cached rows and relies entirely on the async `highlightDiffAST` round trip to heal.

## How we hit it

1. An edit session ends (upstream's documented commit pattern: one combined `updateItem` carrying `edit: false` plus the restored `fileDiff` with a fresh `cacheKey`).
2. The session leaves `renderCache.highlighted === true` (session rendering runs locally with the token transformer forced on).
3. The teardown repaint takes the branch above: `newContent` is true, but the highlighted cache blocks the swap, so the stale edited rows keep painting.
4. Pristine pixels arrive only if the queued worker highlight completes, which takes 30ms to seconds, and never arrives at all when the task is invalidated first (we observed this with a concurrent `setRenderOptions` theme sync).

Reproduced empirically with a real `CodeView` plus worker pool under happy-dom: after the combined restore write, the DOM still contained the edited buffer at t+1000ms in the invalidation case. Our regression test for the workaround is `packages/review-editor/edit/discardRestoreRender.test.tsx`.

## Suggested upstream fix

In the worker branch, let `newContent` force the cold path even when the cache is highlighted: swap `renderCache.diff`, drop `highlighted` to false, and rebuild the plain AST result synchronously (the same UX as any diff switch: plaintext first, async highlight after). Alternatively, treat a highlighted cache with `newContent` the same as `renderCache.result == null`.

## Workaround on our side

`writeRestore` in `packages/review-editor/edit/useEditSession.ts` clears the live instance's render cache and calls `rerender()` after the restore `updateItem`, forcing the next paint down the cold-render path. This reaches into the protected `hunksRenderer` and should be removed once upstream honors `newContent` for highlighted caches.

---

*This issue was drafted with AI assistance; the root cause was established with an empirical repro harness against the unpatched dist.*

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.