Repository navigation
Add a WebView record cache and page client - #529
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
11a1e6a to
4f4a059
Compare
84c1ae1 to
cc6fc1f
Compare
cc6fc1f to
1faaebd
Compare
imnasnainaec
left a comment
There was a problem hiding this comment.
❓ Discards. The comment on #452 asked for the done-when to cover them, and Blast Radius says neither the tests nor perf:pages run discardBook or discardAll against the cache. A discard can replace whole books and always puts the scope record. So it takes the first-scope-reply path (see the comment on recordCache.ts), and its reply carries its whole ChangeSet. Two options:
- Add
RecordCachetests that apply adiscardBookreply and adiscardAllreply and comparepageAtwith a refetch. AdddiscardBookto theperf:pagescommand list. - Leave discards to 4a, and comment on #453: its end-to-end tests must cover discards with the flag on, and 4a decides whether the cache applies or refetches a
discardAllreply onlarge-loadable.
⛏️ The done-when says "in tests over the #407 datasets". The #407 coverage comes only from running perf:pages by hand; CI runs only the unit tests, which use a small fixture. Could Evidence say so? The same applies to #527 and #528.
Drafted by Claude Opus 5.5; barely reviewed by me.
|
Discards: took option 1. Evidence: added a line to #527, #528 and #529 saying the #407 tiers are checked only by |
033656c to
1038475
Compare
imnasnainaec
left a comment
There was a problem hiding this comment.
Review for reinvented or duplicated code (things the repo or paranext-core already has, or repeated within the PR). Drafted by Claude Opus 5.5 (claude-opus-5-5); inline comments not reviewed by me.
5985851 to
d35d30a
Compare
d35d30a to
59de47f
Compare
5fc3d0b to
414ef83
Compare
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec partially reviewed 25 files and all commit messages, and resolved 20 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
perf:pages, per review
…ks from the reply, per review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… send that skips its own events, per review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
discards and splits, per review
414ef83 to
b273692
Compare
Part of #426; closes #452. Stacked on #528.
Summary
This PR adds the WebView's record cache and page client. Both are built and tested, but nothing calls them yet.
RecordCacheholds what a view has fetched. That covers the segments, the links and payloads on them, pool buckets, phrase spans and placed stale free translations.pageAtserves a held range in the host'sSegmentPageshape, so a reply applied here can be compared with a refetch.revisioncounts the replies applied. A page request notes the revision when it is sent.absorbthen skips any record or pool bucket that a later reply wrote or removed.applysays what it can't apply itself. It returns'outline'when the reply'sresegmentedBooksnames the book. It returns'placement'when the reply'sstaleTranslationBooksnames the book, because the cache can't place stale translations or count orphaned cards. That refetches the held pages and the outline's counts, and the held pages are served meanwhile. It returns'pages'when a reply puts a phrase over a token that the cache doesn't hold. Otherwise it returns'none'.PageClientkeeps the behavior of Serve book outlines and segment pages from the host (#436) #464'suseSegmentPagesand Render the continuous view from segment pages #467's per-view ranges. Pages have a fixed size, and a failed load gets bounded retries. The client keeps the latest range of each view ('list' | 'strip'), reloads those ranges after a new outline, and drops a range onrelease. Each page has one fetch state (in flight, backing off or failed), changed through one transition, so only one retry chain runs. A page that fails every retry, or an outline read that fails, waits forretryFailed(Tell the reader when a segment page fails to load #471) or a later outline read. Outline reads run one at a time.sendsends a command under a change id it mints, applies the reply and fetches whatever the reply leaves out of date.heartakes the session'sonAnalysisChangedchange and refetches the held pages in its ranges. It also refetches the held pages that read one of its analyses or the pool bucket of one of itsforms. A change naming the book whole refetches the outline, and one naming the book andgetStaleSummaryrefetches the placement. It skips the change reported for a command it sent, whether that change arrives before the reply or after it, unless the command throws. A page that a change reaches while its fetch is in flight, a first load included, is fetched again once it lands.outline,holdsandpageAtare read-only, andfailedPagesis a snapshot that stays the same object until the client's version changes.CommandReply.resegmentedBooksnames the books whose boundaries a change moved, so the cache needn't compare scope records.CommandReply.staleTranslationBooksnames the books that the change's event flags withstaleTranslations.AnalysisChange.formsnames the forms whose pool buckets a change may have changed, read as the reply's pool buckets are.PAGE_SIZE,PAGE_RETRY_DELAYS_MS,PageCaller,pagesCoveringandwaitMslive inpageClient.ts, anduseSegmentPagesimports them.Evidence
The done-when, and where each part is checked:
The #407 tiers are checked only by
npm run perf:pages, run by hand. CI runs only the unit tests, which use a small fixture.recordCache.test.tsandpageClient.test.tscomparepageAtagainst a freshgetSegmentsafter an edit.npm run perf:pageschecks this for every change over a Data layer: capture a performance baseline before the database migration #407 tier. Each run applies random commands, undos and redos through aPageClient, then compares every page and the outline with what the session serves.perf:pagesincludessplitSegmentamong its commands and compares the cached outline after every change. Unit tests cover a reply that moves boundaries, and a first reply after open that names the boundaries unchanged.perf:pagesincludesdiscardBook, restoring the book as the draft held it at open.RecordCachetests apply adiscardBookreply and adiscardAllreply and comparepageAtwith a refetch, and check that a discard restoring boundaries asks for the outline.RecordCachetest covers this. Inperf:pages, about 30% of changes also race a refetch sent before the change and absorbed after it. Changes whose reply carries a scope record are left out of the race, because a boundary change refetches the outline anyway. APageClienttest covers a page that lands between a boundary reply and the outline it asks for.perf:pagesfeeds the session's reported changes tohearas they are emitted. It reports 0 differences on every tier:small: 177 changes (37 refetch races), at b273692.medium: 37 changes (9 refetch races), at b273692.large: 10 changes (2 refetch races), at 5985851.large-loadable: 10 changes (2 refetch races), at b273692.RecordCacheand 42PageClientunit tests run over a session insession-source.ts, a realAnalysisSessionsover aDraftRepository, which hands each change it reports to the client in the order it emits them. They cover phrases over unheld tokens, stale free translations, discards, retries, changes during a first load and during backoff, both orders of reply and event, failed outline reads, released ranges, replaced outlines, in-flight refetches and listeners.tscare clean.Decisions with a strong alternative
absorbkeeps whatever a later reply wrote. The alternative was to drop any page sent before the latest reply. That would refetch pages a reply never touched.hearrefetches only the held pages that a change reaches, and events name the forms they touch. Refetching every held page for any change naming analyses would refetch them all for each re-anchor pass during the session-open read (Re-anchor every linked book in the host #507). Analysis ids alone miss a new payload elsewhere whose form a held page shows, so the event names forms too.PageClientsends commands itself rather than callers registering a pending id, so the client knows each id before the session reports its change. 4a builds onsend.Merge Danger
Door: two-way
Nothing imports
src/cache/outside its tests andperf/pages/cli.ts, so no runtime behavior changes.Blast Radius: two new modules, their tests, and
perf/pages/cli.ts.perf:pagesnow also catchesApprovalRuleErrorfrom a random edit and counts the edit as applying nothing. Phase 4 wires the client to the WebView.CommandReplygainsresegmentedBooksandstaleTranslationBooks, and the session form ofonAnalysisChangedgainsforms.RecordTablemoves tosrc/utils/record-table.ts, and itsputandremovebecome public, since the cache holds its records inRecordTables. The discards from #524 reach the cache as ordinary replies: they refetch the outline when they restore boundaries, and their puts and deletes apply otherwise.🤖 Generated with Claude Code
Stack created with GitHub Stacks CLI • Give Feedback 💬
This change is