Reachability-based create detection (replaces eager orphan-purge)#48
Open
matej21 wants to merge 3 commits into
Open
Reachability-based create detection (replaces eager orphan-purge)#48matej21 wants to merge 3 commits into
matej21 wants to merge 3 commits into
Conversation
…an-purge) Resolve the orphan-`create` leak class (#47 + bughunt) structurally instead of patching each detach path. A created entity is a `create` iff it is reachable from a root through live relations; a detached created entity is simply unreachable and produces no mutation. This replaces the eager-purge machinery (cascade, purgeOrphanedCreated, isEntityReferenced refcounts, per-detach-path purges) with a single invariant. - RootRegistry + ReachabilityAnalyzer + RelationStore.getLiveChildIds; id->key index in EntitySnapshotStore for O(1) child resolution - createEntity auto-roots; registerParentChild un-roots (a child is anchored by its parent); top-level <Entity create> / useEntityList adds stay roots - DirtyTracker create branch gated on reachability; ActionDispatcher back to plain relation updates (isNeverPersisted kept only for DELETE_RELATION relation-state semantics: never-persisted has-one target reverts to disconnected, not deleted) - lazy sweep (sweepUnreachableCreated, post-persist) + React unmount cleanup reclaim detached-create snapshots; correctness never depends on either - pessimistic persist failure preserves the user's edits/creates for retry (P2) instead of stranding them at the server view - drop dead relation API (cancelHasManyConnection/Removal); removeFromHasMany returns boolean Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj
- reachabilityCreateDetection: lingering-orphan, diamond, cascade-drop, sweep and pessimistic-window cases proving the gate independent of any eager purge - rewrite orphan / hasMany-remove tests to assert no-create behavior instead of the eager-purge mechanism (a detached snapshot may linger until the lazy sweep) - pessimistic tests assert P2 preserve-for-retry on failure (update + create) - update removeFromHasMany / REMOVE_FROM_LIST call sites for dropped itemType / targetType params Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj
…a loss, unmount cleanup, perf - PERSIST-1: on a failed pessimistic transaction restore ALL captured states (not only the ones whose own mutation failed) and gate the post-persist sweep to full success, so a succeeded-but-reset parent and its inline-created child survive a partial batch failure (the default non-atomic path) instead of being stranded/swept - PERSIST-2: restore relation placeholderData on commit=false so an inline has-one create and its dirty signal survive a pessimistic retry - REACT-1: make the create-mode / list unmount cleanup reachability-aware (unregisterRootEntity + sweepUnreachableCreated) so a draft connected into another live parent (diamond) is preserved instead of hard-removed - REACT-2: re-seed the create-mode draft under the same temp id so the form survives a React StrictMode mount cycle - PERF-1: early-out computeReachableCreated when no never-persisted snapshot exists, keeping the common update-only dirty check off the O(V*(R+H)) graph walk - cleanup: drop dead RootRegistry.has() and EntitySnapshotStore.findByEntityId (isNeverPersisted resolves via keyForId), extract ActionDispatcher disconnectRelation helper, guard idIndex removal against id collisions, document removeEntity's no-inbound-cleanup contract - tests: partial-failure pessimistic (incl. inline-child survival), unmount cleanup (discard / persisted-kept / diamond / StrictMode), getLiveChildIds deleted-edge Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4RHL9A7AGr5gMq6d4Qhk7
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.
What & why
Resolves the orphan-
createleak class — issue #47 and the bug-hunt that followed it — structurally instead of patching each detach path.Root cause:
getAllDirtyEntities()decided create-ness from a single signal (snapshot exists withexistsOnServer=false), while attachment lived in a separateRelationStore. Two sources of truth kept in sync by hand → every detach path had to manually purge the snapshot or it leaked a phantomcreate(a global persist then tried to create a row the user removed). The prior fix grew into whack-a-mole eager-purge (cascade, refcounts, per-path purges).The invariant
createEntityauto-roots;registerParentChildun-roots (a child is anchored by its parent). A top-level<Entity create>/useEntityListadd stays a root.RelationStore(live membership), never the subscription registry.A detached created entity is now simply unreachable → produces no mutation. The whole eager-purge layer is deleted (net -25 LOC of production despite two new modules);
ActionDispatcherreturns to plain relation updates.Also in this PR
cancelHasManyConnection/Removal);removeFromHasManyreturnsboolean.Testing
formRelations › tracks dirty state from has-many relation— is pre-existing and unrelated: it fails identically on cleanmain(a separate has-many-disconnect dirty-tracking bug, out of scope here).packages/example/App.tsxduplicate-graphql-clientinstall error onmain).Follow-ups (not in this PR)
Investigation surfaced a unifying theme — RelationStore should be the single authoritative source for relation state. Candidates: unify the parent-child graph (delete
childToParents; needs eager has-one materialization first), simplify the 5-field has-many state model, and redesign the pessimistic mutate-restore as a presentation flag. Tracked for a later decision.🤖 Generated with Claude Code
https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj