From 395070ca433ba5cca6ce5e2f9454ec1e854e5535 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Mon, 14 Sep 2026 11:07:09 +0000 Subject: [PATCH 1/2] feat(calm-hub-ui): render items catalogues in pattern diagrams A pattern may declare nodes and relationships under items as well as prefixItems. The renderer read only prefixItems, so a catalogue node never appeared and a decision naming one rendered no box at all. items is read as one more choice block. Its members get a decision group like prefixItems alternatives do, so the existing parenting and the existing options lookup handle them with no further change. The catalogue box carries the decision's prompt and choices. A decision is read from prefixItems only. Declaring one in items is rejected by validation, so the renderer does not tolerate it. Drawer's isPatternData accepts nodes.items, or a pattern that declares only a catalogue is misrouted as an architecture and never reaches the transformer. The allOf lookup and the oneOf/anyOf read were each written out four times. Both are now one function. --- .../visualizer/components/drawer/Drawer.tsx | 3 +- .../utils/patternTransformer.test.ts | 121 ++++++++++++++++ .../reactflow/utils/patternTransformer.ts | 129 +++++++++++------- 3 files changed, 204 insertions(+), 49 deletions(-) diff --git a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx index 75ad314f7..a982e337d 100644 --- a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx +++ b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx @@ -19,7 +19,8 @@ function isPatternData(data: unknown): boolean { if (!data || typeof data !== 'object') return false; const obj = data as Record; const props = obj['properties'] as Record | undefined; - return !!(props?.['nodes'] && typeof props['nodes'] === 'object' && (props['nodes'] as Record)['prefixItems']); + const nodes = props?.['nodes'] as Record | undefined; + return !!(nodes && typeof nodes === 'object' && (nodes['prefixItems'] || nodes['items'])); } /** diff --git a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts index 7e1aca850..81b064b9c 100644 --- a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts +++ b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.test.ts @@ -519,3 +519,124 @@ describe('nested container ordering', () => { expect(ids).toEqual(['A', 'B', 'C', 'system']); }); }); +describe('items catalogues', () => { + function optionsRelationship(uniqueId: string, prompt: string, nodeIds: string[]) { + return { + properties: { + 'unique-id': { const: uniqueId }, + description: { const: prompt }, + 'relationship-type': { + properties: { + options: { + prefixItems: [ + { + anyOf: nodeIds.map((id) => ({ + properties: { + description: { const: `Add ${id}` }, + nodes: { const: [id] }, + relationships: { const: [] }, + }, + })), + }, + ], + }, + }, + }, + }, + }; + } + + function withCatalogue(nodes: unknown[], catalogue: unknown[], relationships: unknown[] = []) { + return { + properties: { + nodes: { prefixItems: nodes, items: { oneOf: catalogue } }, + relationships: { prefixItems: relationships }, + }, + }; + } + + it('renders a node declared under items', () => { + const result = parsePatternData(withCatalogue([], [schemaNode('cache', 'Cache', 'service')])); + + expect(result.nodes.map((n) => n.id)).toContain('cache'); + }); + + it('groups items members into one decision box', () => { + const result = parsePatternData( + withCatalogue( + [schemaNode('webapp', 'Web', 'service')], + [schemaNode('cache', 'Cache', 'service'), schemaNode('queue', 'Queue', 'service')] + ) + ); + + const box = result.nodes.find((n) => n.type === 'decisionGroup'); + expect(box).toBeDefined(); + expect(result.nodes.filter((n) => n.parentId === box?.id).map((n) => n.id)).toEqual(['cache', 'queue']); + expect(result.nodes.find((n) => n.id === 'webapp')?.parentId).toBeUndefined(); + }); + + it('attaches a decision prompt to the items box', () => { + const result = parsePatternData( + withCatalogue( + [], + [schemaNode('cache', 'Cache', 'service')], + [optionsRelationship('add-ons', 'Optional add-ons', ['cache'])] + ) + ); + + const box = result.nodes.find((n) => n.type === 'decisionGroup'); + expect(box?.data.prompt).toBe('Optional add-ons'); + expect(box?.data.choices).toHaveLength(1); + }); + + it('reads an anyOf catalogue and labels the box', () => { + const pattern = { + properties: { + nodes: { items: { anyOf: [schemaNode('cache', 'Cache', 'service')] } }, + relationships: { prefixItems: [] }, + }, + }; + + expect(parsePatternData(pattern).nodes.find((n) => n.type === 'decisionGroup')?.data.decisionType).toBe('anyOf'); + }); + + it('keeps a prefixItems decision separate from the items box', () => { + const pattern = { + properties: { + nodes: { + prefixItems: [{ oneOf: [schemaNode('pg', 'Postgres', 'database'), schemaNode('my', 'MySQL', 'database')] }], + items: { oneOf: [schemaNode('cache', 'Cache', 'service')] }, + }, + relationships: { prefixItems: [] }, + }, + }; + + const boxes = parsePatternData(pattern).nodes.filter((n) => n.type === 'decisionGroup'); + expect(boxes).toHaveLength(2); + expect(new Set(boxes.map((b) => b.id)).size).toBe(2); + }); + + it('draws an edge for a relationship declared under items', () => { + const pattern = { + properties: { + nodes: { prefixItems: [schemaNode('a', 'A', 'service'), schemaNode('b', 'B', 'service')] }, + relationships: { items: { oneOf: [connectsRelationship('a-b', 'a', 'b')] } }, + }, + }; + + const edges = parsePatternData(pattern).edges; + expect(edges).toHaveLength(1); + expect([edges[0].source, edges[0].target]).toEqual(['a', 'b']); + }); + + it('ignores an items schema that offers no choice', () => { + const pattern = { + properties: { + nodes: { prefixItems: [schemaNode('a', 'A', 'service')], items: { $ref: 'core.json#/defs/node' } }, + relationships: { prefixItems: [] }, + }, + }; + + expect(parsePatternData(pattern).nodes.map((n) => n.id)).toEqual(['a']); + }); +}); diff --git a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts index 08160fa02..e264e1e06 100644 --- a/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts +++ b/calm-hub-ui/src/visualizer/components/reactflow/utils/patternTransformer.ts @@ -25,21 +25,43 @@ type SchemaObject = Record; // ---- Schema traversal helpers ---- /** - * Gets the prefixItems for a given top-level key (e.g. 'nodes' or 'relationships') - * from a pattern, handling allOf structures. + * A pattern may declare its nodes and relationships directly, or inside an allOf branch. */ -function getPrefixItems(pattern: SchemaObject, key: string): SchemaObject[] { - if (pattern['properties']?.[key]?.['prefixItems']) { - return pattern['properties'][key]['prefixItems']; +function findDeclarations(pattern: SchemaObject, key: string, member: string): SchemaObject | undefined { + if (pattern['properties']?.[key]?.[member]) { + return pattern['properties'][key]; } - if (pattern['allOf'] && Array.isArray(pattern['allOf'])) { + if (Array.isArray(pattern['allOf'])) { for (const schema of pattern['allOf']) { - if (schema['properties']?.[key]?.['prefixItems']) { - return schema['properties'][key]['prefixItems']; + if (schema['properties']?.[key]?.[member]) { + return schema['properties'][key]; } } } - return []; + return undefined; +} + +function getPrefixItems(pattern: SchemaObject, key: string): SchemaObject[] { + return findDeclarations(pattern, key, 'prefixItems')?.['prefixItems'] ?? []; +} + +function getItems(pattern: SchemaObject, key: string): SchemaObject | undefined { + return findDeclarations(pattern, key, 'items')?.['items']; +} + +interface Choice { + groupType: 'oneOf' | 'anyOf'; + alternatives: SchemaObject[]; +} + +function readChoice(schema: SchemaObject | undefined): Choice | undefined { + if (Array.isArray(schema?.['oneOf'])) { + return { groupType: 'oneOf', alternatives: schema['oneOf'] }; + } + if (Array.isArray(schema?.['anyOf'])) { + return { groupType: 'anyOf', alternatives: schema['anyOf'] }; + } + return undefined; } /** @@ -160,6 +182,9 @@ function extractNodeFromSchemaItem(item: SchemaObject): ExtractedNode | null { }; } +const NODE_ITEMS_GROUP = 'node-decision-items'; +const RELATIONSHIP_ITEMS_GROUP = 'rel-decision-items'; + interface DecisionGroup { groupId: string; groupType: 'oneOf' | 'anyOf'; @@ -167,40 +192,44 @@ interface DecisionGroup { } function extractNodesFromPattern(pattern: SchemaObject): { nodes: ExtractedNode[]; decisionGroups: DecisionGroup[] } { - const prefixItems = getPrefixItems(pattern, 'nodes'); const nodes: ExtractedNode[] = []; const decisionGroups: DecisionGroup[] = []; - prefixItems.forEach((item: SchemaObject, index: number) => { - const hasOneOf = Array.isArray(item['oneOf']); - const hasAnyOf = Array.isArray(item['anyOf']); + function addChoice(groupId: string, choice: Choice): void { + const groupNodeIds: string[] = []; - if (hasOneOf || hasAnyOf) { - const groupType: 'oneOf' | 'anyOf' = hasOneOf ? 'oneOf' : 'anyOf'; - const alternatives: SchemaObject[] = hasOneOf ? item['oneOf'] : item['anyOf']; - const groupId = `node-decision-${index}`; - const groupNodeIds: string[] = []; - - alternatives.forEach((alt: SchemaObject) => { - const node = extractNodeFromSchemaItem(alt); - if (node) { - node.decisionGroupId = groupId; - nodes.push(node); - groupNodeIds.push(node.uniqueId); - } - }); - - if (groupNodeIds.length > 0) { - decisionGroups.push({ groupId, groupType, nodeIds: groupNodeIds }); - } - } else { - const node = extractNodeFromSchemaItem(item); + choice.alternatives.forEach((alt: SchemaObject) => { + const node = extractNodeFromSchemaItem(alt); if (node) { + node.decisionGroupId = groupId; nodes.push(node); + groupNodeIds.push(node.uniqueId); } + }); + + if (groupNodeIds.length > 0) { + decisionGroups.push({ groupId, groupType: choice.groupType, nodeIds: groupNodeIds }); + } + } + + getPrefixItems(pattern, 'nodes').forEach((item: SchemaObject, index: number) => { + const choice = readChoice(item); + if (choice) { + addChoice(`node-decision-${index}`, choice); + return; + } + + const node = extractNodeFromSchemaItem(item); + if (node) { + nodes.push(node); } }); + const catalogue = readChoice(getItems(pattern, 'nodes')); + if (catalogue) { + addChoice(NODE_ITEMS_GROUP, catalogue); + } + return { nodes, decisionGroups }; } @@ -327,6 +356,16 @@ function extractRelationshipsFromPattern(pattern: SchemaObject): { const relationships: ExtractedRelationship[] = []; const optionsMetadata: OptionsMetadata[] = []; + function addAlternatives(groupId: string, choice: Choice): void { + choice.alternatives.forEach((alt: SchemaObject) => { + const rel = extractSingleRelationship(alt); + if (rel) { + rel.decisionGroupId = groupId; + relationships.push(rel); + } + }); + } + prefixItems.forEach((item: SchemaObject, index: number) => { // Check for options relationship first if (isOptionsRelationship(item)) { @@ -337,21 +376,9 @@ function extractRelationshipsFromPattern(pattern: SchemaObject): { return; } - // Check for oneOf/anyOf wrapped relationships - const hasOneOf = Array.isArray(item['oneOf']); - const hasAnyOf = Array.isArray(item['anyOf']); - - if (hasOneOf || hasAnyOf) { - const alternatives: SchemaObject[] = hasOneOf ? item['oneOf'] : item['anyOf']; - const groupId = `rel-decision-${index}`; - - alternatives.forEach((alt: SchemaObject) => { - const rel = extractSingleRelationship(alt); - if (rel) { - rel.decisionGroupId = groupId; - relationships.push(rel); - } - }); + const choice = readChoice(item); + if (choice) { + addAlternatives(`rel-decision-${index}`, choice); return; } @@ -362,6 +389,12 @@ function extractRelationshipsFromPattern(pattern: SchemaObject): { } }); + // A decision is always positional, so items is read for relationships but never for decisions. + const catalogue = readChoice(getItems(pattern, 'relationships')); + if (catalogue) { + addAlternatives(RELATIONSHIP_ITEMS_GROUP, catalogue); + } + return { relationships, optionsMetadata }; } From 491bfcc5058b7972562ed725b1f71773223f8f7f Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Fri, 18 Sep 2026 06:17:25 +0000 Subject: [PATCH 2/2] docs(calm-hub-ui): thin the comments in the visualiser drawer Most of them restated the line below. The ones that carry a reason nobody can read from the code stay: why useDropzone takes no accept filter, why a dropped file must not borrow the viewport key, and why the default layout is withheld alongside it. Those three were paragraphs guarding one or two lines each. Same facts, fewer words, and the viewport-key note is now one point per line rather than four run together. --- .../visualizer/components/drawer/Drawer.tsx | 81 +++++-------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx index 5c2788714..d55db385e 100644 --- a/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx +++ b/calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx @@ -12,8 +12,8 @@ import { colors } from '../../../theme/colors.js'; import type { DrawerProps, Control, Decorator } from '../../contracts/contracts.js'; /** - * Detect whether JSON data is a CALM pattern (JSON Schema) or an architecture instance. - * Patterns have properties.nodes.prefixItems; architectures have nodes directly. + * A pattern declares its nodes under `properties.nodes`, as `prefixItems` or `items`. + * An architecture carries `nodes` directly. */ function isPatternData(data: unknown): boolean { if (!data || typeof data !== 'object') return false; @@ -23,9 +23,6 @@ function isPatternData(data: unknown): boolean { return !!(nodes && typeof nodes === 'object' && (nodes['prefixItems'] || nodes['items'])); } -/** - * Extract the unique-id from a CALM node or relationship - */ function extractId(item: CalmNodeSchema | CalmRelationshipSchema): string { return item?.['unique-id'] || ''; } @@ -43,13 +40,9 @@ export function Drawer({ const [calmInstance, setCALMInstance] = useState(undefined); const [patternInstance, setPatternInstance] = useState | undefined>(undefined); const [fileInstance, setFileInstance] = useState | undefined>(undefined); - // Set when a dropped/browsed file can't be read as JSON, so the empty state - // can surface the failure instead of throwing an unhandled rejection. const [dropError, setDropError] = useState(undefined); const [decoratorsState, setDecoratorsState] = useState([]); - // Default to collapsed as per user request const [isMetadataCollapsed, setIsMetadataCollapsed] = useState(true); - // Height of the metadata panel when expanded (in pixels) const [metadataPanelHeight, setMetadataPanelHeight] = useState(250); const onDrop = useCallback(async (acceptedFiles: File[]) => { @@ -60,54 +53,37 @@ export function Drawer({ setDropError(undefined); setFileInstance(parsed); } catch { - // Non-JSON or malformed file: surface the failure rather than - // accepting it and throwing an unhandled rejection downstream. + // Surfaced in the empty state rather than thrown, which would go unhandled. setDropError( "Couldn't read that file — expected CALM JSON (architecture / pattern)." ); } }, []); - // Clear any prior error as soon as a new drag begins, so a fresh attempt - // starts from a clean slate. const onDragEnter = useCallback(() => setDropError(undefined), []); - // No `accept` filter: CALM JSON is often saved with a non-.json extension - // (.calm, .txt, none), and an extension/MIME filter would reject those before - // onDrop ever runs. onDrop parses the file and surfaces a clear dropError on - // anything that isn't valid JSON, so validation lives there, not in the filter. + // No `accept` filter on purpose: CALM JSON is often saved as .calm, .txt or with no + // extension, and a filter would reject those silently before onDrop ran. const { getRootProps, getInputProps, isDragActive } = useDropzone({ onDrop, onDragEnter, }); - // Identifies the diagram (ignoring version) so its viewport can be remembered - // across version/moment switches and refreshes. A dropped file has no identity, - // so it must never resolve to `viewportKeyOverride` (DiagramSection's resolved - // namespace/numeric-architectureId for the *currently loaded* architecture) — - // otherwise dragging a node on a dropped file would write scratch positions, - // and "Save as default layout" would write server positions, under the loaded - // architecture's key using the dropped file's unrelated layout. - // `viewportKeyOverride` takes precedence when present and no file is dropped, - // so scratch storage and the server layout share one key regardless of whether - // this architecture was reached via a slug or numeric route. An explicit `null` - // override (a slug that finished resolving with no match) suppresses the - // fallback rather than triggering it — falling back to the slug here would - // reintroduce exactly the key split the override exists to close. - // The calmType component guards against a second, independent collision: - // architecture ids and pattern ids are allocated from separate counters, so - // an Architecture 7 and a Pattern 7 can coexist in the same namespace — - // without it they'd share one scratch-position entry and one viewport entry. + // Remembers pan and zoom for one diagram across version switches and refreshes. + // + // A dropped file has no identity, so it must never borrow the override: its positions + // would then be saved under the loaded architecture's key. + // A `null` override means a slug resolved to nothing, so it suppresses the fallback + // rather than triggering it. + // `calmType` is in the key because architecture and pattern ids come from separate + // counters, so Architecture 7 and Pattern 7 would otherwise collide. const computedViewportKey = !fileInstance && data ? buildViewportKey(data.name, data.calmType, data.id) : undefined; const viewportKey = fileInstance || viewportKeyOverride === null ? undefined : (viewportKeyOverride ?? computedViewportKey); - // `defaultLayout`/`layoutEpoch` must collapse alongside `viewportKey` for the same - // reason: they describe the currently-*loaded* resource's saved server layout, not - // whatever was just dropped. Without this, dragging in a locally-edited copy of the - // same file would re-apply the loaded resource's saved positions (matched by node id) - // onto the dropped file's freshly-parsed nodes instead of a fresh auto-layout — the - // graph's `awaitingDefaultLayout` gate only reads these two props, it doesn't know - // about `fileInstance`. + // Dropped alongside `viewportKey`, and for the same reason: these describe the loaded + // resource's saved layout. Otherwise dropping an edited copy of an open file re-applies + // the saved positions by node id instead of laying it out fresh. The graph's + // `awaitingDefaultLayout` gate reads only these two props and cannot see `fileInstance`. const effectiveDefaultLayout = fileInstance ? undefined : defaultLayout; const effectiveLayoutEpoch = fileInstance ? undefined : layoutEpoch; @@ -138,7 +114,6 @@ export function Drawer({ const decorators = decoratorsProp ?? decoratorsState; - // Extract ADR links from CALM data const adrs = useMemo((): string[] => { const calmData = calmInstance as CalmArchitectureSchema & { adrs?: unknown }; const rawAdrs = calmData?.adrs; @@ -151,7 +126,6 @@ export function Drawer({ .filter((adr) => adr.length > 0); }, [calmInstance]); - // Extract controls from CALM data (from root, nodes, and relationships) const controls = useMemo((): Record => { const calmData = calmInstance as CalmArchitectureSchema & { controls?: Record; @@ -162,7 +136,6 @@ export function Drawer({ const nodeControls: Record = {}; const relationshipControls: Record = {}; - // Extract controls from nodes const nodes = calmData.nodes || []; nodes.forEach((node) => { if (node.controls) { @@ -179,7 +152,6 @@ export function Drawer({ } }); - // Extract controls from relationships const relationships = calmData.relationships || []; relationships.forEach((relationship) => { if (relationship.controls) { @@ -196,7 +168,6 @@ export function Drawer({ } }); - // Merge all control sources (root-level takes precedence) return { ...nodeControls, ...relationshipControls, ...rootControls }; }, [calmInstance]); @@ -208,7 +179,6 @@ export function Drawer({ onItemSelect?.(null); }, [onItemSelect]); - // Pattern-specific click handlers const handlePatternNodeClick = useCallback((nodeData: Record) => { onItemSelect?.({ data: toSidebarNodeData(nodeData) }); }, [onItemSelect]); @@ -226,7 +196,6 @@ export function Drawer({ }, [onItemSelect]); - // Handle node click from controls panel const handleControlNodeClick = useCallback( (nodeId: string) => { const node = calmInstance?.nodes?.find((n) => n['unique-id'] === nodeId); @@ -273,9 +242,6 @@ export function Drawer({ viewportKey={viewportKey} defaultLayout={effectiveDefaultLayout} layoutEpoch={effectiveLayoutEpoch} - // See ReactFlowVisualizer's onPositionsChange below: never - // reported for a dropped file, which has no stable identity - // to save a shared default layout against. onPositionsChange={fileInstance ? undefined : onPositionsChange} /> ) : calmInstance ? ( @@ -287,15 +253,10 @@ export function Drawer({ viewportKey={viewportKey} defaultLayout={effectiveDefaultLayout} layoutEpoch={effectiveLayoutEpoch} - // Never reported for a dropped file: `onPositionsChange` - // ultimately feeds DiagramSection's "Save as default - // layout", which is scoped to the *loaded architecture* - // (via `viewportKeyOverride`/`defaultLayoutState`, not - // this component's local `fileInstance` state). Passing - // it through unconditionally would let a locally-dropped - // file's on-screen positions be saved as the shared - // default layout for the architecture actually being - // viewed. + // Withheld for a dropped file. This feeds DiagramSection's + // "Save as default layout", which is scoped to the loaded + // architecture, so passing it through would save a dropped + // file's positions as that architecture's shared default. onPositionsChange={fileInstance ? undefined : onPositionsChange} /> ) : null}