Skip to content

test(diff): pin endnote-scope N-way note composition + fix stale comment (closes #231)#270

Merged
JSv4 merged 1 commit into
mainfrom
claude/issue-231-evaluation-fix-c1kbdw
Jul 12, 2026
Merged

test(diff): pin endnote-scope N-way note composition + fix stale comment (closes #231)#270
JSv4 merged 1 commit into
mainfrom
claude/issue-231-evaluation-fix-c1kbdw

Conversation

@JSv4

@JSv4 JSv4 commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Summary

Issue #231 asked the DocxDiff composite merger to compose footnote/endnote content edits across reviewers (NoteOps composition) and to remove the NoteOps-null tripwire. Evaluation finding: that capability is already fully implemented and tested — it landed after the issue was filed (2026-06-14), via #250 ("close the N-way Consolidate gap — note merge…") and #261 ("cross-kind note fix"). The issue was simply never closed.

This PR closes the loop by landing the two residual gaps the evaluation surfaced — both squarely in the IR-based DiffDocx engine, no WmlComparer changes and no engine behavior change:

  1. Endnote-scope regression coverage was missing. The dedicated IrCompositeNoteTests fixture is footnote-only, yet issue feat(diff): composite — merge footnote/endnote content edits across reviewers (NoteOps composition) #231's acceptance criterion names "footnote/endnote text". Footnotes and endnotes run the same MergeNoteScopes kind-loop ({ Footnote, Endnote }) and the same ApplyCompositeNoteDiffs render dispatch, but the endnote branch had no committed test. New IrCompositeEndnoteTests mirrors the footnote suite for endnotes.
  2. A stale code comment in IrCompositeMerger.LowerStructuralOps still read "the composite path does not yet compose them across reviewers" — a pre-feat(diff): close the N-way Consolidate gap — note merge, column composition, native split/merge/row-moves, cell-shell visibility #250 leftover that flatly contradicts MergeNoteScopes. Corrected (the architecture doc at ir_diff_engine.md line 248 was already updated; the comment was missed).

Verification of #231's acceptance criteria in the current tree

Acceptance criterion Status Evidence
MergeNoteScopes composes note diffs across reviewers (both kinds) IrCompositeMerger.cs kind-loop over Footnote and Endnote
NoteOps-null Debug.Assert tripwire removed IrCompositeMarkupRenderer.cs renders when NoteOps present
Disjoint compose · same-note conflict per policy · delete-vs-edit conflict · reviewer-inserted · attributed revisions IrCompositeNoteTests + IrCompositeCrossKindNoteTests (22 cases)
Endnote text specifically (AC1) new IrCompositeEndnoteTests (6 cases)
reject ≡ base (incl. notes) under all policies asserted in every note test
Base 2-way parity stays 179/179 IrParityScoreboardTests: Total: 179 PASS: 179 DEVIATION: 0 FAIL: 0
Full note/composite suite green note suites 29/29; composite/consolidate/footnote slice 200/200

Changes

  • Docxodus.Tests/Ir/Diff/IrCompositeEndnoteTests.cs (new): endnote sibling of IrCompositeNoteTests.
    • Disjoint_endnote_edits_compose_both_land (×3 policies) — both reviewers' disjoint endnote edits land on accept; endnotes restore to base on reject.
    • Same_endnote_conflicting_edits_resolve_per_policy (×3 policies) — same-endnote edits recorded as a conflict with both authors; accept follows BaseWins/FirstReviewerWins/StackAll; reject ≡ base.
    • Consolidated_revisions_surface_endnote_edits_with_attribution — endnote edits appear as attributed revisions.
  • Docxodus/Ir/Diff/IrCompositeMerger.cs: corrected the LowerStructuralOps note-passthrough comment.

Testing

  • dotnet build Docxodus/Docxodus.csproj -c Release → 0 warnings, 0 errors (warnings-as-errors).
  • IrCompositeEndnoteTests + IrCompositeNoteTests + IrCompositeCrossKindNoteTests29/29 pass.
  • IrParityScoreboardTests (2-way parity) → 179/179, deviation 0.
  • Composite / consolidate / footnote slice → 200/200 pass.

Closes #231.


Generated by Claude Code

… comment

Issue #231 (compose footnote/endnote content edits across reviewers in the
DocxDiff composite merger) was already implemented by #250/#261: MergeNoteScopes
iterates { Footnote, Endnote } and ApplyCompositeNoteDiffs renders both scopes;
the NoteOps-null tripwire is gone. This closes the two residual gaps that
evaluation surfaced, both squarely in the IR-based DiffDocx engine:

- Endnote regression coverage was missing. The dedicated IrCompositeNoteTests
  fixture is footnote-only, yet the acceptance criterion names "footnote/endnote
  text". IrCompositeEndnoteTests mirrors it for the endnote branch: disjoint
  endnote edits compose, same-endnote edits conflict per policy, consolidated
  revisions attribute endnote edits, and reject == base under all policies.

- A stale comment in LowerStructuralOps still claimed "the composite path does
  not yet compose them across reviewers" (a pre-#250 leftover). Corrected:
  note scopes pass through body-op lowering unchanged and are composed later by
  MergeNoteScopes.

No engine behavior change. Base 2-way parity stays 179/179; note suites green.
@JSv4 JSv4 merged commit 7bf5876 into main Jul 12, 2026
12 checks passed
@JSv4 JSv4 deleted the claude/issue-231-evaluation-fix-c1kbdw branch July 12, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(diff): composite — merge footnote/endnote content edits across reviewers (NoteOps composition)

2 participants