Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions docs/design/2026-09-dashboard-plugin-test-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -686,9 +686,9 @@ The semantic normalizer is `packages/rad-components/src/graphRecord.ts`. It emit
and edge meaning: ids, labels, types, the currently unpopulated icon/status semantics recorded as
the explicit `not-yet-populated` sentinel, resolved endpoints, and direction. It records no layout
coordinates. All fourteen records are committed under
`src/__fixtures__/graph-records/`; `graph-expected-changes.md` is empty. GU-22 rejects undeclared
`src/__fixtures__/graph-records/`; `graph-expected-changes.md` declares the two edge removals for #353. GU-22 rejects undeclared
record changes, GU-23 reports known defects whose own invariant is still violated as carried
forward, and GU-24 keeps the manifest empty between extraction phases.
forward, and GU-24 limits the manifest to these two corrections.

GU-22 alone is not sufficient, because it compares generated records against the committed ones: a
change that edits a record file and the renderer together satisfies it without consulting the
Expand Down Expand Up @@ -743,8 +743,10 @@ which is the argument for doing this before the extraction rather than after:
- An **unparseable connection id is not skipped**. The parse result only gates the direction
rewrite; the edge-building loop that follows runs over every connection regardless, so the
connection becomes an edge to a node that does not exist and the dependency vanishes from the
diagram silently (GU-05a).
- A **connection to a resource absent from the graph** produces the same dangling edge (GU-04).
diagram silently (GU-05a). Fixed for #353: connections with no matching node are omitted,
and the graph shows a warning with the owning resource and target IDs.
- A **connection to a resource absent from the graph** produced the same dangling edge (GU-04).
Fixed for #353 with the same endpoint check and visible warning.
- A **self-referential connection** produces a self-loop (GU-06a), and **duplicate resource ids**
produce duplicate node ids, one of which React Flow silently discards.

Expand Down Expand Up @@ -1043,7 +1045,7 @@ its issue is fixed, and that failure is the signal the fix landed, not a regress
| #41 | Selecting a graph node does not reveal dismissible resource details | GU-18 |
| #89 | Graph nodes do not expose deployment status | GU-21, GU-23 |
| #352 | `parseResourceId` rejects legal names and types; `ResourceLink` then throws | RU-02, AC-08, EC-08 |
| #353 | Graph silently drops connections whose target cannot be resolved | GU-04, GU-05a |
| #353 | Graph silently drops connections whose target cannot be resolved — **fixed; omitted connections are reported in the graph** | GU-04, GU-05a |
| #354 | `initialNodes` mutates the graph payload it is given | GU-05b |
| #355 | Graph layout state leaks between applications via a module-level Dagre graph — **fixed in this PR at maintainer request; GU-08 is now a positive invariant, not a characterization pin** | GU-08 (regression) |
| #356 | Cluster selection disagrees between `RadiusApi` and the graph request | CN-03, CN-04 |
Expand Down Expand Up @@ -1491,9 +1493,9 @@ expected-change manifest.
| GU-02 | A | Retained connections exactly match fixture-owned source/target multisets — **done**; invalid/self connections are separate defect pins |
| GU-02a | A | Redirected, reversed, missing, and extra edges fail the topology assertions — **done** |
| GU-03 | A | Every edge endpoint resolves to a node present in the same graph — **done** |
| GU-04 | A | A connection to a resource absent from the graph is dropped or stubbed, never left dangling — **done, KNOWN-DEFECT** |
| GU-04 | A | A connection to a resource absent from the graph is omitted and reported, never left dangling — **done, regression test for #353** |
| GU-05 | A | An unparseable connection does not drop its owning node — **done** |
| GU-05a | A | Unparseable connections should be skipped; today's dangling edge is a **KNOWN-DEFECT pin**, not desired behavior |
| GU-05a | A | A connection with an unparseable ID and no matching node is omitted and reported — **done, regression test for #353** |
| GU-05b| A | Building the model does not mutate the caller's graph — **done, KNOWN-DEFECT** |
| GU-06 | A | A self-referential connection produces no duplicate node and no self-loop — **done, KNOWN-DEFECT** |
| GU-07 | A | Building the same fixture twice yields the same model — **done**, including rendered remount determinism |
Expand All @@ -1518,7 +1520,7 @@ expected-change manifest.
| GU-23a | C | A defect clears only when its own invariant is repaired, not when an unrelated one is — **done** |
| GU-23b | C | Each defect predicate detects its own condition and reports repair on a healthy record — **done** |
| GU-23c | C | A defect naming a fixture with no record is rejected rather than silently ignored — **done** |
| GU-24 | C | The manifest is empty at the end of each extraction phase — **done** |
| GU-24 | C | The manifest contains only the two missing-endpoint corrections for #353 — **done** |
| GU-25 | C | A committed record edited relative to the base branch maps to a manifest entry — **done** |
| GU-25a | C | A mutated committed record is reported as an unapproved change — **done** |

