diff --git a/CHANGELOG.md b/CHANGELOG.md index 373c885..8939230 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,44 @@ platform API version. globs, the openability probe and the raw sidecar commands accept any of them; the new-file scaffold takes the first. `sourceExtension` stays honoured and is all a Python product needs. +- `DiagramStorageOptions.platformEntityItems` (and the same on the toolkit's + profile input): `false` leaves the palette's Entities category to the + product's own `entityPaletteItems`, for a product whose nodes are none of the + platform's kinds. Default `true`, so every existing palette is unchanged. +- Sidecar toolkit, for a sidecar that is not Python: `openability.decoratorName` + is optional (with only a probe, an undecidable probe opens the file instead of + scanning for a decorator); `navigation: 'none'` installs no Python navigation + provider and skips the Python caller scan; `chat.sourceMimeType` names what + the agent receives the file as. +- A producer may place a port with its bundle instead of by its direction. A + port's `side` (`west`/`east`) and a boundary node's `meta.side` say where it + sits; `group` names the bundle. A handshake's `ready` then sits beside the + `valid` and `data` it answers, and the wire between two nodes is a straight + segment with the arrow pointing back instead of a loop around both. The + direction is untouched: a `ready` output stays an output. The side reaches + the shared port anchor (`cal:portSide`), ELK's port sides and row ordering, + the label pass, the client hit areas and a mirrored boundary glyph + (`cal:boundarySide`). Absent, everything is as before. An edge whose + `meta.role` is `bidirectional` is drawn without an arrowhead. +- `documentErrorsToProblems` on the sidecar toolkit's profile input and runtime + config: `true` publishes a loaded document's own root-level `errors` to the + Problems panel with their file, line and column. Default `false`: a Python + runtime's load errors stay in the diagram's overlay, as the diagnostics + parity tests pin. +- A location in a graph document (`source`, `referencedSource`, `authoredSource` + on a node, `source` on a port or an edge) may leave out its `file` when it is + the graph's own, `graph.meta.file`; the reader puts it back before anything + navigates. A hierarchy export of a large netlist repeated one absolute path on + every element, a third of the export. A document with every file spelled out + reads exactly as before. + +### Fixed + +- ELK is told the side the producer placed a port on. It was told the side + from the direction alone, so a bundle's `ready`, placed in the other margin, + was routed to as if it sat on the far edge: the wire came around the outside + of the margin and crossed the whole node, label included, to reach the glyph. + It now arrives from the side the glyph faces, in one straight run. ## [0.7.0] - 2026-09-01 diff --git a/packages/diagram-client/src/libavoid-edge-router.ts b/packages/diagram-client/src/libavoid-edge-router.ts index 27cd7b0..1034f00 100644 --- a/packages/diagram-client/src/libavoid-edge-router.ts +++ b/packages/diagram-client/src/libavoid-edge-router.ts @@ -124,6 +124,7 @@ function anchorOf(port: any): { x: number; y: number; side: PortSide } | undefin absolute: absolutePosition(port), size: port.size ?? port.bounds, direction: port.args?.['cal:portDirection'] as string | undefined, + side: port.args?.['cal:portSide'] as string | undefined, type: port.type }); } diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index acff3f7..8092cfd 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -44,7 +44,7 @@ import { import { EDGE_OPEN_ANCHOR_X_ARG, EDGE_OPEN_ANCHOR_Y_ARG } from './edge-open-mouse-listener'; import { clientBehavior } from './profile'; import { isIncidentEdgeInActiveDrag } from './elk-live-drag-router'; -import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramMetadata, WorkflowDiagramTypes, normalizePortSide } from '@dialogram/shared'; import { WorkflowDiagramConstants } from '@dialogram/shared'; /** Walk to the model root; the drag-active window is tracked per root. */ @@ -1077,6 +1077,12 @@ export class ProxyNodeView extends ShapeView { * Boundary input port node (left margin) - network's input exposed to internal entities * Styled as a terminal connector shape with port name centered */ +/** Whether the producer marked the connection as carrying data both ways (`edge-role-bidirectional`). */ +function isBidirectionalEdge(edge: Readonly): boolean { + const classes = (edge as { cssClasses?: unknown }).cssClasses; + return Array.isArray(classes) && classes.includes('edge-role-bidirectional'); +} + /** Whether an edge terminates on a boundary port, whose glyph is already an arrow. */ function endsAtBoundaryPort(edge: Readonly): boolean { let element: any = (edge as any).target; @@ -1090,6 +1096,18 @@ function endsAtBoundaryPort(edge: Readonly): boolean { return false; } +/** Whether the server placed this boundary node in the margin opposite its direction (`cal:boundarySide`). */ +function boundaryMirrored(node: unknown, isInput: boolean): boolean { + const margin = normalizePortSide((node as { args?: Record })?.args?.[WorkflowDiagramMetadata.BOUNDARY_SIDE]); + return margin !== undefined && margin !== (isInput ? 'WEST' : 'EAST'); +} + +/** Which side of its node a port sits on, as the server placed it: `cal:portSide`, else its direction. */ +function portSideOf(port: unknown, fallbackEast: boolean): 'WEST' | 'EAST' { + return normalizePortSide((port as { args?: Record })?.args?.[WorkflowDiagramMetadata.PORT_SIDE]) + ?? (fallbackEast ? 'EAST' : 'WEST'); +} + /** One of a boundary node's own text values, from the args the server set. */ function boundaryText(node: unknown, key: string): string { const value = (node as { args?: Record })?.args?.[key]; @@ -1112,24 +1130,31 @@ function boundaryText(node: unknown, key: string): string { * wire misses the arrow — both read `BoundaryPortGeometry` for exactly that * reason. */ -function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: string): VNode[] { +function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: string, mirrored = false): VNode[] { const G = BoundaryPortGeometry; const axis = G.AXIS_Y_PX; const arrow = isInput ? G.SOURCE_ARROW : G.SINK_ARROW; - // Inputs occupy the node's right edge, outputs its left. - const glyphX = isInput ? nodeWidth - G.glyphWidth(true) : 0; + // Inputs occupy the node's right edge, outputs its left — unless the + // producer placed the node in the other margin with its bundle + // (`cal:boundarySide`), when everything mirrors: the glyph takes the + // other edge and points the other way, the text runs out the other side. + // The shape (bare arrow or arrow and bar) still says input or output. + const glyphEast = isInput !== mirrored; + const pointsEast = !mirrored; + const glyphX = glyphEast ? nodeWidth - G.glyphWidth(isInput) : 0; const top = axis - arrow.height / 2; - const tipX = glyphX + arrow.width; - const textX = isInput ? nodeWidth - G.textOffset(true) : G.textOffset(false); - const anchor = isInput ? 'end' : 'start'; + const tipX = pointsEast ? glyphX + arrow.width : glyphX; + const baseX = pointsEast ? glyphX : glyphX + arrow.width; + const textX = glyphEast ? nodeWidth - G.textOffset(isInput) : G.textOffset(isInput); + const anchor = glyphEast ? 'end' : 'start'; // How far the text reaches, from the glyph outward. Approximate by design — // see approximateTextWidth; it sizes these rectangles and nothing else. const nameReach = G.approximateTextWidth(name, G.NAME_FONT_PX); const textReach = Math.max(nameReach, G.approximateTextWidth(type, G.TYPE_FONT_PX)); - const spanFrom = (reach: number): { x: number; width: number } => isInput - ? { x: textX - reach, width: reach + G.textOffset(true) } - : { x: 0, width: G.textOffset(false) + reach }; + const spanFrom = (reach: number): { x: number; width: number } => glyphEast + ? { x: textX - reach, width: reach + G.textOffset(isInput) } + : { x: 0, width: G.textOffset(isInput) + reach }; // Two rectangles, because clicking and highlighting are not the same extent // — the diagram already draws that distinction and the port was not. @@ -1161,14 +1186,14 @@ function boundaryPort(isInput: boolean, nodeWidth: number, name: string, type: s // input, into the bar for an output. svg('path', { class: { 'boundary-glyph': true }, - attrs: { d: `M ${glyphX} ${top} L ${tipX} ${axis} L ${glyphX} ${top + arrow.height} Z` } + attrs: { d: `M ${baseX} ${top} L ${tipX} ${axis} L ${baseX} ${top + arrow.height} Z` } }) ]; if (!isInput) { parts.push(svg('rect', { class: { 'boundary-glyph': true, 'boundary-glyph-bar': true }, attrs: { - x: tipX, + x: pointsEast ? tipX : tipX - G.SINK_BAR.width, y: axis - G.SINK_BAR.height / 2, width: G.SINK_BAR.width, height: G.SINK_BAR.height @@ -1228,7 +1253,7 @@ export class BoundaryInputNodeView extends ShapeView { // No body: the symbol on the axis IS the port — glyph, name and // type together, drawn as one thing. ...boundaryPort(true, width, boundaryText(node, WorkflowDiagramMetadata.PORT_NAME), - boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE)), + boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE), boundaryMirrored(node, true)), // The port element (an anchor only) and the label elements, which // render nothing — see WorkflowLabelView. ...context.renderChildren(node) @@ -1263,7 +1288,7 @@ export class BoundaryOutputNodeView extends ShapeView { }, // No body — see the input view above. ...boundaryPort(false, width, boundaryText(node, WorkflowDiagramMetadata.PORT_NAME), - boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE)), + boundaryText(node, WorkflowDiagramMetadata.PORT_TYPE), boundaryMirrored(node, false)), ...context.renderChildren(node) ); } @@ -1286,8 +1311,9 @@ export class WorkflowInputPortView extends ShapeView { // IMPORTANT: Do not expand the hit area past the node boundary. // If the hit target extends outward, edge anchoring can use the larger bounds, // which makes connections (and thus labels) look "far" from the visible port. - // For input ports (left side), expand inward (to the right). - const hitX = 0; + // For input ports (left side), expand inward (to the right) — or to the + // left when the producer placed this input on the east side with its bundle. + const hitX = portSideOf(port, false) === 'EAST' ? portWidth - hitSize : 0; const hitY = (portHeight - hitSize) / 2; const children: VNode[] = [ @@ -1365,8 +1391,9 @@ export class WorkflowOutputPortView extends ShapeView { // Non-visual hit target to make connecting reliable without changing visible port size. const hitSize = Math.max(14, portWidth, portHeight); // For output ports (right side), expand inward (to the left) so the hit area right edge - // matches the visible port right edge. - const hitX = portWidth - hitSize; + // matches the visible port right edge — or to the right when the producer placed + // this output on the west side with its bundle. + const hitX = portSideOf(port, true) === 'EAST' ? portWidth - hitSize : 0; const hitY = (portHeight - hitSize) / 2; const children: VNode[] = [ @@ -2282,6 +2309,11 @@ export class WorkflowEdgeView extends PolylineEdgeView { if (edge && endsAtBoundaryPort(edge)) { return svg('g', {}); } + // A wire that carries data both ways (`role: bidirectional`, an HDL + // `inout` net) has no head: an arrow would claim a direction it lacks. + if (edge && isBidirectionalEdge(edge)) { + return svg('g', {}); + } if (segments.length < 2) { return svg('g', {}); } @@ -2396,7 +2428,10 @@ export class WorkflowLabelView implements IView { const direction = parentPort?.args?.['cal:portDirection'] as string | undefined; const parentType = parentPort?.type as string | undefined; const isOutputByType = parentType?.includes('output') ?? false; - const isOutput = direction === 'output' || isOutputByType; + // "Output" here means the EAST side: where the label anchors depends on + // which edge the port sits on, which is its direction unless the producer + // placed it with its bundle (`cal:portSide`). + const isOutput = portSideOf(parentPort, direction === 'output' || isOutputByType) === 'EAST'; // Input labels: do not add extra inner padding. The label box already has a // PORT_LABEL_GAP_PX from the port marker; additional inset makes inputs look diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index 48a89ae..d4a3bad 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -1,5 +1,5 @@ import type { StorageRuntimeOptions } from '../server/storage-runtime-options'; -import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramConstants, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramConstants, WorkflowDiagramMetadata, WorkflowDiagramTypes, normalizePortSide, type PortSide } from '@dialogram/shared'; import * as path from 'node:path'; import { findFeedbackEdges } from './feedback-edges'; import { URI } from 'vscode-uri'; @@ -10,6 +10,15 @@ export type PyGraphPort = { direction: 'in' | 'out'; type?: string; role?: string; + /** + * `west` or `east`: the producer placing the port with its bundle rather + * than by its direction (a handshake's `ready` beside its `valid`). Absent, + * inputs sit west and outputs east. + */ + side?: string; + /** The bundle the port belongs to (`s_axis`). */ + group?: string; + width?: number; source?: { file?: string; line?: number } | null; }; @@ -50,6 +59,11 @@ export type PyGraph = { meta?: Record; }; +/** + * A graph document. A location (`source`, `referencedSource`, `authoredSource` + * on a node, `source` on a port or an edge) may leave out its `file` when it + * is the graph's own, `graph.meta.file`; see `resolveSourceFiles`. + */ export type PyGraphDocument = { version: string; graph: PyGraph; @@ -193,6 +207,7 @@ export class GraphGModelSource { constructor(private readonly storageOptions?: StorageRuntimeOptions) {} transform(doc: PyGraphDocument): { graph: GGraph } { + doc = resolveSourceFiles(doc); const children: GModelElement[] = []; const nodeMap = new Map(); const portIdMap = new Map(); @@ -235,8 +250,11 @@ export class GraphGModelSource { } continue; } - const inputs = node.ports.filter(p => p.direction === 'in'); - const outputs = node.ports.filter(p => p.direction === 'out'); + // Ports are laid out by SIDE, which is the direction unless the + // producer placed the port with its bundle (`port.side`). + const sideOf = (p: PyGraphPort): PortSide => normalizePortSide(p.side) ?? (p.direction === 'in' ? 'WEST' : 'EAST'); + const inputs = node.ports.filter(p => sideOf(p) === 'WEST'); + const outputs = node.ports.filter(p => sideOf(p) === 'EAST'); const portW = WorkflowDiagramConstants.PORT_WIDTH_PX; const portH = WorkflowDiagramConstants.PORT_HEIGHT_PX; @@ -539,9 +557,11 @@ export class GraphGModelSource { 'wf:footerTypeLabel': typeLabel }; } - const mkPort = (port: PyGraphPort, direction: 'input' | 'output', idx: number): GPort => { + const mkPort = (port: PyGraphPort, side: PortSide, idx: number): GPort => { const portId = `${stableNodeId}_port_${port.name}`; - const x = direction === 'output' ? size.width : -portW; + const direction: 'input' | 'output' = port.direction === 'out' ? 'output' : 'input'; + const east = side === 'EAST'; + const x = east ? size.width : -portW; const y = yStart + idx * portRowH; const labelText = port.name; const labelSize = this.estimateLabelSize(labelText, WorkflowDiagramConstants.PORT_LABEL_FONT_SIZE_PX); @@ -559,10 +579,12 @@ export class GraphGModelSource { [WorkflowDiagramMetadata.PORT_ROLE]: port.role, // Kept beside the documented key: the property panel and // the port views already read this one. - 'wf:portRole': port.role + 'wf:portRole': port.role, + ...(normalizePortSide(port.side) ? { [WorkflowDiagramMetadata.PORT_SIDE]: side } : {}), + ...(port.group ? { [WorkflowDiagramMetadata.PORT_GROUP]: port.group } : {}) }, layoutOptions: { - 'elk.port.side': direction === 'output' ? 'EAST' : 'WEST' + 'elk.port.side': side } }; gport.cssClasses = [ @@ -590,10 +612,10 @@ export class GraphGModelSource { cssClasses: [ WorkflowDiagramCss.LABEL, 'port-label', - direction === 'input' ? 'input-port-label' : 'output-port-label' + east ? 'output-port-label' : 'input-port-label' ], position: { - x: direction === 'input' ? portW + 1 : -1 - labelBoxWidth, + x: east ? -1 - labelBoxWidth : portW + 1, y: Math.round((portH - labelSize.height) / 2) }, size: { width: labelBoxWidth, height: labelSize.height }, @@ -630,8 +652,8 @@ export class GraphGModelSource { return gport; }; - inputs.forEach((p, i) => gnode.children!.push(mkPort(p, 'input', i))); - outputs.forEach((p, i) => gnode.children!.push(mkPort(p, 'output', i))); + inputs.forEach((p, i) => gnode.children!.push(mkPort(p, 'WEST', i))); + outputs.forEach((p, i) => gnode.children!.push(mkPort(p, 'EAST', i))); if (node.kind === 'if' || node.kind === 'loop') { const branches = this.createControlBranchPorts(node.id, node.kind); @@ -1175,6 +1197,13 @@ export class GraphGModelSource { private createBoundaryNode(node: PyGraphNode, stableNodeId: string): GNode { const port = node.ports[0]; const isInput = node.kind === 'wf-input'; + // Which margin the node sits in. By its direction, unless the producer + // placed it with its bundle (`meta.side`): an output `ready` belongs + // beside the input `valid` and `data` it answers. Then the glyph faces + // the other way and the port sits on the other edge. + const margin: PortSide = normalizePortSide(node.meta?.['side']) ?? (isInput ? 'WEST' : 'EAST'); + const mirrored = margin !== (isInput ? 'WEST' : 'EAST'); + const facesEast = margin === 'WEST'; // One row tall now, not a 50px box — the port is a symbol on the wire's // axis rather than a container. Width stays fixed across every boundary // node so their inner edges, and therefore their glyphs, line up in a @@ -1197,15 +1226,17 @@ export class GraphGModelSource { // puts the anchor on the axis — the NAME's line, not the node's // centre, which is what used to run the wire between the two lines. position: { - x: isInput ? size.width - glyph.width : 0, + x: facesEast ? size.width - glyph.width : 0, y: BoundaryPortGeometry.AXIS_Y_PX - glyph.height / 2 }, size: glyph, args: { [WorkflowDiagramMetadata.PORT_NAME]: port?.name ?? '', - [WorkflowDiagramMetadata.PORT_DIRECTION]: isInput ? 'output' : 'input' + [WorkflowDiagramMetadata.PORT_DIRECTION]: isInput ? 'output' : 'input', + ...(mirrored ? { [WorkflowDiagramMetadata.PORT_SIDE]: facesEast ? 'EAST' : 'WEST' } : {}), + ...(port?.group ? { [WorkflowDiagramMetadata.PORT_GROUP]: port.group } : {}) }, - layoutOptions: { 'elk.port.side': isInput ? 'EAST' : 'WEST' } + layoutOptions: { 'elk.port.side': facesEast ? 'EAST' : 'WEST' } }; const typeText = port?.type ?? ''; @@ -1253,7 +1284,7 @@ export class GraphGModelSource { size, layoutOptions: { 'elk.portConstraints': 'FIXED_POS', - 'elk.layered.layering.layerConstraint': isInput ? 'FIRST_SEPARATE' : 'LAST_SEPARATE' + 'elk.layered.layering.layerConstraint': margin === 'WEST' ? 'FIRST_SEPARATE' : 'LAST_SEPARATE' }, children: [ { @@ -1280,6 +1311,7 @@ export class GraphGModelSource { [WorkflowDiagramMetadata.PORT_TYPE]: typeText, 'wf:boundaryKind': isInput ? 'input' : 'output', 'wf:portName': port?.name ?? '', + ...(mirrored ? { [WorkflowDiagramMetadata.BOUNDARY_SIDE]: margin } : {}), ...boundaryNavigation } }; @@ -1426,3 +1458,50 @@ export class GraphGModelSource { return this.isExternalNodeKind(node.kind) || this.isExternalByMeta(node); } } + +type SourceLocation = { file?: string; line?: number } | null | undefined; + +/** + * A location without a `file` is in the graph's own file. + * + * A hierarchy export repeats one absolute path on every node, port and edge of + * every graph; a producer may leave it out where it is the graph's `meta.file` + * (a third of a large netlist's export), and this puts it back before anything + * reads a location. A document whose locations all name their file, or whose + * graph names no file, is returned as it is. + */ +export function resolveSourceFiles(doc: PyGraphDocument): PyGraphDocument { + const own = doc.graph?.meta?.['file']; + if (typeof own !== 'string' || own === '') { + return doc; + } + let changed = false; + const fill = (loc: unknown): unknown => { + const at = loc as SourceLocation; + if (at && typeof at === 'object' && typeof at.line === 'number' && typeof at.file !== 'string') { + changed = true; + return { ...at, file: own }; + } + return loc; + }; + const fillKeys = >(obj: T, keys: string[]): T => { + let out: Record = obj; + for (const key of keys) { + if (key in out) { + const filled = fill(out[key]); + if (filled !== out[key]) { + out = out === obj ? { ...obj } : out; + out[key] = filled; + } + } + } + return out as T; + }; + const nodes = (doc.graph.nodes ?? []).map(node => { + const meta = node.meta ? fillKeys(node.meta, ['source', 'referencedSource', 'authoredSource']) : node.meta; + const ports = (node.ports ?? []).map(port => fillKeys(port, ['source'])); + return meta === node.meta && ports.every((port, i) => port === node.ports[i]) ? node : { ...node, meta, ports }; + }); + const edges = (doc.graph.edges ?? []).map(edge => fillKeys(edge, ['source'])); + return changed ? { ...doc, graph: { ...doc.graph, nodes, edges } } : doc; +} diff --git a/packages/diagram-server/src/operations/change-routing-points-handler.ts b/packages/diagram-server/src/operations/change-routing-points-handler.ts index 67bb8c5..7a54848 100644 --- a/packages/diagram-server/src/operations/change-routing-points-handler.ts +++ b/packages/diagram-server/src/operations/change-routing-points-handler.ts @@ -14,7 +14,7 @@ import { inject, injectable } from 'inversify'; import { WorkflowDiagramMetadata } from '@dialogram/shared'; import { LayoutPersistenceService } from '../services/layout-persistence-service'; import { WORKFLOW_LAYOUT_PERSISTENCE_KEY, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; -import { WorkflowDiagramConstants } from '@dialogram/shared'; +import { WorkflowDiagramConstants, portSide } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { GModelSerializer } from '@eclipse-glsp/server'; import { layoutTargetOf } from '../server/layout-target'; @@ -140,12 +140,12 @@ export class WorkflowChangeRoutingPointsOperationHandler extends OperationHandle const w = (port.size?.width && port.size.width > 0) ? port.size.width : fallbackPortWidth; const h = (port.size?.height && port.size.height > 0) ? port.size.height : fallbackPortHeight; - const direction = (port as any).args?.['cal:portDirection'] as string | undefined; - const side = direction === 'output' ? 'EAST' - : direction === 'input' ? 'WEST' - : (port.type as string | undefined)?.includes('output') ? 'EAST' - : (port.type as string | undefined)?.includes('input') ? 'WEST' - : 'WEST'; + const side = portSide({ + absolute: nodeAbs, + direction: (port as any).args?.['cal:portDirection'] as string | undefined, + side: (port as any).args?.['cal:portSide'] as string | undefined, + type: port.type as string | undefined + }); const absPortX = nodeAbs.x + (portRel.x ?? 0); const absPortY = nodeAbs.y + (portRel.y ?? 0); diff --git a/packages/diagram-server/src/operations/layout-serpentine-mesh-handler.ts b/packages/diagram-server/src/operations/layout-serpentine-mesh-handler.ts index a568187..23f79b2 100644 --- a/packages/diagram-server/src/operations/layout-serpentine-mesh-handler.ts +++ b/packages/diagram-server/src/operations/layout-serpentine-mesh-handler.ts @@ -22,7 +22,7 @@ import { import { WORKFLOW_LAYOUT_PERSISTENCE_KEY, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; -import { WorkflowDiagramConstants } from '@dialogram/shared'; +import { WorkflowDiagramConstants, portSide } from '@dialogram/shared'; import { layoutTargetOf } from '../server/layout-target'; export const WORKFLOW_LAYOUT_SERPENTINE_MESH_OPERATION_KIND = 'dialogram.layoutSerpentineMesh' as const; @@ -742,12 +742,12 @@ export class WorkflowLayoutSerpentineMeshOperationHandler extends OperationHandl const w = port.size?.width && port.size.width > 0 ? port.size.width : WorkflowDiagramConstants.PORT_WIDTH_PX; const h = port.size?.height && port.size.height > 0 ? port.size.height : WorkflowDiagramConstants.PORT_HEIGHT_PX; - const direction = (port as any).args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined; - const side = direction === 'output' ? 'EAST' - : direction === 'input' ? 'WEST' - : port.type?.includes('output') ? 'EAST' - : port.type?.includes('input') ? 'WEST' - : 'WEST'; + const side = portSide({ + absolute: nodePos, + direction: (port as any).args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: (port as any).args?.[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, + type: port.type + }); const absPortX = nodePos.x + portPos.x; const absPortY = nodePos.y + portPos.y; diff --git a/packages/diagram-server/src/operations/reroute-edges-avoid-overlaps-handler.ts b/packages/diagram-server/src/operations/reroute-edges-avoid-overlaps-handler.ts index cc653c8..1abbbd8 100644 --- a/packages/diagram-server/src/operations/reroute-edges-avoid-overlaps-handler.ts +++ b/packages/diagram-server/src/operations/reroute-edges-avoid-overlaps-handler.ts @@ -224,6 +224,7 @@ export class WorkflowRerouteEdgesAvoidOverlapsOperationHandler extends Operation absolute: this.absPos(port), size: this.sizeOf(port), direction: (port as any).args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: (port as any).args?.[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, type: port.type }); return { ...anchor, nodeId: String((parentNode as any).id) }; diff --git a/packages/diagram-server/src/routing/persisted-edge-routes.ts b/packages/diagram-server/src/routing/persisted-edge-routes.ts index a499654..3239b8e 100644 --- a/packages/diagram-server/src/routing/persisted-edge-routes.ts +++ b/packages/diagram-server/src/routing/persisted-edge-routes.ts @@ -56,6 +56,7 @@ function portAnchorAbsolute(index: GModelIndex, portId: string | undefined): Poi absolute: absolutePosition(port), size: (port as any).size as { width?: number; height?: number } | undefined, direction: (port as any).args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: (port as any).args?.[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, type: port.type }); } diff --git a/packages/diagram-server/src/server/diagram-action-handlers.ts b/packages/diagram-server/src/server/diagram-action-handlers.ts index cbb1669..a75a20a 100644 --- a/packages/diagram-server/src/server/diagram-action-handlers.ts +++ b/packages/diagram-server/src/server/diagram-action-handlers.ts @@ -47,7 +47,8 @@ import { WorkflowDiagramConstants, WorkflowDiagramTypes, WORKFLOW_NETWORK_MODEL_KEY, - WORKFLOW_LAYOUT_PERSISTENCE_KEY + WORKFLOW_LAYOUT_PERSISTENCE_KEY, + portSide } from '@dialogram/shared'; import { LayoutPersistenceService } from '../services/layout-persistence-service'; import { AgentStructuralEditSignal } from './agent-structural-edit-signal'; @@ -340,8 +341,15 @@ export class WorkflowModelSubmissionHandler extends ModelSubmissionHandler { if (element instanceof GPort) { const portW = element.size?.width ?? WorkflowDiagramConstants.PORT_WIDTH_PX; const portH = element.size?.height ?? WorkflowDiagramConstants.PORT_HEIGHT_PX; - const direction = (element.args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined) - ?? (element.type === WorkflowDiagramTypes.PORT_OUTPUT ? 'output' : 'input'); + // The label sits inside the node, on the port's SIDE — which is + // the direction unless the producer placed the port with its + // bundle (`cal:portSide`). + const east = portSide({ + absolute: { x: 0, y: 0 }, + direction: element.args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: element.args?.[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, + type: element.type + }) === 'EAST'; const children = (element.children as any[] | undefined) ?? []; for (const child of children) { @@ -358,7 +366,7 @@ export class WorkflowModelSubmissionHandler extends ModelSubmissionHandler { // sits to the *left* of the port marker with a fixed gap. Client-side alignment // transforms for port labels are intentionally disabled (see ComputedBounds handling), // so we use the label's top-left origin here. - const x = direction === 'output' + const x = east ? -portLabelGapPx - labelW : portW + portLabelGapPx; @@ -1034,11 +1042,15 @@ export class WorkflowComputedBoundsActionHandler implements ActionHandler { return; } - const direction = (parentPort.args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined) - ?? (portType === WorkflowDiagramTypes.PORT_OUTPUT ? 'output' : 'input'); + const east = portSide({ + absolute: { x: 0, y: 0 }, + direction: parentPort.args?.[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: parentPort.args?.[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, + type: portType + }) === 'EAST'; const y = Math.round((portH - labelH) / 2); - const x = direction === 'output' + const x = east ? -portLabelGapPx - labelW : portW + portLabelGapPx; diff --git a/packages/diagram-server/src/server/elk-layout-engine.ts b/packages/diagram-server/src/server/elk-layout-engine.ts index 24b8d27..3bf1bee 100644 --- a/packages/diagram-server/src/server/elk-layout-engine.ts +++ b/packages/diagram-server/src/server/elk-layout-engine.ts @@ -15,7 +15,18 @@ import { GlspElkLayoutEngine, ElkFactory, ElementFilter, LayoutConfigurator } fr import { ElkExtendedEdge, ElkNode, ElkPort, ElkShape } from 'elkjs'; import { injectable } from 'inversify'; import { WorkflowDiagramTypes } from '@dialogram/shared'; -import { WorkflowDiagramConstants } from '@dialogram/shared'; +import { WorkflowDiagramConstants, WorkflowDiagramMetadata, portSide, type PortSide } from '@dialogram/shared'; + +/** Which side of its node a GPort sits on: the producer's placement, else its direction or type. */ +function gportSide(port: GPort): PortSide { + const args = (port.args ?? {}) as Record; + return portSide({ + absolute: { x: 0, y: 0 }, + direction: args[WorkflowDiagramMetadata.PORT_DIRECTION] as string | undefined, + side: args[WorkflowDiagramMetadata.PORT_SIDE] as string | undefined, + type: port.type + }); +} import { snapRouteEndpoints } from '../routing/persisted-edge-routes'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -198,8 +209,8 @@ export class WorkflowElkLayoutEngine extends GlspElkLayoutEngine { * move with them; nothing else about the node changes. */ private regridPorts(node: GNode, elkPorts: ElkPort[]): void { - for (const input of [true, false]) { - const ports = this.findChildren(node, GPort).filter(p => (p.type === WorkflowDiagramTypes.PORT_INPUT) === input); + for (const side of ['WEST', 'EAST'] as const) { + const ports = this.findChildren(node, GPort).filter(p => gportSide(p) === side); if (ports.length < 2) { continue; } @@ -857,13 +868,8 @@ export class WorkflowElkLayoutEngine extends GlspElkLayoutEngine { const w = (port.size?.width && port.size.width > 0) ? port.size.width : fallbackPortWidth; const h = (port.size?.height && port.size.height > 0) ? port.size.height : fallbackPortHeight; - // Determine port side from direction or type - const direction = (port as any).args?.['cal:portDirection'] as string | undefined; - const side = direction === 'output' ? 'EAST' - : direction === 'input' ? 'WEST' - : port.type?.includes('output') ? 'EAST' - : port.type?.includes('input') ? 'WEST' - : 'WEST'; + // Determine port side: the producer's placement, else direction or type + const side = gportSide(port); const absPortX = nodePos.x + portPos.x; const absPortY = nodePos.y + portPos.y; diff --git a/packages/diagram-server/src/server/layout-configurator.ts b/packages/diagram-server/src/server/layout-configurator.ts index b6ac533..98d86fe 100644 --- a/packages/diagram-server/src/server/layout-configurator.ts +++ b/packages/diagram-server/src/server/layout-configurator.ts @@ -10,7 +10,12 @@ import { GGraph, GNode, GEdge, GPort, GLabel, GModelElement } from '@eclipse-glsp/server'; import { AbstractLayoutConfigurator, LayoutOptions } from '@eclipse-glsp/layout-elk'; import { injectable } from 'inversify'; -import { WorkflowDiagramTypes } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes, normalizePortSide } from '@dialogram/shared'; + +/** The margin a boundary node was placed in by its producer, when it was not left to its direction. */ +function boundaryMargin(node: GNode): 'WEST' | 'EAST' | undefined { + return normalizePortSide((node.args as Record | undefined)?.[WorkflowDiagramMetadata.BOUNDARY_SIDE]); +} /** * Workflow-specific layout configurator for ELK. @@ -244,10 +249,12 @@ export class WorkflowLayoutConfigurator extends AbstractLayoutConfigurator { // FIXED_SIZE prevents ELK from resizing the node: the port is positioned at // x = node.width (right edge), so any ELK-computed size change would push the // port outside the node bounds and break edge routing/anchoring. + // Unless the producer placed it with its bundle on the other margin + // (`cal:boundarySide`), in which case it goes with the outputs. return { 'elk.nodeSize.constraints': 'FIXED_SIZE', 'elk.portConstraints': 'FIXED_POS', - 'elk.layered.layering.layerConstraint': 'FIRST_SEPARATE' + 'elk.layered.layering.layerConstraint': boundaryMargin(node) === 'EAST' ? 'LAST_SEPARATE' : 'FIRST_SEPARATE' }; } @@ -259,7 +266,7 @@ export class WorkflowLayoutConfigurator extends AbstractLayoutConfigurator { return { 'elk.nodeSize.constraints': 'FIXED_SIZE', 'elk.portConstraints': 'FIXED_POS', - 'elk.layered.layering.layerConstraint': 'LAST_SEPARATE' + 'elk.layered.layering.layerConstraint': boundaryMargin(node) === 'WEST' ? 'FIRST_SEPARATE' : 'LAST_SEPARATE' }; } @@ -319,8 +326,14 @@ export class WorkflowLayoutConfigurator extends AbstractLayoutConfigurator { * Configure port side based on direction and let ELK handle label placement. */ protected override portOptions(port: GPort): LayoutOptions | undefined { - const direction = (port as any).args?.['cal:portDirection'] as string | undefined; - const side = direction === 'output' ? 'EAST' : 'WEST'; + // The side the producer placed the port on wins over its direction + // (`cal:portSide`, a bundle's `ready` beside its `valid`); this is the + // side ELK routes to, so it has to be the one the anchor and the + // glyph use, or a wire reaches the port from across its own node. + // Without one, the direction decides exactly as before. + const args = (port as any).args as Record | undefined; + const direction = args?.['cal:portDirection'] as string | undefined; + const side = normalizePortSide(args?.[WorkflowDiagramMetadata.PORT_SIDE]) ?? (direction === 'output' ? 'EAST' : 'WEST'); return { 'elk.port.side': side, diff --git a/packages/diagram-server/src/server/storage-runtime-options.ts b/packages/diagram-server/src/server/storage-runtime-options.ts index 26609bf..ab10f37 100644 --- a/packages/diagram-server/src/server/storage-runtime-options.ts +++ b/packages/diagram-server/src/server/storage-runtime-options.ts @@ -26,6 +26,15 @@ export interface StorageRuntimeOptions { * describes an entry instead, and the platform builds it. */ entityPaletteItems?: EntityPaletteItemSpec[]; + /** + * Whether the platform's own Entities entries (either of the two lists + * above) are offered. Default `true`. `false` leaves the Entities category + * to {@link entityPaletteItems} alone: a product whose nodes are none of + * the platform's kinds would otherwise offer entries that mean nothing in + * it, and the platform naming that product's kinds instead is the thing a + * neutral platform does not do. + */ + platformEntityItems?: boolean; /** * The node families the product recognises — see `NodeFamilySpec`. * The class is derived from each annotation, so core names none. diff --git a/packages/diagram-server/src/server/tool-palette-provider.ts b/packages/diagram-server/src/server/tool-palette-provider.ts index 3c67ede..c3bcdaa 100644 --- a/packages/diagram-server/src/server/tool-palette-provider.ts +++ b/packages/diagram-server/src/server/tool-palette-provider.ts @@ -60,10 +60,14 @@ export class WorkflowToolPaletteItemProvider extends ToolPaletteItemProvider { } const useAlternateEntityPalette = this.storageOptions?.useAlternateEntityPalette ?? false; + const platformEntityItems = this.storageOptions?.platformEntityItems ?? true; const items: PaletteItem[] = []; - // Create Entities category - const entityItems: PaletteItem[] = useAlternateEntityPalette + // Create Entities category: the platform's own entries unless the + // product's nodes are none of these kinds, then the product's alone. + const entityItems: PaletteItem[] = !platformEntityItems + ? [] + : useAlternateEntityPalette ? [ this.createNodeItem(WorkflowDiagramTypes.NODE_TASK, 'Actor', 'Create an Actor instance', 'symbol-class'), this.createNodeItem(WorkflowDiagramTypes.NODE_WORKFLOW, 'Network', 'Create a Network instance', 'symbol-namespace') diff --git a/packages/diagram-server/test/elk-routes-to-the-producers-side.test.ts b/packages/diagram-server/test/elk-routes-to-the-producers-side.test.ts new file mode 100644 index 0000000..79d63a9 --- /dev/null +++ b/packages/diagram-server/test/elk-routes-to-the-producers-side.test.ts @@ -0,0 +1,122 @@ +/** + * A wire reaches a port from the side the producer put it on. + * + * A handshake's `ready` answers backwards: the boundary output sits in the + * west margin beside the `valid` it answers, its glyph faces east, and the + * wire from the instance should arrive from the east in one straight run. + * ELK was told the side from the direction alone, so it routed the wire + * around the far west and in through the node, across the label. This pins + * the route ELK gives, on the layout the server really runs. + */ +import { describe, expect, it } from 'vitest'; +import ELK from 'elkjs/lib/elk.bundled.js'; +import { GEdge, GGraph, GNode } from '@eclipse-glsp/server'; +import { DefaultElementFilter } from '@eclipse-glsp/layout-elk'; +import { WorkflowElkLayoutEngine } from '../src/server/elk-layout-engine'; +import { WorkflowLayoutConfigurator } from '../src/server/layout-configurator'; +import { GraphGModelSource, type PyGraphDocument } from '../src/model/graph-gmodel-source'; +import { convertToGModelRoot } from '../src/server/gmodel-convert'; + +function find(element: any, id: string): any { + if (!element) { + return undefined; + } + if (element.id === id) { + return element; + } + for (const child of element.children ?? []) { + const hit = find(child, id); + if (hit) { + return hit; + } + } + return undefined; +} + +function collect(element: any, cls: any, out: T[] = []): T[] { + if (element instanceof cls) { + out.push(element); + } + for (const child of element.children ?? []) { + collect(child, cls, out); + } + return out; +} + +const DOC: PyGraphDocument = { + version: '1.1', + graph: { + id: 'wf:top', + nodes: [ + { id: 'wf:input:valid', kind: 'wf-input', label: 'valid', scope: 'scope:root', + ports: [{ id: 'p:valid', name: 'valid', direction: 'out', type: 'logic', group: 's' }] }, + { id: 'wf:output:ready', kind: 'wf-output', label: 'ready', scope: 'scope:root', meta: { side: 'west' }, + ports: [{ id: 'p:ready', name: 'ready', direction: 'in', type: 'logic', side: 'east', group: 's' }] }, + { id: 'wf:input:m_ready', kind: 'wf-input', label: 'm_ready', scope: 'scope:root', meta: { side: 'east' }, + ports: [{ id: 'p:m_ready', name: 'm_ready', direction: 'out', type: 'logic', side: 'west', group: 'm' }] }, + { id: 'wf:output:m_valid', kind: 'wf-output', label: 'm_valid', scope: 'scope:root', + ports: [{ id: 'p:m_valid', name: 'm_valid', direction: 'in', type: 'logic', group: 'm' }] }, + { id: 'node:u', kind: 'instance', label: 'u', type: 'stage', scope: 'scope:root', + ports: [ + { id: 'p:u:in_valid', name: 'in_valid', direction: 'in', type: 'logic', group: 'in' }, + { id: 'p:u:in_ready', name: 'in_ready', direction: 'out', type: 'logic', group: 'in', side: 'west' }, + { id: 'p:u:out_valid', name: 'out_valid', direction: 'out', type: 'logic', group: 'out' }, + { id: 'p:u:out_ready', name: 'out_ready', direction: 'in', type: 'logic', group: 'out', side: 'east' } + ] } + ], + edges: [ + { id: 'e1', from: 'p:valid', to: 'p:u:in_valid', scope: 'scope:root' }, + { id: 'e2', from: 'p:u:in_ready', to: 'p:ready', scope: 'scope:root' }, + { id: 'e3', from: 'p:u:out_valid', to: 'p:m_valid', scope: 'scope:root' }, + { id: 'e4', from: 'p:m_ready', to: 'p:u:out_ready', scope: 'scope:root' } + ] + } +}; + +async function layout(): Promise { + const root = convertToGModelRoot(new GraphGModelSource({} as any).transform(DOC) as any) as unknown as GGraph; + const modelState: any = { + root, + index: { get: (id: string) => find(root, id), findByClass: (id: string) => find(root, id) } + }; + const engine = new WorkflowElkLayoutEngine( + () => new (ELK as any)({ algorithms: ['layered'] }), + new DefaultElementFilter(), + new WorkflowLayoutConfigurator(), + modelState + ); + await engine.layout(); + return root; +} + +describe('a ready wire reaches its boundary port from the side the glyph faces', () => { + it('west margin: the wire arrives from the east, and the port column is one', async () => { + const root = await layout(); + const nodes = collect(root, GNode); + const ready = nodes.find(n => n.id === 'ready')!; + const valid = nodes.find(n => n.id === 'valid')!; + const u = nodes.find(n => n.id === 'u')!; + expect(Math.abs(ready.position.x - valid.position.x)).toBeLessThanOrEqual(10); // one column, west margin + expect(u.position.x).toBeGreaterThan(ready.position.x + ready.size.width); + const edge = collect(root, GEdge).find(e => e.targetId === 'ready_port_ready')!; + const route = ((edge as any).routingPoints ?? []) as Array<{ x: number; y: number }>; + const glyphTip = ready.position.x + ready.size.width; + // Every point of the route is east of the glyph: nothing runs across the node. + expect(route.length).toBeGreaterThanOrEqual(2); + for (const p of route) { + expect(p.x).toBeGreaterThanOrEqual(glyphTip - 1); + } + }); + + it('east margin: the wire leaves westward', async () => { + const root = await layout(); + const nodes = collect(root, GNode); + const mReady = nodes.find(n => n.id === 'm_ready')!; + const edge = collect(root, GEdge).find(e => e.sourceId === 'm_ready_port_m_ready')!; + const route = ((edge as any).routingPoints ?? []) as Array<{ x: number; y: number }>; + expect(route.length).toBeGreaterThanOrEqual(2); + for (const p of route) { + expect(p.x).toBeLessThanOrEqual(mReady.position.x + 1); + } + }); +}); diff --git a/packages/diagram-server/test/entity-palette-contributions.test.ts b/packages/diagram-server/test/entity-palette-contributions.test.ts index ed92491..e0434c5 100644 --- a/packages/diagram-server/test/entity-palette-contributions.test.ts +++ b/packages/diagram-server/test/entity-palette-contributions.test.ts @@ -44,6 +44,20 @@ describe('entity palette contributions', () => { expect(contributed.actions[0].args).toEqual({ streamblocksNode: true }); }); + it('offers the product’s entries alone when it says the platform’s are not its kinds', async () => { + const entities = await entitiesOf(providerWith({ + settingsNamespace: 'hw', + operationPrefix: 'hw', + useAlternateEntityPalette: true, + platformEntityItems: false, + entityPaletteItems: [{ elementTypeId: 'node:actor', label: 'Module Instance', description: 'x' }] + })); + expect(entities.map((i: any) => i.label)).toEqual(['Module Instance']); + // Ports and connections are still the platform's to offer. + const items = await providerWith({ settingsNamespace: 'hw', operationPrefix: 'hw', platformEntityItems: false }).getItems(); + expect(items.map((i: any) => i.id)).toEqual(['palette-entities', 'palette-ports', 'palette-connections']); + }); + it('keeps the platform’s own entries', async () => { const entities = await entitiesOf(providerWith({ settingsNamespace: 'wfpy', diff --git a/packages/diagram-server/test/port-anchor.test.ts b/packages/diagram-server/test/port-anchor.test.ts index 3cc08cd..ba47db1 100644 --- a/packages/diagram-server/test/port-anchor.test.ts +++ b/packages/diagram-server/test/port-anchor.test.ts @@ -24,6 +24,17 @@ describe('port anchor', () => { })).toEqual({ x: 100, y: 203.5, side: 'WEST' }); }); + it('lets the producer\'s side win over the direction', () => { + // A handshake's `ready` is an output placed on the west edge with its bundle. + expect(portAnchor({ + absolute: { x: 100, y: 200 }, size: { width: 9, height: 7 }, direction: 'output', side: 'WEST' + })).toEqual({ x: 100, y: 203.5, side: 'WEST' }); + expect(portSide({ absolute: { x: 0, y: 0 }, direction: 'input', side: 'east' })).toBe('EAST'); + // Anything that is not a side is ignored, and the direction decides. + expect(portSide({ absolute: { x: 0, y: 0 }, direction: 'input', side: 'NORTH' })).toBe('WEST'); + expect(portSide({ absolute: { x: 0, y: 0 }, direction: 'output', side: undefined })).toBe('EAST'); + }); + it('falls back to the shared constants when size is missing or zero', () => { const missing = portAnchor({ absolute: { x: 0, y: 0 }, direction: 'output' }); const zero = portAnchor({ absolute: { x: 0, y: 0 }, size: { width: 0, height: 0 }, direction: 'output' }); diff --git a/packages/diagram-server/test/port-bundle-sides.test.ts b/packages/diagram-server/test/port-bundle-sides.test.ts new file mode 100644 index 0000000..1cff311 --- /dev/null +++ b/packages/diagram-server/test/port-bundle-sides.test.ts @@ -0,0 +1,136 @@ +/** + * A producer may place a port with its bundle rather than by its direction: + * a handshake's `ready` beside the `valid` and `data` it answers, so the wire + * between two nodes runs straight instead of looping around both. The port + * says `side`; the boundary node says `meta.side`. The direction is untouched. + */ +import { describe, expect, it } from 'vitest'; +import { BoundaryPortGeometry, WorkflowDiagramConstants, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { GraphGModelSource, type PyGraphDocument, type PyGraphNode } from '../src/model/graph-gmodel-source'; +import { WorkflowLayoutConfigurator } from '../src/server/layout-configurator'; + +function transform(nodes: PyGraphNode[]): any[] { + const doc: PyGraphDocument = { version: '1', graph: { id: 'root', nodes, edges: [] } }; + return new GraphGModelSource().transform(doc).graph.children ?? []; +} + +const PORT_W = WorkflowDiagramConstants.PORT_WIDTH_PX; +const ROW_H = WorkflowDiagramConstants.PORT_HEIGHT_PX + WorkflowDiagramConstants.PORT_ROW_VERTICAL_GAP_PX; +const Y0 = WorkflowDiagramConstants.HEADER_HEIGHT_PX + WorkflowDiagramConstants.PORT_AREA_PADDING_PX; + +describe('ELK is told the side the producer placed the port on', () => { + // The configurator's port options are the only side ELK sees. Derived + // from the direction alone, a west-placed output was routed to as an east + // port: the wire came around the far side of the margin and crossed the + // whole node, label and all, to reach the glyph. + // The transform yields plain objects, so call the port rule directly rather than through `apply`'s instanceof dispatch. + const elkSide = (port: any): string | undefined => (new WorkflowLayoutConfigurator() as any).portOptions(port)?.['elk.port.side'] as string | undefined; + + it('for an instance port with a side', () => { + const node = transform([{ + id: 'node:c', kind: 'instance', label: 'c', scope: 'root', + ports: [ + { id: 'p:c:ready', name: 's_ready', direction: 'out', side: 'west' }, + { id: 'p:c:q', name: 'q', direction: 'out' } + ] + }])[0]; + expect(elkSide(node.children.find((c: any) => c.id === 'c_port_s_ready'))).toBe('WEST'); + expect(elkSide(node.children.find((c: any) => c.id === 'c_port_q'))).toBe('EAST'); + }); + + it('for a boundary node placed in the other margin', () => { + const [west, east] = transform([ + { id: 'wf:output:ready', kind: 'wf-output', label: 'ready', scope: 'root', meta: { side: 'west' }, + ports: [{ id: 'p:ready', name: 'ready', direction: 'in' }] }, + { id: 'wf:input:m_ready', kind: 'wf-input', label: 'm_ready', scope: 'root', meta: { side: 'east' }, + ports: [{ id: 'p:m_ready', name: 'm_ready', direction: 'out' }] } + ]); + expect(elkSide(west.children.find((c: any) => c.type.startsWith('port')))).toBe('EAST'); + expect(elkSide(east.children.find((c: any) => c.type.startsWith('port')))).toBe('WEST'); + }); +}); + +describe('port side from the producer', () => { + const consumer: PyGraphNode = { + id: 'node:c', kind: 'instance', label: 'c', scope: 'root', + ports: [ + { id: 'p:c:valid', name: 's_valid', direction: 'in', group: 's' }, + { id: 'p:c:ready', name: 's_ready', direction: 'out', group: 's', side: 'west' }, + { id: 'p:c:data', name: 's_data', direction: 'in', group: 's', width: 16 }, + { id: 'p:c:q', name: 'q', direction: 'out' } + ] + }; + + it('puts an output with side west on the west edge, in the west rows, still an output', () => { + const node = transform([consumer])[0]; + const ready = node.children.find((c: any) => c.id === 'c_port_s_ready'); + expect(ready.type).toBe(WorkflowDiagramTypes.PORT_OUTPUT); + expect(ready.args[WorkflowDiagramMetadata.PORT_DIRECTION]).toBe('output'); + expect(ready.args[WorkflowDiagramMetadata.PORT_SIDE]).toBe('WEST'); + expect(ready.args[WorkflowDiagramMetadata.PORT_GROUP]).toBe('s'); + expect(ready.layoutOptions['elk.port.side']).toBe('WEST'); + expect(ready.position).toEqual({ x: -PORT_W, y: Y0 + 1 * ROW_H }); + // Its label reads inward from the west edge, like its neighbours'. + expect(ready.children[0].position.x).toBe(PORT_W + 1); + expect(ready.children[0].cssClasses).toContain('input-port-label'); + // The west column is valid, ready, data; the east column is q alone. + const data = node.children.find((c: any) => c.id === 'c_port_s_data'); + const q = node.children.find((c: any) => c.id === 'c_port_q'); + expect(data.position).toEqual({ x: -PORT_W, y: Y0 + 2 * ROW_H }); + expect(q.position.y).toBe(Y0); + expect(q.position.x).toBe(node.size.width); + }); + + it('leaves a port without a side exactly as before', () => { + const node = transform([consumer])[0]; + const valid = node.children.find((c: any) => c.id === 'c_port_s_valid'); + expect(valid.args[WorkflowDiagramMetadata.PORT_SIDE]).toBeUndefined(); + expect(valid.layoutOptions['elk.port.side']).toBe('WEST'); + const q = node.children.find((c: any) => c.id === 'c_port_q'); + expect(q.args[WorkflowDiagramMetadata.PORT_SIDE]).toBeUndefined(); + expect(q.args[WorkflowDiagramMetadata.PORT_GROUP]).toBeUndefined(); + expect(q.layoutOptions['elk.port.side']).toBe('EAST'); + }); + + it('sizes the node for the fuller column', () => { + const [node] = transform([consumer]); + const threeRows = Y0 + 3 * ROW_H - WorkflowDiagramConstants.PORT_ROW_VERTICAL_GAP_PX + + WorkflowDiagramConstants.PORT_HEIGHT_PX + WorkflowDiagramConstants.PORT_AREA_PADDING_PX; + expect(node.size.height).toBeGreaterThanOrEqual(threeRows); + }); +}); + +describe('boundary node placed with its bundle', () => { + const tready: PyGraphNode = { + id: 'wf:output:s_axis_tready', kind: 'wf-output', label: 's_axis_tready', scope: 'root', + ports: [{ id: 'port:wf:output:s_axis_tready:in', name: 's_axis_tready', direction: 'in', group: 's_axis', side: 'east' }], + meta: { side: 'west' } + }; + const tdata: PyGraphNode = { + id: 'wf:output:m_axis_tdata', kind: 'wf-output', label: 'm_axis_tdata', scope: 'root', + ports: [{ id: 'port:wf:output:m_axis_tdata:in', name: 'm_axis_tdata', direction: 'in' }] + }; + + it('moves an output to the west margin, its port facing the diagram', () => { + const [node] = transform([tready]); + expect(node.type).toBe(WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT); + expect(node.args[WorkflowDiagramMetadata.BOUNDARY_SIDE]).toBe('WEST'); + expect(node.layoutOptions['elk.layered.layering.layerConstraint']).toBe('FIRST_SEPARATE'); + const port = node.children.find((c: any) => c.type === WorkflowDiagramTypes.PORT_INPUT); + expect(port.args[WorkflowDiagramMetadata.PORT_DIRECTION]).toBe('input'); + expect(port.args[WorkflowDiagramMetadata.PORT_SIDE]).toBe('EAST'); + expect(port.layoutOptions['elk.port.side']).toBe('EAST'); + expect(port.position.x).toBe(node.size.width - BoundaryPortGeometry.glyphWidth(false)); + }); + + it('is placed in the first layer by the configurator, and an ordinary output in the last', () => { + const [moved, plain] = transform([tready, tdata]); + const configurator = new WorkflowLayoutConfigurator(); + expect((configurator as any).nodeOptions(moved)['elk.layered.layering.layerConstraint']).toBe('FIRST_SEPARATE'); + expect((configurator as any).nodeOptions(plain)['elk.layered.layering.layerConstraint']).toBe('LAST_SEPARATE'); + expect(plain.args[WorkflowDiagramMetadata.BOUNDARY_SIDE]).toBeUndefined(); + const plainPort = plain.children.find((c: any) => c.type === WorkflowDiagramTypes.PORT_INPUT); + expect(plainPort.position.x).toBe(0); + expect(plainPort.args[WorkflowDiagramMetadata.PORT_SIDE]).toBeUndefined(); + }); +}); diff --git a/packages/diagram-server/test/source-file-fallback.test.ts b/packages/diagram-server/test/source-file-fallback.test.ts new file mode 100644 index 0000000..406cfdf --- /dev/null +++ b/packages/diagram-server/test/source-file-fallback.test.ts @@ -0,0 +1,101 @@ +/** + * A location without a file is in the graph's own file. + * + * A hierarchy export of a large netlist repeated one absolute path on every + * node, port and edge of every graph, a third of the export. A producer may + * leave the file out where it is the graph's `meta.file`; these pin that the + * reader puts it back everywhere a location is read, that a location naming + * another file keeps it, and that a document with every file spelled out, or + * with no graph file to fall back on, reads exactly as before. + */ + +import { describe, expect, it } from 'vitest'; +import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { GraphGModelSource, resolveSourceFiles, type PyGraphDocument } from '../src/model/graph-gmodel-source'; + +const OWN = '/rtl/top.sv'; +const OWN_URI = 'file:///rtl/top.sv'; + +function doc(withFile: boolean, locationFile?: string): PyGraphDocument { + const at = (line: number) => ({ line, ...(locationFile ? { file: locationFile } : {}) }); + return { + version: '1.1', + graph: { + id: 'wf:top', + meta: withFile ? { file: OWN } : {}, + nodes: [ + { + id: 'wf:input:x', kind: 'wf-input', label: 'x', scope: 'scope:root', + meta: { source: at(3) }, + ports: [{ id: 'port:wf:input:x:out', name: 'x', direction: 'out', type: 'logic', source: at(3) }] + }, + { + id: 'node:u', kind: 'instance', label: 'u_add', type: 'adder', scope: 'scope:root', + meta: { source: at(10), referencedSource: at(2) }, + ports: [{ id: 'port:node:u:a', name: 'a', direction: 'in', type: 'logic', source: at(4) }] + } + ], + edges: [ + { id: 'e1', from: 'port:wf:input:x:out', to: 'port:node:u:a', scope: 'scope:root', source: at(11) } + ] + } + }; +} + +function transform(d: PyGraphDocument): any[] { + return new GraphGModelSource().transform(d).graph.children ?? []; +} + +const uriOf = (element: any): string | undefined => element?.args?.[WorkflowDiagramMetadata.REFERENCED_URI]; +const find = (children: any[], id: string): any => { + for (const child of children) { + if (child.id === id) { + return child; + } + const nested = find(child.children ?? [], id); + if (nested) { + return nested; + } + } + return undefined; +}; + +describe('a location without a file', () => { + it('is read in the graph\'s own file: node, port, edge and boundary', () => { + const children = transform(doc(true)); + const instance = find(children, 'u_add'); + expect(uriOf(instance)).toBe(OWN_URI); + expect(instance.args[WorkflowDiagramMetadata.SOURCE_RANGE].start.line).toBe(9); + expect(instance.args[WorkflowDiagramMetadata.REFERENCED_SOURCE_RANGE].start.line).toBe(1); + const port = find(children, 'u_add')?.children?.find((c: any) => c.args?.[WorkflowDiagramMetadata.PORT_NAME] === 'a'); + expect(uriOf(port)).toBe(OWN_URI); + const edge = children.find(c => c.type?.startsWith('edge')); + expect(uriOf(edge)).toBe(OWN_URI); + const boundary = find(children, 'x'); + expect(JSON.stringify(boundary)).toContain(OWN_URI); + }); + + it('keeps a file it names itself', () => { + const resolved = resolveSourceFiles(doc(true, '/rtl/other.sv')); + expect(resolved.graph.nodes[1].meta?.['source']).toEqual({ line: 10, file: '/rtl/other.sv' }); + expect(uriOf(find(transform(doc(true, '/rtl/other.sv')), 'u_add'))).toBe('file:///rtl/other.sv'); + }); + + it('stays where it is when the graph names no file: no navigation, as before', () => { + const d = doc(false); + expect(resolveSourceFiles(d)).toBe(d); + const instance = find(transform(d), 'u_add'); + expect(uriOf(instance)).toBeUndefined(); + }); + + it('returns the same document when nothing was missing, and a new one otherwise, leaving the input alone', () => { + const complete = doc(true, OWN); + expect(resolveSourceFiles(complete)).toBe(complete); + const sparse = doc(true); + const before = JSON.stringify(sparse); + const resolved = resolveSourceFiles(sparse); + expect(resolved).not.toBe(sparse); + expect(JSON.stringify(sparse)).toBe(before); + expect(resolved.graph.edges[0].source).toEqual({ line: 11, file: OWN }); + }); +}); diff --git a/packages/extension-core/src/api.ts b/packages/extension-core/src/api.ts index 0b3ce9f..57b4763 100644 --- a/packages/extension-core/src/api.ts +++ b/packages/extension-core/src/api.ts @@ -97,6 +97,9 @@ export interface DiagramStorageOptions { /** Extra Entities-palette entries this product contributes. Plain data; the * platform builds the actual palette item. */ entityPaletteItems?: EntityPaletteItemSpec[]; + /** Whether the platform's own Entities entries are offered beside them. + * Default `true`; `false` leaves the Entities category to the product's. */ + platformEntityItems?: boolean; nodeFamilies?: NodeFamilySpec[]; /** How nested workflows are navigated; in place, a nested view's layout is * the root's, per instance (diagram-server `layout-target.ts`). */ diff --git a/packages/shared/src/diagram-types.ts b/packages/shared/src/diagram-types.ts index be95f0a..ad077a7 100644 --- a/packages/shared/src/diagram-types.ts +++ b/packages/shared/src/diagram-types.ts @@ -276,6 +276,22 @@ export namespace WorkflowDiagramMetadata { export const PORT_INDEX = 'cal:portIndex'; export const PORT_DIRECTION = 'cal:portDirection'; export const IS_INPUT_PORT = 'cal:isInputPort'; + /** + * Which side of its node the port sits on, `'WEST'` or `'EAST'`, when the + * producer places it with its bundle rather than by its direction: a + * handshake's `ready` beside the `valid` and `data` it answers, so the wire + * between two nodes runs straight instead of looping around both. Absent, + * the side follows the direction. The direction itself is untouched. + */ + export const PORT_SIDE = 'cal:portSide'; + /** The bundle the port belongs to (`s_axis`), a producer's fact. */ + export const PORT_GROUP = 'cal:portGroup'; + /** + * On a boundary node: `'WEST'` or `'EAST'` when the producer places it with + * its bundle rather than by its direction (an output `ready` beside the + * input bundle it belongs to). The glyph, the text and the port mirror. + */ + export const BOUNDARY_SIDE = 'cal:boundarySide'; export const IS_ARRAY_PORT = 'cal:isArrayPort'; export const ARRAY_SIZE = 'cal:arraySize'; diff --git a/packages/shared/src/port-anchor.ts b/packages/shared/src/port-anchor.ts index 86bd4bf..8ca9a33 100644 --- a/packages/shared/src/port-anchor.ts +++ b/packages/shared/src/port-anchor.ts @@ -25,6 +25,8 @@ export interface PortAnchorGeometry { size?: { width?: number; height?: number }; /** `cal:portDirection`, when the model carries it. */ direction?: string; + /** `cal:portSide`, when the producer placed the port with its bundle. Wins over `direction`. */ + side?: string; /** The element type, used only when `direction` is missing or unrecognised. */ type?: string; } @@ -35,15 +37,29 @@ export interface PortAnchor { side: PortSide; } +/** `'WEST'` or `'EAST'` from a producer's value in either case; anything else is nothing. */ +export function normalizePortSide(value: unknown): PortSide | undefined { + if (typeof value !== 'string') { + return undefined; + } + const upper = value.toUpperCase(); + return upper === 'WEST' || upper === 'EAST' ? upper : undefined; +} + /** * Which side of its node a port sits on. * - * `direction` wins when it is one of the two values the model uses; otherwise - * fall back to the type string. An unrecognised port is treated as an input, - * matching the model's own default. + * An explicit `side` wins: it is the producer placing the port with its bundle + * (see `WorkflowDiagramMetadata.PORT_SIDE`). Then `direction`, when it is one + * of the two values the model uses; otherwise fall back to the type string. An + * unrecognised port is treated as an input, matching the model's own default. */ export function portSide(geometry: PortAnchorGeometry): PortSide { const { direction, type } = geometry; + const explicit = normalizePortSide(geometry.side); + if (explicit) { + return explicit; + } if (direction === 'output') { return 'EAST'; } diff --git a/packages/sidecar-toolkit/src/diagram-openability.ts b/packages/sidecar-toolkit/src/diagram-openability.ts index 9ff846b..071b7d9 100644 --- a/packages/sidecar-toolkit/src/diagram-openability.ts +++ b/packages/sidecar-toolkit/src/diagram-openability.ts @@ -32,8 +32,11 @@ export interface DiagramOpenabilityConfig { /** Every extension it handles, for a language with several (`['.sv', '.v']`); * `sourceExtension` alone when absent. */ sourceExtensions?: string[]; - /** Decorator name the fallback checker scans for (e.g. `network`/`workflow`). */ - decoratorName: string; + /** Decorator name the fallback checker scans for (e.g. `network`/`workflow`). + * Absent, there is no fallback: an undecidable probe answers *unknown* and + * the platform opens the file, which is right for a language whose files + * carry no decorator to scan for. */ + decoratorName?: string; } export type DiagramOpenabilityProbe = ( @@ -123,7 +126,7 @@ export async function shouldOpenDiagram( requestedName?: string, probe: DiagramOpenabilityProbe = probeDiagramOpenability, definitionChecker: DecoratedDefinitionChecker = hasRequestedDecoratedDefinition -): Promise { +): Promise { if (targetUri.scheme !== 'file' || !hasSourceExtension(targetUri.fsPath, cfg)) { return false; } @@ -133,6 +136,10 @@ export async function shouldOpenDiagram( return probeResult; } + if (cfg.decoratorName === undefined) { + return undefined; + } + try { return await definitionChecker(targetUri.fsPath, cfg.decoratorName, requestedName); } catch { diff --git a/packages/sidecar-toolkit/src/server/graph-payload-to-doc.ts b/packages/sidecar-toolkit/src/server/graph-payload-to-doc.ts index 4506fd2..c452e9f 100644 --- a/packages/sidecar-toolkit/src/server/graph-payload-to-doc.ts +++ b/packages/sidecar-toolkit/src/server/graph-payload-to-doc.ts @@ -131,6 +131,13 @@ export function graphPayloadToDoc(graphPayload: Record): GraphD name: String(port?.name ?? ''), direction: (port?.direction === 'in' ? 'in' : 'out') as 'in' | 'out', type: typeof port?.type === 'string' ? port.type : undefined, + // A producer's facts about the port, passed through as the + // document schema has them: what it is for, the bundle it + // travels with and the side that bundle puts it on, its width. + ...(typeof port?.role === 'string' ? { role: port.role } : {}), + ...(typeof port?.side === 'string' ? { side: port.side } : {}), + ...(typeof port?.group === 'string' ? { group: port.group } : {}), + ...(typeof port?.width === 'number' ? { width: port.width } : {}), source: port?.location && typeof port.location === 'object' ? { file: typeof port.location.file === 'string' ? port.location.file : undefined, diff --git a/packages/sidecar-toolkit/src/server/sidecar-model-source.ts b/packages/sidecar-toolkit/src/server/sidecar-model-source.ts index 2626c77..b275e7e 100644 --- a/packages/sidecar-toolkit/src/server/sidecar-model-source.ts +++ b/packages/sidecar-toolkit/src/server/sidecar-model-source.ts @@ -86,14 +86,21 @@ export class SidecarModelSource implements DiagramModelSource { constructor(private readonly cfg: SidecarRuntimeConfig) {} - /** Language-specific source-analysis capabilities, delegating to the moved analysis module. */ + /** Language-specific source-analysis capabilities, delegating to the moved + * analysis module. With `navigation: 'none'` the two that walk the + * workspace's Python files answer empty: for a runtime whose "used by" comes + * from its own hierarchy export, a `.py` scan finds nothing and costs a walk. */ readonly analysis: GraphSourceAnalysis = { findPackageRootForFile, pickDefaultWorkflowName, resolveWorkflowDefinitionRange, analyzeWorkflowRelationships, - collectSourceFilesUnderRoots: collectPythonFilesUnderRoots, - discoverCrossFileWorkflowCallers, + collectSourceFilesUnderRoots: this.cfg.navigation === 'none' + ? async () => [] + : collectPythonFilesUnderRoots, + discoverCrossFileWorkflowCallers: this.cfg.navigation === 'none' + ? async () => [] + : discoverCrossFileWorkflowCallers, buildOverlayAstPathCandidates: buildViewerOverlayAstPathCandidates, buildOverlaySignatureCandidates: buildViewerOverlaySignatureCandidates, buildOverlayNodeIdentityCandidates: buildViewerOverlayNodeIdentityCandidates, @@ -155,9 +162,21 @@ export class SidecarModelSource implements DiagramModelSource { * returned `graph`. */ attachDiagnostics(result: SidecarModelSourceResult, rawPayload?: unknown): SidecarModelSourceResult { - const payload = rawPayload && typeof rawPayload === 'object' + let payload = rawPayload && typeof rawPayload === 'object' ? rawPayload as Record : (result.graph && typeof result.graph === 'object' ? result.graph as Record : {}); + // A document's own `errors` sit beside `graph`, not in it, so the walk + // over the graph never sees them: on the CLI path a Python runtime's + // load errors stay in the overlay, and diagnostics-parity.test.ts pins + // that. A compiler's errors are a different matter, and a product that + // says so gets them in the Problems panel with their file and line. + if (this.cfg.documentErrorsToProblems && !rawPayload) { + const docErrors = Array.isArray((result as { errors?: unknown }).errors) ? (result as { errors: unknown[] }).errors : []; + if (docErrors.length > 0) { + const graphErrors = Array.isArray(payload.errors) ? payload.errors : []; + payload = { ...payload, errors: [...docErrors, ...graphErrors] }; + } + } return { ...result, diagnostics: this.normalizeGraphDiagnostics(payload) }; } diff --git a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts index 6378c6f..139a509 100644 --- a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts +++ b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts @@ -200,6 +200,16 @@ export interface SidecarRuntimeConfig { cliCommandSettingKey: string; cliCommandDefault: string; cliPythonModule?: string; + /** `'none'` turns off the Python caller scan (`discoverCrossFileWorkflowCallers`) + * and the `.py` workspace walk behind it; default `'python'`. */ + navigation?: 'python' | 'none'; + /** + * `true` publishes a graph document's own `errors` (the compiler's, at the + * document root) to the Problems panel on a successful load, with their + * file, line and column. Default `false`: a Python runtime's document + * errors stay in the diagram's overlay, as they always have. + */ + documentErrorsToProblems?: boolean; operationKinds: { createEntityPort: string; deleteEntityPort: string }; /** Op prefixes this runtime accepts when rewriting foreign-prefixed ops * (replaces the hardcoded product-prefix literal check). */ diff --git a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts index 9663534..379b65c 100644 --- a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts +++ b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts @@ -170,6 +170,13 @@ export interface SidecarProfileInput { supportsElementCreation?: boolean; /** Extra Entities-palette entries this product contributes; plain data. */ entityPaletteItems?: EntityPaletteItemSpec[]; + /** + * Whether the platform's own Entities entries are offered beside + * `entityPaletteItems`. Default `true`. A product whose vocabulary is not + * the platform's (a hardware module is neither a task nor an actor) sets + * `false` and the palette's Entities are its own entries alone. + */ + platformEntityItems?: boolean; nodeFamilies?: NodeFamilySpec[]; // Chat edit backend. @@ -189,8 +196,25 @@ export interface SidecarProfileInput { newContainer: { label: string; decorator: string; importLine: string }; identifierNoun?: string; - // Openability (omit for "always openable"). - openability?: { probeOp?: string; decoratorName: string }; + // Openability (omit for "always openable"). With only `probeOp`, the + // sidecar's answer decides and an undecidable probe opens the file. + openability?: { probeOp?: string; decoratorName?: string }; + + /** + * How cross-file navigation targets are resolved. `'python'` (the default) + * uses the Python symbol providers and a `.py` workspace search, and scans + * sibling Python files for callers; `'none'` resolves nothing itself: the + * runtime emits absolute source locators and the platform navigates to them. + */ + navigation?: 'python' | 'none'; + + /** + * Whether a loaded document's own `errors` go to the Problems panel with + * their file, line and column. A compiler's errors belong there; a Python + * runtime's recoverable load errors stay in the diagram's overlay. Default + * `false`. + */ + documentErrorsToProblems?: boolean; // Run driver. runOutputDirSettingKey: string; @@ -224,6 +248,8 @@ export interface SidecarProfileInput { fullName: string; skill?: string; nodeCommands?: Array<{ command: string; nodeType: string; description: string }>; + /** The MIME type the agent receives the source file as. Default `text/x-python`. */ + sourceMimeType?: string; }; // Diagram source-file watch globs (default derived from the source extensions). @@ -258,7 +284,9 @@ function sidecarRuntimeConfig(input: SidecarProfileInput): SidecarRuntimeConfig undoLabelSuffix: input.undoLabelSuffix, createNodeStrings: input.createNodeStrings, createNodeBehavior: input.createNodeBehavior, - createNodeVariants: input.createNodeVariants + createNodeVariants: input.createNodeVariants, + navigation: input.navigation, + documentErrorsToProblems: input.documentErrorsToProblems }; } @@ -453,11 +481,14 @@ export function createSidecarDiagramProfile(input: SidecarProfileInput) { operationPrefix: input.sidecarOperationPrefix, useAlternateEntityPalette: input.useAlternateEntityPalette, entityPaletteItems: input.entityPaletteItems, + platformEntityItems: input.platformEntityItems, nodeFamilies: input.nodeFamilies, nestedNavigation: input.clientBehavior?.nestedNavigation }, watch: { globs: input.watchGlobs ?? sourceExtensions.map(ext => `**/*${ext}`) }, - navigation: createPythonNavigationProvider(), + // No provider at all for a product that resolves nothing itself: the + // platform then navigates straight to the referenced target. + navigation: input.navigation === 'none' ? undefined : createPythonNavigationProvider(), canOpenSource, editBackend, // GLSP-MCP opt-in (0.5.0): sidecar-backed products ship it ON by default. @@ -473,8 +504,8 @@ export function createSidecarDiagramProfile(input: SidecarProfileInput) { nodeCommands: input.chat.nodeCommands, // Read-only sidecar-registry tools bridged to GLSP-MCP by the platform adapter. tools: chatTools, - // The libcst edit backend rewrites Python source; agents get the file as text/x-python. - sourceMimeType: 'text/x-python', + // What the agent receives the file as; Python unless the product says. + sourceMimeType: input.chat.sourceMimeType ?? 'text/x-python', // The ACP connector: the setting the chat and the property panel // read. Without it the chat keeps its opencode default. acpConnector: input.acpConnectorSettingKey && input.acpConnectorFiles diff --git a/packages/sidecar-toolkit/test/diagram-openability.test.ts b/packages/sidecar-toolkit/test/diagram-openability.test.ts index 2eb8149..f395f3d 100644 --- a/packages/sidecar-toolkit/test/diagram-openability.test.ts +++ b/packages/sidecar-toolkit/test/diagram-openability.test.ts @@ -41,6 +41,19 @@ describe('shouldOpenDiagram', () => { expect(definitionChecker).toHaveBeenCalledWith(targetUri.fsPath, 'network', 'top_level'); }); + it('answers unknown, not no, when there is no decorator to scan for and the probe cannot say', async () => { + const cfg: DiagramOpenabilityConfig = { ...OPENABILITY_CONFIG, decoratorName: undefined }; + const definitionChecker = vi.fn(async () => false); + await expect( + shouldOpenDiagram(vscode.Uri.file('/rtl/top.py'), cfg, undefined, vi.fn(async () => undefined), definitionChecker) + ).resolves.toBeUndefined(); + expect(definitionChecker).not.toHaveBeenCalled(); + // The probe's answer still decides when it has one. + await expect( + shouldOpenDiagram(vscode.Uri.file('/rtl/top.py'), cfg, undefined, vi.fn(async () => false), definitionChecker) + ).resolves.toBe(false); + }); + it('accepts any of several source extensions and nothing else', async () => { const cfg: DiagramOpenabilityConfig = { ...OPENABILITY_CONFIG, sourceExtension: '.sv', sourceExtensions: ['.sv', '.v'] }; expect(hasSourceExtension('/rtl/top.sv', cfg)).toBe(true); diff --git a/packages/sidecar-toolkit/test/document-errors-to-problems.test.ts b/packages/sidecar-toolkit/test/document-errors-to-problems.test.ts new file mode 100644 index 0000000..71fbc0b --- /dev/null +++ b/packages/sidecar-toolkit/test/document-errors-to-problems.test.ts @@ -0,0 +1,56 @@ +/** + * A compiler's errors belong in the Problems panel. A graph document carries + * them at its root, beside `graph`, where the diagnostics walk over the graph + * never looks: on the CLI path a Python runtime's load errors stay in the + * overlay (diagnostics-parity.test.ts pins that). `documentErrorsToProblems` + * is the product's say that its document errors are a compiler's. + */ +import { describe, expect, it } from 'vitest'; +import { SidecarModelSource, type SidecarRuntimeConfig } from '../src/index'; +import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config'; + +const BASE: SidecarRuntimeConfig = { + settingsNamespace: 'hdl', + sidecarOperationPrefix: 'sv', + sidecarCommandSettingKey: 'svSidecarCommand', + sidecarCommandDefault: 'sv-sidecar', + cliCommandSettingKey: 'svCommand', + cliCommandDefault: 'hdl', + cliPythonModule: undefined, + operationKinds: { createEntityPort: 'op.createEntityPort', deleteEntityPort: 'op.deleteEntityPort' }, + ...NEUTRAL_CREATE_NODE_CONFIG, + acceptedOperationPrefixes: ['sv'], + graphAcquisition: 'cli-plan', + cliGraphArgs: (file: string) => ['plan', file, '--format', 'graph'] +}; + +const DOC = { + version: '1.1', + partial: true, + errors: [ + { message: "unknown module 'vendor_ip'", severity: 'error', file: '/rtl/wrapper.sv', line: 8, column: 3 }, + { message: 'implicit net', severity: 'warning', file: '/rtl/wrapper.sv', line: 9 } + ], + graph: { id: 'wf:wrapper', nodes: [], edges: [], subgraphs: [] } +} as any; + +describe('documentErrorsToProblems', () => { + it('leaves a document\'s root errors out of the diagnostics by default', () => { + const source = new SidecarModelSource(BASE); + expect(source.attachDiagnostics({ ...DOC }).diagnostics).toEqual([]); + }); + + it('publishes them with file, line, column and severity when the product asks', () => { + const source = new SidecarModelSource({ ...BASE, documentErrorsToProblems: true }); + expect(source.attachDiagnostics({ ...DOC }).diagnostics).toEqual([ + { severity: 'error', message: "unknown module 'vendor_ip'", uri: '/rtl/wrapper.sv', startLine: 8, startColumn: 3 }, + { severity: 'warning', message: 'implicit net', uri: '/rtl/wrapper.sv', startLine: 9 } + ]); + }); + + it('does not double up errors that already came through a raw payload', () => { + const source = new SidecarModelSource({ ...BASE, documentErrorsToProblems: true }); + const raw = { nodes: [], edges: [], errors: DOC.errors }; + expect(source.attachDiagnostics({ ...DOC }, raw).diagnostics).toHaveLength(2); + }); +}); diff --git a/packages/sidecar-toolkit/test/graph-payload-port-facts.test.ts b/packages/sidecar-toolkit/test/graph-payload-port-facts.test.ts new file mode 100644 index 0000000..5f9a5da --- /dev/null +++ b/packages/sidecar-toolkit/test/graph-payload-port-facts.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import { SidecarModelSource, type SidecarRuntimeConfig } from '../src/index'; +import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config'; + +const CFG: SidecarRuntimeConfig = { + settingsNamespace: 'wfLang', + sidecarOperationPrefix: 'wfpy', + sidecarCommandSettingKey: 'wfpySidecarCommand', + sidecarCommandDefault: 'wfpy-sidecar', + cliCommandSettingKey: 'wfpyCommand', + cliCommandDefault: 'wfpy', + cliPythonModule: undefined, + operationKinds: { createEntityPort: 'op.createEntityPort', deleteEntityPort: 'op.deleteEntityPort' }, + ...NEUTRAL_CREATE_NODE_CONFIG, + acceptedOperationPrefixes: ['wfpy'], + graphAcquisition: 'sidecar-export' +}; + +describe('graph payload port facts', () => { + it('passes a port\'s role, bundle, side and width through to the document', () => { + const doc = new SidecarModelSource(CFG).graphPayloadToDoc({ + network: 'top', + nodes: [{ + id: 'node:scope:root:u', kind: 'instance', label: 'u', scope: 'scope:root', + ports: [ + { id: 'p:valid', name: 's_valid', direction: 'in', type: 'logic', group: 's', width: 1 }, + { id: 'p:ready', name: 's_ready', direction: 'out', type: 'logic', group: 's', side: 'west' }, + { id: 'p:sda', name: 'sda', direction: 'out', type: 'wire', role: 'bidirectional' }, + { id: 'p:plain', name: 'q', direction: 'out', type: 'logic', side: 42, width: 'wide' } + ], + metadata: {} + }], + edges: [] + }) as any; + const ports = doc.graph.nodes[0].ports; + expect(ports[0]).toMatchObject({ name: 's_valid', direction: 'in', group: 's', width: 1 }); + expect(ports[0].side).toBeUndefined(); + expect(ports[1]).toMatchObject({ name: 's_ready', direction: 'out', group: 's', side: 'west' }); + expect(ports[2]).toMatchObject({ name: 'sda', role: 'bidirectional' }); + // Values of the wrong type are dropped, not passed along. + expect(ports[3].side).toBeUndefined(); + expect(ports[3].width).toBeUndefined(); + }); +}); diff --git a/packages/sidecar-toolkit/test/navigation-none-analysis.test.ts b/packages/sidecar-toolkit/test/navigation-none-analysis.test.ts new file mode 100644 index 0000000..f405d10 --- /dev/null +++ b/packages/sidecar-toolkit/test/navigation-none-analysis.test.ts @@ -0,0 +1,42 @@ +/** + * A runtime that resolves navigation itself gets no Python caller scan. + * + * `discoverCrossFileWorkflowCallers` walks the workspace's `.py` files for + * callers of a workflow; for a product whose "used by" comes from its own + * hierarchy export, that walk finds nothing and costs a directory scan on + * every load. `navigation: 'none'` on the runtime config answers empty instead. + */ +import { describe, expect, it } from 'vitest'; +import { SidecarModelSource } from '../src/server/sidecar-model-source'; +import type { SidecarRuntimeConfig } from '../src/server/sidecar-runtime-config'; + +const BASE: SidecarRuntimeConfig = { + settingsNamespace: 'p', + sidecarOperationPrefix: 'p', + sidecarCommandSettingKey: 'sidecarCommand', + sidecarCommandDefault: 'p-sidecar', + cliCommandSettingKey: 'cliCommand', + cliCommandDefault: 'p', + operationKinds: { createEntityPort: 'p.createEntityPort', deleteEntityPort: 'p.deleteEntityPort' }, + acceptedOperationPrefixes: ['p'], + graphAcquisition: 'sidecar-export', + undoLabelSuffix: ' (p)', + createNodeStrings: { + newTypeNamePrompt: () => '', typeLabel: () => '', classNamePlaceholder: () => '', + sidecarDisplayName: '', invalidCapabilitiesResponse: '', missingCapabilities: () => '', invalidListResponse: () => '' + }, + createNodeBehavior: { capabilityProbeBeforeCreate: false, mergeProjectDiscoveredTypes: false, surfaceSidecarListErrors: false } +}; + +describe('navigation: none', () => { + it('answers the caller scan and the source walk with nothing', async () => { + const source = new SidecarModelSource({ ...BASE, navigation: 'none' }); + await expect(source.analysis.discoverCrossFileWorkflowCallers('/x/a.sv', 'top', ['/x/b.py'])).resolves.toEqual([]); + await expect(source.analysis.collectSourceFilesUnderRoots(['/x'], '/x/a.sv')).resolves.toEqual([]); + }); + + it('leaves the Python scan in place by default', () => { + const source = new SidecarModelSource(BASE); + expect(source.analysis.discoverCrossFileWorkflowCallers.name).toBe('discoverCrossFileWorkflowCallers'); + }); +}); diff --git a/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts b/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts index 278105b..c818087 100644 --- a/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts +++ b/packages/sidecar-toolkit/test/sidecar-diagram-profile.test.ts @@ -125,6 +125,24 @@ describe('createSidecarDiagramProfile', () => { expect(createSidecarDiagramProfile(baseInput({ sourceExtensions: [' ', ''] })).sourceExtensions).toEqual(['.py']); }); + it('drops the Python navigation provider, and carries the palette and MIME choices, when the product asks', () => { + const python = createSidecarDiagramProfile(baseInput()); + expect(python.navigation).toBeDefined(); + expect(python.chat?.sourceMimeType).toBe('text/x-python'); + expect(python.storageOptions?.platformEntityItems).toBeUndefined(); + + const other = createSidecarDiagramProfile(baseInput({ + navigation: 'none', + platformEntityItems: false, + entityPaletteItems: [{ elementTypeId: 'node:actor', label: 'Module Instance', description: 'x' }], + chat: { name: 'hw', fullName: 'HW', sourceMimeType: 'text/x-systemverilog' } + })); + expect(other.navigation).toBeUndefined(); + expect(other.chat?.sourceMimeType).toBe('text/x-systemverilog'); + expect(other.storageOptions?.platformEntityItems).toBe(false); + expect(other.storageOptions?.entityPaletteItems?.map(i => i.label)).toEqual(['Module Instance']); + }); + it('turns on resuming at a step exactly when the product says how', () => { const without = createSidecarDiagramProfile(baseInput({ clientBehavior: { noneSentinel: 'x' } })); expect((without.clientBehavior as any)?.resumeAtStep).toBeUndefined();