Skip to content

refactor(layout): pre-compute SDT container keys in resolved layout#2812

Open
tupizz wants to merge 1 commit intotadeu/sd-2563-02-fragment-metadatafrom
tadeu/sd-2563-03-sdt-container-keys
Open

refactor(layout): pre-compute SDT container keys in resolved layout#2812
tupizz wants to merge 1 commit intotadeu/sd-2563-02-fragment-metadatafrom
tadeu/sd-2563-03-sdt-container-keys

Conversation

@tupizz
Copy link
Copy Markdown
Contributor

@tupizz tupizz commented Apr 14, 2026

Summary

  • Add sdtContainerKey to all resolved item types
  • Create sdtContainerKey.ts helper in layout-resolved (duplicates pure key derivation logic)
  • Compute SDT container key during resolution for para, list-item, table fragments
  • computeSdtBoundaries() in painter prefers resolved keys with legacy fallback

PR Stack (SD-2563: Dumb Painter Refactor)

# PR Title Status
1 #2810 Lift page metadata into ResolvedPage
2 #2811 Lift fragment metadata into resolved paint items
3 #2812 Pre-compute SDT container keys in resolved layout 👈 this PR
4 #2813 Pre-compute paragraph border data in resolved layout
5 #2814 Move change detection into resolved layout stage

Test plan

  • 71/71 layout-resolved tests pass (12 new)
  • painter-dom types compile cleanly
  • No rendering behavior changes

@linear
Copy link
Copy Markdown

linear bot commented Apr 14, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55044fe203

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +6904 to +6908
const containerKeys: (string | null)[] = resolvedItems
? resolvedItems.map((item) => {
if ('sdtContainerKey' in item) {
const key = (item as { sdtContainerKey?: string | null }).sdtContainerKey;
return key ?? null;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back to block metadata when resolved SDT key is absent

computeSdtBoundaries now treats resolvedItems as authoritative, but when a resolved item omits sdtContainerKey it returns null rather than recomputing from blockLookup; since sdtContainerKey is optional in the resolved item contracts, older or partially populated resolved layouts (including external callers/tests that construct ResolvedLayout directly) will silently lose SDT container grouping/labels even though the fragment still has SDT metadata. Add a per-item fallback to getFragmentSdtContainerKey(fragment, blockLookup) when the resolved key is missing.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant