Skip to content

Commit 16bad0d

Browse files
authored
refactor: migrate paragraph consumers to directionContext (SD-2777) (#3263)
* refactor: read paragraph direction from directionContext (SD-2777) Migrates Phase A paragraph consumers onto Wave 1a's resolver context (SD-2776). Adds getParagraphInlineDirection helper in @superdoc/contracts that reads directionContext.inlineDirection first, with a compat fallback to legacy attrs.direction / attrs.dir / attrs.rtl / paragraphProperties.rightToLeft until SD-2778 collapses the duplicates. Behavior is unchanged because pm-adapter still mirrors directionContext.inlineDirection onto the legacy fields, so hash output stays identical. Phase A scope (consumers, paragraph-axis only): - layout-bridge: paragraph-hash-utils, cache (paragraph + cell sites) - layout-resolved: versionSignature (block + cell sites) - painter-dom: renderer (block + cell hash sites) - super-editor: listBoundaryNavigationPlugin Out of scope (deferred): - Table-axis consumers (renderTableFragment / renderTableCell / renderTableRow). Wait for PR #3227 to settle since it overlaps these. - SD-2779 rename of the rtl-paragraph DomPainter feature module. Kept separate to avoid diff noise. - ParagraphNodeView.inferParagraphRtlFromRuns - heuristic violates the resolver's no-infer-from-runs rule; removing it is a behavior change. * chore(comments): apply comment-policy fixes to Phase A - Drop paraphrase from three `// Direction` section labels in layout-bridge. The label now matches the surrounding `// Shading` / `// Tabs` neighbors; the helper's name + JSDoc already document the read order. - Reshape the compat-fallback comment on getParagraphInlineDirection into the policy's canonical `AIDEV-NOTE: compat-fallback - <trigger>. Retire once <condition>.` form per comment-policy.md. No behavior change. * test(pm-adapter): pin direction <-> directionContext.inlineDirection invariant (SD-2777) Verifies the load-bearing property of the Phase A migration: pm-adapter must write `paragraphAttrs.direction` and `paragraphAttrs.directionContext.inlineDirection` as either the same value or both undefined. This is what makes `getParagraphInlineDirection(attrs)` produce a hash byte-identical to the legacy `attrs.direction` read on FlowBlock attrs. Layout-compare against 462 R2 docs returned 0 changed snapshots; this test codifies the invariant so a future change to the writer can't silently break hash stability. Also rename `getParagraphInlineDirection` test 'returns undefined when directionContext is present with no inlineDirection' to 'falls back past directionContext when inlineDirection is null' so the title matches the assertion. * refactor: migrate remaining FlowBlock attrs direction reads to helper (SD-2777) Catches four `attrs?.direction === 'rtl'` reads the original audit's regex missed (the optional-chain form `attrs?.direction` didn't match `attrs\.direction`): - painters/dom/src/renderer.ts:3182, 3289 - render-path isRtl lookups - painters/dom/src/features/rtl-paragraph/rtl-styles.ts - isRtlParagraph helper - super-editor/.../CaretGeometry.ts - caret RTL adjustment Behavior is unchanged: the pm-adapter invariant test added in the previous commit proves `direction` and `directionContext.inlineDirection` are always paired (or both undefined). These call sites resolve to the same value via the helper as they did via the legacy scalar. Caught via codex-bot review comment on PR #3263 plus a broader grep for `paragraphProperties.rightToLeft` reads. Two upstream consumers remain out of scope: `headless-toolbar/helpers/ paragraph.ts` and `extensions/text-align/text-align.js` read `paragraphProperties.rightToLeft` at the style-resolved layer, upstream of pm-adapter where directionContext doesn't exist yet.
1 parent 27c76c6 commit 16bad0d

11 files changed

Lines changed: 192 additions & 66 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { getParagraphInlineDirection } from './direction-context.js';
3+
4+
describe('getParagraphInlineDirection', () => {
5+
it('returns undefined for null/undefined attrs', () => {
6+
expect(getParagraphInlineDirection(undefined)).toBeUndefined();
7+
expect(getParagraphInlineDirection(null)).toBeUndefined();
8+
});
9+
10+
it('prefers directionContext.inlineDirection over legacy fields', () => {
11+
const attrs = {
12+
directionContext: { inlineDirection: 'rtl' as const },
13+
direction: 'ltr',
14+
rtl: false,
15+
};
16+
expect(getParagraphInlineDirection(attrs)).toBe('rtl');
17+
});
18+
19+
it('falls back past directionContext when inlineDirection is null', () => {
20+
// Per resolver semantics, inlineDirection=null/undefined means "no explicit
21+
// w:bidi"; the legacy `direction` field is the next fallback in the chain.
22+
const attrs = { directionContext: { inlineDirection: null }, direction: 'rtl' };
23+
expect(getParagraphInlineDirection(attrs)).toBe('rtl');
24+
});
25+
26+
it('falls back to attrs.direction', () => {
27+
expect(getParagraphInlineDirection({ direction: 'rtl' })).toBe('rtl');
28+
expect(getParagraphInlineDirection({ direction: 'ltr' })).toBe('ltr');
29+
});
30+
31+
it('falls back to attrs.dir', () => {
32+
expect(getParagraphInlineDirection({ dir: 'rtl' })).toBe('rtl');
33+
expect(getParagraphInlineDirection({ dir: 'ltr' })).toBe('ltr');
34+
});
35+
36+
it('falls back to attrs.rtl boolean', () => {
37+
expect(getParagraphInlineDirection({ rtl: true })).toBe('rtl');
38+
expect(getParagraphInlineDirection({ rtl: false })).toBe('ltr');
39+
});
40+
41+
it('falls back to paragraphProperties.rightToLeft', () => {
42+
expect(getParagraphInlineDirection({ paragraphProperties: { rightToLeft: true } })).toBe('rtl');
43+
expect(getParagraphInlineDirection({ paragraphProperties: { rightToLeft: false } })).toBe('ltr');
44+
});
45+
46+
it('returns undefined when no signal is present', () => {
47+
expect(getParagraphInlineDirection({})).toBeUndefined();
48+
expect(getParagraphInlineDirection({ directionContext: {} })).toBeUndefined();
49+
expect(getParagraphInlineDirection({ paragraphProperties: {} })).toBeUndefined();
50+
});
51+
});

packages/layout-engine/contracts/src/direction-context.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,40 @@ export type RunScriptContext = {
156156
eastAsian?: string;
157157
};
158158
};
159+
160+
/**
161+
* Read a paragraph's inline base direction from its attributes.
162+
*
163+
* Prefers the resolved {@link ParagraphDirectionContext} (SD-2776) when
164+
* present, then falls back to the legacy scalar fields (`direction`,
165+
* `dir`, `rtl`, `paragraphProperties.rightToLeft`) for compatibility
166+
* until SD-2778 collapses the duplicates.
167+
*
168+
* Consumers should call this instead of inspecting attrs ad hoc so the
169+
* direction source check stays in one place.
170+
*/
171+
export function getParagraphInlineDirection(
172+
attrs:
173+
| {
174+
directionContext?: { inlineDirection?: BaseDirection | null } | null;
175+
direction?: string | null;
176+
dir?: string | null;
177+
rtl?: boolean | null;
178+
paragraphProperties?: { rightToLeft?: boolean | null } | null;
179+
}
180+
| null
181+
| undefined,
182+
): BaseDirection | undefined {
183+
const fromContext = attrs?.directionContext?.inlineDirection;
184+
if (fromContext != null) return fromContext;
185+
// AIDEV-NOTE: compat-fallback - used when ParagraphAttrs.directionContext.inlineDirection is absent.
186+
// Retire once SD-2778 collapses the duplicate scalar fields onto directionContext.
187+
const ppRtl = attrs?.paragraphProperties?.rightToLeft;
188+
if (attrs?.direction === 'rtl' || attrs?.dir === 'rtl' || attrs?.rtl === true || ppRtl === true) {
189+
return 'rtl';
190+
}
191+
if (attrs?.direction === 'ltr' || attrs?.dir === 'ltr' || attrs?.rtl === false || ppRtl === false) {
192+
return 'ltr';
193+
}
194+
return undefined;
195+
}

packages/layout-engine/contracts/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export type {
1616
RunBidiContext,
1717
RunScriptContext,
1818
} from './direction-context.js';
19+
export { getParagraphInlineDirection } from './direction-context.js';
1920
import type { ParagraphDirectionContext, RunBidiContext, RunScriptContext } from './direction-context.js';
2021

2122
// Export table contracts

packages/layout-engine/layout-bridge/src/cache.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1-
import type {
2-
DrawingBlock,
3-
FlowBlock,
4-
ImageBlock,
5-
ImageRun,
6-
ListBlock,
7-
TableBlock,
8-
ParagraphBlock,
9-
ParagraphAttrs,
10-
ParagraphFrame,
11-
TableAttrs,
12-
TableCellAttrs,
13-
Run,
1+
import {
2+
getParagraphInlineDirection,
3+
type DrawingBlock,
4+
type FlowBlock,
5+
type ImageBlock,
6+
type ImageRun,
7+
type ListBlock,
8+
type TableBlock,
9+
type ParagraphBlock,
10+
type ParagraphAttrs,
11+
type ParagraphFrame,
12+
type TableAttrs,
13+
type TableCellAttrs,
14+
type Run,
1415
} from '@superdoc/contracts';
1516
import { fieldAnnotationKey } from './field-annotation-key.js';
1617
import { hasTrackedChange, resolveTrackedChangesEnabled } from './tracked-changes-utils.js';
@@ -392,7 +393,8 @@ const hashRuns = (block: FlowBlock): string => {
392393
}
393394

394395
// Direction
395-
if (attrs.direction) parts.push(`dir:${attrs.direction}`);
396+
const cellDir = getParagraphInlineDirection(attrs);
397+
if (cellDir) parts.push(`dir:${cellDir}`);
396398

397399
if (parts.length > 0) {
398400
cellHashes.push(`pa:${parts.join(':')}`);
@@ -547,7 +549,8 @@ const hashRuns = (block: FlowBlock): string => {
547549
}
548550

549551
// Direction
550-
if (attrs.direction) parts.push(`dir:${attrs.direction}`);
552+
const dir = getParagraphInlineDirection(attrs);
553+
if (dir) parts.push(`dir:${dir}`);
551554

552555
// Pagination properties
553556
if (attrs.keepNext) parts.push('kn');

packages/layout-engine/layout-bridge/src/paragraph-hash-utils.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import type {
2-
ParagraphAttrs,
3-
ParagraphBorders,
4-
ParagraphBorder,
5-
Run,
6-
TableBorders,
7-
TableBorderValue,
8-
CellBorders,
9-
BorderSpec,
1+
import {
2+
getParagraphInlineDirection,
3+
type ParagraphAttrs,
4+
type ParagraphBorders,
5+
type ParagraphBorder,
6+
type Run,
7+
type TableBorders,
8+
type TableBorderValue,
9+
type CellBorders,
10+
type BorderSpec,
1011
} from '@superdoc/contracts';
1112

1213
/**
@@ -202,7 +203,8 @@ export const hashParagraphAttrs = (attrs: ParagraphAttrs | undefined): string =>
202203
}
203204

204205
// Direction
205-
if (attrs.direction) parts.push(`dir:${attrs.direction}`);
206+
const dir = getParagraphInlineDirection(attrs);
207+
if (dir) parts.push(`dir:${dir}`);
206208

207209
return parts.join(':');
208210
};

packages/layout-engine/layout-resolved/src/versionSignature.ts

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
1-
import type {
2-
DrawingBlock,
3-
FieldAnnotationRun,
4-
FlowBlock,
5-
Fragment,
6-
ImageBlock,
7-
ImageDrawing,
8-
ImageRun,
9-
ParagraphAttrs,
10-
ParagraphBlock,
11-
SdtMetadata,
12-
ShapeGroupDrawing,
13-
SourceAnchor,
14-
TableAttrs,
15-
TableBlock,
16-
TableCellAttrs,
17-
TextRun,
18-
VectorShapeDrawing,
1+
import {
2+
getParagraphInlineDirection,
3+
type DrawingBlock,
4+
type FieldAnnotationRun,
5+
type FlowBlock,
6+
type Fragment,
7+
type ImageBlock,
8+
type ImageDrawing,
9+
type ImageRun,
10+
type ParagraphAttrs,
11+
type ParagraphBlock,
12+
type SdtMetadata,
13+
type ShapeGroupDrawing,
14+
type SourceAnchor,
15+
type TableAttrs,
16+
type TableBlock,
17+
type TableCellAttrs,
18+
type TextRun,
19+
type VectorShapeDrawing,
1920
} from '@superdoc/contracts';
2021
import { hashParagraphBorders } from './paragraphBorderHash.js';
2122
import {
@@ -293,7 +294,7 @@ export const deriveBlockVersion = (block: FlowBlock): string => {
293294
attrs.borders ? hashParagraphBorders(attrs.borders) : '',
294295
attrs.shading?.fill ?? '',
295296
attrs.shading?.color ?? '',
296-
attrs.direction ?? '',
297+
getParagraphInlineDirection(attrs) ?? '',
297298
attrs.tabs?.length ? JSON.stringify(attrs.tabs) : '',
298299
].join(':')
299300
: '';
@@ -437,7 +438,7 @@ export const deriveBlockVersion = (block: FlowBlock): string => {
437438
hash = hashNumber(hash, attrs.indent?.hanging ?? 0);
438439
hash = hashString(hash, attrs.shading?.fill ?? '');
439440
hash = hashString(hash, attrs.shading?.color ?? '');
440-
hash = hashString(hash, attrs.direction ?? '');
441+
hash = hashString(hash, getParagraphInlineDirection(attrs) ?? '');
441442
if (attrs.borders) {
442443
hash = hashString(hash, hashParagraphBorders(attrs.borders));
443444
}

packages/layout-engine/painters/dom/src/features/rtl-paragraph/rtl-styles.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77
* @ooxml w:pPr/w:bidi — paragraph bidirectional flag
88
* @spec ECMA-376 §17.3.1.1 (bidi)
99
*/
10-
import type { ParagraphAttrs } from '@superdoc/contracts';
10+
import { getParagraphInlineDirection, type ParagraphAttrs } from '@superdoc/contracts';
1111

1212
/**
1313
* Returns true when the paragraph attributes indicate right-to-left direction.
1414
*/
15-
export const isRtlParagraph = (attrs: ParagraphAttrs | undefined): boolean => attrs?.direction === 'rtl';
15+
export const isRtlParagraph = (attrs: ParagraphAttrs | undefined): boolean =>
16+
getParagraphInlineDirection(attrs) === 'rtl';
1617

1718
/**
1819
* Compute the effective CSS text-align for a paragraph.

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
computeLinePmRange,
6262
expandRunsForInlineNewlines,
6363
getCellSpacingPx,
64+
getParagraphInlineDirection,
6465
normalizeColumnLayout,
6566
normalizeBaselineShift,
6667
resolveBaseFontSizeForVerticalText,
@@ -3178,7 +3179,7 @@ export class DomPainter {
31783179
fragment.markerTextWidth,
31793180
resolvedLine.indentOffset,
31803181
);
3181-
const isRtl = block.attrs?.direction === 'rtl';
3182+
const isRtl = getParagraphInlineDirection(block.attrs) === 'rtl';
31823183
const lineEl = this.renderLine(
31833184
block,
31843185
resolvedLine.line,
@@ -3285,7 +3286,7 @@ export class DomPainter {
32853286
const paraIndent = block.attrs?.indent;
32863287
const paraIndentLeft = paraIndent?.left ?? 0;
32873288
const paraIndentRight = paraIndent?.right ?? 0;
3288-
const isRtl = block.attrs?.direction === 'rtl';
3289+
const isRtl = getParagraphInlineDirection(block.attrs) === 'rtl';
32893290
const {
32903291
anchorIndentPx: paraMarkerAnchorIndent,
32913292
firstLinePx: markerFirstLine,
@@ -7774,7 +7775,7 @@ const deriveBlockVersion = (block: FlowBlock): string => {
77747775
attrs.borders ? hashParagraphBorders(attrs.borders) : '',
77757776
attrs.shading?.fill ?? '',
77767777
attrs.shading?.color ?? '',
7777-
attrs.direction ?? '',
7778+
getParagraphInlineDirection(attrs) ?? '',
77787779
attrs.tabs?.length ? JSON.stringify(attrs.tabs) : '',
77797780
].join(':')
77807781
: '';
@@ -7960,7 +7961,7 @@ const deriveBlockVersion = (block: FlowBlock): string => {
79607961
hash = hashNumber(hash, attrs.indent?.hanging ?? 0);
79617962
hash = hashString(hash, attrs.shading?.fill ?? '');
79627963
hash = hashString(hash, attrs.shading?.color ?? '');
7963-
hash = hashString(hash, attrs.direction ?? '');
7964+
hash = hashString(hash, getParagraphInlineDirection(attrs) ?? '');
79647965
if (attrs.borders) {
79657966
hash = hashString(hash, hashParagraphBorders(attrs.borders));
79667967
}

packages/layout-engine/pm-adapter/src/attributes/paragraph.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,37 @@ describe('computeParagraphAttrs', () => {
348348
expect(paragraphAttrs.direction).toBeUndefined();
349349
});
350350

351+
// SD-2777 invariant: pm-adapter must keep `direction` and
352+
// `directionContext.inlineDirection` aligned (or both absent). This is the
353+
// load-bearing property that lets `getParagraphInlineDirection` produce a
354+
// hash byte-identical to the legacy `attrs.direction` read across the
355+
// entire R2 corpus. If this breaks, the helper-based hash sites in
356+
// layout-bridge / layout-resolved / painter-dom will drift from the
357+
// pre-migration output.
358+
describe('SD-2777: direction and directionContext.inlineDirection are paired', () => {
359+
const cases: Array<{ name: string; rightToLeft: boolean | undefined; expected: 'rtl' | 'ltr' | undefined }> = [
360+
{ name: 'rightToLeft=true', rightToLeft: true, expected: 'rtl' },
361+
{ name: 'rightToLeft=false', rightToLeft: false, expected: 'ltr' },
362+
{ name: 'rightToLeft=undefined', rightToLeft: undefined, expected: undefined },
363+
];
364+
365+
for (const { name, rightToLeft, expected } of cases) {
366+
it(`${name}: attrs.direction === directionContext.inlineDirection (${String(expected)})`, () => {
367+
const paragraph: PMNode = {
368+
type: { name: 'paragraph' },
369+
attrs: {
370+
paragraphProperties: rightToLeft === undefined ? {} : { rightToLeft },
371+
},
372+
};
373+
374+
const { paragraphAttrs } = computeParagraphAttrs(paragraph as never);
375+
376+
expect(paragraphAttrs.direction).toBe(expected);
377+
expect(paragraphAttrs.directionContext?.inlineDirection).toBe(expected);
378+
});
379+
}
380+
});
381+
351382
it('inherits writing mode from body section context (§17.3.1.41)', () => {
352383
// When the paragraph omits w:textDirection, it should pick up writing-mode
353384
// from the section. This test feeds a pre-resolved sectionDirectionContext

packages/super-editor/src/editors/v1/core/presentation-editor/selection/CaretGeometry.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,16 @@ import {
88
pmPosToCharOffset,
99
extractParagraphIndent,
1010
} from '@superdoc/layout-bridge';
11-
import type {
12-
FlowBlock,
13-
Layout,
14-
Line,
15-
Measure,
16-
ParaFragment,
17-
TableBlock,
18-
TableFragment,
19-
TableMeasure,
11+
import {
12+
getParagraphInlineDirection,
13+
type FlowBlock,
14+
type Layout,
15+
type Line,
16+
type Measure,
17+
type ParaFragment,
18+
type TableBlock,
19+
type TableFragment,
20+
type TableMeasure,
2021
} from '@superdoc/contracts';
2122
import { computeTableCaretLayoutRectFromDom } from '../tables/TableCaretDomGeometry.js';
2223
import { getPageElementByIndex } from '../../../dom-observer/PageDom.js';
@@ -196,7 +197,7 @@ export function computeCaretLayoutRectGeometry(
196197

197198
const availableWidth = Math.max(0, fragment.width - (indentAdjust + indent.right));
198199
const charX = measureCharacterX(block, line, pmOffset, availableWidth);
199-
const isRtlParagraph = block.attrs?.direction === 'rtl';
200+
const isRtlParagraph = getParagraphInlineDirection(block.attrs) === 'rtl';
200201
const resolvedCharX = isRtlParagraph ? Math.max(0, availableWidth - charX) : charX;
201202
const localX = fragment.x + indentAdjust + resolvedCharX;
202203
const lineOffset = lineHeightBeforeIndex(measure.lines, fragment.fromLine, index);

0 commit comments

Comments
 (0)