refactor(layout): pre-compute SDT container keys in resolved layout#2812
refactor(layout): pre-compute SDT container keys in resolved layout#2812tupizz wants to merge 1 commit intotadeu/sd-2563-02-fragment-metadatafrom
Conversation
There was a problem hiding this comment.
💡 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".
| const containerKeys: (string | null)[] = resolvedItems | ||
| ? resolvedItems.map((item) => { | ||
| if ('sdtContainerKey' in item) { | ||
| const key = (item as { sdtContainerKey?: string | null }).sdtContainerKey; | ||
| return key ?? null; |
There was a problem hiding this comment.
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 👍 / 👎.
Summary
sdtContainerKeyto all resolved item typessdtContainerKey.tshelper in layout-resolved (duplicates pure key derivation logic)computeSdtBoundaries()in painter prefers resolved keys with legacy fallbackPR Stack (SD-2563: Dumb Painter Refactor)
Test plan