refactor(fig): clarify imported instance reconciliation

packages/core/src/clipboard.ts

@@ -4,6 +4,7 @@ import { populateAndApplyOverrides } from '@open-pencil/fig/instance-overrides'
 import type { InstanceNodeChange } from '@open-pencil/fig/instance-overrides'
 import {
   nodeChangeToProps,
+  linkImportedInstanceChildren,
   shouldImportTextAsAutoSize,
   sortChildren
 } from '@open-pencil/fig/node-change'
@@ -12,8 +13,6 @@ import type { GUID, NodeChange as KiwiNodeChange } from '@open-pencil/kiwi/fig/c
 import { decodeBinarySchema, compileSchema, ByteBuffer } from '@open-pencil/kiwi/schema-runtime'
 import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph'
 
-import { linkImportedInstanceChildren } from '#core/kiwi/fig/import-linkage'
-
 import { decodeBase64, decodeBase64Text, encodeBase64, encodeBase64Text } from './bytes'
 import { shapeTextForClipboard } from './canvas/text/clipboard'
 import {

packages/core/src/kiwi/fig/import.ts

@@ -8,6 +8,7 @@ import {
   getOpenPencilPluginValue,
   guidToString,
   importCanvasGuides,
+  linkImportedInstanceChildren,
   nodeChangeToProps,
   shouldImportTextAsAutoSize,
   sortChildren,
@@ -23,7 +24,6 @@ import type {
 } from '@open-pencil/scene-graph'
 
 import { BLACK } from '#core/constants'
-import { linkImportedInstanceChildren } from '#core/kiwi/fig/import-linkage'
 import { setLazyFigImportContext } from '#core/kiwi/fig/lazy-import'
 
 type AssetRef = { key: string; version?: string }

packages/fig/src/node-change/index.ts

@@ -7,7 +7,7 @@ export * from './export-node'
 export * from './export-runtime'
 export * from './font/features'
 export * from './font/style'
-export * from './font/variations'
+export * from './instance-linkage'
 export * from './paint'
 export * from './path/commands'
 export * from './plugin-data'

packages/fig/src/node-change/instance-linkage/index.ts

@@ -67,11 +67,15 @@ function linkSubtree(
   // without stable keys, and for legacy files without overrideKey).
   const remainingComp = compChildren.filter((c) => !linkedComp.has(c.id))
   const remainingInst = instChildren.filter((c) => !linkedInst.has(c.id))
-  const count = Math.min(remainingComp.length, remainingInst.length)
-  for (let i = 0; i < count; i++) {
+  // Positional evidence is safe only when every remaining slot has a matching type.
+  // Otherwise preserve all unmatched instance children and let sync clone missing ones.
+  const compatible =
+    remainingComp.length === remainingInst.length &&
+    remainingComp.every((child, index) => child.type === remainingInst[index]?.type)
+  if (!compatible) return
+  for (let i = 0; i < remainingComp.length; i++) {
     const compChild = remainingComp[i]
     const instChild = remainingInst[i]
-    if (compChild.type !== instChild.type) continue
     linkPair(compChild, instChild)
   }
 }

packages/scene-graph/src/instances.ts

@@ -150,26 +150,30 @@ function matchFallbackChildren(
   instChildMap: Map<string, SceneNode>,
   usedInstChildIds: Set<string>
 ): void {
-  const unmatchedCompChildIds = compParent.childIds.filter((id) => !instChildMap.has(id))
-  const unmatchedInstChildren = instParent.childIds
-    .map((id) => graph.nodes.get(id))
-    .filter((n): n is SceneNode => n !== undefined && !usedInstChildIds.has(n.id))
+  const fallbackByType = new Map<SceneNode['type'], Map<string, SceneNode[]>>()
+  for (const childId of instParent.childIds) {
+    const child = graph.nodes.get(childId)
+    if (!child || usedInstChildIds.has(child.id)) continue
+    let byName = fallbackByType.get(child.type)
+    if (!byName) {
+      byName = new Map()
+      fallbackByType.set(child.type, byName)
+    }
+    const queue = byName.get(child.name)
+    if (queue) queue.push(child)
+    else byName.set(child.name, [child])
+  }
 
-  if (unmatchedCompChildIds.length === 0 || unmatchedInstChildren.length === 0) return
-
-  // Match by name and type with forward (FIFO) iteration to preserve sibling order.
-  for (const compChildId of unmatchedCompChildIds) {
+  // Match by name and type with FIFO queues to preserve sibling order in linear time.
+  for (const compChildId of compParent.childIds) {
+    if (instChildMap.has(compChildId)) continue
     const compChild = graph.nodes.get(compChildId)
     if (!compChild) continue
-    const matchIdx = unmatchedInstChildren.findIndex(
-      (instChild) => instChild.type === compChild.type && instChild.name === compChild.name
-    )
-    if (matchIdx !== -1) {
-      const [instChild] = unmatchedInstChildren.splice(matchIdx, 1)
-      instChildMap.set(compChildId, instChild)
-      usedInstChildIds.add(instChild.id)
-      linkMatchedChild(overrides, instParentId, instChild, compChildId)
-    }
+    const match = fallbackByType.get(compChild.type)?.get(compChild.name)?.shift()
+    if (!match) continue
+    instChildMap.set(compChildId, match)
+    usedInstChildIds.add(match.id)
+    linkMatchedChild(overrides, instParentId, match, compChildId)
   }
 }
 
@@ -183,31 +187,18 @@ function sortInstanceChildren(
   const orderMap = new Map<string, number>()
   for (let i = 0; i < compChildOrder.length; i++) orderMap.set(compChildOrder[i], i)
 
-  // Children whose resolved mapping (sourceComponentId override, else componentId)
-  // is one of this component's children sort in component order; children with no
-  // such mapping (e.g. imported extras or user-added nodes inside the instance)
-  // sort to the END rather than the front, keeping their relative order among
-  // themselves. This keeps component children in canonical order instead of yanking
-  // unmapped extras to the front on every sync.
-  instParent.childIds.sort((a, b) => {
-    const nodeA = graph.nodes.get(a)
-    const nodeB = graph.nodes.get(b)
-    const sourceA = nodeA
-      ? getInstanceOverride(overrides, instParentId, nodeA.id, 'sourceComponentId')
+  const ranks = new Map<string, number>()
+  for (let index = 0; index < instParent.childIds.length; index++) {
+    const childId = instParent.childIds[index]
+    const node = graph.nodes.get(childId)
+    const source = node
+      ? getInstanceOverride(overrides, instParentId, node.id, 'sourceComponentId')
       : undefined
-    const sourceB = nodeB
-      ? getInstanceOverride(overrides, instParentId, nodeB.id, 'sourceComponentId')
-      : undefined
-
-    const mappedA = typeof sourceA === 'string' ? sourceA : nodeA?.componentId
-    const mappedB = typeof sourceB === 'string' ? sourceB : nodeB?.componentId
-    const idxA = mappedA ? (orderMap.get(mappedA) ?? -1) : -1
-    const idxB = mappedB ? (orderMap.get(mappedB) ?? -1) : -1
-    if (idxA === -1 && idxB === -1) return 0
-    if (idxA === -1) return 1
-    if (idxB === -1) return -1
-    return idxA - idxB
-  })
+    const mapped = typeof source === 'string' ? source : node?.componentId
+    const componentIndex = mapped ? orderMap.get(mapped) : undefined
+    ranks.set(childId, componentIndex ?? compChildOrder.length + index)
+  }
+  instParent.childIds.sort((left, right) => (ranks.get(left) ?? 0) - (ranks.get(right) ?? 0))
 }
 
 /** True when syncing `compParentId` into `instParentId` would form a cycle. */

tests/engine/editor/clipboard/import-nodes/instance-linkage.test.ts

@@ -1,7 +1,9 @@
 import { describe, expect, it } from 'bun:test'
 
-import { importClipboardNodes, SceneGraph } from '@open-pencil/core'
+import { importClipboardNodes } from '@open-pencil/core'
 import type { NodeChange } from '@open-pencil/core'
+import { linkImportedInstanceChildren } from '@open-pencil/fig/node-change'
+import { SceneGraph } from '@open-pencil/scene-graph'
 
 import { getNodeOrThrow } from '#tests/helpers/assert'
 
@@ -85,9 +87,7 @@ describe('importClipboardNodes: instance child linkage', () => {
     expect(instChild.height).toBe(50)
   })
 
-  it('does not mis-link an extra same-type child inserted before a renamed serialized child (overrideKey)', async () => {
-    const { linkImportedInstanceChildren } = await import('#core/kiwi/fig/import-linkage')
-    const { SceneGraph } = await import('@open-pencil/scene-graph')
+  it('does not mis-link an extra same-type child inserted before a renamed serialized child (overrideKey)', () => {
     const graph = new SceneGraph()
     const page = graph.addPage('Test')
 
@@ -135,4 +135,25 @@ describe('importClipboardNodes: instance child linkage', () => {
     expect(inst.childIds[0]).toBe(renamed.id)
     expect(inst.childIds[1]).toBe(extra.id)
   })
+
+  it('leaves ambiguous positional children unmapped when cardinality differs', () => {
+    const graph = new SceneGraph()
+    const page = graph.addPage('Test')
+    const component = graph.createNode('COMPONENT', page.id, { name: 'Card' })
+    graph.createNode('FRAME', component.id, { name: 'Header' })
+    const instance = graph.createNode('INSTANCE', page.id, {
+      name: 'Card',
+      componentId: component.id
+    })
+    const extra = graph.createNode('FRAME', instance.id, { name: 'Badge' })
+    const serialized = graph.createNode('FRAME', instance.id, { name: 'Header v2' })
+
+    linkImportedInstanceChildren(graph, new Set([instance.id]))
+
+    expect(extra.componentId).toBeNull()
+    expect(serialized.componentId).toBeNull()
+    graph.syncInstances(component.id)
+    expect(instance.childIds).toHaveLength(3)
+    expect(instance.childIds).toEqual(expect.arrayContaining([extra.id, serialized.id]))
+  })
 })

tests/engine/scene-graph/instance-sync-duplication.test.ts

@@ -1,9 +1,8 @@
 import { describe, expect, test } from 'bun:test'
 
+import { linkImportedInstanceChildren } from '@open-pencil/fig/node-change'
 import { SceneGraph, recordInstanceOverride } from '@open-pencil/scene-graph'
 
-import { linkImportedInstanceChildren } from '#core/kiwi/fig/import-linkage'
-
 describe('instance synchronization child deduplication', () => {
   test('syncInstances on instance with imported children without componentId does not duplicate children', () => {
     const graph = new SceneGraph()