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
46 changes: 46 additions & 0 deletions openspec/changes/add-markdoc-structural-validation/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Design: Structural validation at the resolved-operation boundary

## Decision

Validators consume a resolved, read-only operation view: source paragraph, positional anchor, intended insertion level/style peer, and the ordered outline window around the anchor. This is late enough to know the actual DOCX structure and early enough to remain transactional.

The shared result is a structured diagnostic rather than harness control flow: stable `code` and `severity` (`warning` or `error`), operation and source/anchor IDs, observed and intended hierarchy levels, human/agent-facing message, and an optional deterministic `suggested_anchor_id`.

Markdoc compilation treats unsafe structural placement as fail-closed by default. Interactive editing tools return the same diagnostic as a warning or retryable error according to the tool's mutation contract. Retry counts and "warn once, then allow" policy remain application concerns and are not ported.

## Parent-child slicing

For a section-level insertion, scan forward from the positional anchor until a shallower ancestor boundary. If deeper descendants occur before that boundary and the inserted level would separate them from their parent, diagnose slicing and identify the last descendant as the corrective anchor. Insertion at or below the first child's level does not slice the hierarchy.

Content-based section-header detection from the harness is not authoritative in Safe DOCX. Markdoc operation kind plus resolved numbering/style hierarchy must drive applicability; content heuristics may only provide advisory evidence.

## Rollout

1. Introduce the diagnostic/result contract and parent-child slicing validator.
2. Integrate it with Markdoc `validate` and compile preflight.
3. Expose matching diagnostics from insertion/edit tools.
4. Port level and list-renumbering rules one at a time with shared fixtures.

## Bonded run-in paragraph pairs

The NVCA form represents a run-in provision as two adjacent paragraphs with
different roles: a heading paragraph and a body follower. Repeated adjacent
style transitions in the source establish that pairing. Validation requires
both insertions, distinct structural peers, and an application order that
produces heading then body. Text casing and punctuation are not authoritative.
Pairing is one-to-one. `AFTER` operations name the body first because repeated
insertion reverses around the anchor; `BEFORE` operations name the heading
first. Multiple repeated followers for one heading style are resolved only when
the submitted body peer makes the choice unique, otherwise validation emits an
ambiguity error.

`batch_edit` keeps same-slot collisions hard by default. The sole exception is
an explicit two-step `bonded_pair_id` group that the source-derived validator
recognizes as a correctly ordered heading/body pair with distinct style peers.
Three-step groups and unrelated inserts at that slot remain conflicts.

Junior Harness's current live hooks do not enforce this exact two-paragraph
construction. Its same-paragraph regex is heuristic and its header consistency
hook checks sibling formatting. Safe DOCX therefore implements the structural
pair rule directly; harness retry state, Aspose bindings, legal-content
classification, and warn-once policy remain unported.
20 changes: 20 additions & 0 deletions openspec/changes/add-markdoc-structural-validation/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Change: Add Markdoc structural validation and edit warnings

## Why

Safe DOCX preserves formatting reliably when the caller supplies the correct structural peer, but a syntactically valid insertion can still choose an anchor that slices a parent from its descendants or inherits the wrong hierarchy. The Junior harness already contains battle-tested deterministic rules for these mistakes, but those rules currently live above Safe DOCX and cannot protect Markdoc authors or other editing-tool callers.

## What Changes

- Add a product-neutral structural-validator contract to `docx-markdoc` with stable codes, severity/outcome, source location, evidence, and a suggested corrective anchor when one is deterministic.
- Port the semantics of the harness parent-child-slicing rule to the Safe DOCX document/operation model; do not couple Safe DOCX to the harness's Python hook registry, retry state, Aspose objects, or legal-content classifiers.
- Run structural validation after Markdoc schema validation and source resolution, before mutation or output writes.
- Surface the same diagnostics from `docx-markdoc validate`, compilation, and applicable editing-tool responses so agents receive actionable warnings.
- Start with parent-child slicing, then migrate level mismatch and mid-list renumbering rules behind the same registry when their semantics are proven against Safe DOCX fixtures.

## Impact

