Repository navigation
Add derived indexes with a rebuild-and-diff command - #487
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
📝 Walkthrough
Merge Risk: 🔵 Low · up to
|
69856f8 to
0958f7d
Compare
0958f7d to
a11b7b4
Compare
ea64ae0 to
8a50049
Compare
8a50049 to
132a682
Compare
|
On the follow-ups in the first review's summary:
|
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 9 files and all commit messages, made 2 comments, and resolved 26 discussions.
Reviewable status: 9 of 27 files reviewed, 2 unresolved discussions (waiting on alex-rawlings-yyc).
src/__tests__/utils/json-copy.test.ts line 7 at r3 (raw file):
describe('jsonCopy', () => { it('copies a JSON-plain value deeply, sharing nothing with it', () => { const value = { gloss: { en: 'and' }, morphemes: [{ form: 'and' }] };
⛏️ Could we add some other atomic types in this test value?
src/services/recordTable.ts line 107 at r3 (raw file):
private put(record: T): void { this.remove(record.id);
❓ From 3j (#449): this line changes the order lookups return records in, and the re-anchor pass depends on that order.
- Removing before re-filing moves a replaced record to the end of
recordsand of each index set.applyChangeSetkeeps a replaced record in place. - 3j's pass will read a book through
findLinksInBooks([book]). reanchorAnalysisToBookbreaks ties by list order: of two stale links contending for one target, only the earlier revives. So it matches the WebView only when it gets a book's links in stored order.
Keeping a replaced record's place would give a one-book lookup stored order. Map.set already keeps it, so put would only need to re-file the index keys that change.
Is that worth doing here? If not, 3j can sort by a key of its own, accepting tie-breaks that differ from main's where two links contend for one target.
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc made 2 comments and resolved 2 discussions.
Reviewable status: 8 of 27 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc and imnasnainaec).
src/__tests__/utils/json-copy.test.ts line 7 at r3 (raw file):
Previously, imnasnainaec (D. Ror.) wrote…
⛏️ Could we add some other atomic types in this test
value?
Done; the value now also carries a number, a boolean and null.
src/services/recordTable.ts line 107 at r3 (raw file):
Previously, imnasnainaec (D. Ror.) wrote…
❓ From 3j (#449): this line changes the order lookups return records in, and the re-anchor pass depends on that order.
- Removing before re-filing moves a replaced record to the end of
recordsand of each index set.applyChangeSetkeeps a replaced record in place.- 3j's pass will read a book through
findLinksInBooks([book]).reanchorAnalysisToBookbreaks ties by list order: of two stale links contending for one target, only the earlier revives. So it matches the WebView only when it gets a book's links in stored order.Keeping a replaced record's place would give a one-book lookup stored order.
Map.setalready keeps it, soputwould only need to re-file the index keys that change.Is that worth doing here? If not, 3j can sort by a key of its own, accepting tie-breaks that differ from main's where two links contend for one target.
Done. put keeps a replaced record's place and re-files only the keys that change, so a one-book lookup returns that book's links in stored order, which DraftRepository's own tests pin. The seam still promises no order, so a later backend needn't keep it.
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec partially reviewed 18 files.
Reviewable status: 24 of 27 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 4 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
Part of #426; closes #442. Stacked on #486.
Summary
Derived indexes, built from the records on first use and kept current by
apply, and a debug command that rebuilds them and reports differences.AnalysisIndexanswers the lookups for bothDraftRepositoryandPt9ImportRepository, through a sharedIndexedRepositorybase that hands out copies.Evidence
The done-when, checked in Node with
npm run perf:indexesover the #407 tiers. It applies random change sets with undos, compares lookups with a scan of what storage holds, and runscheckIndexes. In the app the PAPI command reports nothing until edits go through a session, which #443's commands make possible.Decisions with a strong alternative
findLinksInBookstakes several books, since the contract has lookups take keys in batches.checkIndexesis part of the seam; a backend keeping no derived indexes reports nothing. It reports nothing until a lookup or apply has built an index.exportAlland a whole save drop them. Copying the draft on every load instead cost about 1.3 s per load on large-loadable. A session's first edit still re-reads the draft, which 4a can move to the session's open.reanchorAnalysisToBook's tie-breaks need in 3j. The seam still promises no order.Follow-up
#505 scopes each apply's write to the books it reaches.
Merge Danger
Door: two-way
Nothing calls the lookups or the command yet.
Blast Radius:
DraftRepositoryandPt9ImportRepository, which keep a draft and its index after their first lookup or apply;savePt9ImportanddeleteProject, which drop an open import repository's records🤖 Generated with Claude Code
This change is
Summary by CodeRabbit