Expand Down Expand Up @@ -1582,7 +1584,7 @@ names, element nesting, or coordinates of any kind.

The records are frozen under `packages/rad-components/src/__fixtures__/graph-records/` and diffed
in Phase 4 against `packages/rad-components/src/__fixtures__/graph-expected-changes.md`. The
manifest is currently empty. Fixtures tagged `KNOWN-DEFECT` declare the invariant they violate
manifest declares the two missing-endpoint edge removals for #353. Fixtures tagged `KNOWN-DEFECT` declare the invariant they violate
through `knownGraphDefects` in `graphRecord.ts`; GU-23 reports every defect whose invariant is still
violated, so only an actual repair retires an entry.

Expand Down
32 changes: 32 additions & 0 deletions packages/rad-components/e2e-tests/appGraph.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,38 @@ const graphContract = async (page: Page) => ({
});

test.describe('real AppGraph renderer', () => {
for (const [story, target] of [
[
'missing-target',
'/planes/radius/local/resourceGroups/demo/providers/Applications.Datastores/redisCaches/absent',
],
['unparseable-connection', 'not-a-resource-id'],
]) {
test(`GU-04 / GU-05a: reports ${story} without hiding the graph`, async ({
page,
}) => {
await page.goto(storyUrl(story));

const warning = page.getByRole('alert');
await expect(warning).toContainText('The graph is incomplete.');
await expect(node(page, 'webapp')).toBeVisible();
await expect(page.locator('.react-flow__edge')).toHaveCount(0);
const disclosure = warning.getByText('Show unresolved connections');
await disclosure.focus();
await page.keyboard.press('Enter');
await expect(warning.getByText(target, { exact: true })).toBeVisible();
await expect(
warning.getByText(
'/planes/radius/local/resourceGroups/demo/providers/Applications.Core/containers/webapp',
{ exact: true },
),
).toBeVisible();
await expect(
page.getByRole('button', { name: 'Fit View' }),
).toBeVisible();
});
}

test('GU-11: unmounting and remounting preserves positions without leaking scheduled work', async ({
page,
}) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ is updated. Clear the table at the end of each extraction phase.

| Fixture | Field | Old value | New value | Reason |
| ------- | ----- | --------- | --------- | ------ |
| missing-target | edges.0 | {"source":"/planes/radius/local/resourceGroups/demo/providers/Applications.Datastores/redisCaches/absent","target":"/planes/radius/local/resourceGroups/demo/providers/Applications.Core/containers/webapp","direction":"source-to-target"} | <absent> | #353: Omit the edge with a missing endpoint and report the connection in the graph warning. |
| unparseable-connection | edges.0 | {"source":"not-a-resource-id","target":"/planes/radius/local/resourceGroups/demo/providers/Applications.Core/containers/webapp","direction":"source-to-target"} | <absent> | #353: Omit the edge with a missing endpoint and report the connection in the graph warning. |
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,5 @@
"statusBadge": "not-yet-populated"
}
],
"edges": [
{
"source": "/planes/radius/local/resourceGroups/demo/providers/Applications.Datastores/redisCaches/absent",
"target": "/planes/radius/local/resourceGroups/demo/providers/Applications.Core/containers/webapp",
"direction": "source-to-target"
}
]
"edges": []
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,5 @@
"statusBadge": "not-yet-populated"
}
],
"edges": [
{
"source": "not-a-resource-id",
"target": "/planes/radius/local/resourceGroups/demo/providers/Applications.Core/containers/webapp",
"direction": "source-to-target"
}
]
"edges": []
}
48 changes: 9 additions & 39 deletions packages/rad-components/src/__test__/graphInvariants.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ const expectedRelationships: Record<string, Relationship[]> = {
],
),
'managed-cluster': [],
'missing-target': [],
'unparseable-connection': [],
'deploy-status-matrix': [],
'unknown-type': [],
'duplicate-ids': [],
Expand All @@ -110,13 +112,8 @@ const expectedRelationships: Record<string, Relationship[]> = {
),
};

const defectConnectionFixtures = [
'missing-target',
'unparseable-connection',
'self-reference',
];
const retainedConnectionFixtures = allFixtures.filter(
([name]) => !defectConnectionFixtures.includes(name),
([name]) => name !== 'self-reference',
);

const assertRelationships = (model: GraphModel, expected: Relationship[]) => {
Expand All @@ -140,7 +137,7 @@ describe('graph invariants', () => {
});

describe('GU-02: every retained connection yields exactly one edge', () => {
// Invalid/self connections are defect pins below, not desired topology.
// Self connections remain a separate defect pin below.
it.each(retainedConnectionFixtures)('%s', (name, fixture) => {
expect(expectedRelationships).toHaveProperty(name);
assertRelationships(
Expand Down Expand Up @@ -181,15 +178,7 @@ describe('graph invariants', () => {
});

describe('GU-03: every edge endpoint resolves to a node in the same graph', () => {
// `missing-target` and `unparseable-connection` are excluded and covered by
// GU-04 and GU-05a, which state the current behavior for endpoints that do
// not resolve.
const resolvable = allFixtures.filter(
([name]) =>
name !== 'missing-target' && name !== 'unparseable-connection',
);

it.each(resolvable)('%s', (_name, fixture) => {
it.each(allFixtures)('%s', (_name, fixture) => {
const model = buildGraphModel(load(fixture));
const ids = new Set(model.nodes.map(node => node.id));

Expand All @@ -200,19 +189,10 @@ describe('graph invariants', () => {
});
});

/**
* KNOWN-DEFECT: a connection whose target is not in the graph produces an
* edge pointing at a node that does not exist. React Flow drops such an edge
* silently, so a real dependency simply vanishes from the diagram with no
* error anywhere. The requirement is that it is dropped or stubbed
* deliberately; today it is neither.
*/
it('GU-04: KNOWN-DEFECT a connection to an absent resource leaves a dangling edge', () => {
it('GU-04: a connection to an absent resource produces no dangling edge', () => {
const model = buildGraphModel(load(missingTarget));
const ids = new Set(model.nodes.map(node => node.id));

expect(model.edges).toHaveLength(1);
expect(ids.has(model.edges[0].source)).toBe(false);
expect(model.edges).toHaveLength(0);
});

it('GU-05: an unparseable connection id does not drop its owning node', () => {
Expand All @@ -222,20 +202,10 @@ describe('graph invariants', () => {
expect(model.nodes[0].label).toBe('webapp');
});

/**
* KNOWN-DEFECT: the parse result is used only to decide whether to rewrite the
* direction; the edge-building loop that follows runs over every connection
* regardless. So an unparseable connection id is *not* skipped — it produces
* an edge to a node that does not exist, and the dependency disappears from
* the diagram with no error. GU-05 protects the owning node; this test separately
* pins the incorrect connection handling until it is fixed.
*/
it('GU-05a: KNOWN-DEFECT an unparseable connection still produces a dangling edge', () => {
it('GU-05a: an unparseable connection with no matching node produces no dangling edge', () => {
const model = buildGraphModel(load(unparseableConnection));
const ids = new Set(model.nodes.map(node => node.id));

expect(model.edges).toHaveLength(1);
expect(ids.has(model.edges[0].source)).toBe(false);
expect(model.edges).toHaveLength(0);
});

/**
Expand Down
21 changes: 11 additions & 10 deletions packages/rad-components/src/__test__/graphRecords.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,6 @@ describe('graph records', () => {
}
});

expect(
byFixture('missing-target').isPresent({
...healthy,
edges: [
{ source: 'a', target: 'absent', direction: 'source-to-target' },
],
}),
).toBe(true);
expect(
byFixture('self-reference').isPresent({
...healthy,
Expand Down Expand Up @@ -601,7 +593,16 @@ describe('graph records', () => {
).not.toEqual([]);
});

it('GU-24: keeps the checked-in expected-change manifest empty', () => {
expect(parseManifest(fs.readFileSync(manifestPath, 'utf8'))).toEqual([]);
it('GU-24: limits expected changes to the missing endpoint fixes for #353', () => {
expect(parseManifest(fs.readFileSync(manifestPath, 'utf8'))).toEqual(
['missing-target', 'unparseable-connection'].map(fixture => ({
fixture,
field: 'edges.0',
oldValue: expect.any(String),
newValue: '<absent>',
reason:
'#353: Omit the edge with a missing endpoint and report the connection in the graph warning.',
})),
);
});
});
75 changes: 62 additions & 13 deletions packages/rad-components/src/components/appgraph/AppGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,62 @@ const LayoutFlow = (props: { graph: AppGraphData }) => {
// - We're using Dagre for layout: https://reactflow.dev/learn/layouting/layouting#dagre

return (
<ReactFlow
defaultNodes={nodes}
defaultEdges={edges}
defaultEdgeOptions={{ type: 'bezier', animated: true }}
nodeTypes={nodeTypes}
onNodesChange={onNodesChange}
onEdgesChange={onEdgesChange}
fitView
>
<Controls showInteractive={false} />
</ReactFlow>
<>
{initial.unresolvedConnections.length > 0 && (
<div
role="alert"
style={{
maxHeight: '30%',
overflow: 'auto',
overflowWrap: 'anywhere',
}}
>
<strong>Some connections cannot be shown.</strong> Their target
resources are not in this graph. The graph is incomplete.
<details>
<summary>Show unresolved connections</summary>
<ul>
{initial.unresolvedConnections.map((connection, index) => (
<li key={index}>
Resource: <code>{connection.resourceId}</code>
<br />
Connection target: <code>{connection.connectionId}</code>
</li>
))}
</ul>
</details>
</div>
)}
<div style={{ flex: 1, minHeight: 0 }}>
<ReactFlow
defaultNodes={nodes}
defaultEdges={edges}
defaultEdgeOptions={{ type: 'bezier', animated: true }}
nodeTypes={nodeTypes}
onNodesChange={onNodesChange}
onEdgesChange={onEdgesChange}
fitView
>
<Controls showInteractive={false} />
</ReactFlow>
</div>
</>
);
};

export type AppGraphProps = { graph: AppGraphData };

function AppGraph(props: AppGraphProps) {
return (
<div {...props} style={{ height: '100%', width: '100%' }}>
<div
{...props}
style={{
height: '100%',
width: '100%',
display: 'flex',
flexDirection: 'column',
}}
>
<ReactFlowProvider>
<LayoutFlow graph={props.graph} />
</ReactFlowProvider>
Expand All @@ -94,9 +131,13 @@ const isResourceType = (type: string, types: Set<string>) =>
export function initialNodes(graph: AppGraphData): {
nodes: Node<Resource>[];
edges: Edge[];
unresolvedConnections: { resourceId: string; connectionId: string }[];
} {
const nodes: Node<Resource>[] = [];
const edges: Edge[] = [];
const unresolvedConnections: { resourceId: string; connectionId: string }[] =
[];
const resourceIds = new Set(graph.resources.map(resource => resource.id));

// Very simple layout scheme here for nodes.
const orderData: { [order: number]: number } = {};
Expand Down Expand Up @@ -142,6 +183,14 @@ export function initialNodes(graph: AppGraphData): {
}

for (const connection of resource.connections) {
if (!resourceIds.has(connection.id)) {
unresolvedConnections.push({
resourceId: resource.id,
connectionId: connection.id,
});
continue;
}

if (connection.direction === 'Inbound') {
edges.push({
id: `${connection.id}-${resource.id}`,
Expand All @@ -159,7 +208,7 @@ export function initialNodes(graph: AppGraphData): {
}
}

return { nodes, edges };
return { nodes, edges, unresolvedConnections };
}

export function getLayoutedElements(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import * as AppGraph from './AppGraph.stories.tsx';

AppGraph displays the topology of different applications.

If a connection target is not in the graph response, AppGraph shows a warning
that the graph is incomplete. Expand **Show unresolved connections** to see
the owning resource ID and the target ID for each omitted connection.
Resources and connections with both endpoints present remain visible.

#### Example: Demo Application

<Canvas of={AppGraph.Demo} />
Expand Down
Loading
Loading