- Affected specs: `docx-markdoc`, `mcp-server`
- Affected code: `packages/docx-markdoc`, `packages/docx-mcp`, shared document outline/numbering inspection primitives
- Dependency: builds on `add-brownfield-markdoc-authoring`
- Compatibility: diagnostics are additive; strict compilation may newly reject structurally unsafe operations before writing output
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## ADDED Requirements

### Requirement: Resolved Markdoc operations receive structural validation

The system SHALL validate resolved Markdoc operations against the pinned source document's ordered hierarchy before mutating a document or writing output. Diagnostics SHALL have stable codes, severity, operation and anchor identity, structural evidence, and a corrective anchor when one is deterministic.

#### Scenario: Parent-child slicing fails before mutation

- **GIVEN** a section-level insertion anchored between a parent paragraph and its existing descendants
- **WHEN** the inserted level would separate those descendants from their parent
- **THEN** validation SHALL emit a parent-child-slicing diagnostic
- **AND** SHALL identify the last descendant before the ancestor boundary as the suggested anchor
- **AND** strict compilation SHALL write no output

#### Scenario: Nested peer insertion is not misdiagnosed

- **GIVEN** an insertion whose intended level is at or below the first following child's level
- **WHEN** structural validation runs
- **THEN** the parent-child-slicing validator SHALL pass

#### Scenario: Validation output is actionable and stable

- **GIVEN** a structurally unsafe resolved operation
- **WHEN** `docx-markdoc validate` or compilation preflight reports it
- **THEN** both surfaces SHALL use the same stable diagnostic code and evidence fields

#### Scenario: Bonded run-in subsection requires two paragraphs

- **GIVEN** the source repeatedly pairs a deterministic heading style with a distinct body-follower style
- **WHEN** an insertion supplies only the heading half or orders the two insertions incorrectly
- **THEN** strict validation SHALL fail before mutation
- **AND** SHALL identify both structural peer styles without relying on title-case text
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
## ADDED Requirements

### Requirement: Editing tools surface structural placement warnings

Applicable paragraph insertion and restructuring tools SHALL expose structural diagnostics derived from the same product-neutral validators used by Markdoc.

#### Scenario: Unsafe insertion returns corrective guidance

- **GIVEN** an insertion request that would slice a parent from existing descendants
- **WHEN** the tool resolves the requested anchor and intended hierarchy
- **THEN** the tool response SHALL identify the unsafe relationship
- **AND** SHALL include a deterministic suggested anchor when available

#### Scenario: Atomic bonded pair shares one insertion slot

- **GIVEN** exactly two batch insertion steps declare one bonded-pair identity, one anchor and position, and distinct source-proven heading/body peers
- **WHEN** the requested operation order yields heading followed by body
- **THEN** the batch SHALL permit the shared slot and apply both steps atomically
- **AND** an unrelated third insertion at that slot SHALL remain a hard conflict
28 changes: 28 additions & 0 deletions openspec/changes/add-markdoc-structural-validation/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## 1. Shared contract

- [x] 1.1 Define stable structural diagnostic and validator interfaces
- [x] 1.2 Build a resolved, read-only outline/operation context
- [x] 1.3 Add registry aggregation and deterministic ordering tests

## 2. Parent-child slicing

- [x] 2.1 Port the position-based slicing algorithm without harness dependencies
- [x] 2.2 Cover parent, child, sibling, ancestor-boundary, and intentional nested insertion cases
- [x] 2.3 Return a deterministic last-descendant anchor suggestion

## 3. Surfaces

- [x] 3.1 Include structural diagnostics in Markdoc validation output
- [x] 3.2 Run strict compile preflight before any mutation or output write
- [x] 3.3 Surface equivalent warnings/retry guidance from applicable MCP editing tools

## 4. Follow-on validators

- [x] 4.1 Port list-level mismatch validation with Safe DOCX fixtures
- [x] 4.2 Port mid-list renumbering avoidance with Safe DOCX fixtures
- [x] 4.3 Document which harness hooks remain product-specific and intentionally unported

## 5. Verification

