fix(document-api): anchor pure-insert word-diff groups to preceding EQUAL token (SD-3044)#3368
Open
tupizz wants to merge 1 commit into
Open
fix(document-api): anchor pure-insert word-diff groups to preceding EQUAL token (SD-3044)#3368tupizz wants to merge 1 commit into
tupizz wants to merge 1 commit into
Conversation
…QUAL token (SD-3044) In tracked `text.rewrite`, when the word-level diff produces multiple delete/insert groups separated by EQUAL tokens, pure-insert groups were anchoring to the previous result op's end (`oldTo` / `insertAt`) instead of the EQUAL token that precedes the group. `prevStep = steps[i - 1]` read the last step of the just-processed group (always delete/insert), so the `prevStep.type === 'equal'` branch was dead code. For inputs like `[insert] of [insert` → `John James Smith of [insert address`, the diff produced three groups with EQUAL tokens between them (`of`, ` `, `[insert`). Both pure-insert groups fell back to `insertAt = 8`, piling all granular insertions on the first deletion site and producing strings like `JohnJames Smith address of [insert]...` after accept. Capture `groupStart` before the inner while loop and read `steps[groupStart - 1]` so the EQUAL-anchor branch actually fires. Adds unit coverage for `getWordChanges` and two integration tests mirroring the Lighthouse SAFE-document repro shape.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Demo
Summary
Fixes SD-3044: tracked
text.rewritewas inserting replacement text at shifted positions, producing strings likeJohnJames Smith address of [insert]...instead ofJohn James Smith of [insert address].... The bug survived accept-all, so it was written into exported DOCX.Root cause
In
packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/word-diff.ts,getWordChangescomputes a word-level diff and groups consecutive delete/insert ops into single ops. For pure-insert groups (no delete, just inserts surrounded by EQUAL tokens), it tried to anchor the insertion to the preceding EQUAL token:But by the time this code runs, the inner
whileloop has already advancedipast the group, sosteps[i - 1]is the last delete/insert of the current group — never an EQUAL. TheprevStep.type === 'equal'branch was dead code, and every pure-insert group fell back to the previous result op's end.The bug only triggers when Myers can find EQUAL tokens between insert-only groups. The customer payload happens to do this because the prefix/suffix trim splits
[insert]into[insert(without the], which is part of the shared suffix), making[inserttoken-equal between old and new. Two insert-only groups in[insert] of [insert→John James Smith of [insert addressboth gotinsertAt = 8, pilingJohn,James Smithandaddressat the first deletion site.Fix
Capture
groupStartbefore the inner while loop and use it to look at the step BEFORE the group:One-line change. The previously-dead
prevStep.type === 'equal'branch now runs and the existingprevToken.offset + prevToken.text.lengthlogic produces the correct anchor.How to reproduce
The customer fixture is
Simple Agreement for future equity (SAFE) (5).docx. Steps:pnpm devand open the SuperDoc dev app.Upload the SAFE fixture.
Run the Lighthouse payload via the dev console:
Click "Accept tracked changes".
Before the fix:
After the fix:
Test plan
packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/word-diff.test.ts(8 tests) — covers identity, empty-side, single replace, insert-only with surrounding EQUAL, the exact Lighthouse suffix-trim shape, and prefix-only equal anchoring.tracked-rewrite.integration.test.ts:SD-3044: tracked rewrite with shared suffix anchors inserts correctly— parties-investor shape; explicit guards againstJohnJamesandSmith address.SD-3044: tracked rewrite of long block preserves spacing across multiple equal anchors— parties-company shape; guards againstPtyTitle,AustraliaNew,(ACNPark.super-editorsuite passes (1033 files, 13083 tests).pnpm dev: applying the Lighthouse payload then accepting all tracked changes now produces the expected text for all four blocks (3BD84854,7D51E57C,6FDB207C,447A152E).Acceptance criteria (from ticket)
JohnJames,PtyTitleGroup,TitleGroup(concatenated), orAustraliaNewZealand.tracked-rewrite.integration.test.ts+word-diff.test.ts).tracked-rewrite.integration.test.tscases regressed.Related