From 605a6ba9f33d4533699b24a5a0b9882e3832e570 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 14:05:37 -0700 Subject: [PATCH] refactor(builder): consolidate buildNodeTree onto shared flattener; delete dead serializeTreeForCraft MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildNodeTree now delegates its structural walk (ColumnLayout linkedNodes, SHELL_INNER wrapping, style:[]->{} normalization) to flattenTreeForCraft, materializing each flat node into a real Craft.js Node via query.parseFreshNode — except the synthetic SHELL_INNER wrapper, which is still hand-built (parseFreshNode would merge in Container's default craft.props and change its look). sanitizeAiTree now lives in craft-tree.ts and is re-exported here for existing callers. Delete the dead serializeTreeForCraft (only its own tests called it) and its now-orphaned tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/utils/apply-ai-response.test.ts | 60 +----- craft/src/utils/apply-ai-response.ts | 218 +++------------------- 2 files changed, 26 insertions(+), 252 deletions(-) diff --git a/craft/src/utils/apply-ai-response.test.ts b/craft/src/utils/apply-ai-response.test.ts index 13bc8e7..2853a62 100644 --- a/craft/src/utils/apply-ai-response.test.ts +++ b/craft/src/utils/apply-ai-response.test.ts @@ -1,63 +1,5 @@ import { describe, test, expect, vi } from 'vitest'; -import { serializeTreeForCraft, sanitizeAiTree, __test } from './apply-ai-response'; - -describe('serializeTreeForCraft', () => { - test('flattens nested tree', () => { - const tree = { - type: { resolvedName: 'Section' }, - props: { aiName: 'Hero', node_id: 'ai-hero-1' }, - nodes: [ - { - type: { resolvedName: 'Heading' }, - props: { aiName: 'Title', node_id: 'ai-h-1', text: 'Welcome' }, - nodes: [], - }, - ], - }; - const out = serializeTreeForCraft(tree); - expect(out.rootNodeId).toBe('ai-hero-1'); - expect((out.nodes['ai-hero-1'] as any).nodes).toEqual(['ai-h-1']); - expect((out.nodes['ai-h-1'] as any).parent).toBe('ROOT'); - }); - - test('auto-generates ids when node_id is missing', () => { - const tree = { type: { resolvedName: 'Heading' }, props: {}, nodes: [] }; - const out = serializeTreeForCraft(tree); - expect(typeof out.rootNodeId).toBe('string'); - expect(out.nodes[out.rootNodeId]).toBeDefined(); - }); - - test('sets isCanvas true for layout components', () => { - const tree = { - type: { resolvedName: 'Container' }, - props: { node_id: 'c1' }, - nodes: [], - }; - const out = serializeTreeForCraft(tree); - expect((out.nodes['ROOT'] as any).isCanvas).toBe(true); - }); - - test('sets isCanvas false for leaf components', () => { - const tree = { - type: { resolvedName: 'Heading' }, - props: { node_id: 'h1' }, - nodes: [], - }; - const out = serializeTreeForCraft(tree); - expect((out.nodes['ROOT'] as any).isCanvas).toBe(false); - }); - - test('aliases root node to ROOT key', () => { - const tree = { - type: { resolvedName: 'Section' }, - props: { node_id: 'ai-section-1' }, - nodes: [], - }; - const out = serializeTreeForCraft(tree); - expect(out.nodes['ROOT']).toBeDefined(); - expect((out.nodes['ROOT'] as any).parent).toBeNull(); - }); -}); +import { sanitizeAiTree, __test } from './apply-ai-response'; describe('findNodeIdByAiNodeId', () => { const query = { diff --git a/craft/src/utils/apply-ai-response.ts b/craft/src/utils/apply-ai-response.ts index 1a2894f..cfcc8cf 100644 --- a/craft/src/utils/apply-ai-response.ts +++ b/craft/src/utils/apply-ai-response.ts @@ -2,20 +2,13 @@ import { useEditor } from '@craftjs/core'; import type { NodeTree } from '@craftjs/core'; import { usePages } from '../state/PageContext'; import { SitesmithResponse, SerializedTreeNode } from '../types/sitesmith'; -import { componentResolver } from '../components/resolver'; +import { sanitizeAiTree, flattenTreeForCraft } from './craft-tree'; -/** Only Container is a "real" Craft.js canvas in serialized state. Layout - * shells (Section/HeroSimple/ColumnLayout/etc) use linkedNodes - * internally — their own node must serialize with isCanvas:false or - * toNodeTree's walker hits an Invariant because the shell claims to be a - * canvas but its render ignores `data.nodes`. */ -const CANVAS_TYPES = new Set(['Container']); - -const SHELL_INNER: Record = { - Section: 'section-inner', - BackgroundSection: 'bg-section-inner', - FormContainer: 'form-inner', -}; +// Re-exported for existing callers/tests that import sanitizeAiTree from +// this module — the implementation now lives in craft-tree.ts alongside the +// shared flattener, since treeToState (PageContext.tsx) needs the same +// validation this module pioneered. +export { sanitizeAiTree }; /** * update_props may never touch these keys, regardless of what the AI sends. @@ -25,119 +18,6 @@ const SHELL_INNER: Record = { */ const PROTECTED_UPDATE_PROPS_KEYS = new Set(['node_id']); -/** True if `name` is a registered component in the resolver (i.e. safe to - * hand to `actions.addNodeTree` without it throwing "does not exist in - * resolver" at render time). */ -function isKnownResolvedName(name: unknown): name is string { - return typeof name === 'string' && Object.prototype.hasOwnProperty.call(componentResolver, name); -} - -/** - * Validate + sanitize an AI-supplied tree before it is handed to - * `buildNodeTree`/`query.parseFreshNode`. Fails SOFT — never throws: - * - * - **resolvedName allowlist**: any node whose `type.resolvedName` is not a - * key of `componentResolver` is dropped (it and its subtree), with a - * `console.warn`. If the tree's ROOT is itself invalid, this returns - * `null` and the caller must skip the whole op/tree. - * - **id policy — REGENERATE (not skip)**: a node id that is `'ROOT'`, - * empty/non-string, or collides with an id already in `usedIds` is - * replaced with a fresh `ai-auto-N` id (and a warning is logged). The - * node itself is kept — only structurally invalid *types* are dropped; - * a merely-bad *id* is repaired so we never lose otherwise-valid AI - * content over an id collision. - * - * `usedIds` should be seeded with the live document's existing node ids - * (see `buildNodeTree`) so AI-supplied ids can never collide with content - * already on the canvas; it is also used internally to keep ids unique - * within the tree being sanitized. - */ -export function sanitizeAiTree( - tree: SerializedTreeNode, - usedIds: Set = new Set(), - idCounter: { n: number } = { n: 0 }, -): SerializedTreeNode | null { - const resolvedName = tree?.type?.resolvedName; - if (!isKnownResolvedName(resolvedName)) { - console.warn(`sitesmith: dropping node with unknown component "${String(resolvedName)}"`); - return null; - } - - const rawId = tree.props?.node_id; - let id = typeof rawId === 'string' ? rawId.trim() : ''; - if (!id) { - id = `ai-auto-${idCounter.n++}`; - } else if (id === 'ROOT') { - console.warn('sitesmith: rejecting AI-supplied node id "ROOT" (reserved); regenerating'); - id = `ai-auto-${idCounter.n++}`; - } else if (usedIds.has(id)) { - console.warn(`sitesmith: AI-supplied node id "${id}" collides with an existing id; regenerating`); - id = `ai-auto-${idCounter.n++}`; - } - // Belt-and-suspenders: keep drawing fresh ids in the (pathological) case - // the freshly generated id itself collides. - while (usedIds.has(id)) id = `ai-auto-${idCounter.n++}`; - usedIds.add(id); - - const children: SerializedTreeNode[] = []; - for (const child of tree.nodes ?? []) { - const sanitizedChild = sanitizeAiTree(child, usedIds, idCounter); - if (sanitizedChild) children.push(sanitizedChild); - } - - return { - type: tree.type, - props: { ...tree.props, node_id: id }, - nodes: children, - }; -} - -/** - * Flatten a SerializedTreeNode tree into a Craft.js node map ready for - * `actions.deserialize()`. - * - * Returns `{ rootNodeId, nodes }` where `nodes` is a flat map keyed by node id. - * The root entry is also aliased under 'ROOT' so Craft.js can find it when - * calling `actions.deserialize(JSON.stringify(nodes))`. - */ -export function serializeTreeForCraft(tree: SerializedTreeNode): { rootNodeId: string; nodes: Record } { - const idCounter = { n: 0 }; - const nodes: Record = {}; - - const walk = (node: SerializedTreeNode, parent: string | null): string => { - const id = (node.props.node_id as string | undefined) || `ai-auto-${idCounter.n++}`; - nodes[id] = { - type: node.type, - props: node.props, - displayName: node.type.resolvedName, - isCanvas: CANVAS_TYPES.has(node.type.resolvedName), - parent, - nodes: [] as string[], - hidden: false, - custom: {}, - linkedNodes: {}, - }; - for (const child of node.nodes ?? []) { - const childId = walk(child, id); - nodes[id].nodes.push(childId); - } - return id; - }; - - const rootId = walk(tree, null); - - // Craft.js frame expects a 'ROOT' key; alias it if the AI gave a different id - if (rootId !== 'ROOT') { - nodes['ROOT'] = { ...nodes[rootId], parent: null }; - // Fix children's parent reference to 'ROOT' - for (const childId of nodes['ROOT'].nodes as string[]) { - if (nodes[childId]) nodes[childId].parent = 'ROOT'; - } - } - - return { rootNodeId: rootId, nodes }; -} - /** * Build a Craft.js `NodeTree` from a `SerializedTreeNode` using `query.parseFreshNode`. * This is the correct way to construct a tree for `actions.addNodeTree()` when @@ -157,79 +37,31 @@ function buildNodeTree(query: any, tree: SerializedTreeNode): NodeTree { throw new Error('sitesmith: AI tree root has an unknown/invalid component type; nothing to build'); } - const craftNodes: Record = {}; + // sanitizeAiTree() has already guaranteed every node has a valid, unique, + // non-'ROOT' node_id and a registered resolvedName — the shared flattener + // just needs to do the structural walk (linkedNodes/SHELL_INNER/style + // normalization) common to every tree-flattening caller in this codebase. + const flat = flattenTreeForCraft(sanitized); - const walk = (node: SerializedTreeNode, parent: string | null): string => { - // sanitizeAiTree() has already guaranteed every node here has a valid, - // unique, non-'ROOT' node_id — no fallback/collision handling needed. - const id = node.props.node_id as string; - const craftNode = (query.parseFreshNode({ - id, - data: { - type: node.type, - props: node.props, - displayName: node.type.resolvedName, - isCanvas: CANVAS_TYPES.has(node.type.resolvedName), - parent, - nodes: [], - linkedNodes: {}, - hidden: false, - custom: {}, - }, - }) as any).toNode() as any; - craftNodes[id] = craftNode; - for (const child of node.nodes ?? []) { - const childId = walk(child, id); - craftNodes[id].data.nodes.push(childId); - } - // ColumnLayout uses linkedNodes (col-0, col-1, ...) — not direct children. - if (node.type.resolvedName === 'ColumnLayout' && craftNodes[id].data.nodes.length > 0) { - const linked: Record = {}; - craftNodes[id].data.nodes.forEach((cid: string, i: number) => { - linked[`col-${i}`] = cid; - if (craftNodes[cid]) craftNodes[cid].data.isCanvas = true; - }); - craftNodes[id].data.nodes = []; - craftNodes[id].data.linkedNodes = linked; - const colCount = Object.keys(linked).length; - if (!craftNodes[id].data.props.columns || craftNodes[id].data.props.columns !== colCount) { - craftNodes[id].data.props.columns = colCount; - } - } - // Section/BackgroundSection/FormContainer wrap their content in a single - // . Pre-create the linkedNode - // so Craft.js doesn't auto-create one with a malformed type field. - const innerKey = SHELL_INNER[node.type.resolvedName]; - if (innerKey && craftNodes[id].data.nodes.length > 0) { - const innerId = `${id}__${innerKey}`; - const childIds: string[] = [...craftNodes[id].data.nodes]; - craftNodes[innerId] = { - id: innerId, - data: { - type: { resolvedName: 'Container' }, - props: { tag: 'div' }, - displayName: 'Container', - isCanvas: true, - parent: id, - nodes: childIds, - linkedNodes: {}, - hidden: false, - custom: {}, - }, + const craftNodes: Record = {}; + for (const [id, flatNode] of Object.entries(flat.nodes)) { + if (flat.syntheticIds.has(id)) { + // The SHELL_INNER wrapper Container is synthesized by the flattener, + // not present in the AI's tree — build it directly rather than + // through query.parseFreshNode(), which would merge in Container's + // default `craft.props` (e.g. default padding) and change its look. + craftNodes[id] = { + id, + data: { ...flatNode }, events: { selected: false, hovered: false, dragged: false }, rules: { canDrag: () => true, canMoveIn: () => true, canMoveOut: () => true, canDrop: () => true }, }; - for (const cid of childIds) { - if (craftNodes[cid]) craftNodes[cid].data.parent = innerId; - } - craftNodes[id].data.nodes = []; - craftNodes[id].data.linkedNodes = { [innerKey]: innerId }; + } else { + craftNodes[id] = (query.parseFreshNode({ id, data: { ...flatNode } }) as any).toNode(); } - return id; - }; + } - const rootId = walk(sanitized, null); - return { rootNodeId: rootId, nodes: craftNodes }; + return { rootNodeId: flat.rootNodeId, nodes: craftNodes }; } /**