- [x] 5.1 Add NVCA SPA Section 2 insertion regression cases
- [x] 5.2 Run package tests and repository pre-submit checks
1 change: 1 addition & 0 deletions packages/docx-core/src/primitives/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './document.js';
export * from './document_view.js';
export * from './structural_validation.js';
export * from './errors.js';
export * from './list_labels.js';
export * from './layout.js';
Expand Down
128 changes: 128 additions & 0 deletions packages/docx-core/src/primitives/structural_validation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { describe, expect } from 'vitest';
import { testAllure as test } from '../testing/allure-test.js';
import type { DocumentViewNode } from './document_view-types.js';
import { validateStructuralInsertion, validateStructuralInsertions } from './structural_validation.js';

function node(id: string, level: number | null, numId = level == null ? null : '1'): DocumentViewNode {
return {
id, list_label: '', header: '', style: level == null ? 'body' : `Heading${level}`,
text: id, clean_text: id, tagged_text: id,
list_metadata: { list_level: level == null ? -1 : level - 1, label_type: null, label_string: '', header_text: null, header_style: null, header_formatting: null, is_auto_numbered: level != null },
style_fingerprint: { list_level: level == null ? -1 : level - 1, left_indent_pt: 0, first_line_indent_pt: 0, style_name: '', alignment: 'LEFT' },
paragraph_style_id: null, paragraph_style_name: '', paragraph_alignment: 'LEFT',
paragraph_indents_pt: { left: 0, first_line: 0 },
numbering: { num_id: numId, ilvl: level == null ? null : level - 1, is_auto_numbered: level != null },
heading: level == null ? undefined : { text: id, source: 'word_style', level },
header_formatting: null, body_run_formatting: null,
};
}

