Conversation
Snap: implement saveModifiedTree (asset-preserving SQL replay) - Copy original .sps verbatim then replay pendingMutations as targeted UPDATE/INSERT - addButton inserts Button + ElementReference + CommandSequence + one ElementPlacement per PageLayout (Visible=1 at free cell, Visible=0 out-of-bounds otherwise) - capabilities.preservesAssetsOnSave: false → true - load path uses _loadButton to keep pendingMutations clean - Tested round-trip + addButton on Core First Scanning .sps; loads cleanly in TD Snap on dashboard and topic pages
The cherry-picked saveModifiedTree (parent commit) was untested. This adds test/snapProcessor.saveModifiedTree.test.ts with 6 cases: - capabilities flag is preservesAssetsOnSave: true - zero-mutation round-trip is byte-identical (full 23-table schema preserved) - addButton inserts a complete Button + ElementReference + CommandSequence + one ElementPlacement per existing PageLayout for the target page, with all the modal-non-NULL columns TD Snap requires (ContentType=6, CommandFlags=8, ForegroundColor / BackgroundColor set, ElementType=0, fresh GUID, MessageAction:0 in CommandSequence) - updateButton patches Label/Message on the matching Button.Id - removeButton flips Visible=0 on every placement - WordList mutations are silent no-ops (capability: wordList=none) Bug surfaced + fixed by the tests: ElementPlacement.GridSpan is NOT NULL, and not every Snap schema has a default of '1,1' (Core_First_Scanning.sps does, test/assets/snap/example.sps does not). All 5 ElementPlacement INSERTs now specify GridSpan='1,1' explicitly. Restores main CI: coverage thresholds were dropping below limit because saveModifiedTree was added without tests in the merged PR. Co-Authored-By: Claude Opus 4.7 (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.
Snap: ship saveModifiedTree properly on main
main was missing two things vs feature-mutationAPI:
The whole SnapProcessor.saveModifiedTree method (~280 lines) — copies the original .sps verbatim, then replays pendingMutations as targeted SQL INSERT/UPDATE for Button, ElementReference, CommandSequence, ElementPlacement. Capability flag preservesAssetsOnSave flipped false → true.
Tests — the merged PR didn't include any, which dragged Snap coverage from ~58% → 54%, pulled global coverage below thresholds, and red-CI'd main.
This branch cherry-picks the implementation onto main, then layers two more changes on top:
test/snapProcessor.saveModifiedTree.test.ts — 6 cases: capabilities flag, zero-mutation round-trip preserving the full 23-table schema, addButton full row + per-layout placement invariant, updateButton, removeButton, WordList no-op.
ElementPlacement.GridSpan='1,1' set explicitly on every INSERT. Surfaced by the tests: Core_First_Scanning.sps declares the column with DEFAULT '1,1', but test/assets/snap/example.sps doesn't, so the implicit default crashes on stricter schemas.
CI locally: npm run lint ✓, npm run type-check ✓, npm run test:ci ✓ (87 suites, 756 tests, coverage above thresholds). Tested in TD Snap on Core First Scanning — both Topic: My Family and Google Home Speaker (the dashboard page that previously crashed) load and render correctly with the new buttons.
Closes the regression introduced by PR #45.