From eb977f06e18e98cd20815b22a471d8e9eb82c51b Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 08:29:55 -0300 Subject: [PATCH 1/2] refactor(direction): centralize last paragraph isRtl reads on helper (SD-2778) Migrate the two remaining direct reads of attrs.direction/.dir on the paragraph inline-direction axis onto getParagraphInlineDirection: - layout-bridge/src/position-hit.ts: isRtlBlock - layout-resolved/src/resolveParagraph.ts: isRtl Behavior is unchanged on the typed directionContext path and strictly broader on fallback (the helper also covers paragraphProperties.rightToLeft). After this, no consumer outside the helper reads the legacy scalar fields; a follow-up can stop pm-adapter from writing them and drop them from ParagraphAttrs. --- .../layout-bridge/src/position-hit.ts | 26 +++++-------------- .../layout-resolved/src/resolveParagraph.ts | 4 +-- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/position-hit.ts b/packages/layout-engine/layout-bridge/src/position-hit.ts index a504642c09..fbcb239695 100644 --- a/packages/layout-engine/layout-bridge/src/position-hit.ts +++ b/packages/layout-engine/layout-bridge/src/position-hit.ts @@ -23,7 +23,12 @@ import type { ParagraphBlock, ParagraphMeasure, } from '@superdoc/contracts'; -import { adjustAvailableWidthForTextIndent, computeLinePmRange, getFirstLineIndentOffset } from '@superdoc/contracts'; +import { + adjustAvailableWidthForTextIndent, + computeLinePmRange, + getFirstLineIndentOffset, + getParagraphInlineDirection, +} from '@superdoc/contracts'; import { charOffsetToPm, findCharacterAtX } from './text-measurement.js'; import type { PageGeometryHelper } from './page-geometry-helper.js'; @@ -120,26 +125,9 @@ export const getAtomicPmRange = (fragment: AtomicFragment, block: FlowBlock): { export const isRtlBlock = (block: FlowBlock): boolean => { if (block.kind !== 'paragraph') return false; - const attrs = block.attrs as Record | undefined; - if (!attrs) return false; - // AIDEV-NOTE: The typed directionContext.inlineDirection (SD-2776) is the source of - // truth for paragraph inline direction. Check the value, not the key — `inlineDirection` - // can be `undefined` per the resolver contract (no explicit w:bidi anywhere in the - // cascade), and we should fall through to the legacy field in that case. // Do NOT consult attrs.textDirection here: that's writing-mode (ECMA §17.18.93, // values lrTb/tbRl/btLr/lrTbV/tbRlV/tbLrV) which is a separate axis from inline RTL. - const directionContext = attrs.directionContext as { inlineDirection?: string } | undefined; - if (directionContext?.inlineDirection != null) { - return directionContext.inlineDirection === 'rtl'; - } - // AIDEV-NOTE: compat-fallback — `attrs.direction` / `attrs.dir` are the legacy scalar - // duplicates of `directionContext.inlineDirection`. Retire once SD-2778 collapses - // them on `ParagraphAttrs`. - const directionAttr = attrs.direction ?? attrs.dir; - if (typeof directionAttr === 'string' && directionAttr.toLowerCase() === 'rtl') { - return true; - } - return false; + return getParagraphInlineDirection(block.attrs) === 'rtl'; }; export const determineColumn = (layout: Layout, fragmentX: number): number => { diff --git a/packages/layout-engine/layout-resolved/src/resolveParagraph.ts b/packages/layout-engine/layout-resolved/src/resolveParagraph.ts index 60b005e037..de829837d8 100644 --- a/packages/layout-engine/layout-resolved/src/resolveParagraph.ts +++ b/packages/layout-engine/layout-resolved/src/resolveParagraph.ts @@ -9,7 +9,7 @@ import type { ResolvedDropCapItem, ResolvedListMarkerItem, } from '@superdoc/contracts'; -import { adjustAvailableWidthForTextIndent } from '@superdoc/contracts'; +import { adjustAvailableWidthForTextIndent, getParagraphInlineDirection } from '@superdoc/contracts'; import { isMinimalWordLayout, resolveListMarkerGeometry, @@ -93,7 +93,7 @@ export function resolveParagraphContent( const paraIndent = (block.attrs as ParagraphAttrs | undefined)?.indent; const paraIndentLeft = paraIndent?.left ?? 0; const paraIndentRight = paraIndent?.right ?? 0; - const isRtl = (block.attrs as ParagraphAttrs | undefined)?.direction === 'rtl'; + const isRtl = getParagraphInlineDirection(block.attrs) === 'rtl'; const { anchorIndentPx: paraMarkerAnchorIndent, firstLinePx: markerFirstLine, From 2699e3ae8b341ea201e3d088be73d9c83283204b Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 09:05:10 -0300 Subject: [PATCH 2/2] test(direction): pin SD-2778 migration via typed-path + broader-fallback cases Two coverage gaps caught in review: - layout-resolved/src/resolveLayout.test.ts ("preserves increasing first-line marker anchor for nested RTL list levels") used attrs.direction: 'rtl'. The pre-migration code read attrs.direction directly, so that fixture would have passed against the old implementation. Switch to directionContext.inlineDirection so the test only passes through the new helper-driven typed path. - layout-bridge/test/position-hit.test.ts: switching isRtlBlock to getParagraphInlineDirection is strictly broader on fallback (the helper also picks up paragraphProperties.rightToLeft when no directionContext is present). Pin that case so the broadening is intentional and not a regression vector. --- .../layout-bridge/test/position-hit.test.ts | 9 +++++++++ .../layout-resolved/src/resolveLayout.test.ts | 7 ++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/layout-engine/layout-bridge/test/position-hit.test.ts b/packages/layout-engine/layout-bridge/test/position-hit.test.ts index 1eb0210384..eddb24e2fb 100644 --- a/packages/layout-engine/layout-bridge/test/position-hit.test.ts +++ b/packages/layout-engine/layout-bridge/test/position-hit.test.ts @@ -61,4 +61,13 @@ describe('isRtlBlock', () => { ), ).toBe(true); }); + + // SD-2778: switching to getParagraphInlineDirection is strictly broader on + // fallback than the prior inline read. Specifically, the helper picks up + // paragraphProperties.rightToLeft when neither directionContext nor the legacy + // scalar field is present. Pin that case so the broader fallback is intentional. + it('falls back to paragraphProperties.rightToLeft when no other direction signal is present', () => { + expect(isRtlBlock(paragraph({ paragraphProperties: { rightToLeft: true } }))).toBe(true); + expect(isRtlBlock(paragraph({ paragraphProperties: { rightToLeft: false } }))).toBe(false); + }); }); diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts index 525329512c..dc8f46a19e 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts @@ -1777,7 +1777,12 @@ describe('resolveLayout', () => { id, runs: [{ kind: 'text', text: 'RTL list item' }], attrs: { - direction: 'rtl', + // SD-2778: use directionContext so this test only passes through the + // new helper-driven typed path. The pre-migration code read + // attrs.direction directly, so the prior `direction: 'rtl'` fixture + // would have passed against the old implementation too and didn't + // actually prove the migration. + directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' }, indent: { right, hanging: -24 }, wordLayout: { marker: {