fix: draw a placeholder for missing or cyclic composite elements

A composite whose definition is missing (connection disabled or element removed)
now draws a 'Missing Element' placeholder instead of nothing. Track the composite
types being expanded, so a composite that contains itself draws an '∞'
placeholder instead of recursing forever.

Author: Julusian

companion/lib/Graphics/ConvertGraphicsElements.ts

@@ -134,7 +134,8 @@ export async function ConvertSomeButtonGraphicsElementForDrawing(
 		globalReferences,
 		processedElementIds,
 		currentLocationStr,
-		getRenderAtLocation
+		getRenderAtLocation,
+		new Set()
 	)
 
 	const newElements = await convertElements(context, elements, '')
@@ -224,8 +225,11 @@ async function convertElements(
 			if (cacheEntry.referencedLocation)
 				context.globalReferences.referencedLocations.drawn.add(cacheEntry.referencedLocation)
 
-			// If this is a group, populate the children
-			if (element.type === 'group' || element.type === 'composite') {
+			if (cacheEntry.compositeElement?.isPlaceholder) {
+				// Placeholder already carries its children
+				return cacheEntry.drawElement
+			} else if (element.type === 'group' || element.type === 'composite') {
+				// If this is a group, populate the children
 				const childIdPrefix = cacheEntry.compositeElement ? cacheEntry.compositeElement.childIdPrefix : idPrefix
 
 				const children =
@@ -234,8 +238,11 @@ async function convertElements(
 						: (context.resolveCompositeElement(element.connectionId, element.elementId)?.elements ?? [])
 
 				if (cacheEntry.drawElement?.type === 'group') {
-					const childContext = cacheEntry.compositeElement?.childPropOverrides
-						? context.withPropOverrides(cacheEntry.compositeElement.childPropOverrides)
+					const childContext = cacheEntry.compositeElement
+						? context.withCompositeChild(
+								cacheEntry.compositeElement.elementId,
+								cacheEntry.compositeElement.childPropOverrides
+							)
 						: context
 
 					// Recurse to process children
@@ -577,64 +584,54 @@ async function convertCompositeElementForDrawing(
 	// The full ID of this composite element (with any parent prefixes)
 	const compositeFullId = idPrefix + element.id
 
+	// Perform enabled check first, to avoid executing expressions when not needed
+	const enabled = helper.getBoolean('enabled', true)
+
 	const childElement = context.resolveCompositeElement(element.connectionId, element.elementId)
-	if (!childElement) {
-		return {
-			drawElement: null,
-			references,
-			compositeElement: {
-				elementId: compositeElementId,
-				childIdPrefix: compositeFullId + '/',
-				childPropOverrides: {},
-			},
-		}
+	const placeholderText = !childElement
+		? 'Missing\nElement'
+		: context.compositeAncestors.has(compositeElementId)
+			? '\u221e'
+			: null
+
+	let propOverrides: VariableValues = {}
+	let childIdPrefix = compositeFullId + '/'
+	if (childElement && placeholderText === null) {
+		propOverrides = parseCompositeElementChildOptions(helper, childElement, element)
+		const propOverridesHash = createHash('sha256').update(JSON.stringify(propOverrides)).digest('hex')
+		childIdPrefix = compositeFullId + '-' + propOverridesHash + '/'
 	}
 
-	const propOverrides = parseCompositeElementChildOptions(helper, childElement, element)
-	const propOverridesHash = createHash('sha256').update(JSON.stringify(propOverrides)).digest('hex')
-
-	const childIdPrefix = compositeFullId + '-' + propOverridesHash + '/'
-
-	// Perform enabled check first, to avoid executing expressions when not needed
-	const enabled = helper.getBoolean('enabled', true)
-	if (!enabled && context.onlyEnabled)
-		return {
-			drawElement: null,
-			references,
-			compositeElement: {
-				elementId: compositeElementId,
-				childIdPrefix,
-				childPropOverrides: propOverrides,
-			},
-		}
+	const compositeElement: ElementConversionCacheEntry['compositeElement'] = {
+		elementId: compositeElementId,
+		childIdPrefix,
+		childPropOverrides: propOverrides,
+		isPlaceholder: placeholderText !== null,
+	}
 
-	const opacity = helper.getNumber('opacity', 1, 0.01)
-	const bounds = convertDrawBounds(helper)
+	if (!enabled && context.onlyEnabled) return { drawElement: null, references, compositeElement }
 
 	// Note: Composite elements render as groups - hash is shallow (children have their own hashes)
 	const drawElement: ButtonGraphicsGroupDrawElement = {
 		id: compositeFullId,
 		type: 'group',
 		usage: element.usage,
 		enabled,
-		opacity,
-		...bounds,
+		opacity: helper.getNumber('opacity', 1, 0.01),
+		...convertDrawBounds(helper),
 		rotation: helper.getNumber('rotation', 0),
 		squareCoords: false,
-		children: [], // Will be filled in by caller
+		children: [], // Will be filled in by caller, unless a placeholder
 		contentHash: '', // Will be computed below
 	}
 
 	drawElement.contentHash = computeElementContentHash(drawElement)
-	return {
-		drawElement,
-		references,
-		compositeElement: {
-			elementId: compositeElementId,
-			childIdPrefix,
-			childPropOverrides: propOverrides,
-		},
+
+	if (placeholderText !== null) {
+		drawElement.children = makeReferencePlaceholder(compositeFullId, placeholderText)
 	}
+
+	return { drawElement, references, compositeElement }
 }
 
 async function convertImageElementForDrawing(
@@ -930,7 +927,7 @@ function collectChildElementIds(
 		// If this child is a composite, we need to check if we have a cached entry
 		// to determine the correct childIdPrefix for its children
 		if (child.type === 'composite') {
-			if (cachedEntry?.compositeElement) {
+			if (cachedEntry?.compositeElement && !cachedEntry.compositeElement.isPlaceholder) {
 				const compositeChildren = context.resolveCompositeElement(child.connectionId, child.elementId)?.elements
 				if (compositeChildren) {
 					collectChildElementIds(context, compositeChildren, cachedEntry.compositeElement.childIdPrefix)

companion/lib/Graphics/ConvertGraphicsElements/Helper.ts

@@ -520,11 +520,14 @@ export interface ParseElementsContext {
 	/** Callback to fetch the last-rendered ImageResult for a given location */
 	readonly getRenderAtLocation: ((location: ControlLocation) => ImageResult | null) | null
 
+	/** Composite element types currently being expanded above this point, used for cycle detection */
+	readonly compositeAncestors: ReadonlySet<CompositeElementIdString>
+
 	/**
-	 * Create a child factory for recursive conversion (e.g., inside composite elements)
-	 * with prop overrides injected into the parser
+	 * Create a child factory for converting the children of a composite element,
+	 * with prop overrides injected into the parser and the composite added to the ancestors
 	 */
-	withPropOverrides(propOverrides: VariableValues): ParseElementsContext
+	withCompositeChild(compositeElementId: CompositeElementIdString, propOverrides: VariableValues): ParseElementsContext
 
 	/**
 	 * Resolve a composite element definition by connection and element ID
@@ -548,7 +551,8 @@ export function createParseElementsContext(
 	globalReferences: ExpressionReferences,
 	processedElementIds: Set<string>,
 	currentLocationStr: string | null,
-	getRenderAtLocation: ((location: ControlLocation) => ImageResult | null) | null
+	getRenderAtLocation: ((location: ControlLocation) => ImageResult | null) | null,
+	compositeAncestors: ReadonlySet<CompositeElementIdString>
 ): ParseElementsContext {
 	return {
 		cache,
@@ -558,6 +562,7 @@ export function createParseElementsContext(
 		drawPixelBuffers,
 		currentLocationStr,
 		getRenderAtLocation,
+		compositeAncestors,
 
 		createHelper<T extends { readonly id: string }>(
 			element: T
@@ -568,7 +573,10 @@ export function createParseElementsContext(
 			return { helper, references }
 		},
 
-		withPropOverrides(propOverrides: VariableValues): ParseElementsContext {
+		withCompositeChild(
+			compositeElementId: CompositeElementIdString,
+			propOverrides: VariableValues
+		): ParseElementsContext {
 			return createParseElementsContext(
 				compositeElementStore,
 				parser.createIsolatedChildParser(propOverrides),
@@ -579,7 +587,8 @@ export function createParseElementsContext(
 				globalReferences,
 				processedElementIds,
 				currentLocationStr,
-				getRenderAtLocation
+				getRenderAtLocation,
+				new Set([...compositeAncestors, compositeElementId])
 			)
 		},
 

companion/lib/Graphics/ElementConversionCache.ts

@@ -21,6 +21,8 @@ export interface ElementConversionCacheEntry {
 		readonly childPropOverrides: VariableValues
 		/** Prefix applied to child element IDs */
 		readonly childIdPrefix: string
+		/** Whether the draw element is a placeholder (missing definition or cycle), with no children to convert */
+		readonly isPlaceholder: boolean
 	} | null
 
 	/**

companion/test/Graphics/ConvertGraphicsElements.test.ts

@@ -1687,56 +1687,6 @@ describe('ConvertSomeButtonGraphicsElementForDrawing', () => {
 			})
 		})
 
-		test('returns nothing for missing composite definition', async () => {
-			const compositeDefinition: CompositeElementDefinition = {
-				id: 'composite1',
-				name: 'Test Composite',
-				sortKey: null,
-				description: '',
-				options: [],
-				elements: [],
-			}
-
-			const instanceDefs = createMockInstanceDefinitions({
-				'test-connection': {
-					composite1: compositeDefinition,
-				},
-			})
-
-			const elements: SomeButtonGraphicsElement[] = [
-				{
-					id: 'comp1',
-					name: '',
-					type: 'composite',
-					rotation: val(0),
-					pinnedProperties: [],
-					usage: USAGE,
-					enabled: val(true),
-					opacity: val(100),
-					x: val(0),
-					y: val(0),
-					width: val(100),
-					height: val(100),
-					connectionId: 'test-connection',
-					elementId: 'nonexistent',
-				},
-			]
-
-			const result = await ConvertSomeButtonGraphicsElementForDrawing(
-				instanceDefs,
-				createMockParser(),
-				mockDrawPixelBuffers,
-				elements,
-				new Map(),
-				true,
-				null,
-				null,
-				null
-			)
-
-			expect(result.elements).toHaveLength(0)
-		})
-
 		function makeCompositeEl(elementId: string, overrides: Partial<ButtonGraphicsCompositeElement> = {}) {
 			return {
 				id: 'comp1',
@@ -1779,6 +1729,12 @@ describe('ConvertSomeButtonGraphicsElementForDrawing', () => {
 			)
 		}
 
+		function placeholderText(element: SomeButtonGraphicsDrawElement | undefined): string | undefined {
+			const group = element as ButtonGraphicsGroupDrawElement
+			const text = group.children.find((c) => c.type === 'text')
+			return text?.text
+		}
+
 		test('applies rotation to the group', async () => {
 			const instanceDefs = createMockInstanceDefinitions({
 				'test-connection': { composite1: makeDefinition('composite1', [makeTextEl({ id: 'child1' })]) },
@@ -1799,6 +1755,61 @@ describe('ConvertSomeButtonGraphicsElementForDrawing', () => {
 
 			expect(result.elements[0]).toMatchObject({ type: 'group', rotation: 0 })
 		})
+
+		test('draws a placeholder for a missing composite definition', async () => {
+			const instanceDefs = createMockInstanceDefinitions({
+				'test-connection': { composite1: makeDefinition('composite1', []) },
+			})
+
+			const result = await convert(instanceDefs, [makeCompositeEl('nonexistent')], null)
+
+			expect(result.elements).toHaveLength(1)
+			expect(result.elements[0].type).toBe('group')
+			expect(placeholderText(result.elements[0])).toBe('Missing\nElement')
+			expect([...result.compositeElements.drawn]).toEqual(['test-connection:nonexistent'])
+		})
+
+		test('draws nothing for a disabled composite with a missing definition', async () => {
+			const instanceDefs = createMockInstanceDefinitions({})
+
+			const result = await convert(instanceDefs, [makeCompositeEl('nonexistent', { enabled: val(false) })], null)
+
+			expect(result.elements).toHaveLength(0)
+		})
+
+		test('draws a placeholder for a composite that contains itself', async () => {
+			const instanceDefs = createMockInstanceDefinitions({
+				'test-connection': {
+					loop: makeDefinition('loop', [makeTextEl({ id: 'text' }), makeCompositeEl('loop', { id: 'self' })]),
+				},
+			})
+
+			const result = await convert(instanceDefs, [makeCompositeEl('loop')], null)
+
+			const outer = result.elements[0] as ButtonGraphicsGroupDrawElement
+			expect(outer.children).toHaveLength(2)
+			expect(outer.children[0].type).toBe('text')
+			expect(placeholderText(outer.children[1])).toBe('\u221e')
+		})
+
+		test('draws a placeholder for an indirect composite cycle', async () => {
+			const instanceDefs = createMockInstanceDefinitions({
+				'test-connection': {
+					a: makeDefinition('a', [makeCompositeEl('b', { id: 'to-b' })]),
+					b: makeDefinition('b', [makeCompositeEl('a', { id: 'to-a' })]),
+				},
+			})
+
+			const cache = new ElementConversionCache()
+			for (let i = 0; i < 2; i++) {
+				// Second pass is served from the cache
+				const result = await convert(instanceDefs, [makeCompositeEl('a')], cache)
+
+				const a = result.elements[0] as ButtonGraphicsGroupDrawElement
+				const b = a.children[0] as ButtonGraphicsGroupDrawElement
+				expect(placeholderText(b.children[0])).toBe('\u221e')
+			}
+		})
 	})
 
 	describe('id prefixing', () => {

companion/test/Graphics/ConvertGraphicsElements/Helper.test.ts

@@ -151,7 +151,8 @@ function makeCtx(
 		makeGlobalRefs(),
 		new Set(),
 		null,
-		null
+		null,
+		new Set()
 	)
 }
 
@@ -1195,25 +1196,34 @@ describe('createParseElementsContext', () => {
 		})
 	})
 
-	describe('withPropOverrides', () => {
+	describe('withCompositeChild', () => {
 		test('variables injected via propOverrides are resolved in the child context', () => {
 			const ctx = makeCtx()
-			const child = ctx.withPropOverrides({ 'ns:x': 'injected' })
+			const child = ctx.withCompositeChild('conn:elem', { 'ns:x': 'injected' })
 			const { helper } = child.createHelper({ id: 'el1', strProp: expr<string>('$(ns:x)') })
 			expect(helper.getString('strProp', '')).toBe('injected')
 		})
 
 		test('injected propOverrides do not affect the parent context', () => {
 			const ctx = makeCtx()
-			ctx.withPropOverrides({ 'ns:x': 'injected' })
+			ctx.withCompositeChild('conn:elem', { 'ns:x': 'injected' })
 			// Parent parser has no ns:x; missing variable resolves to undefined → defaultValue
 			const { helper } = ctx.createHelper({ id: 'el1', strProp: expr<string>('$(ns:x)') })
 			expect(helper.getString('strProp', 'default')).toBe('default')
 		})
 
+		test('adds the composite to the ancestors of the child context only', () => {
+			const ctx = makeCtx()
+			const child = ctx.withCompositeChild('conn:elem', {})
+			const grandchild = child.withCompositeChild('conn:other', {})
+			expect(ctx.compositeAncestors).toEqual(new Set())
+			expect(child.compositeAncestors).toEqual(new Set(['conn:elem']))
+			expect(grandchild.compositeAncestors).toEqual(new Set(['conn:elem', 'conn:other']))
+		})
+
 		test('child context is isolated from parent global variable values', () => {
 			const ctx = makeCtx({ variableValues: { ns: { existing: 'base' } } })
-			const child = ctx.withPropOverrides({ 'ns:injected': 'extra' })
+			const child = ctx.withCompositeChild('conn:elem', { 'ns:injected': 'extra' })
 			// The child can only see its own propOverrides, not the parent's global variables
 			const { helper } = child.createHelper({ id: 'el1', strProp: expr<string>('$(ns:existing)') })
 			expect(helper.getString('strProp', 'default')).toBe('default')