Skip to content

Diff each scene revision once when loading a review - #499

Merged
mtaanquist merged 1 commit into
developfrom
fix/review-anchor-diff-reuse
Jul 29, 2026
Merged

Diff each scene revision once when loading a review#499
mtaanquist merged 1 commit into
developfrom
fix/review-anchor-diff-reuse

Conversation

@mtaanquist

Copy link
Copy Markdown
Owner

The stall

Opening the Review tab on a story with a lot of review history hangs for several seconds before rendering. On a restored production copy, the story "Temptations Above the Cloud" (20 scenes, 98 threads, 53 suggested edits) takes about 4.2s of server time to load, and the whole of it is CPU, not SQL.

listThreads and listSuggestions re-anchor every comment and suggested edit onto the current text by running diffChars between the revision the note was left on and the scene as it stands now. That is one character-level diff per anchor. Most of them are the same diff: this story's threads collapse to 5 distinct (base revision, scene) pairs across 26 diffs, and 8 of them sit on one 45k-character scene that costs about 260ms each on its own.

The diffs run inside the load's Promise.all, but they are synchronous CPU on one thread, so parallelism buys nothing and the event loop is blocked for the duration.

The change

createAnchorMapper() diffs each pair of texts once and maps every anchor through that one result. The review routes make one mapper per request and hand it to both listThreads and listSuggestions, so a scene revision is diffed once for the page rather than once per note.

Measured on that story, warm, same process, A/B against the per-anchor path:

before after
diffs run 26 5
threads + suggestions ~4.2s ~0.55s
whole load wave 4.1-4.5s ~0.57s

reanchorRange and reanchorPoint keep their signatures and behaviour. The offset maths moved into shared mapRange/mapPoint helpers so the cached and uncached paths cannot drift.

The guest review route (/review/[token]) awaited the two calls serially, so it paid the same cost twice over; it now shares one mapper too. The Assistant's review-reply endpoint and the account export's review loader get the same treatment.

What this does not fix

The underlying growth is still there: a thread's anchor stays pinned to the revision it was left on, so the diff against the current text gets bigger the longer the author keeps writing. This change makes the cost per scene revision instead of per note, which is a large constant-factor win, but a scene that has moved a long way from its notes still costs a real diff on every load. Rebasing anchors forward after a scene save (the worker already runs on that event) would take it to near zero. Worth doing, but it rewrites stored anchors, so it is a separate change with its own risk.

Testing

  • npm run lint, npm run check: clean.
  • npx vitest run: 1077 passed, 1 skipped. Includes new tests that check the mapper against reanchorRange/reanchorPoint over every offset of a set of edit shapes, so the cached path is verified to return exactly what the uncached one does.
  • Review e2e (review, review-sidebar, review-entity-card, author-review): pass.

Two specs (author-review.spec.ts:116, review-entity-card.spec.ts:5) flake locally. I ran both interleaved against origin/develop with a clean worker slate before each run and they fail at the same rate on both sides, and both fail before any review code runs, so they are not from this change. The local e2e database has accumulated ~860 universes, which is likely what pushes them over their 10s assertions.

🤖 Generated with Claude Code

@mtaanquist
mtaanquist marked this pull request as ready for review July 29, 2026 05:41
The Review tab re-anchored every comment and suggested edit with its own
character diff of the scene it sits in. A story whose scenes have moved on
since the notes were left pays that diff once per anchor: on a 45k-character
scene it is around 260ms each, and the load runs them back to back on one
thread, so the page hangs before it renders.

The threads and suggestions on a scene almost all share the same pair of
texts, so an anchor mapper now diffs each pair once and maps every anchor
through that one result, shared between listThreads and listSuggestions for
the page. On a restored production story (20 scenes, 98 threads, 53
suggestions) that takes the load's 26 diffs down to 5, and the load itself
from about 4.2s to 570ms.

reanchorRange and reanchorPoint keep their signatures and behaviour; the
offset maths moved into shared helpers so the cached and uncached paths
cannot drift, and the new tests check the mapper against them exhaustively.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mtaanquist
mtaanquist force-pushed the fix/review-anchor-diff-reuse branch from 78847a9 to 00b8f84 Compare July 29, 2026 06:01
@mtaanquist
mtaanquist merged commit a52b7ec into develop Jul 29, 2026
4 checks passed
@mtaanquist
mtaanquist deleted the fix/review-anchor-diff-reuse branch July 29, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant