Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions openspec/changes/remove-accepted-ghost-section-break/design.md
Original file line number Diff line number Diff line change
@@ -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.

33 changes: 33 additions & 0 deletions openspec/changes/remove-accepted-ghost-section-break/proposal.md
Original file line number Diff line number Diff line change
@@ -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`

Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions openspec/changes/remove-accepted-ghost-section-break/tasks.md
Original file line number Diff line number Diff line change
@@ -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
64 changes: 64 additions & 0 deletions packages/docx-compare/src/tagged/formattingFidelity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`<w:p><w:pPr><w:sectPr><w:pgSz w:w="12000" w:h="16000"/></w:sectPr></w:pPr>` +
`<w:r><w:t>Invented first section</w:t></w:r></w:p>` +
`<w:p><w:r><w:t>Invented continuation</w:t></w:r></w:p>` +
`<w:sectPr><w:pgSz w:w="12240" w:h="15840"/></w:sectPr>`,
);
const revised = docXml(
`<w:p><w:r><w:t>Invented first section</w:t></w:r></w:p>` +
`<w:p><w:r><w:t>Invented continuation</w:t></w:r></w:p>` +
`<w:sectPr><w:pgSz w:w="12240" w:h="15840"/></w:sectPr>`,
);
const candidate = docXml(
`<w:p><w:pPr><w:sectPr><w:sectPrChange w:id="1" w:author="Invented" ` +
`w:date="2026-09-14T00:00:00Z"><w:sectPr><w:pgSz w:w="12000" ` +
`w:h="16000"/></w:sectPr></w:sectPrChange></w:sectPr></w:pPr>` +
`<w:r><w:t>Invented first section</w:t></w:r></w:p>` +
`<w:p><w:r><w:t>Invented continuation</w:t></w:r></w:p>` +
`<w:sectPr><w:pgSz w:w="12240" w:h="15840"/></w:sectPr>`,
);

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 =
`<w:p><w:pPr><w:sectPr><w:pgSz w:w="12000" w:h="16000"/></w:sectPr></w:pPr>` +
`<w:r><w:t>Invented removable boundary</w:t></w:r></w:p>` +
`<w:p><w:r><w:t>Invented following section</w:t></w:r></w:p>`;
const revisedBody =
`<w:p><w:r><w:t>Invented removable boundary</w:t></w:r></w:p>` +
`<w:p><w:r><w:t>Invented following section</w:t></w:r></w:p>`;
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) => {
Expand Down
26 changes: 26 additions & 0 deletions packages/docx-compare/src/tagged/trackChangesAcceptorAst.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `<?xml version="1.0"?>
<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
<w:body>
<w:p><w:pPr><w:sectPr><w:pgMar w:top="900"/>
<w:sectPrChange w:id="1"><w:sectPr><w:pgMar w:top="800"/></w:sectPr></w:sectPrChange>
</w:sectPr></w:pPr><w:r><w:t>Invented live section</w:t></w:r></w:p>
<w:sectPr><w:pgSz w:w="12240" w:h="15840"/></w:sectPr>
</w:body>
</w:document>`;

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;
Expand Down
25 changes: 25 additions & 0 deletions packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
Loading