From 082c8eed6c849f4404be0b4682a3d7a9e5277f35 Mon Sep 17 00:00:00 2001 From: Matthew Connelly Date: Thu, 28 May 2026 18:24:44 -0400 Subject: [PATCH] fix(layout-engine): include SDT alias/tag/appearance in version hash The DomPainter's cache invalidation system wasn't including SDT mutable properties (alias, tag, appearance) in the version hash. When only the alias changed via contentControls.patch(), the computed version stayed the same, so the painter skipped repainting. Changes: - Add getSdtMetadataAlias/Tag/Appearance helpers to versionSignature.ts - Include all mutable SDT properties in getSdtMetadataVersion() - Add getSdtMetadataVersion(textRun.sdt) to text run version signature - Fix buildSdtCacheKey to include mutable props for structured content - Fix resolveNodeSdtMetadata to not pass explicit cacheKey for structured content (allowing full attrs-based cache key) Co-Authored-By: Claude Opus 4.5 --- .../layout-resolved/src/versionSignature.ts | 35 +++++++++++++++++- .../pm-adapter/src/sdt/metadata.ts | 36 +++++-------------- .../layout-engine/style-engine/src/index.ts | 9 +++++ 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/packages/layout-engine/layout-resolved/src/versionSignature.ts b/packages/layout-engine/layout-resolved/src/versionSignature.ts index 89d56326f8..f79b0838eb 100644 --- a/packages/layout-engine/layout-resolved/src/versionSignature.ts +++ b/packages/layout-engine/layout-resolved/src/versionSignature.ts @@ -50,9 +50,40 @@ const getSdtMetadataLockMode = (metadata: SdtMetadata | null | undefined): strin return metadata.type === 'structuredContent' ? (metadata.lockMode ?? '') : ''; }; +const getSdtMetadataAlias = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + if ('alias' in metadata && metadata.alias != null) { + return String(metadata.alias); + } + return ''; +}; + +const getSdtMetadataTag = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + if ('tag' in metadata && metadata.tag != null) { + return String(metadata.tag); + } + return ''; +}; + +const getSdtMetadataAppearance = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + if (metadata.type === 'structuredContent' && 'appearance' in metadata && metadata.appearance != null) { + return String(metadata.appearance); + } + return ''; +}; + const getSdtMetadataVersion = (metadata: SdtMetadata | null | undefined): string => { if (!metadata) return ''; - return [metadata.type, getSdtMetadataLockMode(metadata), getSdtMetadataId(metadata)].join(':'); + return [ + metadata.type, + getSdtMetadataLockMode(metadata), + getSdtMetadataId(metadata), + getSdtMetadataAlias(metadata), + getSdtMetadataTag(metadata), + getSdtMetadataAppearance(metadata), + ].join(':'); }; const getTrackedChangeLayers = (run: TextRun): TrackedChangeMeta[] => { @@ -359,6 +390,8 @@ export const deriveBlockVersion = (block: FlowBlock): string => { textRun.comments?.length ?? 0, // SD-3098: DomPainter reads run.bidi to apply dir + RLM injection; signature must include it. textRun.bidi ? JSON.stringify(textRun.bidi) : '', + // Include inline SDT metadata (alias, tag, etc.) so changes trigger repaint. + getSdtMetadataVersion(textRun.sdt), ].join(','); }) .join('|'); diff --git a/packages/layout-engine/pm-adapter/src/sdt/metadata.ts b/packages/layout-engine/pm-adapter/src/sdt/metadata.ts index 322bad72e5..ad5eb55aa6 100644 --- a/packages/layout-engine/pm-adapter/src/sdt/metadata.ts +++ b/packages/layout-engine/pm-adapter/src/sdt/metadata.ts @@ -6,29 +6,10 @@ * document sections, TOC entries, structured content blocks, etc. */ -import type { - FlowBlock, - TableBlock, - ListBlock, - SdtMetadata, - FieldAnnotationMetadata, - StructuredContentMetadata, - DocumentSectionMetadata, - DocPartMetadata, -} from '@superdoc/contracts'; +import type { FlowBlock, TableBlock, ListBlock, SdtMetadata } from '@superdoc/contracts'; import type { PMNode } from '../types.js'; import { resolveSdtMetadata } from '@superdoc/style-engine'; -type SdtMetadataForOverride = TOverride extends 'fieldAnnotation' - ? FieldAnnotationMetadata - : TOverride extends 'structuredContent' | 'structuredContentBlock' - ? StructuredContentMetadata - : TOverride extends 'documentSection' - ? DocumentSectionMetadata - : TOverride extends 'docPartObject' - ? DocPartMetadata - : SdtMetadata; - /** * Type guard to check if a node has instruction attribute. */ @@ -76,16 +57,17 @@ export function getDocPartObjectId(node: PMNode): string | undefined { * @param overrideType - Optional type override (e.g., 'documentSection', 'docPartObject') * @returns Resolved SDT metadata, or undefined if none */ -export function resolveNodeSdtMetadata( - node: PMNode, - overrideType?: TOverride, -): SdtMetadataForOverride | undefined { +export function resolveNodeSdtMetadata(node: PMNode, overrideType?: string): SdtMetadata | undefined { const attrs = node.attrs; if (!attrs) return undefined; const nodeType = overrideType ?? node.type; if (!nodeType) return undefined; - const cacheKey = - typeof attrs.hash === 'string' + // Don't pass cacheKey for structuredContent - let buildSdtCacheKey use full attrs + // This ensures mutable properties are included in the cache key + const isStructuredContent = nodeType === 'structuredContent' || nodeType === 'structuredContentBlock'; + const cacheKey = isStructuredContent + ? undefined + : typeof attrs.hash === 'string' ? attrs.hash : typeof attrs.id === 'string' ? attrs.id @@ -96,7 +78,7 @@ export function resolveNodeSdtMetadata | undefined; + }); } /** diff --git a/packages/layout-engine/style-engine/src/index.ts b/packages/layout-engine/style-engine/src/index.ts index 7fa06ce00e..579a3bf216 100644 --- a/packages/layout-engine/style-engine/src/index.ts +++ b/packages/layout-engine/style-engine/src/index.ts @@ -408,6 +408,15 @@ function buildSdtCacheKey( const id = toOptionalString(attrs.id); if (id) { + // For structuredContent, include mutable properties in the cache key + // so that changes to alias/tag/lockMode/appearance cause cache misses. + if (nodeType === 'structuredContent' || nodeType === 'structuredContentBlock') { + const alias = toOptionalString(attrs.alias) ?? ''; + const tag = toOptionalString(attrs.tag) ?? ''; + const lockMode = toOptionalString(attrs.lockMode) ?? ''; + const appearance = toOptionalString(attrs.appearance) ?? ''; + return `${nodeType}:${id}:${alias}:${tag}:${lockMode}:${appearance}`; + } return `${nodeType}:${id}`; }