fix(data): observe.select re-emits on membership contraction#144
Merged
Conversation
observeSelectEntities did not re-emit when a transaction removed an entity from the result set by clearing a required (membership) component that the caller selected but did not filter or order on. The in-set branch of the per-transaction check only re-tested `order` and `where` components; it never re-derived archetype membership, so a removed `include` component migrated the row out of the result archetype silently and the observer kept emitting a stale list. Add the symmetric counterpart to the existing entry-path check: when an entity currently in the set has an `include` component changed, re-derive membership from the authoritative store (`store.locate` + `isSupersetOf(includeSet)`) and re-emit if it has left the set. `whereSet` is a subset of `includeSet`, so the existing where-predicate re-check is folded into the same single locate. Preserves O(changes): the loop still iterates only changed entities, with at most one O(1) locate each; value-only transactions still hit the early-exit without notifying. A full O(result-rows) re-query happens only on a genuine membership/order/where change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the per-entity O(|include|) `isSupersetOf(include)` scan in the observe.select transaction handler with a `Map<ArchetypeId, boolean>` memoized per subscription. Archetype component sets never change and ids are stable/dense, so each archetype is classified at most once; every subsequent entity in it is an O(1) map lookup. Archetypes created after subscription are classified on first sight, so late migrations into a brand-new qualifying archetype are still detected. Unify the in-set and entry branches through the shared `qualifies` helper. This also fixes a latent staleness bug: the reactive path previously ignored the `exclude` clause entirely (only the initial `store.select` honored it), so an entity gaining or losing an excluded component never re-emitted. The top-level guard and the per-entity gate now test `include ∪ exclude`; when there is no exclude clause the extra `isDisjointFrom(∅)` is a free O(1) check, preserving the fast path. Still O(changes): the loop iterates only changed entities, each costing one O(1) locate + O(1) memoized lookup; value-only transactions still hit the early-exit without notifying. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two `rowPredicate(location.archetype as any, ...)` calls discarded the whole table type. Localize them into one `matchesFilter` helper that asserts only the fuller `ReadonlyTable<Pick<…, Include>>` shape the predicate expects (`store.locate` types just the base `id` column). The invariant is real and compiler-invisible: callers reach it only after `qualifies` confirms the archetype is a superset of `include`, and `where` keys are a subset of `include`, so every column the predicate reads is present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An order-key change on an in-set entity always forces a requery — a row that stays a member may reorder, one that migrated out is removed — so the membership locate is unnecessary in that case. Move the order-key test ahead of store.locate to skip the lookup entirely for order-value churn (common in ordered/timeline queries). Behavior is unchanged; only the redundant locate is avoided. Co-Authored-By: Claude Opus 4.8 <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.
Description
db.observe.select(include)did not re-emit when a transaction removed an entity from the result set by clearing a required (membership) component that the caller selected but did notwhere/orderon. The observer kept emitting a stale list still containing the removed entity.Root cause
The per-transaction check in
observeSelectEntitieshas two branches for each changed entity:store.locate(...).archetype.components.isSupersetOf(includeSet).orderandwherecomponents. It never re-derived archetype membership, so removing anincludecomponent that isn't also awhere/orderkey migrated the row out of the result archetype silently.Note the top-level
changedComponents.isDisjointFrom(includeSet)guard is not the culprit:where/orderkeys are type-constrained to be withininclude, and a component removal always lands inchangedComponents, so the guard never wrongly bails on a contraction.Fix
Add the symmetric counterpart to the entry-path check in the in-set branch: when an entity currently in the set has an
includecomponent changed, re-derive membership from the authoritative store and re-emit if it has left the set.whereSet ⊆ includeSet, so the existing where-predicate re-check is folded into the same singlestore.locate.Performance
Preserves the O(changes) contract: the loop still iterates only changed entities, with at most one O(1)
store.locateeach; value-only transactions still hit the early-exit without notifying. A full O(result-rows) re-query happens only on a genuine membership / order / where change (unavoidable, since results are returned as fresh arrays).Tests
includecomponent drops the entity and triggers a re-emit (fails onmain, passes here).@adobe/datadatabase suite (709 tests) + O(changes) performance test green; lint + typecheck clean.Downstream impact
This is a behavior change:
observe.selectnow correctly re-emits on membership contraction. Consumers that worked around the gap by separately subscribing to the membership component can drop that extra subscription.