diff --git a/openspec/changes/remove-accepted-ghost-section-break/design.md b/openspec/changes/remove-accepted-ghost-section-break/design.md new file mode 100644 index 00000000..a99336c9 --- /dev/null +++ b/openspec/changes/remove-accepted-ghost-section-break/design.md @@ -0,0 +1,31 @@ +## Context + +`w:sectPrChange` stores the prior section-property state. In the observed removal +shape, the live paragraph-owned `w:sectPr` has no current property children; its +only element child is the change snapshot. Removing only the snapshot converts +that revision record into an empty but semantically active section break. + +## Goals / Non-Goals + +- Goals: make accept-all reflect removal of the paragraph-level section break; + keep reject-all restoration unchanged; preserve exact fidelity enforcement. +- Non-Goals: change section alignment, lower safety thresholds, special-case any + publisher document, or reinterpret nonempty/live section properties. + +## Decisions + +- Record removable section containers before deleting property-change records. + A container qualifies only when it is `w:pPr > w:sectPr`, has a direct + `w:sectPrChange`, and has no other direct element children. +- Never remove `w:body > w:sectPr`; it is the final section-properties container, + not a paragraph-level section break. +- Keep the fidelity oracle and its exact threshold unchanged. The regression + exercises the real accept/reject projections over independently invented XML. + +## Risks / Trade-offs + +- Malformed empty paragraph-owned section properties without a change snapshot + remain untouched and visible to safety checks. +- A paragraph section with any live property child remains a live section break, + even if it also carries a historical snapshot. + diff --git a/openspec/changes/remove-accepted-ghost-section-break/proposal.md b/openspec/changes/remove-accepted-ghost-section-break/proposal.md new file mode 100644 index 00000000..14e9758f --- /dev/null +++ b/openspec/changes/remove-accepted-ghost-section-break/proposal.md @@ -0,0 +1,33 @@ +# Change: Remove accepted ghost section breaks + +## Why + +Accepting a tracked removal of a paragraph-level section break currently removes +the `w:sectPrChange` record but leaves its now-empty `w:sectPr` container. That +container remains a real section break in the accepted projection, so source- +projected formatting fidelity correctly fails even when the revised source has +no corresponding break. + +This was reproduced with wholly invented OOXML and with a private document pair; +the private observation is limited to structural metrics and is not a source of +committed fixture content. + +## What Changes + +- Treat a paragraph-owned `w:sectPr` whose only element child is its + `w:sectPrChange` snapshot as a tracked removal of that section break when + accepting revisions. +- Remove that paragraph-level section-properties container in the accept-all + projection while preserving body-level final section properties and live + section formatting. +- Add an invented regression proving both accept and reject projections retain + exact formatting fidelity without lowering or bypassing the fidelity gate. + +## Impact + +- Affected specs: `docx-comparison` +- Affected code: `packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts` +- Affected tests: tagged accept/reject projection and formatting-fidelity tests +- Dependency: UseJunior/legal-context#918 +- Base: UseJunior/safe-docx `31b1d8fd2e9f0f285cc6167906ef6e2c9f220699` + diff --git a/openspec/changes/remove-accepted-ghost-section-break/specs/docx-comparison/spec.md b/openspec/changes/remove-accepted-ghost-section-break/specs/docx-comparison/spec.md new file mode 100644 index 00000000..0c717a14 --- /dev/null +++ b/openspec/changes/remove-accepted-ghost-section-break/specs/docx-comparison/spec.md @@ -0,0 +1,29 @@ +## MODIFIED Requirements + +### Requirement: Projection-Based Candidate Formatting Comparison + +The system SHALL provide `compareProjectedFormattingFidelity(expectedCandidateXml, actualCandidateXml)` that compares the accept-all projections and the reject-all projections of two tracked-changes candidates and returns both formatting-fidelity reports plus an overall score equal to the minimum of the two projection scores, so that revision-markup granularity differences between reconstruction modes do not register as formatting divergence. Accepting a tracked removal of a paragraph-level section break SHALL remove the paragraph-owned section-properties container when the change snapshot is its only element child; it SHALL NOT remove the body-level final section-properties container or a section-properties container with live formatting children. + +#### Scenario: projected fidelity ignores revision markup granularity differences + +- **WHEN** two candidates encode the same insertion with different `w:ins` wrapper and run granularity but identical formatting +- **THEN** the overall projected score is exactly 1.0 + +#### Scenario: pipeline inplace and rebuild candidates are measurable end-to-end + +- **WHEN** the comparison pipeline produces an inplace candidate and a rebuild candidate for the same original and revised documents +- **THEN** the projected formatting-fidelity comparison of the two candidates returns well-formed accept and reject reports with scores in [0, 1] + +#### Scenario: accepting a tracked section-break removal does not leave a ghost section + +- **GIVEN** an invented candidate whose paragraph-owned `w:sectPr` has a `w:sectPrChange` snapshot and no live section-property child +- **WHEN** accept-all and reject-all projections are compared with their corresponding revised and original source views +- **THEN** both formatting-fidelity scores are exactly 1.0 +- **AND** accept-all contains no paragraph-owned section-properties container for the removed break +- **AND** reject-all restores the prior section properties + +#### Scenario: live and final section properties are preserved + +- **GIVEN** section-properties containers with live formatting children or a body-level final section-properties container +- **WHEN** revisions are accepted +- **THEN** those section-properties containers remain present diff --git a/openspec/changes/remove-accepted-ghost-section-break/tasks.md b/openspec/changes/remove-accepted-ghost-section-break/tasks.md new file mode 100644 index 00000000..2ff2f95f --- /dev/null +++ b/openspec/changes/remove-accepted-ghost-section-break/tasks.md @@ -0,0 +1,7 @@ +## 1. Implementation + +- [x] 1.1 Add conformance-cited accept-all handling for tracked removal of a paragraph-level section break +- [x] 1.2 Add an independently invented projection regression covering accept and reject formatting fidelity +- [x] 1.3 Re-run the private comparison and retain only ignored, metadata-only evidence +- [x] 1.4 Run the documented pre-submit suite and package leakage audit +- [x] 1.5 Conduct and adjudicate supplemental same-model self-review diff --git a/packages/docx-compare/src/tagged/formattingFidelity.test.ts b/packages/docx-compare/src/tagged/formattingFidelity.test.ts index 52b1684d..6eba6a39 100644 --- a/packages/docx-compare/src/tagged/formattingFidelity.test.ts +++ b/packages/docx-compare/src/tagged/formattingFidelity.test.ts @@ -264,6 +264,70 @@ describe('Formatting-fidelity comparison check', () => { expect(result.score).toBe(1); }); + humanReadableTest + .conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.32' }) + .openspec('accepting a tracked section-break removal does not leave a ghost section')( + 'Scenario: accepting a tracked section-break removal does not leave a ghost section', + (_: AllureBddContext) => { + const original = docXml( + `` + + `Invented first section` + + `Invented continuation` + + ``, + ); + const revised = docXml( + `Invented first section` + + `Invented continuation` + + ``, + ); + const candidate = docXml( + `` + + `Invented first section` + + `Invented continuation` + + ``, + ); + + const result = compareSourceProjectedFormattingFidelity(original, revised, candidate); + + expect(result.accept.score).toBe(1); + expect(result.accept.sectionFormatting).toEqual({ compared: 1, divergent: 0, score: 1 }); + expect(result.reject.score).toBe(1); + expect(result.reject.sectionFormatting).toEqual({ compared: 2, divergent: 0, score: 1 }); + expect(result.score).toBe(1); + }, + ); + + test('invented DOCX section-break removal clears the exact publication gate', async () => { + const originalBody = + `` + + `Invented removable boundary` + + `Invented following section`; + const revisedBody = + `Invented removable boundary` + + `Invented following section`; + const [original, revised] = await Promise.all([ + buildDocxFromBodyXml(originalBody), + buildDocxFromBodyXml(revisedBody), + ]); + + const result = await compareDocumentsAtomizer(original, revised, { + author: 'Invented Fixture', + date: new Date('2026-09-14T00:00:00Z'), + }); + const [originalXml, revisedXml, candidateXml] = await Promise.all([ + DocxArchive.load(original).then((archive) => archive.getDocumentXml()), + DocxArchive.load(revised).then((archive) => archive.getDocumentXml()), + DocxArchive.load(result.document).then((archive) => archive.getDocumentXml()), + ]); + + const fidelity = compareSourceProjectedFormattingFidelity(originalXml, revisedXml, candidateXml); + expect(result.engine).toBe('tagged-tree'); + expect(fidelity.accept.score).toBe(1); + expect(fidelity.reject.score).toBe(1); + }); + humanReadableTest.openspec('pipeline inplace and rebuild candidates are measurable end-to-end')( 'Scenario: pipeline inplace and rebuild candidates are measurable end-to-end', async (_: AllureBddContext) => { diff --git a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.test.ts b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.test.ts index a36a29ec..67d9ba09 100644 --- a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.test.ts +++ b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.test.ts @@ -226,6 +226,32 @@ describe('trackChangesAcceptorAst', () => { }); }); + test + .conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.32' })( + 'preserves live paragraph and final section properties when accepting revisions', + () => { + const input = ` + + + + + Invented live section + + + `; + + const result = acceptAllChanges(input); + const root = parseDocumentXml(result); + const sections = findAllByTagName(root, 'w:sectPr'); + + expect(sections).toHaveLength(2); + expect(result).toContain('w:pgMar w:top="900"'); + expect(result).toContain('w:pgSz w:w="12240" w:h="15840"'); + expect(result).not.toContain('w:sectPrChange'); + expect(result).not.toContain('w:pgMar w:top="800"'); + }, + ); + describe('rejectAllChanges', () => { test('should remove w:ins elements entirely', async ({ given, when, then, and }: AllureBddContext) => { let input: string; diff --git a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts index f51b88be..0565b125 100644 --- a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts +++ b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts @@ -634,6 +634,7 @@ export function acceptAllChanges(documentXml: string): string { removeAllByTagName(root, 'w:pPrChange'); removeAllByTagName(root, 'w:trPrChange'); removeAllByTagName(root, 'w:tcPrChange'); + removeAcceptedSectionBreakRemovals(root); removeAllByTagName(root, 'w:sectPrChange'); removeEmptyTablePropertyContainers(root); @@ -842,6 +843,30 @@ function removeEmptyTablePropertyContainers(root: Element): void { } } +/** + * Remove a paragraph-level section break whose tracked prior state is the only + * content of its live section-properties container. Accepting that revision + * means accepting the absence of the break; retaining an empty `w:sectPr` + * would instead create a ghost section. Body-level final section properties + * and paragraph sections with any live property remain untouched. + * + * @conformance ECMA-376 edition 5, Part 1 ยง 17.13.5.32 + */ +function removeAcceptedSectionBreakRemovals(root: Element): void { + for (const change of findAllByTagName(root, 'w:sectPrChange')) { + const sectionProperties = parentElement(change); + const paragraphProperties = sectionProperties ? parentElement(sectionProperties) : undefined; + if ( + sectionProperties?.tagName === 'w:sectPr' && + paragraphProperties?.tagName === 'w:pPr' && + childElements(sectionProperties).length === 1 && + childElements(sectionProperties)[0] === change + ) { + paragraphProperties.removeChild(sectionProperties); + } + } +} + function restoreSectionPropertiesFromChanges(root: Element): void { for (const change of findAllByTagName(root, 'w:sectPrChange')) { const live = change.parentNode as Element | null;