fix(track-changes): expose stable id on trackChanges.list (SD-3084)#3233
Open
caio-pizzol wants to merge 7 commits into
Open
fix(track-changes): expose stable id on trackChanges.list (SD-3084)#3233caio-pizzol wants to merge 7 commits into
caio-pizzol wants to merge 7 commits into
Conversation
The id on TrackChangeInfo was a positional/content hash recomputed every time the document changed. Callers who stored it from list() and passed it to decide() after an edit hit TARGET_NOT_FOUND. The stable raw mark id is now the canonical id, so id matches comment.commentId from onCommentsUpdate and survives edits. Keep the old derived id resolvable as a one-release compat fallback inside findMatchingChange, marked AIDEV-NOTE: temporary.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…-3084) Address self-review nits: - Drop paraphrase opener on the `id: change.rawId` site; the rest of the comment carries the load-bearing context. - Document `toCanonicalTrackedChangeId`'s broadened "any known form, returns canonical" contract and rename the param from `rawId` to `id` to match. - Simplify the lookup predicate to `item.rawId === id`; the previous `item.id === id || item.rawId === id` was redundant because `id` and `rawId` are equal by construction. The existing "id === rawId" invariant test guards the simplification. - Sharpen the AIDEV-NOTE removal trigger to reference SD-3095. - Lock the broadened `toCanonicalTrackedChangeId` semantics with an extra assertion in the soft-fallback test.
Add a "Tracked-change ids" section to both the built-in UI and Custom UI track-changes pages. Names the contract surfaced by SD-3084: - `items[i].id` (and `address.entityId`) survives across edits. - Same value as `payload.changeId` / `payload.comment.commentId` on `onCommentsUpdate`. - Same value `decide` and `get` accept as `target.id`. Story scope guidance: same id can appear in body and a footnote; pair with `address.story` when listing across stories.
Contributor
Five Playwright scenarios that exercise the SD-3084 contract through the public Document API in a real browser. They simulate the consumer pattern that motivated the fix: list -> cache id -> async work / intervening edits -> decide() with the cached id. - items[i].id matches the commentId emitted by onCommentsUpdate - id is stable across a position-shifting edit - decide() accepts a cached id after a wait and an intervening edit - accepting one change does not invalidate the ids of other changes - handle.ref is exposed but the public id alone is sufficient for decide Passes 15/15 across Chromium, Firefox, and WebKit.
…onIds (SD-3084) Three related corrections from review feedback: - Built-in UI and Custom UI docs: "survives across edits" was overbroad. Replace with "stable across edits for the loaded document instance" and add a Source-correlation subsection naming `wordRevisionIds.insert / .delete / .format` as the bridge for source-correlated workflows. - `TrackChangeInfo.id` JSDoc: scope identically and call out that the SuperDoc id is not the OOXML `w:id`. - E2e position-shift test: previous setup inserted AFTER the existing change, so positions didn't move. Pre-SD-3084 hash (type|from|to|...) would have passed too. Rewritten to seed an anchor, make a tracked replacement there, then insert non-tracked text BEFORE it so the change's range actually moves. Added a sanity assertion proving the positions shifted. 15/15 pass on Chromium + Firefox + WebKit.
…I (SD-3084) The built-in UI page is reference, not tutorial. Collapse the "Tracked-change ids" section to a compact contract note: id is SuperDoc's tracked-change id for the loaded document instance, matches address.entityId / payload.changeId / payload.comment.commentId, accepted by get() and decide(). For source correlation, use wordRevisionIds. For story-scoped ids and cached-row patterns, see the Custom UI page. Also tighten the TrackChangeInfo.id field description in the response table to match.
…nce (SD-3084) Make the runtime id feel like one obvious handle and treat OOXML provenance as advanced metadata. - Replace the prose section on the Custom UI page with a "Which id do I use?" decision table and a 4-line `decide()` example using only `item.id`. - Move story-scope and source-correlation guidance into clearly-labeled subsections so the simple case is uncluttered. - Shorten `TrackChangeInfo.id` JSDoc to a 3-line summary that points at `wordRevisionIds` for source correlation. - Reframe `TrackChangeWordRevisionIds` and its fields as source provenance from the imported DOCX, not "an alternate id."
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
2 participants
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.
The id on TrackChangeInfo was a positional/content hash recomputed every time the document changed. Callers who stored it from list() and passed it to decide() after an edit hit TARGET_NOT_FOUND. The stable raw mark id is now the canonical id, so id matches comment.commentId from onCommentsUpdate and survives edits.
Keep the old derived id resolvable as a one-release compat fallback inside findMatchingChange, marked AIDEV-NOTE: temporary.