Keep the node index honest across an in-place subtree edit - #17
Merged
Conversation
A host that owns its own undo stack does not mutate the tree through the session's forms: it edits nodes in place and re-registers the source to make the index describe the result. That re-registration means re-parsing the whole document, which on a 75 MB dictionary costs seconds per keystroke — but skipping it leaves the index wrong, because a node's id is a PATH, so renaming an entry rekeys the entry and everything under it, and adding or removing a child adds or strips an entry the session hands out. mutateSubtree does the same repair at subtree scope. It snapshots the ids before the mutation and rebuilds them after, so a rename rekeys descendants too, and releases the selection for nodes the mutation removed — the same release deleteOneNode does, computed by identity so a rename (same nodes, new ids) releases nothing. The snapshot is taken inside the wrapper because `id` is a live getter: a caller that snapshotted after the fact would delete exactly the entries it meant to keep. The repair runs in a `finally` so a mutation that throws still leaves the index describing the tree as it stands.
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
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.
Why
A host that owns its own undo stack — the VS Code custom editor for a
compressed-binary
.sldd— does not mutate the tree throughdeleteNodesByIdorthe other session forms. It edits nodes in place and then re-registers the source
so
nodeIndexdescribes the result. On a real customer dictionary (75 MB ofdata/chunk0.xml, 31k entries) that re-registration is a 3.2 s parse, and theedit path was paying it twice per committed cell: 7.5 s of host time to express a
one-cell change.
Simply skipping the re-registration is not an option, because it leaves the index
wrong. A node's
idis a path, so renaming an entry rekeys the entry andevery descendant; adding or removing a child adds or strips an id the session
hands out. The edit itself looks fine — it is the next edit on an affected row
that fails to resolve, or routes into a detached node.
What
mutateSubtree(root, mutate)performs the same repair at subtree scope:root.flatten()ids before the mutation and rebuilds them after,so a rename rekeys descendants too;
beforeminusafterby identity, so a rename (same nodes, new ids) releases nothing;
idis a live getter, so acaller snapshotting after the fact would delete exactly the entries it meant to
keep;
finally, so a mutation that throws still leaves the indexdescribing the tree as it stands.
Scope is
root.flatten()on both sides, matching whatindexSource/deindexSourceindex for a whole source. No change-notification is published —the caller that owns the mutation owns telling its own view.
Tests
test/mutateSubtree.test.ts, 10 cases: a rename rekeys the node and itsdescendants while the rest of the document stays indexed and the selection
survives; an added child becomes findable; a removed name-keyed child becomes
null; a removed positional child stops being handed out even though a siblinginherits its id; a selection on a removed node is released; the wrapper returns
the mutation's value; the index is repaired even when the mutation throws.
npm run verifygreen: 4093 tests, dist rebuilt,check:packandcheck:leakOK.