fix(watcher): suppress echo view-rebuild (U2b) + refuse mis-splittable id recovery (U1d)#3
Merged
Merged
Conversation
… id recovery (U1d)
U2b — own-write echo no longer triggers a redundant view rebuild:
process_single_watcher_event now returns Result<bool> (did the index
change), and apply_watcher_events marks a collection affected only on a
real change. A suppressed echo, benign vanished-file skip, or stale Remove
returns false, so on_view_change fires exactly once per API write instead of
twice (the ledger already prevented the index mutation; this stops the
redundant rebuild+notify). Root cause confirmed empirically by Donovan.
U1d — id recovery from filenames is only sound when a non-terminal field's
value alphabet excludes the delimiter that follows it. ulid (Crockford
base32) and date-format fields are safe; uuid ('-'), nanoid ('-','_') and
slugged string/ref values ('-') are not. A mis-splittable template silently
produces wrong ids on reindex — and since ids are path-derived, not stored,
such a store cannot survive index reconstruction from disk.
Fix, honoring "no silent wrong ids" without "no store opens":
- Detect mis-splittable collections at open (generalized non-terminal-
delimiter rule); warn loudly per collection with a 3-part message (reads
may be empty/incomplete, reindex refused, migration hint). Open never
fails on this.
- id_for_path now looks up the index BY PATH first (authoritative for any id
strategy once indexed); extract is only a fallback for unindexed files, and
is treated as untrusted for mis-splittable collections (returns None).
- Reindex/rebuild refuse mis-splittable collections (full/incremental scan
skip with a warn so boot still succeeds; explicit rebuild + watcher-index
of an unindexed file return a targeted error → ChangeEvent::Error) rather
than write wrong ids. API writes are unaffected (id known at write time).
- Reconcile (directory-wins frontmatter rewrite) is skipped for
mis-splittable collections, so a mis-split extraction can't overwrite
correct front matter (Arabella finding #3).
U1c (explicit-id decorated + on_conflict:suffix clobber) is subsumed: such
templates are mis-splittable → reindex refused → the clobber can't occur;
ulid-decorated never suffixes (unique ids, no path collision).
Adds SystemDb::get_id_by_path. +4 tests (U2b echo-no-rebuild, mis-splittable
detection across schemas, open+reads-work/reindex-refused, reconcile-no-
corruption). 151 grounddb / 173 workspace green, 0 new warnings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Amendment from PR #3 review (liz + arabella). Two-tier enforcement (liz): a mis-splittable template no longer always "opens and degrades". The tier depends on whether the collection already has documents on disk: - Has data → open succeeds, loud warn, reindex/rebuild/reconcile refused (never lock a user out of data they must migrate). - No data (new config / template change before first write) → HARD reject at load. Nothing to lock out, and a config that cannot survive index reconstruction from disk is broken, so fail fast. Adds collection_has_documents() to pick the tier. Review nits (arabella): - The ulid alphabet arm now checks is_ascii_alphanumeric() rather than returning false unconditionally, so an alphanumeric literal delimiter (e.g. {id}x{title}) is correctly flagged (ulid can emit 'x'). - Documented the intentional rebuild(None) warn-skip vs rebuild(Some) error asymmetry. README updated: the constraint is now enforced at load (reject new / warn+refuse existing), not "planned". +2 tests (hard-reject on no data; ulid-before-alphanumeric-delimiter hazard) and the existing degrade-path tests now seed on-disk data so they exercise the open-and-degrade tier. 152 grounddb / 174 workspace green, 0 new warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Follow-up to #1/#2. Two latent-in-main issues from Arabella's review + Donovan's live probe. Do not merge until liz + arabella review this diff and Donovan re-runs his probe (per team norm).
U2b — own-write echo triggered a redundant view rebuild
The self-write ledger suppressed the index mutation for a recognized echo, but
apply_watcher_eventsstill marked the collection affected → rebuilt the view →notify_view. Soon_view_changefired 2× per API write (Donovan measured; he corrected an earlier JOIN hypothesis). Fix:process_single_watcher_eventreturnsResult<bool>(did the index change); mark affected only ontrue. Echo / benign-skip / stale-remove →false→ no rebuild. Now exactly 1 fire.U1d — id recovery from filenames is unsound for separator-colliding alphabets
extract()splits a non-terminal field at the next delimiter — only unambiguous when the field's alphabet excludes that delimiter. ulid (Crockford base32) + date-format fields are safe; uuid (-), nanoid (-,_), and slugged string/ref values (-) are not → wrong ids on reindex → the U1 duplicate-row bug returns. And because ids are path-derived (not stored), such a store can't survive index reconstruction from disk.Fix — invariant is "no silent wrong ids", not "no store opens":
id_for_pathlooks up the index by path first (authoritative for any id strategy once indexed); extract is a fallback for unindexed files, treated as untrusted for mis-splittable collections.ChangeEvent::Error). API writes unaffected (id known at write).U1c (explicit-id decorated +
on_conflict:suffixclobber) is subsumed: such templates are mis-splittable → reindex refused → clobber impossible; ulid-decorated never suffixes (unique ids).Adds
SystemDb::get_id_by_path. +4 tests. 151 grounddb / 173 workspace green, 0 new warnings.🤖 Generated with Claude Code