feat(cll): surface downstream impact of whole-model changes (DRC-3341) by danyelf · Pull Request #1381 · DataRecce/recce

danyelf · 2026-05-19T00:42:33Z

Summary

Clean v2 of #1367.

Same feature; rebuilt from main to drop ~30 commits of prototype drift (dead wholeModelImpactCauseMap state, unused washBg token, stale JSDoc, duplicated badge-meta lookups, storybook that re-implemented NodeView's header JSX).

--whole-model-impact CLI flag (implies --new-cll-experience)
Compute whole-model impact via BFS over the lineage graph
Title chip + [COLUMN]/[ADD] badge + left stripe accent in NodeView; matching badge on LineageNode
Changed-wins (Q11) enforced in the helper short-circuit AND at the consumer boundary via pickWholeModelFlags
One-place palette (wholeModelTreatment.ts) + shared primitives (TreatmentChip, getBadgeMeta, pickWholeModelFlags)
Storybook coverage rendering real components

Supersedes #1367. Closes DRC-3341.

Test plan

pnpm test passes (3724 passed / 5 skipped)
pnpm type:check clean
pnpm lint clean
make flake8 + pytest tests/test_cli.py -k downstream clean
Visual smoke against Snowflake: recce server --downstream-of-breaking --target snowflake_dev — /api/flag reports downstream_of_breaking: true, new_cll_experience: true; brown / amber / green surfaces render on changed / impacted / additive models

🤖 Generated with Claude Code

codecov · 2026-05-19T00:49:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
recce/cli.py	`68.82% <100.00%> (+0.08%)`	⬆️
tests/test_cli.py	`100.00% <100.00%> (ø)`

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Adds an opt-in “whole-model impact” mode to Recce’s column-level lineage experience, propagating whole-model changes downstream and surfacing them in the graph + sidebar UI so reviewers can quickly distinguish table-wide effects from column-only changes.

Changes:

Introduces a server CLI flag (--whole-model-impact) that implies --new-cll-experience, and threads the flag through /api/flag to the frontend.
Computes whole-model downstream impact via BFS over the lineage graph and exposes changed/impacted node ID sets through the lineage view context.
Adds shared visual primitives/tokens (chips + badges) and updates NodeView/LineageNode rendering, plus tooltip and test/story coverage.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_cli.py	Adds CLI tests asserting `--whole-model-impact` implies `new_cll_experience` and sets the expected server flags.
recce/cli.py	Adds the `--whole-model-impact` Click option and ensures it enables the corresponding server flags.
js/src/components/lineage/tests/GraphNode.test.tsx	Updates mocked lineage view context to include whole-model impact fields.
js/packages/ui/src/contexts/lineage/types.ts	Extends `LineageViewContextType` with `wholeModelImpact` and whole-model changed/impacted node ID sets.
js/packages/ui/src/components/ui/DataTypeIcon/tooltipText.ts	Adds an “impacted” suffix to the column tooltip when appropriate.
js/packages/ui/src/components/ui/DataTypeIcon/tests/tooltipText.test.ts	Tests new tooltip suffix behavior and ordering relative to the CLL suffix.
js/packages/ui/src/components/schema/ColumnNameCell.tsx	Threads the new `impacted` tooltip flag based on existing badge render rules.
js/packages/ui/src/components/lineage/wholeModelTreatment.ts	Introduces centralized palette tokens and kind resolution for changed/impacted/additive treatments.
js/packages/ui/src/components/lineage/wholeModelHelpers.ts	Adds shared badge metadata + “changed-wins” flag picking helper.
js/packages/ui/src/components/lineage/TreatmentChip.tsx	Adds a reusable MUI Box-based chip primitive for badges/title chips.
js/packages/ui/src/components/lineage/NodeViewOss.tsx	Injects whole-model flags into the base NodeView via context-aware helpers.
js/packages/ui/src/components/lineage/NodeView.tsx	Adds NodeView title chip, inline badge, and left stripe accent for whole-model changed/impacted nodes; refactors tab layout.
js/packages/ui/src/components/lineage/nodes/LineageNode.tsx	Adds TABLE/ADD badge rendering and suppresses change-category labels when whole-model mode is enabled.
js/packages/ui/src/components/lineage/LineageViewOss.tsx	Wires server flag → impact set computation/publication → context values, including whole-model sets.
js/packages/ui/src/components/lineage/index.ts	Re-exports `wholeModelTreatmentTokens` for storybook/consumers.
js/packages/ui/src/components/lineage/hooks/usePublishedImpactSets.ts	Publishes and stores whole-model changed/impacted node ID sets alongside existing impacted sets.
js/packages/ui/src/components/lineage/hooks/usePublishedImpactSets.test.tsx	Tests publishing whole-model sets through the hook.
js/packages/ui/src/components/lineage/GraphNodeOss.tsx	Injects whole-model flags and `wholeModelImpact` into LineageNode props.
js/packages/ui/src/components/lineage/computeWholeModelImpact.ts	Adds BFS-based downstream propagation for whole-model impact sets.
js/packages/ui/src/components/lineage/tests/computeWholeModelImpact.test.ts	Adds unit tests for propagation, multi-source unioning, and cycle termination.
js/packages/ui/src/components/lineage/tests/NodeView.test.tsx	Tests NodeView whole-model surfaces (changed/impacted) and additive non-rendering behavior.
js/packages/ui/src/components/lineage/tests/LineageNode.test.tsx	Updates change-analysis label expectations and adds tests for whole-model badges.
js/packages/ui/src/components/index.ts	Re-exports `wholeModelTreatmentTokens` from the top-level UI components entrypoint.
js/packages/ui/src/api/flag.ts	Extends the typed server flags interface with `whole_model_impact`.
js/packages/storybook/stories/lineage/WholeModelImpact.stories.tsx	Adds storybook visual smoke stories rendering the real primitives/components for whole-model treatment.

danyelf · 2026-05-19T05:05:53Z

+@click.option(
+    "--whole-model-impact",
+    is_flag=True,
+    help="Highlight models downstream of a whole-model change. Implies --new-cll-experience.",
+    envvar="RECCE_WHOLE_MODEL_IMPACT",
+)


Fixed description

+const EMPTY_RESULT: WholeModelImpactSets = {
+  wholeModelImpactedNodeIds: new Set<string>(),
+  wholeModelChangedNodeIds: new Set<string>(),
+};
+
+/**
+ * BFS the lineage graph downstream from every node whose CLL
+ * `change_category === "breaking"`. Cheap — runs only when the
+ * `whole_model_impact` server flag is on.
+ */
+export function computeWholeModelImpact(
+  lineageGraph: LineageGraph,
+  cll: ColumnLineageData,
+): WholeModelImpactSets {
+  const changedNodes: string[] = [];
+  for (const [nodeId, cllNode] of Object.entries(cll.current.nodes)) {
+    if (cllNode.change_category === "breaking") {
+      changedNodes.push(nodeId);
+    }
+  }
+  if (changedNodes.length === 0) return EMPTY_RESULT;
+


danyelf · 2026-05-19T05:06:55Z

+// Used when `--whole-model-impact` is on: the TABLE / ADD badges in
+// the title row carry the breaking/non_breaking/partial_breaking signal
+// instead, so we suppress the text label to avoid double-display.
+// "Unknown" keeps its text label since no badge covers that case.
+const CHANGE_CATEGORY_LABELS_WHOLE_MODEL: Record<
+  ChangeCategory,
+  string | null
+> = {
+  breaking: null,
+  non_breaking: null,
+  partial_breaking: null,


By design. It is intentional that there not be a badge for this.

Surface a single flag that turns on whole-model impact highlighting in the new CLL UI. The flag implies --new-cll-experience. - Add `--downstream-of-breaking` option to the `server` command - Initialize `downstream_of_breaking: False` in the flag dict - When set, both `downstream_of_breaking` and `new_cll_experience` publish as True via AppState - Mirror the flag on `RecceServerFlags` so the frontend sees it - Pin the implication in tests/test_cli.py (positive + negative case) Part of DRC-3341 (v2). Supersedes #1367. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Add the propagation half of the downstream-of-breaking pipeline. The CLI flag publishes; this commit consumes it. - computeWholeModelImpact: BFS the lineage graph from every node whose CLL change_category === "breaking". Returns the impacted set (including the changed nodes themselves) and the changed-only subset. Terminates on cycles via a visited set. - LineageViewContextType: two new fields, wholeModelChangedNodeIds and wholeModelImpactedNodeIds. JSDoc describes the title chip + [TABLE] badge + left stripe surfaces the next commit paints. - usePublishedImpactSets: extend with two new backing state slots; the publish callback defaults each to an empty Set when omitted. - LineageViewOss: wire compute behind serverFlags.downstream_of_breaking and thread both sets into contextValue. - GraphNode.test mock: pin the three new Set fields so the OSS test context type-checks against the new LineageViewContextType shape. Part of DRC-3341 (v2). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

…schema Light up brown / amber / green surfaces wherever the user can see a model: the graph badge on each lineage node, the title chip + inline badge + left stripe on the sidebar header, and the per-column impacted tooltip. Primitives: - wholeModelTreatment.ts — palette helper. Kind values are "changed" | "impacted" | "additive" (no source/downstream/breaking residue). Tokens have no washBg field — the chip background lives in badgeBg, the stripe uses stripeAccent. wholeModelTreatmentKind short-circuits changed-first (Q11 defence-in-depth pair with pickWholeModelFlags). - TreatmentChip.tsx — single Box+sx primitive for every chip/badge surface. Defaults to 16px height, 3px radius; callers override via sx. - wholeModelHelpers.ts — getBadgeMeta (text/tooltip/aria/testId per kind) and pickWholeModelFlags (zeros isWholeModelImpacted when isWholeModelChanged is true). Paint sites: - LineageNode: brown TABLE / amber TABLE / green ADD via getBadgeMeta + TreatmentChip; CHANGE_CATEGORY_LABELS now null for breaking / non_breaking / partial_breaking, only "Unknown" still renders. - NodeView: title chip wraps node name (6px radius), inline [TABLE]/[ADD] badge, 3px left stripe; tab indices hoisted out of IIFE; TABS_HEIGHT hoisted with a comment noting the calc(100% - 48px) coupling. - GraphNodeOss + NodeViewOss: resolve flags via pickWholeModelFlags, no hand-rolled changed-wins zeroing. - SchemaView: no functional change; per-column impacted highlighting still reads from impactedColumnIds. - ColumnNameCell + tooltipText: thread an `impacted` flag into the unified tooltip; appended " · impacted" mirrors the `!` badge render rule. Public surface: - wholeModelTreatmentTokens re-exported from @datarecce/ui/components for Storybook. Everything else stays package-internal. Part of DRC-3341 (v2). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Six stories covering the four kinds across both NodeView and LineageNode, one dimension of variation each. Render the real components with realistic fixtures cribbed from NodeView.stories.tsx; no PanelFixture, no three-panel comparison. Also extends NodeView's whole-model treatment resolver to the additive case so the AdditiveTitleChip story exercises the §4.3 contract — when node.data.change.category === "non_breaking" and neither whole-model flag is set, the sidebar paints a green title chip + [ADD] badge + green stripe (the same surfaces brown/amber paint). Part of DRC-3341 (v2). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

…op NodeView additive surfaces Two review fixes to the unmerged DRC-3341 work: 1. Thread `downstream_of_breaking` through `LineageViewContextType` and into NodeView / LineageNode. When the flag is off, no whole-model UI renders AND the original "Breaking / Non Breaking / Partial Breaking" text labels on the graph node are restored. This stops `--new-cll-experience` users from losing the change-category text labels with nothing to replace them. 2. Drop the NodeView treatment for additive (non_breaking) changes. Additive is per-column, not whole-table — so no green title chip, no green left stripe, no [ADD] badge in the sidebar. The [ADD] badge on the LineageNode graph stays as a small per-model indicator. Tests: 3730 passed / 5 skipped. New coverage: flag-off path on LineageNode (text labels render), flag-off path on NodeView (no treatment), additive path on NodeView (no treatment). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Aligns the user-facing CLI flag with the in-flight vocabulary migration (state: changed/impacted; kind: whole-model/column/additive). The TS implementation already used `wholeModel*` identifiers; only the flag name and its derived snake/camel forms lagged. Surface map: --downstream-of-breaking → --whole-model-impact RECCE_DOWNSTREAM_OF_BREAKING → RECCE_WHOLE_MODEL_IMPACT downstream_of_breaking → whole_model_impact (server flag) downstreamOfBreaking → wholeModelImpact (TS prop/field) Backend wire enums (change_category=breaking/non_breaking/partial_breaking) are untouched — only user-facing and TS surfaces move. Follow-up DRC-3480 tracks migrating CHANGE_CATEGORY_LABELS off legacy "Breaking" strings on the default-off path. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

The shared EMPTY_RESULT constant returned the same Set instances on every no-changes call. Any accidental .add() by a consumer would have leaked globally across renders. Drop the constant and allocate fresh sets inline. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Users found "TABLE" confusing — it read as descriptive ("this thing is a table") rather than indicative ("the whole table changed"). Switching to "OVERALL Δ" (with delta) makes the change-vs-shape signal explicit. "ADD Δ" keeps the trio symmetric (Δ on every change-kind badge). Changed and impacted variants both render "OVERALL Δ"; color (brown vs amber) continues to carry the changed-vs-impacted distinction, matching the previous TABLE/TABLE behavior. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

The Copilot Autofix in 240004e converted sx={{...sx}} to sx=[..., sx], but MUI's array form rejects 'SxProps | undefined' as an element. Use the documented spread pattern: spread when array, wrap when defined, omit when undefined. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Whole-model changes/impacts already carry a strong visual signal via NodeView's title chip + left stripe (color treatment). Adding a "MODEL" text badge to the graph node was redundant noise. Column-only kinds have no such color signal, so the badge is their only marker — keep those. New badge set: COLUMN (brown) — own column-only change (change_category=partial_breaking) COLUMN (amber) — column-only impact (isImpacted, no own change, no whole-model impact) ADD (green) — additive change (change_category=non_breaking) Whole-model kinds (changed, impacted) still resolve in wholeModelTreatmentKind() because NodeView's title-chip + stripe needs the resolution. They no longer render a graph badge or an inline NodeView badge — only the existing chip + stripe carries the signal. Precedence is unchanged (whole-model > additive > column-changed > column-impacted); the new column variants only fire when whole-model resolution returns null. "Do not change logic" preserved. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

danyelf · 2026-05-19T23:08:59Z

Force pushes break history, but approving the Copilot suggestion broke DCO

…MODEL→COLUMN cutover Post-review cleanup of the MODEL→COLUMN label work. The previous `getBadgeMeta` returned a 4-field record (`text` / `tooltip` / `ariaLabel` / `testId`) for every kind, but the consumers used different subsets: NodeView read only `tooltip` + `ariaLabel`, LineageNode read all four, and each site had its own copy of the "which surface paints this kind?" partition. That meant dead fields, dead branches, and the partition expressed twice with opposite polarities. Helpers: - Split `getBadgeMeta` into `getGraphBadgeMeta` (additive, column-changed, column-impacted) and `getTitleChipMeta` (changed, impacted). Each returns null for the kinds it doesn't paint, so the surface partition lives in one canonical place instead of two consumer-side if-checks. - `TitleChipMeta` carries only the two fields NodeView actually reads. - `wholeModelTreatmentTokens` rewritten as an exhaustive switch — the impacted/column-impacted amber pair is now explicit instead of a default fall-through, matching the changed/column-changed brown pair. Consumers: - NodeView: drop dead `isAdditive` derivation (it fed a kind that was always filtered out) and the `isWholeModelTreatment` predicate (replaced by the helper's null return). Net 13 fewer lines. - LineageNode: drop the explicit `if (kind === "changed" || "impacted") return null` short-circuit — `getGraphBadgeMeta` returns null for those. Documentation: - `wholeModelTreatment.ts` file-level JSDoc rewritten: 5 kinds across 2 surfaces (NodeView title chip + stripe; LineageNode graph badge), 3 palettes (brown/amber/green). - `wholeModelTreatmentKind` precedence list extended to 5 steps. - `LineageNodeProps` JSDoc for `isWholeModelChanged` / `isWholeModelImpacted` no longer claims they "paint brown/amber TABLE" — whole-model kinds paint nothing on LineageNode now. - `CHANGE_CATEGORY_LABELS_WHOLE_MODEL` comment clarifies the no-badge-no-label case for `breaking`. - `LineageViewContextType` JSDoc for `wholeModelImpact`, `wholeModelChangedNodeIds`, `wholeModelImpactedNodeIds` drops the removed `[TABLE]/[ADD]` badge references and describes the two-surface model accurately. Stories: - Drop `LineageNode_ChangedBadge` / `LineageNode_ImpactedBadge` — they asserted a brown/amber TABLE badge that the production code no longer renders, so they tested nothing. Leave a comment pointing to the NodeView title-chip stories for those kinds. - File-level and per-story descriptions corrected. No behavior change. Tests: 3734 passed / 5 skipped (unchanged). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Resolved conflicts in NodeView.tsx and NodeView.test.tsx: - NodeView.tsx: kept main's tab ordering (Columns at index 0 — DRC-3468 default landing tab); dropped the TABS_HEIGHT constant and inlined its comment next to the calc(100% - 48px) use site. - NodeView.test.tsx: kept both new describe blocks ("whole-model treatment" from this branch + "default landing tab" from main). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

…view - Dedupe palette: move changed/additive badge tokens into styles.tsx alongside cllImpactedBadge*, drop the third RGB sync point in wholeModelTreatment.ts. - Drop dead wholeModelTreatmentTokens re-export from the lineage and components barrels (no consumers, misleading "Storybook imports this" comment). - Extract LineageNode title-row IIFE into resolveLineageNodeBadge() and collapse the parallel guard chain — wholeModelTreatmentKind already enforces changed-wins precedence. - Mirror the same pattern in NodeView with resolveTitleChip(), replacing the three-variable null chain with a single resolved object. - Merge wholeModelHelpers.ts into wholeModelTreatment.ts and reorganize as a top-to-bottom pipeline: pickWholeModelFlags → kind → tokens / surface metadata. Delete the helpers file. - Add variant: "badge" | "titleChip" to TreatmentChip so the title chip no longer overrides seven base style tokens via sx. - Replace CHANGE_CATEGORY_LABELS_WHOLE_MODEL map with a guard expression at the render site; three of four entries were null. Type-check, lint, and 3735 frontend tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Resolves conflicts in: - GraphNodeOss.tsx: keep wholeModelImpact destructure; drop unused showColumnLevelLineage/setChangeAnalysisMode/isActionAvailable since main removed the onShowImpactRadius callback path. - NodeView.tsx: combine HEAD's title chip (whole-model treatment) with main's formatNodeTooltip wrapping on the non-chip Typography. - nodes/LineageNode.tsx: keep wholeModelBadge + changeCategoryLabel; drop hover-toggled icons + impact-radius button in favor of main's always-visible descriptor/status and floating NodeToolbar kebab. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

After the main merge, the title chip path showed only the whole-model treatment label and the plain path showed only `name - kind`. Combine them into a single MuiTooltip so both branches share the same base `name - kind` text, with the treatment label appended when a chip is present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

even-wei · 2026-05-21T06:04:08Z

Code Review: PR #1381

SHA c9f86cff · Verdict GO

Notes

js/packages/ui/src/components/lineage/LineageViewOss.tsx:679 — The useLayoutEffect dependency list is [lineageGraph, serverFlags?.impact_at_startup], but the effect body reads serverFlags?.whole_model_impact (line 636) when computing the impacted-set snapshot.
Evidence: whole_model_impact ships from the same query as impact_at_startup, so in the normal startup path they arrive in the same render and the existing impact_at_startup dep is sufficient. The risk surfaces only if a future change starts mutating whole_model_impact independently (e.g. a runtime toggle without a re-fetch), at which point the published sets would stay stale until the next CLL action. Acceptable today; worth a comment on the dep list if whole_model_impact ever becomes user-toggleable mid-session.
Pass A.
js/packages/ui/src/components/lineage/computeWholeModelImpact.ts:25 — BFS treats change_category === "breaking" as the only "whole-model change" source. partial_breaking (column-only) is intentionally excluded, matching the design — but the helper's contract (the docstring on WholeModelImpactSets) is the only place this is documented. If a future contributor extends change_category (e.g. breaking_schema), they'll need to revisit this filter; a one-line comment at the filter site naming the design decision would make that drift visible.
Evidence: if (cllNode.change_category === "breaking") — the precise string is load-bearing and only re-stated in the docstring.
Pass F.
recce/cli.py:1344 — The --whole-model-impact flag implies --new-cll-experience by force-setting flag["new_cll_experience"] = True, which is the documented behavior. There is no warning if a user passes --whole-model-impact without --new-cll-experience explicitly. That's fine since the flag is additive, but the --help text ("Implies --new-cll-experience") is the only signal. Acceptable; flagging only because future flag interactions could compound this pattern silently.
Pass F.

Verification

pnpm test (changed-file scope, 4 files): 82/82 pass
pnpm type:check: clean
pnpm lint (biome): clean
uv run pytest tests/test_cli.py -k whole_model: 2/2 pass

What was checked

BFS correctness: cycle termination via visited ✓ (test "terminates on a cycle"), multi-source union ✓, changed-self in impacted set ✓.
Changed-wins (Q11) invariant: enforced in wholeModelTreatmentKind (line 121, "changed" precedes "impacted") AND in pickWholeModelFlags (line 95, masks isWholeModelImpacted when isWholeModelChanged). Defense-in-depth as documented.
Surface separation: getGraphBadgeMeta returns null for whole-model kinds; getTitleChipMeta returns null for per-column kinds. The two surfaces are mutually exclusive by construction.
Package boundary: storybook imports use @datarecce/ui/advanced and @datarecce/ui/primitives, not deep ui/src/* paths.
Type integrity: LineageViewContextType now requires wholeModelImpact, wholeModelChangedNodeIds, wholeModelImpactedNodeIds; GraphNode.test.tsx updated mock context accordingly.
Hook contract: usePublishedImpactSets keeps state-based publishing (regression test for PR fix(lineage): make impacted-column snapshot trigger sidebar re-render #1315) and now also publishes the two whole-model sets with empty-Set fallback.
Race guard with empty changed nodes: computeWholeModelImpact short-circuits to empty sets when no node has change_category === "breaking" — avoids unnecessary BFS allocations.

No blockers. No issues. No findings on the Copilot review thread that go unaddressed in the current code.

even-wei

Code Review: PR #1381

SHA c9f86cff · Verdict GO

Notes

js/packages/ui/src/components/lineage/LineageViewOss.tsx:679 — The useLayoutEffect dependency list is [lineageGraph, serverFlags?.impact_at_startup], but the effect body reads serverFlags?.whole_model_impact (line 636) when computing the impacted-set snapshot.
Evidence: whole_model_impact ships from the same query as impact_at_startup, so in the normal startup path they arrive in the same render and the existing impact_at_startup dep is sufficient. The risk surfaces only if a future change starts mutating whole_model_impact independently (e.g. a runtime toggle without a re-fetch), at which point the published sets would stay stale until the next CLL action. Acceptable today; worth a comment on the dep list if whole_model_impact ever becomes user-toggleable mid-session.
Pass A.
js/packages/ui/src/components/lineage/computeWholeModelImpact.ts:25 — BFS treats change_category === "breaking" as the only "whole-model change" source. partial_breaking (column-only) is intentionally excluded, matching the design — but the helper's contract (the docstring on WholeModelImpactSets) is the only place this is documented. If a future contributor extends change_category (e.g. breaking_schema), they'll need to revisit this filter; a one-line comment at the filter site naming the design decision would make that drift visible.
Evidence: if (cllNode.change_category === "breaking") — the precise string is load-bearing and only re-stated in the docstring.
Pass F.
recce/cli.py:1344 — The --whole-model-impact flag implies --new-cll-experience by force-setting flag["new_cll_experience"] = True, which is the documented behavior. There is no warning if a user passes --whole-model-impact without --new-cll-experience explicitly. That's fine since the flag is additive, but the --help text ("Implies --new-cll-experience") is the only signal. Acceptable; flagging only because future flag interactions could compound this pattern silently.
Pass F.

Verification

pnpm test (changed-file scope, 4 files): 82/82 pass
pnpm type:check: clean
pnpm lint (biome): clean
uv run pytest tests/test_cli.py -k whole_model: 2/2 pass

What was checked

BFS correctness: cycle termination via visited ✓ (test "terminates on a cycle"), multi-source union ✓, changed-self in impacted set ✓.
Changed-wins (Q11) invariant: enforced in wholeModelTreatmentKind (line 121, "changed" precedes "impacted") AND in pickWholeModelFlags (line 95, masks isWholeModelImpacted when isWholeModelChanged). Defense-in-depth as documented.
Surface separation: getGraphBadgeMeta returns null for whole-model kinds; getTitleChipMeta returns null for per-column kinds. The two surfaces are mutually exclusive by construction.
Package boundary: storybook imports use @datarecce/ui/advanced and @datarecce/ui/primitives, not deep ui/src/* paths.
Type integrity: LineageViewContextType now requires wholeModelImpact, wholeModelChangedNodeIds, wholeModelImpactedNodeIds; GraphNode.test.tsx updated mock context accordingly.
Hook contract: usePublishedImpactSets keeps state-based publishing (regression test for PR #1315) and now also publishes the two whole-model sets with empty-Set fallback.
Race guard with empty changed nodes: computeWholeModelImpact short-circuits to empty sets when no node has change_category === "breaking" — avoids unnecessary BFS allocations.

No blockers. No issues. No findings on the Copilot review thread that go unaddressed in the current code.

NodeTitle now accepts an optional extraTooltip suffix; LineageNode passes the wholeModelBadge tooltip when present, so hovering a node card with a whole-model treatment shows "name - kind - Whole-model change" / "... - Whole-model impact" instead of just "name - kind". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Resolves NodeView.tsx conflict from main's sidebar redesign (#1386): - Keep HEAD's whole-model props (isWholeModelChanged/Impacted, wholeModelImpact) alongside main's new rowCountDisplay prop. - Keep HEAD's title chip + unified tooltip but adopt main's header flex styling for the non-chip Typography and add the new ResourceTypeTag slot. Drop ExploreHeaderButtons from the header (main moved it to the action-buttons row). - Take main's IIFE-wrapped Tabs/TabPanels block verbatim (including the SchemaView headerAction prop) and drop the now-redundant top-level columnsTabIndex/codeTabIndex/lineageTabIndex const declarations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

The NodeView title chip was using light-mode tokens in dark mode because `useTheme().palette.mode === "dark"` returns false under this codebase's MUI colorSchemes setup. Switched to the project's `useThemeColors()` hook (same one GraphNodeOss already uses). Also refreshed the dark-mode `impacted` palette so the chip border/bg no longer share the exact text hue (was muddy at low contrast), and mirrored the change to the parallel `--schema-badge-impacted-*` CSS vars so the schema sidebar stays in sync. Tooltip work, while in this area: - Extracted `getTitleRowTooltip(node, flags)` in wholeModelTreatment.ts so LineageNode and NodeView share one helper. - LineageNode now hovers the whole top row (not just the title text) and includes the treatment label for ANY of the five kinds — previously only the chip surfaces (additive/column-only) carried it. - Added `isImpacted` prop to NodeView (wired from NodeViewOss via `impactedNodeIds`) so the sidebar can resolve `column-impacted` even though it doesn't render a chip for that kind. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

`useTheme().palette.mode === "dark"` returns false under this codebase's MUI colorSchemes setup, so the Check page rendered the light-mode border color even in dark mode. Same root cause as the NodeView chip fix in 28e2c04. Switched both CheckPageLoadingOss and CheckPageContentOss to the project's useThemeColors() hook, matching GraphNodeOss / NodeView. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Six fixes from an antagonistic re-review of PR #1381 (DRC-3341): - NodeTag: render RowCountSummary Stack as inline span so it no longer emits a <div> inside the NodeView Row Count <button> (fixes validateDOMNesting hydration warning). - NodeView: drop ariaLabel on the title-chip wrapper so the visible model name serves as the chip's accessible name; treatment is conveyed via the MuiTooltip's aria-describedby. - NodeView: reserve a 3px transparent left border baseline so toggling whole-model treatment doesn't shift content horizontally. - usePublishedImpactSets: reuse a module-level EMPTY_SET for the whole-model fallback path so refreshes with whole-model off don't burn extra renders on Object.is mismatches. Adds regression test. - styles / wholeModelTreatment: make cllAdditiveBadgeBg a { light, dark } map matching its sibling tokens (structural fix; preserves current visual). - NodeView / LineageNode tests: replace tautological assertions for phantom badge testIds with structural [data-testid$="-badge"] queries that catch a regression under any naming. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

Split the 5-way `WholeModelTreatmentKind` enum into two surface-specific classifiers (`pickTitleChip`, `pickGraphBadge`) that each return a fully-baked resolution object — no more null-routing per surface, no more duplicated changeCategory→flag mapping at consumer sites. Move precedence (changed-wins, Q11) entirely into the classifiers so `pickWholeModelFlags` becomes a pure 2-line set lookup with no defence-in-depth layer. Hoist NodeView's IIFEs for the title-row tooltip and tab indices. Net: −122 LOC across the three files. No behavior change; all 497 lineage tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

danyelf · 2026-05-21T18:14:16Z

@even-wei , thanks for the note!

I made a few changes after your approval:

Merged into main (that had a bit of a footprint)
Fixed a small dark mode bug
Cleaned up the treatment pipeline structurally after your approval — net −122 LOC, no behavior change, all tests pass. Flagging in case you want a second look before I merge.

gcko

Deep-Dive Antagonistic Review — Verdict: GO ✅

Ran a four-specialist parallel deep dive (algorithm/state, UI components, tests/coverage, backend/cross-cutting). PR is mergeable as-is — no blockers, no high-severity correctness bugs. The post-approval refactor (−122 LOC, structural collapse of the treatment pipeline) is clean: type-check green, 3745 tests pass, lint clean, flake8 clean.

Per-slice verdicts

Slice	Verdict	Worst finding
Algorithm & state (BFS, hooks, Q11)	GO	1 MEDIUM — stale-closure dep-list risk
UI components (TreatmentChip, NodeView, LineageNode)	GO	None above LOW
Tests & coverage	GO	`wholeModelTreatment.ts` has no direct unit test (LOW)
Backend & cross-cutting (CLI, flag plumbing)	GO	None above LOW

Confirmed non-issues (Copilot review thread)

TreatmentChip sx spread — fixed: now uses Array.isArray(sx) ? sx : sx ? [sx] : [], which correctly handles function/array/object/undefined forms.
EMPTY_RESULT shared Set — intentional (regression-tested at usePublishedImpactSets.test.tsx:74-101 to prevent extra renders); no consumer mutates the returned sets. Theoretical only.
partial_breaking hides under whole-model — by design, acknowledged by author.
PR-description flag name — fixed.

The one thing worth a follow-up (not a blocker)

LineageViewOss.tsx:679 — useLayoutEffect dep list reads serverFlags?.whole_model_impact (line 636) but only depends on serverFlags?.impact_at_startup. Today both flags arrive atomically from a single /api/flag response, so it's safe. Becomes a real stale-closure bug the moment those flags ever diverge in arrival (runtime toggle, partial cache hydration). @even-wei flagged the same thing earlier. Cheapest fix: add serverFlags?.whole_model_impact to the dep array — it can't hurt and removes the latent footgun.

Verification performed

pnpm test → 3745 passed / 5 skipped (the 5 skips are in RunResultPane.test.tsx and pre-date this PR).
pnpm type:check → clean.
pnpm lint (biome) → clean.
make flake8 → clean.
pytest tests/test_cli.py -k whole → 2/2 pass.
BFS correctness verified for: cycles (test "terminates on a cycle"), multi-source union, changed-self in impacted set, empty graph short-circuit.
Q11 changed-wins invariant: enforced in BOTH wholeModelTreatmentKind AND pickWholeModelFlags — defense-in-depth as documented.
Surface separation: getGraphBadgeMeta returns null for whole-model kinds; getTitleChipMeta returns null for per-column kinds. Mutually exclusive by construction.
Package boundary: storybook imports use @datarecce/ui/advanced and @datarecce/ui/primitives, never deep ui/src/* paths.
Server-side flag exposure: /api/flag endpoint in recce/server.py:563-567 returns the full flag dict, so whole_model_impact is serialized without further wiring.

Recommended follow-ups (post-merge, all LOW)

Add wholeModelTreatment.test.ts to lock the palette tokens × (light/dark) directly.
Add a CLI env-var test for RECCE_WHOLE_MODEL_IMPACT=true.
Add computeWholeModelImpact tests for disconnected components and dangling-changed-node-not-in-graph.
The hardcoded #1e1e1e/#ffffff in nodes/LineageNode.tsx:378+ are pre-existing tech debt; worth a dark-mode cleanup pass.
CLL palette is duplicated between styles.tsx and schema/style.css (already commented as known-debt) — consolidate.

Hygiene notes

23 commits / 4 merge commits — recommend squash on merge for clean history.
One non-conventional commit message (90dcd512 "Potential fix for pull request finding" — Copilot Autofix). Squash will absorb it.
One unsigned commit is a merge commit (1c9bf5c); standard DCO bots tolerate this.

Ship it — and bump the dep list on useLayoutEffect either now or in a follow-up.

🤖 Generated by Claude Code (deep-dive multi-agent review)

Generated by Claude Code

danyelf mentioned this pull request May 19, 2026

feat(cll): surface downstream impact of whole-model changes (DRC-3341) #1367

Closed

2 tasks

danyelf commented May 19, 2026

View reviewed changes

Comment thread js/packages/ui/src/components/lineage/nodes/LineageNode.tsx Outdated

gcko requested review from Copilot, even-wei and gcko May 19, 2026 02:54

Copilot started reviewing on behalf of gcko May 19, 2026 02:55 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

danyelf and others added 11 commits May 19, 2026 16:06

Potential fix for pull request finding

90dcd51

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>

danyelf force-pushed the feature/drc-3341-downstream-of-breaking-v2 branch from aefdd32 to 2741687 Compare May 19, 2026 23:08

danyelf and others added 6 commits May 19, 2026 16:30

Merge branch 'main' into feature/drc-3341-downstream-of-breaking-v2

1c9bf5c

even-wei approved these changes May 21, 2026

View reviewed changes

danyelf and others added 5 commits May 21, 2026 00:25

gcko reviewed May 22, 2026

View reviewed changes

danyelf merged commit f0e6c04 into main May 22, 2026
24 checks passed

danyelf deleted the feature/drc-3341-downstream-of-breaking-v2 branch May 22, 2026 01:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cll): surface downstream impact of whole-model changes (DRC-3341)#1381

feat(cll): surface downstream impact of whole-model changes (DRC-3341)#1381
danyelf merged 23 commits into
mainfrom
feature/drc-3341-downstream-of-breaking-v2

danyelf commented May 19, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

danyelf May 19, 2026

Uh oh!

Uh oh!

danyelf May 19, 2026

Uh oh!

danyelf commented May 19, 2026

Uh oh!

even-wei commented May 21, 2026

Uh oh!

even-wei left a comment

Uh oh!

danyelf commented May 21, 2026

Uh oh!

gcko left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

danyelf commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

codecov Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

danyelf May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danyelf May 19, 2026

Choose a reason for hiding this comment

Uh oh!

danyelf commented May 19, 2026

Uh oh!

even-wei commented May 21, 2026

Code Review: PR #1381

Notes

Verification

What was checked

Uh oh!

even-wei left a comment

Choose a reason for hiding this comment

Code Review: PR #1381

Notes

Verification

What was checked

Uh oh!

danyelf commented May 21, 2026

Uh oh!

gcko left a comment

Choose a reason for hiding this comment

Deep-Dive Antagonistic Review — Verdict: GO ✅

Per-slice verdicts

Confirmed non-issues (Copilot review thread)

The one thing worth a follow-up (not a blocker)

Verification performed

Recommended follow-ups (post-merge, all LOW)

Hygiene notes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danyelf commented May 19, 2026 •

edited

Loading

codecov Bot commented May 19, 2026 •

edited

Loading