From f8df462ae864248d3cb846b4a8451c60b70c5786 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Thu, 3 Sep 2026 18:14:23 +0200 Subject: [PATCH 1/2] feat: a third navigation target, for a diagram whose file nobody wrote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cal:referencedUri` answers two questions at once — drill into this element, and show me its source — and that works only for as long as both have the same answer. For a diagram generated from something else they come apart. Drilling has to stay inside the generated file, because that is where the definitions being drilled into live; "show me the source" wants the file a person actually typed. One slot means picking which of the two features to break, and the consumer that hit this picked drill-down and lost source navigation entirely. So a producer that knows both files can now say both: `meta.authoredSource` becomes its own pair of keys and its own context-menu item, and the existing two targets are untouched. Nothing here names a language or a toolchain. Any product with a compile step between what someone wrote and what the diagram is built from has this shape; the platform only learns that such a step can exist. Two deliberate refusals in the implementation. The authored target does NOT fall back to the diagram's own file the way the other two do — there, a missing range still means "somewhere in this document", whereas an absent authored file means the producer made no such claim, and offering the generated file under that label would misrepresent which file the reader is being sent to. And both a file and a line are required, because a half-populated locator navigates to the top of a file nobody asked for, which is worse than not offering the item. For every existing product this is inert: no `authoredSource`, no keys, no third menu item, and 850 tests plus all four neutrality gates pass unchanged. --- .../src/go-to-source-context-menu-provider.ts | 32 ++++++- .../src/model/graph-gmodel-source.ts | 17 ++++ .../test/authored-source.test.ts | 89 +++++++++++++++++++ packages/shared/src/diagram-types.ts | 25 ++++++ 4 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 packages/diagram-server/test/authored-source.test.ts diff --git a/packages/diagram-client/src/go-to-source-context-menu-provider.ts b/packages/diagram-client/src/go-to-source-context-menu-provider.ts index 7cfd0ae..f76af50 100644 --- a/packages/diagram-client/src/go-to-source-context-menu-provider.ts +++ b/packages/diagram-client/src/go-to-source-context-menu-provider.ts @@ -82,6 +82,29 @@ function sourceTargetFromElement(element: GModelElement | undefined, fallbackSou return undefined; } +/** + * Where this element was authored, when that is a third file. + * + * Deliberately does NOT fall back to anything. The other two targets fall + * through to the diagram's own file because a missing range there still means + * "somewhere in this document"; here an absent value means the producer did not + * claim a separate authored file, and offering the generated one under that + * label would be a lie about which file the reader is being sent to. + */ +function authoredTargetFromElement(element: GModelElement | undefined): NavigationTarget | undefined { + let current: GModelElement | undefined = element; + while (current) { + const args = (current as unknown as { args?: Args }).args; + const authoredUri = args?.[WorkflowDiagramMetadata.AUTHORED_URI]; + if (typeof authoredUri === 'string') { + const range = args?.[WorkflowDiagramMetadata.AUTHORED_SOURCE_RANGE]; + return isSerializedRange(range) ? { uri: authoredUri, range } : { uri: authoredUri }; + } + current = (current as unknown as { parent?: GModelElement }).parent; + } + return undefined; +} + function toNavigateAction(target: NavigationTarget): LabeledAction['actions'][number] { return NavigateToExternalTargetAction.create({ uri: target.uri, @@ -121,7 +144,8 @@ export class WorkflowGoToSourceContextMenuItemProvider implements IContextMenuIt } const declarationTarget = selected.length > 0 ? declarationTargetFromElement(selected[0], sourceUri) : undefined; const sourceTarget = selected.length > 0 ? sourceTargetFromElement(selected[0], sourceUri) : undefined; - if (!declarationTarget && !sourceTarget) { + const authoredTarget = selected.length > 0 ? authoredTargetFromElement(selected[0]) : undefined; + if (!declarationTarget && !sourceTarget && !authoredTarget) { if (debug) { // eslint-disable-next-line no-console console.log('[cal][context-menu] no navigation target (no menu items)'); @@ -147,6 +171,12 @@ export class WorkflowGoToSourceContextMenuItemProvider implements IContextMenuIt actions: [toNavigateAction(sourceTarget)] }); } + if (authoredTarget) { + items.push({ + label: 'Go to Authored Source', + actions: [toNavigateAction(authoredTarget)] + }); + } return items; } } diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index 0586c18..2c03a26 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -458,6 +458,23 @@ export class GraphGModelSource { [WorkflowDiagramMetadata.REFERENCED_URI]: referencedSourceMetaFile }; } + // Where a person wrote this, when the file on screen is not + // that file. See `AUTHORED_URI`: it is a third target rather + // than a competitor to the other two, because a generated + // diagram needs drill-down to stay in the generated file and + // still wants "go to source" to reach the source. + const authoredSourceMeta = node.meta['authoredSource'] as { file?: string; line?: number } | undefined; + const authoredSourceFile = normalizeNavigationFileUri(authoredSourceMeta?.file); + if (authoredSourceFile && authoredSourceMeta?.line) { + gnode.args = { + ...(gnode.args ?? {}), + [WorkflowDiagramMetadata.AUTHORED_URI]: authoredSourceFile, + [WorkflowDiagramMetadata.AUTHORED_SOURCE_RANGE]: { + start: { line: Math.max(0, authoredSourceMeta.line - 1), character: 0 }, + end: { line: Math.max(0, authoredSourceMeta.line - 1), character: 0 } + } + }; + } const referencedEntityName = node.meta['referencedEntityName']; if (typeof referencedEntityName === 'string' && referencedEntityName.trim() !== '') { gnode.args = { diff --git a/packages/diagram-server/test/authored-source.test.ts b/packages/diagram-server/test/authored-source.test.ts new file mode 100644 index 0000000..26c6692 --- /dev/null +++ b/packages/diagram-server/test/authored-source.test.ts @@ -0,0 +1,89 @@ +/** + * A third navigation target, for a diagram whose file nobody wrote. + * + * `cal:referencedUri` answers two questions at once — drill into this element, + * and show me its source — and that works only while both have the same answer. + * When a diagram is generated from something else they come apart: drilling has + * to stay inside the generated file, because that is where the definitions being + * drilled into live, while "show me the source" wants the file a person typed. + * One slot means choosing which of the two features to break. + * + * These pin the shape of the third slot, and — more importantly — that adding it + * changed nothing for a producer that does not use it. + */ + +import { describe, expect, it } from 'vitest'; +import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { GraphGModelSource, type PyGraphDocument, type PyGraphNode } from '../src/model/graph-gmodel-source'; + +function transform(nodes: PyGraphNode[]): any[] { + const doc: PyGraphDocument = { version: '1', graph: { id: 'root', nodes, edges: [] } }; + return new GraphGModelSource().transform(doc).graph.children ?? []; +} + +function nodeWith(meta: Record): any { + return transform([ + { + id: 'node:a', + kind: 'instance', + label: 'a', + scope: 'root', + ports: [], + meta + } + ]).find(child => child.id === 'a'); +} + +describe('an element authored in another file', () => { + it('carries the authored file and line as their own metadata', () => { + const node = nodeWith({ authoredSource: { file: '/work/src/Top.scala', line: 42 } }); + + expect(node.args[WorkflowDiagramMetadata.AUTHORED_URI]).toBe('file:///work/src/Top.scala'); + expect(node.args[WorkflowDiagramMetadata.AUTHORED_SOURCE_RANGE]).toEqual({ + start: { line: 41, character: 0 }, + end: { line: 41, character: 0 } + }); + }); + + it('does not disturb where drilling goes', () => { + // The whole point. `referencedSource` decides which document a + // drill-down opens, and an authored file must not become that document + // — a generated diagram would then try to open its own source text as a + // diagram, which is not one. + const node = nodeWith({ + referencedSource: { file: '/work/build/Top.fir', line: 3 }, + authoredSource: { file: '/work/src/Top.scala', line: 42 }, + referencedEntityName: 'Stage' + }); + + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBe('file:///work/build/Top.fir'); + expect(node.args[WorkflowDiagramMetadata.AUTHORED_URI]).toBe('file:///work/src/Top.scala'); + }); + + it('is absent when the producer claims no separate authored file', () => { + // Which is every existing product. An empty value here has to stay + // empty rather than falling back to the diagram's own file, or the menu + // would offer a third item that goes where the second one already does. + const node = nodeWith({ source: { file: '/work/Top.py', line: 7 } }); + + expect(node.args[WorkflowDiagramMetadata.AUTHORED_URI]).toBeUndefined(); + expect(node.args[WorkflowDiagramMetadata.AUTHORED_SOURCE_RANGE]).toBeUndefined(); + }); + + it('needs both a file and a line to be offered at all', () => { + // A half-populated locator is worse than none: it would navigate to the + // top of a file the reader did not ask for. + expect(nodeWith({ authoredSource: { file: '/work/src/Top.scala' } }) + .args[WorkflowDiagramMetadata.AUTHORED_URI]).toBeUndefined(); + expect(nodeWith({ authoredSource: { line: 42 } }) + .args[WorkflowDiagramMetadata.AUTHORED_URI]).toBeUndefined(); + }); + + it('resolves a relative path the way every other locator is resolved', () => { + // Not a new convention: the same normalisation the other two use, so a + // producer does not have to learn a second rule for this slot. + const node = nodeWith({ authoredSource: { file: 'src/Top.scala', line: 1 } }); + + expect(node.args[WorkflowDiagramMetadata.AUTHORED_URI]).toMatch(/^file:\/\/\/.*src\/Top\.scala$/); + }); +}); diff --git a/packages/shared/src/diagram-types.ts b/packages/shared/src/diagram-types.ts index d4d7b5d..488ebfe 100644 --- a/packages/shared/src/diagram-types.ts +++ b/packages/shared/src/diagram-types.ts @@ -225,6 +225,31 @@ export namespace WorkflowDiagramMetadata { /** Resolved referenced definition name (not alias text), used for robust drill-down. */ export const REFERENCED_ENTITY_NAME = 'cal:referencedEntityName'; + /** + * Where this element was AUTHORED, when that is a different file again. + * + * A third navigation target, because two were not enough for a diagram + * whose own file nobody wrote by hand. + * + * `REFERENCED_URI` already answers two questions at once — *drill into this + * element* and *show me its source* — and that works only while both have + * the same answer. For a diagram generated from something else they come + * apart: drilling has to stay inside the generated file, because that is + * where the definitions being drilled into live, while "show me the source" + * wants the file a person actually typed. Forcing both through one slot + * means choosing which of the two features to break. + * + * So a producer that knows both says so, and neither has to lose. Nothing + * here names a language or a toolchain: any product with a compile step + * between what someone wrote and what the diagram is built from has exactly + * this shape. + * + * Absent for a diagram whose file IS the authored one, which is the common + * case and behaves exactly as before. + */ + export const AUTHORED_URI = 'cal:authoredUri'; + export const AUTHORED_SOURCE_RANGE = 'cal:authoredSourceRange'; + // When an element refers to a definition in another file (e.g. actor/network type), // this range points into that referenced document. export const REFERENCED_SOURCE_RANGE = 'cal:referencedSourceRange'; From 0d513b90fe08f65bc7abb5eecab5a0acff5512ea Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Thu, 3 Sep 2026 19:36:52 +0200 Subject: [PATCH 2/2] feat: write on a connection what it carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dataflow diagram's edges are not interchangeable. One may carry a 32-bit value and the next a single bit; one may be a token-carrying channel and its neighbour a plain signal. Drawn identically, the only way to tell them apart is to click each one and read the property panel — which is fine for checking a suspicion and useless for forming one. `meta.label` is text the producer formatted, `meta.width` a bit count, and they describe the same thing at different resolutions. Both reach the model, because both are true and a property panel may want either; the view draws exactly one, preferring the label. Two captions on one line would add no information. Formatted by the producer and shown verbatim, for the same reason a port's type is: the platform re-deriving it from anything structural would drift from how the language actually writes it, and differ between products. Drawn above the line, and higher again when a badge is already there, since the queue depth and the open button occupy the midpoint and a caption landing on one makes both unreadable. The halo is `paint-order` rather than a filled pill: every edge that states a width gets one of these, so a pill would end up weighing as much as the edges do, whereas a halo costs nothing over empty space and only shows up where the text would otherwise be illegible against a line or a box. Inert for a producer that states neither. --- .../diagram-client/src/diagram-client.css | 24 ++++++ packages/diagram-client/src/views.ts | 61 +++++++++++++ .../src/model/graph-gmodel-source.ts | 4 + .../test/edge-carried-label.test.ts | 86 +++++++++++++++++++ packages/shared/src/diagram-types.ts | 10 +++ 5 files changed, 185 insertions(+) create mode 100644 packages/diagram-server/test/edge-carried-label.test.ts diff --git a/packages/diagram-client/src/diagram-client.css b/packages/diagram-client/src/diagram-client.css index ac89352..1dd407a 100644 --- a/packages/diagram-client/src/diagram-client.css +++ b/packages/diagram-client/src/diagram-client.css @@ -1047,6 +1047,30 @@ i.cal-toggle-feedback-edges.cal-feedback-hidden-icon { pointer-events: all; } +/* + * What a connection carries, written above it. + * + * A halo rather than a filled pill. Every edge that states a width or a payload + * gets one, so a pill would weigh as much as the edges themselves; + * `paint-order: stroke` costs nothing where the text crosses empty space and + * only shows up where it would otherwise be unreadable against a line or a box. + */ +.workflow-edge .edge-carried-label { + font-size: 9px; + font-family: var(--vscode-editor-font-family, monospace); + fill: var(--vscode-descriptionForeground, #8b949e); + stroke: var(--vscode-editor-background, #1e1e1e); + stroke-width: 3px; + paint-order: stroke fill; + pointer-events: none; + user-select: none; +} + +.workflow-edge.selected .edge-carried-label, +.workflow-edge.hover .edge-carried-label { + fill: var(--vscode-foreground, #cccccc); +} + .workflow-edge .edge-queue-badge { pointer-events: none; } diff --git a/packages/diagram-client/src/views.ts b/packages/diagram-client/src/views.ts index 504c548..be5f644 100644 --- a/packages/diagram-client/src/views.ts +++ b/packages/diagram-client/src/views.ts @@ -1633,6 +1633,22 @@ export class WorkflowEdgeView extends PolylineEdgeView { const viewerLastTokenText = this.formatViewerLastTokenForTooltip(viewerLastToken); const queueSizeRaw = edgeArgs?.[WorkflowDiagramMetadata.QUEUE_SIZE]; const queueSize = typeof queueSizeRaw === 'number' && Number.isFinite(queueSizeRaw) ? Math.max(0, Math.trunc(queueSizeRaw)) : undefined; + // What this connection carries, when the producer says. + // + // `EDGE_LABEL` is text the producer formatted — a payload type, a + // protocol name — and `EDGE_WIDTH` is a bit count. Both describe the + // same thing at different resolutions, so only one is drawn: the label + // when there is one, the width otherwise. Drawing both would put two + // captions on one line for no extra information. + const edgeLabelText = (edgeArgs?.[WorkflowDiagramMetadata.EDGE_LABEL] as string | undefined)?.trim(); + const edgeWidthRaw = edgeArgs?.[WorkflowDiagramMetadata.EDGE_WIDTH]; + const edgeWidth = typeof edgeWidthRaw === 'number' && Number.isFinite(edgeWidthRaw) + ? Math.max(0, Math.trunc(edgeWidthRaw)) + : undefined; + const carriedText = edgeLabelText && edgeLabelText !== '' + ? edgeLabelText + : edgeWidth !== undefined ? String(edgeWidth) : undefined; + const fromEntity = (edgeArgs?.['wf:from'] as string | undefined) ?? undefined; const toEntity = (edgeArgs?.['wf:to'] as string | undefined) ?? undefined; const outPort = (edgeArgs?.['wf:outPort'] as string | undefined) ?? undefined; @@ -1702,6 +1718,13 @@ export class WorkflowEdgeView extends PolylineEdgeView { }), ...(hasViewerToken ? this.renderEdgeOpenButton(workflowEdge as any, normalizedSegments as any) : []), ...(queueSize !== undefined ? this.renderEdgeQueueSizeBadge(normalizedSegments as any, queueSize, hasViewerToken) : []), + ...(carriedText + ? this.renderEdgeCarriedLabel( + normalizedSegments as any, + carriedText, + hasViewerToken || queueSize !== undefined + ) + : []), // Endpoint index labels for list/entity indexing (e.g. bf[0].In) ...(fromIndexLabel && fromIndexPos ? [svg('text', { @@ -2041,6 +2064,44 @@ export class WorkflowEdgeView extends PolylineEdgeView { ]; } + /** + * What a connection carries, written on it. + * + * A dataflow diagram's edges are not interchangeable: one may carry far + * more than the next, or something of an entirely different kind, and drawn + * identically the only way to tell them apart is to click each one. This is + * the smallest thing that fixes that. + * + * Placed ABOVE the line, and higher again when a badge is present, because + * those already occupy the midpoint and a caption landing on a queue depth + * makes both unreadable. The halo comes from `paint-order` in the + * stylesheet rather than a filled pill: every edge stating a width gets one + * of these, so a pill would weigh as much as the edges do, while a halo + * costs nothing over empty space and only appears where the text would + * otherwise be illegible. + */ + protected renderEdgeCarriedLabel( + segments: { x: number; y: number }[], + text: string, + stacked: boolean + ): VNode[] { + const mid = this.getRouteMidpoint(segments); + if (!mid) { + return []; + } + return [ + svg('text', { + class: { 'edge-carried-label': true }, + attrs: { + x: mid.x, + y: mid.y + (stacked ? -14 : -5), + 'text-anchor': 'middle', + 'dominant-baseline': 'auto' + } + }, text) + ]; + } + protected buildPath(segments: { x: number; y: number }[], cornerRadiusPx: number = 8): string { if (segments.length === 0) return ''; if (segments.length === 1) return `M ${segments[0].x},${segments[0].y}`; diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index 0586c18..3e2d6be 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -819,6 +819,10 @@ export class GraphGModelSource { if (typeof edgeWidth === 'number' && Number.isFinite(edgeWidth)) { (gedge.args as Record)[WorkflowDiagramMetadata.EDGE_WIDTH] = edgeWidth; } + const edgeLabel = edgeMeta?.['label']; + if (typeof edgeLabel === 'string' && edgeLabel.trim() !== '') { + (gedge.args as Record)[WorkflowDiagramMetadata.EDGE_LABEL] = edgeLabel.trim(); + } // Producer-declared feedback. Recorded per edge here; whether the // document uses declared or derived feedback is decided once, for // the whole graph, in `markFeedbackEdges`. diff --git a/packages/diagram-server/test/edge-carried-label.test.ts b/packages/diagram-server/test/edge-carried-label.test.ts new file mode 100644 index 0000000..34b6af5 --- /dev/null +++ b/packages/diagram-server/test/edge-carried-label.test.ts @@ -0,0 +1,86 @@ +/** + * What a connection carries, made visible. + * + * The server half of it: `meta.label` and `meta.width` reaching the model as + * args the edge view can draw. The view's own choice between them is asserted + * here too, because it is a rule about the DATA — a label and a width describe + * the same thing at different resolutions, so exactly one of them should ever + * reach a reader. + */ + +import { describe, expect, it } from 'vitest'; +import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { GraphGModelSource, type PyGraphDocument, type PyGraphEdge } from '../src/model/graph-gmodel-source'; + +function edgeWith(meta: Record | undefined): any { + const edges: PyGraphEdge[] = [ + { id: 'e', from: 'p:a:out', to: 'p:b:in', scope: 'root', ...(meta ? { meta } : {}) } + ]; + const doc: PyGraphDocument = { + version: '1', + graph: { + id: 'root', + nodes: [ + { + id: 'node:a', + kind: 'actor', + label: 'a', + scope: 'root', + ports: [{ id: 'p:a:out', name: 'out', direction: 'out' }] + }, + { + id: 'node:b', + kind: 'actor', + label: 'b', + scope: 'root', + ports: [{ id: 'p:b:in', name: 'in', direction: 'in' }] + } + ], + edges + } + }; + return new GraphGModelSource().transform(doc).graph.children?.find((child: any) => child.id === 'e'); +} + +describe('what a connection carries', () => { + it('reaches the model as text the producer formatted', () => { + // Displayed verbatim, for the same reason a port's type is: the + // platform re-deriving it from anything structural would drift from how + // the language writes it, and differ between products. + const edge = edgeWith({ label: 'Decoupled>' }); + + expect(edge.args[WorkflowDiagramMetadata.EDGE_LABEL]).toBe('Decoupled>'); + }); + + it('reaches the model as a width when that is all the producer has', () => { + const edge = edgeWith({ width: 32 }); + + expect(edge.args[WorkflowDiagramMetadata.EDGE_WIDTH]).toBe(32); + expect(edge.args[WorkflowDiagramMetadata.EDGE_LABEL]).toBeUndefined(); + }); + + it('carries both when the producer states both, and lets the view choose', () => { + // The precedence rule lives in the view, because it is a question about + // what to draw rather than about what is true. Both facts survive, so a + // property panel can still show the width of a labelled channel. + const edge = edgeWith({ label: 'Decoupled>', width: 32 }); + + expect(edge.args[WorkflowDiagramMetadata.EDGE_LABEL]).toBe('Decoupled>'); + expect(edge.args[WorkflowDiagramMetadata.EDGE_WIDTH]).toBe(32); + }); + + it('ignores a label that is only whitespace', () => { + // An empty caption is worse than none: it reserves space above the line + // and says nothing. + const edge = edgeWith({ label: ' ' }); + + expect(edge.args[WorkflowDiagramMetadata.EDGE_LABEL]).toBeUndefined(); + }); + + it('says nothing when the producer says nothing', () => { + const edge = edgeWith(undefined); + + expect(edge.args[WorkflowDiagramMetadata.EDGE_LABEL]).toBeUndefined(); + expect(edge.args[WorkflowDiagramMetadata.EDGE_WIDTH]).toBeUndefined(); + }); +}); diff --git a/packages/shared/src/diagram-types.ts b/packages/shared/src/diagram-types.ts index d4d7b5d..f09cc56 100644 --- a/packages/shared/src/diagram-types.ts +++ b/packages/shared/src/diagram-types.ts @@ -211,6 +211,16 @@ export namespace WorkflowDiagramMetadata { * producer's own words and takes precedence where both are given. */ export const EDGE_WIDTH = 'cal:edgeWidth'; + /** + * What a connection carries, in the producer's own words. + * + * Formatted by the producer and displayed verbatim, for the same reason a + * port's type is: the platform re-rendering it from anything structural + * would drift from how the language actually writes it, and differ between + * products. Takes precedence over {@link EDGE_WIDTH}, which describes the + * same thing at lower resolution. + */ + export const EDGE_LABEL = 'cal:edgeLabel'; /** What a connection is for, beyond carrying data. Producer-defined. */ export const EDGE_ROLE = 'cal:edgeRole'; /** What a port is for, beyond carrying data. Producer-defined. */