describe('structural insertion validation', () => {
test('detects a parent/child slice and suggests the last descendant before the boundary', () => {
const nodes = [node('parent', 1), node('child', 2), node('grandchild', 3), node('body', null), node('sibling', 1)];
expect(validateStructuralInsertion(nodes, { operationId: 'op', position: 'AFTER', anchorId: 'parent' }))
.toContainEqual(expect.objectContaining({ code: 'PARENT_CHILD_SLICE', suggested_anchor_id: 'body' }));
});

test('does not diagnose child-peer, sibling, or ancestor-boundary placement', () => {
const cases: Array<[DocumentViewNode[], string, string]> = [
[[node('parent', 1), node('child', 2)], 'parent', 'child'],
[[node('first', 1), node('second', 1)], 'first', 'first'],
[[node('parent', 1), node('next', 1), node('child', 2)], 'parent', 'parent'],
];
for (const [nodes, anchorId, sourceId] of cases) {
expect(validateStructuralInsertion(nodes, { operationId: 'op', position: 'AFTER', anchorId, styleSourceId: sourceId })
.filter((item) => item.code === 'PARENT_CHILD_SLICE')).toEqual([]);
}
});

test('reports an intentional nested level as advisory, not parent slicing', () => {
const diagnostics = validateStructuralInsertion([node('parent', 1), node('child', 2)], {
operationId: 'op', position: 'AFTER', anchorId: 'parent', styleSourceId: 'child',
});
expect(diagnostics.map((item) => [item.code, item.severity])).toEqual([['LIST_LEVEL_MISMATCH', 'warning']]);
});

test('detects a foreign numbering definition inserted into the middle of a list', () => {
const diagnostics = validateStructuralInsertion([node('a', 1, '1'), node('b', 1, '1'), node('foreign', 1, '9')], {
operationId: 'op', position: 'AFTER', anchorId: 'a', styleSourceId: 'foreign',
});
expect(diagnostics).toContainEqual(expect.objectContaining({ code: 'MID_LIST_RENUMBERING', severity: 'error' }));
});

test('aggregates in operation and registry order', () => {
const nodes = [node('p1', 1), node('c1', 2), node('p2', 1), node('c2', 2)];
const diagnostics = validateStructuralInsertions(nodes, [
{ operationId: 'z', position: 'AFTER', anchorId: 'p2' },
{ operationId: 'a', position: 'AFTER', anchorId: 'p1' },
]);
expect(diagnostics.map((item) => item.operation_id)).toEqual(['z', 'a']);
});

test('requires both halves of a repeated run-in style pair without inspecting title text', () => {
const nodes = [
node('h1', 2), { ...node('b1', null), style: 'HeadingPara2' },
node('h2', 2), { ...node('b2', null), style: 'HeadingPara2' },
node('anchor', 1),
];
expect(validateStructuralInsertions(nodes, [{
operationId: 'heading', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h1',
}])).toContainEqual(expect.objectContaining({ code: 'BONDED_PARAGRAPH_PAIR_REQUIRED' }));
});

test('accepts a complete heading/body pair in the insertion order needed for AFTER', () => {
const nodes = [
node('h1', 2), { ...node('b1', null), style: 'HeadingPara2' },
node('h2', 2), { ...node('b2', null), style: 'HeadingPara2' },
node('anchor', 1),
];
const diagnostics = validateStructuralInsertions(nodes, [
{ operationId: 'body', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'b1' },
{ operationId: 'heading', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h1' },
]);
expect(diagnostics.filter((item) => item.code === 'BONDED_PARAGRAPH_PAIR_REQUIRED' || item.code === 'RUN_IN_PAIR_ORDER')).toEqual([]);
});

test('rejects pair order that would put the body before its heading', () => {
const nodes = [
node('h1', 2), { ...node('b1', null), style: 'HeadingPara2' },
node('h2', 2), { ...node('b2', null), style: 'HeadingPara2' },
node('anchor', 1),
];
expect(validateStructuralInsertions(nodes, [
{ operationId: 'heading', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h1' },
{ operationId: 'body', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'b1' },
])).toContainEqual(expect.objectContaining({ code: 'RUN_IN_PAIR_ORDER' }));
});

test('uses each body operation for only one heading operation', () => {
const nodes = [node('h1', 2), { ...node('b1', null), style: 'HeadingPara2' }, node('h2', 2), { ...node('b2', null), style: 'HeadingPara2' }, node('anchor', 1)];
const diagnostics = validateStructuralInsertions(nodes, [
{ operationId: 'body', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'b1' },
{ operationId: 'heading-one', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h1' },
{ operationId: 'heading-two', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h2' },
]);
expect(diagnostics).toContainEqual(expect.objectContaining({ code: 'BONDED_PARAGRAPH_PAIR_REQUIRED', operation_id: 'heading-two' }));
});

test('enforces the opposite source order for repeated BEFORE insertion', () => {
const nodes = [node('h1', 2), { ...node('b1', null), style: 'HeadingPara2' }, node('h2', 2), { ...node('b2', null), style: 'HeadingPara2' }, node('anchor', 1)];
expect(validateStructuralInsertions(nodes, [
{ operationId: 'heading', position: 'BEFORE', anchorId: 'anchor', styleSourceId: 'h1' },
{ operationId: 'body', position: 'BEFORE', anchorId: 'anchor', styleSourceId: 'b1' },
]).some((item) => item.code === 'RUN_IN_PAIR_ORDER')).toBe(false);
expect(validateStructuralInsertions(nodes, [
{ operationId: 'body', position: 'BEFORE', anchorId: 'anchor', styleSourceId: 'b1' },
{ operationId: 'heading', position: 'BEFORE', anchorId: 'anchor', styleSourceId: 'h1' },
])).toContainEqual(expect.objectContaining({ code: 'RUN_IN_PAIR_ORDER' }));
});

test('fails explicitly when one heading style has ambiguous repeated followers', () => {
const nodes = [
node('h1', 2), { ...node('a1', null), style: 'BodyA' }, node('h2', 2), { ...node('a2', null), style: 'BodyA' },
node('h3', 2), { ...node('b1', null), style: 'BodyB' }, node('h4', 2), { ...node('b2', null), style: 'BodyB' }, node('anchor', 1),
];
expect(validateStructuralInsertions(nodes, [{ operationId: 'heading', position: 'AFTER', anchorId: 'anchor', styleSourceId: 'h1' }]))
.toContainEqual(expect.objectContaining({ code: 'BONDED_PARAGRAPH_PAIR_AMBIGUOUS' }));
});
});
Loading
Loading