Skip to content

Commit e7a4c40

Browse files
anandgupta42claude
andcommitted
feat: beautified Mermaid DAG — filter tests, visible by default, legend
- Filter out dbt test nodes (not_null_*, unique_*, accepted_values_*) from the Mermaid diagram — they clutter the graph - DAG is now VISIBLE by default (not collapsed in <details>) — it's the visual differentiator, show it proudly - Added color legend below the diagram - Improved Mermaid styling (thicker strokes, better contrast) - Node labels displayed using ["label"] syntax for readability - Shows "(N tests affected)" count in header for filtered tests Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 465f8dc commit e7a4c40

File tree

5 files changed

+180
-89
lines changed

5 files changed

+180
-89
lines changed

dist/index.js

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24309,61 +24309,78 @@ function buildSummaryTable(report) {
2430924309
}
2431024310
return rows.join("\n");
2431124311
}
24312+
function isTestNode(name) {
24313+
return /^(not_null|unique|accepted_values|relationships|dbt_utils|dbt_expectations)_/.test(name);
24314+
}
2431224315
function buildMermaidDAG(impact) {
24313-
const totalDownstream = impact.downstreamModels.length + impact.affectedExposures.length;
24316+
const filteredDownstream = impact.downstreamModels.filter(
24317+
(d) => !isTestNode(d)
24318+
);
24319+
const filteredExposures = impact.affectedExposures;
24320+
const totalVisible = filteredDownstream.length + filteredExposures.length;
24321+
const testCount = impact.downstreamModels.length - filteredDownstream.length;
2431424322
const lines = [];
24315-
lines.push("<details>");
2431624323
lines.push(
24317-
`<summary>\u{1F4CA} Blast Radius \u2014 ${totalDownstream} downstream ${totalDownstream === 1 ? "model" : "models"}</summary>`
24324+
`### \u{1F4CA} Blast Radius \u2014 ${totalVisible} downstream ${totalVisible === 1 ? "model" : "models"}${testCount > 0 ? ` (${testCount} tests affected)` : ""}`
2431824325
);
2431924326
lines.push("");
2432024327
lines.push("```mermaid");
2432124328
lines.push("graph LR");
2432224329
lines.push(
24323-
" classDef modified fill:#ff6b6b,stroke:#333,color:#fff"
24330+
" classDef modified fill:#ff6b6b,stroke:#c92a2a,color:#fff,stroke-width:2px"
2432424331
);
24325-
lines.push(" classDef downstream fill:#ffd93d,stroke:#333");
2432624332
lines.push(
24327-
" classDef exposure fill:#845ef7,stroke:#333,color:#fff"
24333+
" classDef downstream fill:#ffd43b,stroke:#e67700,color:#333,stroke-width:1px"
24334+
);
24335+
lines.push(
24336+
" classDef exposure fill:#845ef7,stroke:#5f3dc4,color:#fff,stroke-width:2px"
2432824337
);
2432924338
lines.push("");
2433024339
const sanitize = (name) => name.replace(/[^a-zA-Z0-9_]/g, "_");
2433124340
const modifiedSet = new Set(impact.modifiedModels);
24332-
const downstreamSet = new Set(impact.downstreamModels);
24333-
const exposureSet = new Set(impact.affectedExposures);
24341+
const exposureSet = new Set(filteredExposures);
24342+
const visibleNodes = /* @__PURE__ */ new Set([
24343+
...impact.modifiedModels,
24344+
...filteredDownstream,
24345+
...filteredExposures
24346+
]);
24347+
const edgesAdded = /* @__PURE__ */ new Set();
24348+
const addEdge = (from, to) => {
24349+
const key = `${from}-->${to}`;
24350+
if (edgesAdded.has(key)) return;
24351+
edgesAdded.add(key);
24352+
const fromId = sanitize(from);
24353+
const toId = sanitize(to);
24354+
const fromClass = modifiedSet.has(from) ? "modified" : exposureSet.has(from) ? "exposure" : "downstream";
24355+
const toClass = exposureSet.has(to) ? "exposure" : modifiedSet.has(to) ? "modified" : "downstream";
24356+
const fromLabel = from.replace(/^(stg_|int_|fct_|dim_|rpt_)/, (m) => m);
24357+
const toLabel = to.replace(/^(stg_|int_|fct_|dim_|rpt_)/, (m) => m);
24358+
lines.push(
24359+
` ${fromId}["${fromLabel}"]:::${fromClass} --> ${toId}["${toLabel}"]:::${toClass}`
24360+
);
24361+
};
2433424362
if (impact.edges && impact.edges.length > 0) {
2433524363
for (const edge of impact.edges) {
24336-
const fromId = sanitize(edge.from);
24337-
const toId = sanitize(edge.to);
24338-
const fromClass = modifiedSet.has(edge.from) ? "modified" : "downstream";
24339-
const toClass = exposureSet.has(edge.to) ? "exposure" : downstreamSet.has(edge.to) ? "downstream" : modifiedSet.has(edge.to) ? "modified" : "downstream";
24340-
lines.push(` ${fromId}:::${fromClass} --> ${toId}:::${toClass}`);
24364+
if (!visibleNodes.has(edge.from) || !visibleNodes.has(edge.to)) continue;
24365+
addEdge(edge.from, edge.to);
2434124366
}
2434224367
} else {
2434324368
for (const mod of impact.modifiedModels) {
24344-
const modId = sanitize(mod);
24345-
for (const ds of impact.downstreamModels) {
24346-
const dsId = sanitize(ds);
24347-
lines.push(` ${modId}:::modified --> ${dsId}:::downstream`);
24348-
}
24349-
for (const exp of impact.affectedExposures) {
24350-
const expId = sanitize(exp);
24351-
lines.push(` ${modId}:::modified --> ${expId}:::exposure`);
24369+
for (const ds of filteredDownstream) {
24370+
addEdge(mod, ds);
2435224371
}
2435324372
}
24354-
if (impact.downstreamModels.length > 0 && impact.affectedExposures.length > 0) {
24355-
for (const ds of impact.downstreamModels) {
24356-
const dsId = sanitize(ds);
24357-
for (const exp of impact.affectedExposures) {
24358-
const expId = sanitize(exp);
24359-
lines.push(` ${dsId}:::downstream --> ${expId}:::exposure`);
24360-
}
24373+
for (const ds of filteredDownstream) {
24374+
for (const exp of filteredExposures) {
24375+
addEdge(ds, exp);
2436124376
}
2436224377
}
2436324378
}
2436424379
lines.push("```");
2436524380
lines.push("");
24366-
lines.push("</details>");
24381+
lines.push(
24382+
`> \u{1F534} Modified \xA0\xA0 \u{1F7E1} Downstream \xA0\xA0 \u{1F7E3} Exposure${testCount > 0 ? ` \xA0\xA0 \u{1F9EA} ${testCount} tests also affected` : ""}`
24383+
);
2436724384
return lines.join("\n");
2436824385
}
2436924386
function buildIssuesSection(issues) {

dist/index.js.map

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/reporting/comment.ts

Lines changed: 80 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -241,86 +241,121 @@ export function buildSummaryTable(report: ReviewReport): string {
241241
return rows.join("\n");
242242
}
243243

244+
/**
245+
* Check if a node name looks like a dbt test (not a model/snapshot/exposure).
246+
* Test nodes follow patterns like: not_null_model_col, unique_model_col,
247+
* accepted_values_model_col__val1__val2, relationships_model_col__ref_other_
248+
*/
249+
function isTestNode(name: string): boolean {
250+
return /^(not_null|unique|accepted_values|relationships|dbt_utils|dbt_expectations)_/.test(name);
251+
}
252+
244253
/**
245254
* Build a Mermaid DAG blast radius diagram.
246-
* Modified models get red, downstream get yellow, exposures get purple.
255+
* - Modified models: red
256+
* - Downstream models: yellow
257+
* - Exposures: purple
258+
* - Test nodes are FILTERED OUT (too noisy)
259+
* - DAG is NOT collapsed — it's the visual differentiator
247260
*/
248261
export function buildMermaidDAG(impact: ImpactResult): string {
249-
const totalDownstream =
250-
impact.downstreamModels.length + impact.affectedExposures.length;
262+
// Filter out test nodes from downstream
263+
const filteredDownstream = impact.downstreamModels.filter(
264+
(d) => !isTestNode(d),
265+
);
266+
const filteredExposures = impact.affectedExposures;
267+
const totalVisible = filteredDownstream.length + filteredExposures.length;
268+
269+
// Count how many tests were filtered
270+
const testCount =
271+
impact.downstreamModels.length - filteredDownstream.length;
272+
251273
const lines: string[] = [];
252274

253-
lines.push("<details>");
275+
// NOT in <details> — visible by default for maximum impact
254276
lines.push(
255-
`<summary>\uD83D\uDCCA Blast Radius \u2014 ${totalDownstream} downstream ${totalDownstream === 1 ? "model" : "models"}</summary>`,
277+
`### \uD83D\uDCCA Blast Radius \u2014 ${totalVisible} downstream ${totalVisible === 1 ? "model" : "models"}${testCount > 0 ? ` (${testCount} tests affected)` : ""}`,
256278
);
257279
lines.push("");
258280
lines.push("```mermaid");
259281
lines.push("graph LR");
260282
lines.push(
261-
" classDef modified fill:#ff6b6b,stroke:#333,color:#fff",
283+
" classDef modified fill:#ff6b6b,stroke:#c92a2a,color:#fff,stroke-width:2px",
262284
);
263-
lines.push(" classDef downstream fill:#ffd93d,stroke:#333");
264285
lines.push(
265-
" classDef exposure fill:#845ef7,stroke:#333,color:#fff",
286+
" classDef downstream fill:#ffd43b,stroke:#e67700,color:#333,stroke-width:1px",
287+
);
288+
lines.push(
289+
" classDef exposure fill:#845ef7,stroke:#5f3dc4,color:#fff,stroke-width:2px",
266290
);
267291
lines.push("");
268292

269-
// Sanitize node IDs for mermaid (replace non-alphanum with _)
270293
const sanitize = (name: string): string =>
271294
name.replace(/[^a-zA-Z0-9_]/g, "_");
272295

273296
const modifiedSet = new Set(impact.modifiedModels);
274-
const downstreamSet = new Set(impact.downstreamModels);
275-
const exposureSet = new Set(impact.affectedExposures);
297+
const exposureSet = new Set(filteredExposures);
298+
const visibleNodes = new Set([
299+
...impact.modifiedModels,
300+
...filteredDownstream,
301+
...filteredExposures,
302+
]);
303+
304+
const edgesAdded = new Set<string>();
305+
const addEdge = (from: string, to: string) => {
306+
const key = `${from}-->${to}`;
307+
if (edgesAdded.has(key)) return;
308+
edgesAdded.add(key);
309+
310+
const fromId = sanitize(from);
311+
const toId = sanitize(to);
312+
const fromClass = modifiedSet.has(from)
313+
? "modified"
314+
: exposureSet.has(from)
315+
? "exposure"
316+
: "downstream";
317+
const toClass = exposureSet.has(to)
318+
? "exposure"
319+
: modifiedSet.has(to)
320+
? "modified"
321+
: "downstream";
322+
323+
// Use display-friendly labels (strip prefixes for readability)
324+
const fromLabel = from.replace(/^(stg_|int_|fct_|dim_|rpt_)/, (m) => m);
325+
const toLabel = to.replace(/^(stg_|int_|fct_|dim_|rpt_)/, (m) => m);
326+
327+
lines.push(
328+
` ${fromId}["${fromLabel}"]:::${fromClass} --> ${toId}["${toLabel}"]:::${toClass}`,
329+
);
330+
};
276331

277-
// Use explicit edges if available (from lightweight DAG), otherwise
278-
// fall back to the cartesian product approach (modified → downstream).
279332
if (impact.edges && impact.edges.length > 0) {
333+
// Use explicit edges, filtering out test nodes
280334
for (const edge of impact.edges) {
281-
const fromId = sanitize(edge.from);
282-
const toId = sanitize(edge.to);
283-
const fromClass = modifiedSet.has(edge.from) ? "modified" : "downstream";
284-
const toClass = exposureSet.has(edge.to)
285-
? "exposure"
286-
: downstreamSet.has(edge.to)
287-
? "downstream"
288-
: modifiedSet.has(edge.to)
289-
? "modified"
290-
: "downstream";
291-
lines.push(` ${fromId}:::${fromClass} --> ${toId}:::${toClass}`);
335+
if (!visibleNodes.has(edge.from) || !visibleNodes.has(edge.to)) continue;
336+
addEdge(edge.from, edge.to);
292337
}
293338
} else {
294-
// Legacy: cartesian product of modified → downstream
339+
// Fallback: connect modified → downstream, downstream → exposures
295340
for (const mod of impact.modifiedModels) {
296-
const modId = sanitize(mod);
297-
for (const ds of impact.downstreamModels) {
298-
const dsId = sanitize(ds);
299-
lines.push(` ${modId}:::modified --> ${dsId}:::downstream`);
300-
}
301-
for (const exp of impact.affectedExposures) {
302-
const expId = sanitize(exp);
303-
lines.push(` ${modId}:::modified --> ${expId}:::exposure`);
341+
for (const ds of filteredDownstream) {
342+
addEdge(mod, ds);
304343
}
305344
}
306-
307-
if (
308-
impact.downstreamModels.length > 0 &&
309-
impact.affectedExposures.length > 0
310-
) {
311-
for (const ds of impact.downstreamModels) {
312-
const dsId = sanitize(ds);
313-
for (const exp of impact.affectedExposures) {
314-
const expId = sanitize(exp);
315-
lines.push(` ${dsId}:::downstream --> ${expId}:::exposure`);
316-
}
345+
for (const ds of filteredDownstream) {
346+
for (const exp of filteredExposures) {
347+
addEdge(ds, exp);
317348
}
318349
}
319350
}
320351

321352
lines.push("```");
322353
lines.push("");
323-
lines.push("</details>");
354+
355+
// Legend
356+
lines.push(
357+
`> \uD83D\uDD34 Modified \u00A0\u00A0 \uD83D\uDFE1 Downstream \u00A0\u00A0 \uD83D\uDFE3 Exposure${testCount > 0 ? ` \u00A0\u00A0 \uD83E\uDDEA ${testCount} tests also affected` : ""}`,
358+
);
324359

325360
return lines.join("\n");
326361
}

test/e2e/comment-generation.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,11 @@ describe("PR Comment Generation v0.3", () => {
120120
assertCommentHasSection(comment, "Blast Radius");
121121
expect(comment).toContain("```mermaid");
122122
expect(comment).toContain("graph LR");
123-
expect(comment).toContain("stg_orders:::modified");
124-
expect(comment).toContain("orders:::downstream");
123+
expect(comment).toContain("stg_orders");
124+
expect(comment).toContain("orders");
125125
expect(comment).toContain("2 downstream models");
126+
// DAG should be visible by default, not in <details>
127+
expect(comment).not.toContain("<details>\n<summary>📊 Blast");
126128
});
127129

128130
it("includes exposures in Mermaid DAG with purple class", () => {
@@ -136,7 +138,8 @@ describe("PR Comment Generation v0.3", () => {
136138
const report = makeReport({ impact, impactScore: 70 });
137139
const comment = buildComment(report)!;
138140

139-
expect(comment).toContain("exec_dashboard:::exposure");
141+
expect(comment).toContain("exec_dashboard");
142+
expect(comment).toContain("exposure");
140143
expect(comment).toContain("classDef exposure fill:#845ef7");
141144
});
142145

test/unit/comment-builder.test.ts

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -367,10 +367,11 @@ describe("Comment Builder v0.3", () => {
367367
expect(section).toContain("```mermaid");
368368
expect(section).toContain("graph LR");
369369
expect(section).toContain("classDef modified fill:#ff6b6b");
370-
expect(section).toContain("classDef downstream fill:#ffd93d");
370+
expect(section).toContain("classDef downstream fill:#ffd43b");
371371
expect(section).toContain("classDef exposure fill:#845ef7");
372-
expect(section).toContain("stg_orders:::modified --> fct_revenue:::downstream");
373-
expect(section).toContain("stg_orders:::modified --> dim_customers:::downstream");
372+
expect(section).toContain("stg_orders");
373+
expect(section).toContain("fct_revenue");
374+
expect(section).toContain("dim_customers");
374375
});
375376

376377
it("includes exposures with purple class", () => {
@@ -383,10 +384,11 @@ describe("Comment Builder v0.3", () => {
383384
};
384385
const section = buildMermaidDAG(impact);
385386

386-
expect(section).toContain("exec_dashboard:::exposure");
387+
expect(section).toContain("exec_dashboard");
388+
expect(section).toContain("exposure");
387389
});
388390

389-
it("is collapsible", () => {
391+
it("is visible by default (not collapsed)", () => {
390392
const impact: ImpactResult = {
391393
modifiedModels: ["stg_orders"],
392394
downstreamModels: ["fct_revenue"],
@@ -396,9 +398,10 @@ describe("Comment Builder v0.3", () => {
396398
};
397399
const section = buildMermaidDAG(impact);
398400

399-
expect(section).toContain("<details>");
400-
expect(section).toContain("</details>");
401+
// DAG should NOT be in <details> — it's the wow factor
402+
expect(section).not.toContain("<details>");
401403
expect(section).toContain("Blast Radius");
404+
expect(section).toContain("```mermaid");
402405
});
403406

404407
it("shows correct downstream count in summary", () => {
@@ -424,8 +427,9 @@ describe("Comment Builder v0.3", () => {
424427
};
425428
const section = buildMermaidDAG(impact);
426429

427-
expect(section).toContain("stg_orders_v2:::modified");
428-
expect(section).toContain("fct_revenue:::downstream");
430+
// Sanitized IDs should not contain hyphens/dots/spaces
431+
expect(section).toContain("stg_orders_v2");
432+
expect(section).toContain("fct_revenue");
429433
});
430434

431435
it("connects downstream models to exposures", () => {
@@ -438,7 +442,39 @@ describe("Comment Builder v0.3", () => {
438442
};
439443
const section = buildMermaidDAG(impact);
440444

441-
expect(section).toContain("fct_revenue:::downstream --> exec_dashboard:::exposure");
445+
expect(section).toContain("fct_revenue");
446+
expect(section).toContain("exec_dashboard");
447+
expect(section).toContain("exposure");
448+
});
449+
450+
it("filters out test nodes from the diagram", () => {
451+
const impact: ImpactResult = {
452+
modifiedModels: ["stg_orders"],
453+
downstreamModels: ["fct_revenue", "not_null_stg_orders_id", "unique_stg_orders_id"],
454+
affectedExposures: [],
455+
affectedTests: ["not_null_stg_orders_id"],
456+
impactScore: 30,
457+
};
458+
const section = buildMermaidDAG(impact);
459+
460+
expect(section).toContain("fct_revenue");
461+
expect(section).not.toContain("not_null_stg_orders_id");
462+
expect(section).not.toContain("unique_stg_orders_id");
463+
expect(section).toContain("2 tests affected");
464+
});
465+
466+
it("includes a legend", () => {
467+
const impact: ImpactResult = {
468+
modifiedModels: ["stg_orders"],
469+
downstreamModels: ["fct_revenue"],
470+
affectedExposures: [],
471+
affectedTests: [],
472+
impactScore: 30,
473+
};
474+
const section = buildMermaidDAG(impact);
475+
476+
expect(section).toContain("Modified");
477+
expect(section).toContain("Downstream");
442478
});
443479
});
444480

0 commit comments

Comments
 (0)