Read a document once and compare it many times - #629
Merged
Conversation
After #594 and #616 the redundancy left in the engine is ACROSS comparisons: nothing let a caller say "I already read this document". One baseline against many counterparties re-read the baseline once per counterparty; a version chain A->B->C->D read every interior version twice; GetConflicts followed by Consolidate read everything twice. The read is the largest stage of a comparison, so that is the biggest remaining lever. DocxDiff.CreateSnapshot is that statement, and CreateComparison(snapshot, snapshot, settings) produces exactly the same products without reading either side again. Only the input-revision policy reaches a read, so one snapshot serves comparisons that differ in author, granularity, move detection or format comparison; a snapshot read under a different policy is REJECTED rather than silently reused, because serving it would compare a different view of the document than the caller asked for. Creation is free -- nothing is read until a comparison needs it -- and the compatibility pre-flight stays a property of the comparison, so a reused snapshot never makes a subscription go quiet. DocxDiffConsolidation is the N-way twin: the four consolidate statics delegate to one memoized pass a caller can also hold directly. That closes a second gap on the way -- only Consolidate ran the compatibility pre-flight, so a caller asking for conflicts, consolidated revisions or the consolidated edit script with ThrowOnCompatibilityWarning set was silently never told. The N-way half of what #622 fixed on the pairwise side. On the bridges the snapshot becomes a batch. It holds parsed XML and cannot cross a process boundary; exposing it as a handle would put a memory-pinning lifetime on transports with no way to bound it. So the transports get the workload it exists for: CompareBatchJson (facade + WASM), docxDiffCompareBatch (npm), docx_diff_compare_batch (python), docxodus_compare mode=fan_out (MCP) -- one baseline, many candidates, baseline read once, per-candidate products identical to comparing that pair alone. A candidate that fails carries its error instead of products rather than failing the batch. Measured on a heavyweight pair, four comparisons against one baseline: 8219/6526 ms through the statics against 4028/4159 ms sharing the baseline snapshot. Output parity: [parity] OK, all 14,916 corpus digests identical across 678 documents. Suite 4203 passed; npm docx-diff 11 passed; python 9 passed. Also moves the #614 CHANGELOG entries out of the 10.0.0 section, where an insertion at the first "### Fixed" had wrongly put them, into [Unreleased]. Closes #617
JSv4
pushed a commit
that referenced
this pull request
Aug 31, 2026
Third engine movement this branch has tracked, and the first where all three depths moved together: revisions 152 -> 124 ms, redline 181 -> 157, full+HTML 307 from 352, each a median of three 40-frame runs. Ratio against the mutation path is 64x to 165x; the loop runs 3 to 8 fps. The controlled compareProducts pair (fixed inputs, medians of nine) is 106 ms against 170 ms. A saving uniform across depths is the signature of a change on the path every depth shares, which is what #627 is -- it stopped giving the diff engine's reads an identity nothing asks for. That reads differently from #626, whose saving grew with pipeline depth, as a load-path change does. The README now says both, and says plainly that the shape of the movement is what suggests the attribution while the commits are the authority on it; the panel only ever measures the total. #629 landed in the same merge and is deliberately NOT credited. Its snapshot reuse is across comparisons, and the stress loop makes one comparison per frame against a document that changed, so there is nothing for it to reuse. Crediting it because it arrived at the same time would be the same mistake as reading a regression off the stress p50. Also resolves the CHANGELOG conflict from the merge, keeping both Unreleased entries -- #617's snapshot feature and the demo -- neither supersedes the other. Verified on the merged engine: 14/14 browser assertions, 61/61 node checks. #629 changed docxodus_compare's catalog entry (adding mode and outputPaths), which the contract test parses; additive, and the demo does not call that tool.
This was referenced Aug 31, 2026
JSv4
added a commit
that referenced
this pull request
Aug 31, 2026
…rvation channel (#633) The N-way consolidate observations recorded their Warnings and OrderVariance channels as "n/a" on two premises that were both false when written down: DocxDiffConsolidateSettings COMPOSES DocxDiffSettings, so settings.Diff carries the compatibility subscription like any other; and since #617 the four statics each delegate to a single-use DocxDiffConsolidation whose caller-held form shares one memoized read/merge across every product. The first false premise is what let the #629 gap -- three of the four N-way entry points never running the compatibility pre-flight -- hide behind 2,712 rows per run that looked as though the question had been asked and answered. RecordConsolidate is now a structural mirror of the pairwise Record: warnings captured from the same call that produced each result, and all four products re-asked of one caller-held CreateConsolidation in reverse order, which also pins that class against the statics. The settings rotation extends to the N-way path: each document also consolidates with ONE non-default setting -- the pairwise variations wrapped in a consolidate settings object plus the two conflict-resolution policies that exist only there. Verified: consolidate rows record real warnings on 916 of 2,712 default-mode observations (33.8%), the same neighbourhood as pairwise (33.7%); two runs of one build agree on all 15,594 observations; observing the call is result- neutral (probed on WC067, all four products byte-identical); and the fire drill -- disabling the consolidate pre-flight -- moves 1,832 observations (every real-warning consolidate row), 56 of them on the Result channel via the rotated throw-on-compat mode, where the pre-change harness moved zero.
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.
Closes #617.
What was left
#594 removed the redundant work inside one comparison and #616 removed the redundant reads inside one comparison. What remains is the redundancy across comparisons — nothing let a caller say "I already read this document":
GetConflictsbeforeConsolidateon the same inputs — everything read twice.The IR read is still the single largest stage of a comparison, so this is the biggest remaining lever.
The snapshot
The issue flagged two things as needing a decision rather than an assumption, so here they are decided:
What it keys on. Only the input-revision policy reaches a read —
PreAcceptInputRevisionsandPreserveInputRevisions, which together decide whether the document's own tracked changes are flattened away first. Everything else inDocxDiffSettingsis a diff-time or render-time policy, so one snapshot serves comparisons that differ in author, granularity, move detection, format comparison and the rest. That collapses the compatibility key to a single boolean, which is why the type exposes exactly one:InputRevisionsAccepted.A snapshot handed to a comparison asking for the other policy is rejected with an
ArgumentExceptionnaming both sides, not silently reused or quietly re-read. Serving it would compare a different view of the document than the caller asked for, and that is the kind of thing nobody notices until much later.Memory. The type's doc comment says it plainly rather than leaving it to be discovered: a materialized snapshot roots the parsed
XDocumentof every story, not merely the IR values, because the markup renderer clones source elements out of it. That is what makes the reuse possible, and it means a hundred retained snapshots is a hundred parsed documents. Creation itself is free — nothing is read until a comparison needs it.One more property worth pinning, and it has its own test: the compatibility pre-flight is a property of the comparison, not of the read. A reused snapshot must not make a caller's
OnCompatibilityWarningsubscription go quiet on the second comparison.The N-way half, and a bug it turned up
The issue's closing note — the four consolidate statics each read the full reviewer set independently — is fixed by
DocxDiffConsolidation, the N-way twin ofDocxDiffComparison. The four statics delegate to a single-use instance, so there is one implementation rather than two to keep in step, and a caller can hold the instance to inspect conflicts and then consolidate without a second read ofN+1documents.Unifying them surfaced something: of the four entry points, only
Consolidateran the compatibility pre-flight. A caller who setThrowOnCompatibilityWarningand asked for conflicts, consolidated revisions, or the consolidated edit script was silently never told — the same class of gap #622 closed on the pairwise side, and precisely the failure mode #624 describes (the channel is not observed, so no input reveals it). All four now run the same gate.EveryConsolidateEntryPoint_HonorsTheCompatibilitySubscriptionpins it against a fixture that genuinely warns, asserting both that the callback fires and that the throwing form throws, for each of the four.The ripple, and why it is a batch rather than a handle
CLAUDE.md's checklist says a public API addition ripples through every transport. A snapshot, though, is an in-process object holding parsed XML. It cannot cross a process or language boundary, and exposing it as a handle would put a memory-pinning lifetime on transports with no good way to bound it — the worst of both.So the transports get the workload the snapshot exists for, which is also the shape #594 already established here (
CompareProducts→CompareProductsJson→ each bridge):DocxDiff.CreateSnapshot,CreateComparison(snapshot, snapshot, settings),CreateConsolidationDocxDiffOps.CompareBatchJson,DocxDiffOps.ConsolidateProductsDocxDiffBridge.CompareBatchJsondocxDiffCompareBatch+DocxDiffBatchResult/DocxDiffBatchCandidatedocx_diff_compare_batch(sequence → named by index, or a name→bytes mapping)docxodus_comparewithmode: "fan_out"andoutputPathsir_diff_engine.md,docx_agent_server.md, CHANGELOGOne baseline, many candidates, baseline read once, per-candidate products identical to comparing that pair alone. A candidate that fails carries its
errorinstead of products — one malformed counterparty markup must not cost the caller the other ninety-nine. That took a fix during review: the candidate'sWmlDocumentis now constructed inside the per-candidate try, becauseWmlDocument's byte-array constructor validates the package type and was failing the whole batch.The MCP path picked up the N-way win directly:
docxodus_compare's consolidate branch calledConsolidateand thenGetConsolidatedRevisionsJson, reading the base and every reviewer twice. It now uses oneConsolidateProductspass.Measured
Four comparisons against one baseline, on the heaviest pair in the corpus (
WC-BodyBookmarks-Before/After, 2.8 MB and 1.4 MB ofdocument.xml), alternated in one process:The saving is exactly what the mechanism predicts: 8 reads become 5, and the win grows with the fan-out.
Output parity
This one mattered more than usual, because making the consolidate statics delegate switches three of them from
ReadOptstoRenderReadOpts— provenance retained. #594 established that provenance is equality-neutral by construction; the corpus confirms it across every fixture.Suite 4203 passed, 0 failed, 3 skipped.
npm test -- tests/docx-diff.spec.ts11 passed (after a full WASM rebuild).pytest tests/test_docx_diff.py9 passed. Warning baselines move by exactly two each for the two new files, updated inCLAUDE.mdin the same commit.One unrelated fix, carried here
The #614 CHANGELOG entries were sitting in the
[10.0.0]section: that PR inserted at the first### Fixedheading, and[Unreleased]had no### Fixedat the time, so the first one was inside the released section. Moved to[Unreleased]where they belong.