From bddecda8bf7ae3a25b3bb2cad400aadd0a71839e Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Fri, 4 Sep 2026 15:04:08 +0200 Subject: [PATCH] fix: do not call a complete graph partial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The warning chip prefixed its message with "Partial graph:" whenever the producer reported any error, whether or not the graph was actually partial. Those are opposite claims. A graph that is missing nodes and a graph that faithfully draws something faulty need different reactions, and the wrong label sends a reader hunting for content that was never missing instead of reading the problem being reported. The second case is also the more common one: a producer able to describe a fault precisely usually had no trouble drawing the thing. The prefix now follows `wf:partial`, which is the flag that actually means the graph is incomplete. When the graph is whole the message stands on its own — it already says what is wrong, and the chip's styling already says it is a warning. `isPartialGraph` keeps its meaning of "is there anything to warn about", because the centre overlay keys on it and that reading is right for deciding whether to warn at all. The new value is the narrower fact, and only the wording follows it. The wording moved into a small exported function so it could be tested directly. It is one line of string arithmetic, and it is also the whole difference between telling a reader their picture is missing something and telling them their subject has a fault. --- packages/diagram-client/src/navigation-ui.ts | 46 ++++++++++++++++--- .../test/graph-warning-text.test.ts | 41 +++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 packages/diagram-client/test/graph-warning-text.test.ts diff --git a/packages/diagram-client/src/navigation-ui.ts b/packages/diagram-client/src/navigation-ui.ts index 45cbc1c..75bc139 100644 --- a/packages/diagram-client/src/navigation-ui.ts +++ b/packages/diagram-client/src/navigation-ui.ts @@ -57,12 +57,45 @@ type RunOption = { running?: boolean; }; +/** + * What the warning chip says. + * + * "Partial graph" only when the graph really is partial. Every error used to + * earn that prefix, which made a complete and correct picture of something + * faulty announce itself as an incomplete picture. Those are opposite claims, + * and the wrong one sends a reader hunting for missing nodes instead of reading + * the problem being reported — the likelier case by far, since a producer able + * to describe a fault precisely usually had no trouble drawing it. + * + * When the graph is whole the message stands on its own: it already says what + * is wrong, and the chip's styling already says it is a warning. + * + * Exported for its own test. It is one line of string arithmetic, and it is + * also the whole difference between telling a reader their picture is missing + * something and telling them their subject has a fault. + */ +export function graphWarningText(messages: string[], graphIsTruncated: boolean): string { + const first = messages[0] ?? 'Graph export completed with recoverable errors.'; + const extra = Math.max(0, messages.length - 1); + const suffix = extra > 0 ? ` (+${extra} more)` : ''; + return graphIsTruncated ? `Partial graph: ${first}${suffix}` : `${first}${suffix}`; +} + type RenderMeta = { available: string[]; availableRuns: RunOption[]; entryWorkflows: string[]; graphErrors: GraphLoadError[]; isPartialGraph: boolean; + /** + * Whether the graph itself is incomplete, as opposed to complete but + * describing something that has problems. + * + * `isPartialGraph` above is "is there anything to warn about" and stays + * that, because the overlay keys on it. This is the narrower fact, and it + * is the one the wording has to follow. + */ + graphIsTruncated: boolean; parentWorkflows: ParentWorkflowOption[]; renderableNodeCount: number; runtimeProfile?: string; @@ -208,7 +241,8 @@ export class WorkflowNavigationUi { const availableRuns = this.parseRunOptions(args['wf:availableRuns']); const entryWorkflows = Array.isArray(args['wf:rootWorkflows']) ? args['wf:rootWorkflows'] : []; const graphErrors = this.parseGraphLoadErrors(args['wf:errors']); - const isPartialGraph = args['wf:partial'] === true || graphErrors.length > 0; + const graphIsTruncated = args['wf:partial'] === true; + const isPartialGraph = graphIsTruncated || graphErrors.length > 0; const renderableNodeCount = this.countRenderableNodes(root); const parentWorkflows = this.parseParentWorkflowOptions(args['wf:parentWorkflows'], sourceUri); const selected = args['wf:selectedWorkflow'] ?? args['cal:networkName']; @@ -258,6 +292,7 @@ export class WorkflowNavigationUi { entryWorkflows, graphErrors, isPartialGraph, + graphIsTruncated, parentWorkflows, renderableNodeCount, runtimeProfile, @@ -389,11 +424,10 @@ export class WorkflowNavigationUi { document.body.appendChild(chip); this.graphWarningEl = chip; } - const firstMessage = meta.graphErrors[0]?.message ?? 'Graph export completed with recoverable errors.'; - const extraCount = Math.max(0, meta.graphErrors.length - 1); - this.graphWarningEl.textContent = extraCount > 0 - ? `Partial graph: ${firstMessage} (+${extraCount} more)` - : `Partial graph: ${firstMessage}`; + this.graphWarningEl.textContent = graphWarningText( + meta.graphErrors.map(error => error.message), + meta.graphIsTruncated + ); this.graphWarningEl.title = meta.graphErrors.length > 0 ? meta.graphErrors.map(error => this.describeGraphLoadError(error)).join('\n') : 'Graph export completed with recoverable errors.'; diff --git a/packages/diagram-client/test/graph-warning-text.test.ts b/packages/diagram-client/test/graph-warning-text.test.ts new file mode 100644 index 0000000..6d4d93c --- /dev/null +++ b/packages/diagram-client/test/graph-warning-text.test.ts @@ -0,0 +1,41 @@ +/** + * What the warning chip claims, and what it must not. + * + * A graph that is incomplete and a graph that faithfully draws something faulty + * are opposite situations, and the chip used to describe both as "Partial + * graph". The wrong one sends a reader hunting for missing nodes rather than + * reading the problem being reported — and that is the likelier case, since a + * producer able to describe a fault precisely usually had no trouble drawing + * it. + */ + +import { describe, expect, it } from 'vitest'; +import { graphWarningText } from '../src/navigation-ui'; + +describe('the graph warning chip', () => { + it('says "partial" when the graph really is', () => { + expect(graphWarningText(['half the nodes are missing'], true)).toBe( + 'Partial graph: half the nodes are missing' + ); + }); + + it('does not, when the graph is whole and the subject has a fault', () => { + // The message already says what is wrong and the chip already looks + // like a warning, so a prefix claiming the picture is incomplete only + // adds something untrue. + expect(graphWarningText(['detected a cycle'], false)).toBe('detected a cycle'); + }); + + it('counts the rest either way', () => { + expect(graphWarningText(['one', 'two', 'three'], false)).toBe('one (+2 more)'); + expect(graphWarningText(['one', 'two'], true)).toBe('Partial graph: one (+1 more)'); + }); + + it('still says something when there is no message to show', () => { + // Reached when the producer reports an error it cannot describe. A + // blank chip would be worse than a vague one. + expect(graphWarningText([], false)).toBe( + 'Graph export completed with recoverable errors.' + ); + }); +});