Repository navigation
sqlite-shared cleanup: dedupe, lost-write fix, index split, comment pass - #445
Merged
Merged
Conversation
- Trim the barrels to what consumers import; everything else was re-exported but used only inside the package. - Drop the SortField/SORT_FIELDS aliases of core's NativeSortField and NATIVE_SORT_FIELDS. - record-logic: one recordNotFound(), currentVersion(), readForMutation() and hydrate() instead of repeated inline copies. - lock: acquireLock no longer checks existsSync before readLock(), which already treats a missing lock as absent. - query: compute the native sort field once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
The bare-NOT strip ran over the whole search string, so a NOT at the
start of a phrase or after a "(" inside one was deleted from the text
the user searched for. Phrases are now matched first and kept whole.
Output outside phrases is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
isAuthorityAssociation joins the root entry point: an app undoing a journal entry has to route each edit to amendAssociations() or amendAccess(), and the spec's own example called a predicate no package exported. partitionAssociations joins @haverstack/core/adapter for engines that store both halves in one table. sqlite-shared and MemoryAdapter now use these instead of their own copies of the kind check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
record-logic.ts carried the content_index subsystem (its per-type field cache, sync, and kind lookup) inline, and fts5.ts mixed the search-text sanitizer with the FTS5 write strategy. Both indexes are rebuildable from records and are maintained the same way, so they now live together in indexes.ts: fts5Strategy unchanged, and a ContentIndex class that owns the cache and every content_index write. fts5.ts is now only the sanitizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
Every comment block in the package is now at most five lines, outside file-top module comments. Where the reasoning is spec material the comment links to the spec instead; the sanitizer's regex-backtracking rationale, which is not, moves into fts5.ts's module comment. Also removes comments describing earlier implementations (a test's before/after timings, purgeRecord's reference to a write() call), fixes a doc comment attached to the wrong declaration in fts5.ts, and corrects a version-conflict comment that miscounted its call sites. Spec: the SQLite-specific prose in adapters.md sat under § Conformance; it now has its own § SQLite-backed adapters heading, and the two references that meant it point there. data-model.md's allow-list paragraph no longer narrates the strip-list it replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
🦋 Changeset detectedLatest commit: 2237353 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
mutateRecord read the record, awaited, then wrote: a content patch was applied to that earlier read, so two concurrent unguarded patches lost one of them, and content_index could be rebuilt under a stale typeId. mutateRecord, restoreVersion and commitMigration now take a narrow read (version, type, content) inside their transaction and build from it. The pre-transaction readForMutation() is gone: the search-index removal it used to precede is rolled back with the transaction on any failure, so there is no work for an early check to save. amendAssociations' existence check reads the version instead of hydrating the record. Tests that simulated a writer in the old await gap now pin the same guarantees from the outside: racing guarded writes, one wins and the other conflicts; racing unguarded patches, both land. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
replaceAssociationHalves deleted by its own DATA_KINDS/AUTHORITY_KINDS lists, a third spelling of the partition beside core's predicate. Core now exports AUTHORITY_ASSOCIATION_KINDS from @haverstack/core/adapter, isAuthorityAssociation reads it, and the SQL deletes the data half as "not an authority kind", so a new authority kind added to core is covered in both places at once. isAuthorityAssociation also drops its `a?.` guards: its parameter is a typed Association, and every caller passes one already validated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
- fts5Strategy becomes a SearchIndex class holding its executor, like ContentIndex, instead of taking `exec` on every call. - SharedSqlRecordLogic and SharedTokenLogic store `exec` at construction rather than reading it through a getter, so the logic and the indexes it builds are bound to the same handle. - query.ts imports from @haverstack/core once. - PRAGMA_JOURNAL_MODE_WAL moves from the record-only barrel to the full one: its only consumer is the token store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
record-adapter-sqlite and record-adapter-do-sqlite each inline their own copy of sqlite-shared, so a change to it ships in both. Naming only record-adapter-sqlite left the DO adapter's changelog without the entry, even when the DO adapter was released for an unrelated reason. AGENTS.md, CONTRIBUTING.md and expand-changesets' error message now say to name both, and this branch's sqlite-shared changesets do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg
cuibonobo
changed the title
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A review pass over
@haverstack/sqlite-shared, one commit per concern:af5bc5c): trims the barrels to what consumers import; drops theSortField/SORT_FIELDSaliases of core'sNativeSortField/NATIVE_SORT_FIELDS; replaces repeated inline code inrecord-logic.tswith shared helpers; removes a redundantexistsSyncbeforereadLock().NOTinside a search phrase (d8ef10d): the bare-NOTstrip ran over the whole search string, so"cats (not dogs"lost itsnot. Phrases are now matched first and kept whole.c427f43):isAuthorityAssociationfrom@haverstack/core(apps splitting a journal entry's edits betweenamendAssociations()andamendAccess()need it; the spec's own example called a predicate nothing exported), andpartitionAssociationsfrom@haverstack/core/adapter.sqlite-sharedandMemoryAdapterdrop their own copies.indexes.ts(2d9da43): the FTS5 writes plus aContentIndexclass that owns the per-type field cache and everycontent_indexwrite.fts5.tsis now only the sanitizer.cc64114): every block is at most five lines outside file-top comments, linking to the spec where the reasoning lives there.dd7cba0):mutateRecordapplied a content patch to a record read before anawait, so two concurrent unguarded patches lost one of them, andcontent_indexcould be rebuilt under a staletypeId.mutateRecord,restoreVersionandcommitMigrationnow read (version, type, content) inside their transaction and build from it.b7d358d): core exportsAUTHORITY_ASSOCIATION_KINDS;isAuthorityAssociationreads it, and the SQL deletes the data half as "not an authority kind", replacingrecord-logic.ts's own kind lists.e68f4d4):SearchIndexclass besideContentIndex;execstored at construction; import and barrel tidy-ups.2237353):record-adapter-sqliteandrecord-adapter-do-sqliteeach inline sqlite-shared, so both changelogs owe the entry. AGENTS.md, CONTRIBUTING.md andexpand-changesets' error message say so.Spec
docs/spec/journal.md § The inverse: the example now importsisAuthorityAssociationfrom@haverstack/core.docs/spec/adapters.md: the SQLite-specific prose sat under§ Conformance; it now has its own§ SQLite-backed adaptersheading, andevents.mdandschema.tslink there.docs/spec/data-model.md § Capability-gated filters: the allow-list paragraph no longer narrates the strip-list it replaced.Behavior changes, both within existing spec: a
NOTinside a search phrase is preserved (the spec already says phrase text is left alone), and concurrent writes to one record no longer lose updates.Verification
All six checks pass locally:
format:check,check:refs,lint,test,build,typecheck.mutateRecordpatches to different fields (one was lost), now pinned byconcurrent unguarded patches both land.Changesets:
minoron@haverstack/core(new exports);patchon both@haverstack/record-adapter-sqliteand@haverstack/record-adapter-do-sqlitefor the sqlite-shared changes. The comment-only commit carries none.Notes for reviewers
readForMutation()) is gone rather than kept as an early check: the search-index removal it ran ahead of is rolled back with the transaction on any failure. Three tests inrecord-adapter-sqlitesimulated a writer in theawaitthat read created; they are replaced by tests of the same guarantees from the outside (racing guarded writes: one wins, the other conflicts; a non-bumping write racing a purge:StackNotFoundError).query.ts's ~200-linerecordConditionscould split its JSON-path walk into its own module if it grows.🤖 Generated with Claude Code
https://claude.ai/code/session_01DsiVnSgkrjGjpzpqaSr5xg