Skip to content

feat(lineage): document ADD/COLUMN graph badges in the changes legend (DRC-3466) - #1490

Open
gcko wants to merge 2 commits into
mainfrom
feature/drc-3466-new-cll-addcolumn-lineage-node-badges-have-no-entry-in-the
Open

feat(lineage): document ADD/COLUMN graph badges in the changes legend (DRC-3466)#1490
gcko wants to merge 2 commits into
mainfrom
feature/drc-3466-new-cll-addcolumn-lineage-node-badges-have-no-entry-in-the

Conversation

@gcko

@gcko gcko commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Closes DRC-3466.

Problem

With the new CLL experience on, lineage canvas nodes render ADD and COLUMN treatment badges next to the model name. The change-status legend on the same canvas listed only Added / Removed / Modified / Impacted, so users saw badges with no in-view key — right where they'd look to decode them. The only explanation was a hover tooltip covering the whole node title row.

(The originally-reported ALL badge no longer exists on the canvas; whole-model treatment moved to the NodeView title chip and stripe. This PR covers the badges that remain.)

Change

wholeModelTreatment.ts — new getGraphBadgeLegendEntries(isDark) returning all three GraphBadgeResolutions in a fixed display order (benign → actionable), built from the same private GRAPH_BADGE_LABELS and tokensForKind the node badges use. pickGraphBadge and pickTitleChip are untouched.

legend/LineageLegend.tsx — behind variant === "changeStatus" && newCllExperience, append a divider, a Badges caption, and one row per badge. Swatches are <TreatmentChip variant="badge">, so they match the node badges exactly rather than approximating them.

Rendered order with the flag on:

● Added
● Removed
● Modified
● Impacted
──────────────────
BADGES
[ADD]     Additive change        (green)
[COLUMN]  Column-only change     (brown)
[COLUMN]  Column-only impact     (amber)

Three decisions worth reviewing

Two COLUMN rows, not one. The issue left this open. column-changed and column-impacted share the COLUMN text but not the palette — brown vs amber — so a single row could only match half the badges a user actually sees. Two rows costs three lines of panel height and makes every swatch honest.

Explanations are visible text, not tooltips. The copy comes from each entry's tooltip field but renders inline. Fixing "you have to discover a hover" by adding another hover would have missed the point.

useIsDark() rather than a new prop. LineageLegend is a public primitives.ts export, so a required isDark prop would be a breaking API change. The hook matches sibling components (NodeTag, ColumnLevelLineageControl, LineageViewTopBar) and returns false outside a provider, so existing tests were unaffected. The component's "pure presentation" docstring was updated to say so.

Swatch testIds are legend-treatment-<kind>, deliberately not the node testIds — those end in -badge, and LineageNode.test.tsx / NodeView.test.tsx assert structurally that [data-testid$="-badge"] count is 0 for no-badge cases. Those queries are container-scoped so reuse couldn't break them today, but it would be a trap for any future full-view test.

Acceptance criteria

AC Covered by
AC-1: ADD entry, chip matches node badge, explains additive change documents the ADD badge
AC-2: column-only change and column-only impact both explained documents column-only change and column-only impact separately
AC-3: flag off renders exactly as today omits the badge rows when the flag is off + 3 pre-existing tests unchanged
AC-4: labels sourced from GRAPH_BADGE_LABELS sources every row from the graph badge definitions
AC-5: flag-on and flag-off covered, existing tests pass 7 new tests, 3 existing untouched

Plus reads statuses first, then the badge block, a whole-legend textContent assertion that locks the approved layout.

Verification

  • pnpm test — 3982 passed, 5 skipped (187 files)
  • pnpm type:check — clean
  • pnpm lint — clean
  • scripts/check_wire_enum_literals.sh — clean
  • Storybook: ChangeStatusCll docstring updated; new ChangeStatusCllDark for the dark-mode tokens

Scope

Out: SchemaLegend on the Columns tab (the wrong surface, per closed #1434). Badge behavior or classification changes (DRC-3814 / DRC-3815, which may later revise this legend's copy). A legend entry for the NodeView whole-model title chip.

Follow-up worth filing: the legend only renders when isModelsChanged is true, so a node can carry a COLUMN impact badge in states where no legend is on screen. Pre-existing, unrelated to this change, but it means the legend isn't a complete key in every state.

🤖 Generated with Claude Code

The lineage canvas renders ADD and COLUMN treatment badges on nodes when the
new CLL experience is on, but the change-status legend on the same canvas
listed only Added / Removed / Modified / Impacted. Users had no in-view key
for the badges, only a hover tooltip covering the whole node title row.

- wholeModelTreatment.ts: export getGraphBadgeLegendEntries(isDark), which
  resolves all three graph badges from the same GRAPH_BADGE_LABELS and
  tokensForKind the node badges use, in a fixed legend display order. The
  classifiers are unchanged.
- LineageLegend.tsx: behind newCllExperience, append a divider, a Badges
  caption, and one row per badge. Swatches reuse TreatmentChip so they match
  the node badges pixel for pixel; explanations render as visible text rather
  than another hover tooltip.

Column-only change and column-only impact get separate rows: they share the
COLUMN text but not the palette (brown vs amber), so a single row could only
match half the badges a user sees.

Swatch testIds are legend-treatment-<kind>, not the node testIds, which end in
-badge and are structurally asserted to be absent in the node tests.

The legend takes its colour mode from useIsDark rather than a new prop, so the
public primitives export keeps its signature.

DRC-3466

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
@gcko

gcko commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Code Review: PR #1490

SHA 0e3a555 · Verdict NO-GO

Blockers

None.

Issues

  1. js/packages/ui/src/components/lineage/wholeModelTreatment.ts:215GRAPH_BADGE_LEGEND_ORDER is not exhaustiveness-checked against GRAPH_BADGE_LABELS, so a fourth GraphBadgeKind gets no legend row with tsc and all 22 tests green — silently recreating DRC-3466.
    Evidence: the annotation is GraphBadgeKind[], which accepts a partial list. GRAPH_BADGE_LABELS at line 127 is a Record<GraphBadgeKind, …>, so adding a kind to the union forces a labels entry but not an order entry. No test closes the gap: LineageLegend.test.tsx:62 asserts toHaveLength(3) on the output of the function under test, :64 loops over that same output, and the container.textContent lock at :76 would still match a 3-row render. GRAPH_BADGE_LABELS is module-private, so no test can enumerate the real source of truth. The PR names DRC-3814 / DRC-3815 as pending badge-classification work, so a new kind is a live possibility, and tokensForKind (line 243) has no exhaustive switch either — a new kind would fall through to the impacted palette.
    Pass F.

Notes

  1. js/packages/ui/src/components/lineage/wholeModelTreatment.ts:234getGraphBadgeLegendEntries spreads GRAPH_BADGE_LABELS[kind], so every returned entry carries the node testId (column-changed-badge, whole-model-additive-badge) that the legend deliberately must not render.
    Evidence: LineageLegend.tsx:393 overrides with legend-treatment-<kind>, which is the right call — NodeView.test.tsx:216 and LineageNode.test.tsx:712 count container.querySelectorAll('[data-testid$="-badge"]') structurally. A future caller that trusts entry.testId reintroduces the collision. Consider Omit<GraphBadgeResolution, "testId"> as the return type so the trap can't be picked up.
    Pass C.

  2. js/packages/ui/src/components/lineage/legend/LineageLegend.tsx:360 — the new badge block duplicates the Change-Categories block's section chrome verbatim (divider Box sx at :319-326 vs :361-368, caption Typography sx at :327-337 vs :369-379). Two sources of truth for the same visual; a spacing change to one silently diverges the other.
    Evidence: the two sx objects are character-identical apart from the caption text.
    Pass F.

Verification

  • vitest related over both changed source files — 82 files, 1785 passed, 5 skipped, 0 failed
  • LineageLegend.test.tsx + wholeModelTreatment.test.ts — 22 passed
  • tsc --noEmit — clean
  • biome check --diagnostic-level=error — clean, 664 files
  • Base freshness: git merge-tree --write-tree origin/main HEAD exit 0; main has landed nothing since the merge base, so no semantic-collision surface
  • Pass B (security-review + OWASP backstop): no high-confidence findings — every rendered string is a module-level constant, no dangerouslySetInnerHTML, no user input, no network
  • Verified and not flagged: the legend swatch renders through the same TreatmentChip with the same default badge variant and the same tokensForKind output as LineageNode.tsx:622, so it cannot drift; pickTitleChip paints only in NodeView.tsx, never on the canvas, so the change-status legend is a complete key for the canvas; the story's parameters.backgrounds / globals.theme pair matches the two existing dark stories (TopKBarChart, HistogramDiffResultView) exactly; no pre-existing test was weakened (the test-file diff is additions only)

Limits

The formal --request-changes review could not be submitted — gh is authenticated as gcko, the PR author, and GitHub rejects self-review. This comment is the review of record.

Comment on lines +215 to +219
const GRAPH_BADGE_LEGEND_ORDER: GraphBadgeKind[] = [
"additive",
"column-changed",
"column-impacted",
];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GRAPH_BADGE_LEGEND_ORDER is not exhaustiveness-checked against GRAPH_BADGE_LABELS, so a fourth GraphBadgeKind gets no legend row with tsc and all 22 tests green.

Deriving the order from GRAPH_BADGE_LABELS keys and ranking via a Record<GraphBadgeKind, number> makes the omission a compile error, and preserves the docstring's point — reading order stays independent of the classifier's precedence.

Suggested change
const GRAPH_BADGE_LEGEND_ORDER: GraphBadgeKind[] = [
"additive",
"column-changed",
"column-impacted",
];
const GRAPH_BADGE_LEGEND_RANK: Record<GraphBadgeKind, number> = {
additive: 0,
"column-changed": 1,
"column-impacted": 2,
};
/**
* Derived from `GRAPH_BADGE_LABELS` rather than hand-listed, so a new
* `GraphBadgeKind` cannot reach the canvas without a legend row the rank
* record above stops compiling until it is ranked.
*/
const GRAPH_BADGE_LEGEND_ORDER: GraphBadgeKind[] = (
Object.keys(GRAPH_BADGE_LABELS) as GraphBadgeKind[]
).sort((a, b) => GRAPH_BADGE_LEGEND_RANK[a] - GRAPH_BADGE_LEGEND_RANK[b]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in d9beadaa, with the rank record split out so the docstring's original point (reading order independent of classifier precedence) stays explicit:

  • GRAPH_BADGE_LEGEND_RANK: Record<GraphBadgeKind, number> — adding a kind to the union is now a type error until it is ranked.
  • GRAPH_BADGE_LEGEND_ORDER derived from Object.keys(GRAPH_BADGE_LABELS) and sorted by that rank — once ranked, the legend row appears without further edits.

tsc --noEmit, biome check, and vitest related (1785 passed / 5 skipped) all clean at d9beadaa.

@gcko

gcko commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Code Review Loop — fix pushed (d9beadaa)

  • ISSUE 1GRAPH_BADGE_LEGEND_ORDER exhaustiveness — fixed in d9beadaa. The order is now derived from Object.keys(GRAPH_BADGE_LABELS) and ranked through a Record<GraphBadgeKind, number>, so a new badge kind is a type error until it is ranked, and once ranked it gets a legend row automatically. Reading order (benign → actionable) is unchanged, and the original docstring's point — order independent of classifier precedence — is preserved by the rank record.
  • NOTE 1 (entries carry node testIds) and NOTE 2 (duplicated section chrome) — left to the author, per the loop's guard that NOTEs do not block.

Verified at d9beadaa: tsc --noEmit clean, biome check clean, vitest related over both changed source files — 82 files, 1785 passed, 5 skipped, 0 failed. Pre-commit and pre-push hooks ran.

No blocking findings remain. The review state could not be flipped to approve — gh is authenticated as the PR author, and GitHub rejects self-review — so treat this comment as the close-out.

… is unranked

GRAPH_BADGE_LEGEND_ORDER was a hand-listed GraphBadgeKind[], which accepts a
partial list, so a new badge kind could reach the canvas with no legend row
and a green build. Derive the order from GRAPH_BADGE_LABELS keys and rank via
a Record<GraphBadgeKind, number> so an unranked kind is a type error.

Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant