Skip to content

fix(lineage): distinguish modified and impacted with brown/yellow color pair#1312

Merged
danyelf merged 6 commits intomainfrom
fix/cll-color-scheme
Apr 15, 2026
Merged

fix(lineage): distinguish modified and impacted with brown/yellow color pair#1312
danyelf merged 6 commits intomainfrom
fix/cll-color-scheme

Conversation

@danyelf
Copy link
Copy Markdown
Contributor

@danyelf danyelf commented Apr 14, 2026

Summary

Inside the new CLL experience, make "impacted" a peer status to added/removed/modified — modified uses muted brown, impacted uses muted yellow.

The new palette is scoped behind --new-cll-experience so OSS lineage, schema diff checks, summary, minimap, and the schema sidebar's existing .row-changed styling are unchanged for users without the flag.

How it's scoped

  • lineage/styles.tsx exports two parallel sets:
    • changeStatusColors / changeStatusBackgroundsLight/Dark — original Tailwind palette, used by OSS consumers.
    • cllChangeStatusColors / cllChangeStatusBackgroundsLight/Dark — muted brown/yellow palette + impacted, used by CLL consumers.
    • getCllIconForChangeStatus mirrors getIconForChangeStatus but reads the cll set; getStyleForImpacted reads the cll set.
  • CLL consumers swapped to cll exports: LineageColumnNode, LineageEdge, LineageCanvas minimap, lineage.ts ancestry stroke. LineageNode picks the palette based on newCllExperience.
  • LineageLegend gains a newCllExperience prop. When false (default), it shows the original Tailwind legend without the Impacted entry — matching what OSS users have always seen. When true, it renders the muted CLL palette including Impacted.
  • schema/style.css keeps the original pale-yellow/orange --schema-color-changed* and --schema-color-impacted* in :root / .dark. A scoped .cll-experience { ... } block (and .cll-experience.dark / .dark .cll-experience) overrides them with the muted palette and adds the new --schema-color-impacted-accent + impacted badge vars.
  • SchemaView adds cll-experience to the grid wrapper className only when serverFlags?.new_cll_experience is true.
  • Schema rows + ColumnNameCell get the impacted treatment + ! badge — gated by row.isImpacted, which is only populated when the flag is on.

Storybook

LineageLegend.stories.tsx now has both ChangeStatus (default Tailwind, no impacted) and ChangeStatusCll (muted palette, with impacted) variants.

Test plan

  • pnpm vitest run packages/ui/src/components/lineage — 403/403 pass
  • pnpm test (full UI suite) — 3674/3674 pass
  • pnpm type:check — clean
  • pnpm lint:fix — clean
  • Manual verification with two recce server instances (snowflake_dev, jaffle_shop): :8002 (no flag) shows Tailwind palette + no Impacted entry; :8001 (--new-cll-experience) shows muted brown/yellow + Impacted.
  • Visual check in Storybook: Lineage/LineageLegend ChangeStatus vs ChangeStatusCll in light + dark.
  • Visual check in Next.js: confirm OSS lineage / schema diff check view colors are unchanged from main.
image

🤖 Generated with Claude Code

… color pair

Make "impacted" a peer status to added/removed/modified in CLL lineage
visualization. Modified now uses brown (#D4850B) and impacted uses yellow
(amber[300]) so they read as related but distinct.

- Centralize change-status colors in styles.tsx as the single source of truth
  (changeStatusColors, changeStatusBackgroundsLight/Dark)
- getStyleForImpacted returns a full ChangeStatusStyle (was just a barColor)
- LineageColumnNode and LineageNode use impacted style when not otherwise changed
- LineageLegend adds Impacted as a 4th entry
- Schema sidebar (style.css) updates --schema-color-changed-accent to brown to
  match; impacted shades there were already yellow
- Schema rows and column name cell get an impacted treatment + ! badge
- Ancestry edge stroke colors now read from the centralized constants

Signed-off-by: Danyel Fisher <danyel@gmail.com>
@danyelf danyelf marked this pull request as draft April 14, 2026 22:41
danyelf and others added 4 commits April 14, 2026 16:08
Align the lineage column node and schema sidebar on a single muted
palette so the whole change-status system reads as one visual language:
brown for changed, yellow for impacted, plus the existing green/red for
added/removed — all in the softer sidebar shades.

- styles.tsx: switch changeStatusColors and both light/dark background
  maps to the rgb() values used by --schema-color-* in schema/style.css
  (paired-file comment on both sides flags the hand-sync requirement).
- LineageColumnNode: tint background + left accent for added/removed/
  modified (previously only impacted had the treatment); add dark-mode
  paper fallback and force white text so dark-mode columns no longer
  fall through to the MUI light theme.
- CllExperience story: add stacked columns to each model in the
  comparison demo and a Schema Sidebar demo (light + dark) so all three
  surfaces can be eyeballed together while tuning.
- styles.test: update literal expected values to the new rgb() palette;
  drop the stale "TODO: unify CSS and JS" note now that they are aligned.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
The previous commits repurposed the shared changeStatusColors map and
schema sidebar CSS variables to the muted brown/yellow palette, which
silently changed colors for OSS lineage, schema diff checks, summary,
minimap, and the schema sidebar — all outside the new-CLL flag.

Restore the originals and add parallel cll* exports + a .cll-experience
CSS scope. CLL consumers (LineageColumnNode, LineageEdge, LineageCanvas,
LineageLegend, lineage.ts ancestry stroke) read from the cll* set.
LineageNode picks the palette by flag. SchemaView adds the cll-experience
class only when the flag is on. LineageLegend gains a newCllExperience
prop that toggles palette and shows/hides the Impacted entry.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
…ence

Previous commits made added/removed/modified column rows always render
with a tinted background, 3px left accent, and dark hex fallbacks
regardless of flag. LineageColumnNode reaches OSS via GraphColumnNodeOss,
so OSS users would see the new visual treatment.

Mirror the LineageNode pattern: accept newCllExperience as a prop with
data.newCllExperience fallback, and gate the tinted bg, accent border,
dark hex fallbacks, and muted ChangeStatusIndicator color on the flag.
When off, behavior reverts to main: only isImpacted triggers a tint;
MUI tokens for everything else; sharp changeStatusColors for +/-/~.

GraphColumnNodeOss pulls newCllExperience from LineageViewContext and
passes it through.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
Three near-identical helpers (getIconForChangeStatus, getCllIconForChangeStatus,
getStyleForImpacted) collapse into a single parametrized getIconForChangeStatus
that takes a palette key ("default" | "cll"). getStyleForImpacted becomes a
one-line wrapper that calls it with palette="cll" and status="impacted".

Constant exports (changeStatusColors, cllChangeStatusColors, etc.) are kept
as-is — they're imported directly by LineageEdge, LineageCanvas, LineageLegend,
LineageColumnNode, and lineage.ts.

Net: -16 LOC across helpers + tests, single source of truth for the lookup
logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Danyel Fisher <danyel@gmail.com>
@danyelf
Copy link
Copy Markdown
Contributor Author

danyelf commented Apr 15, 2026

Code Review — PR #1312

Summary

Scopes the muted brown/yellow CLL palette behind --new-cll-experience and refactors getIconForChangeStatus to take a palette parameter. Verified: 423/423 lineage+schema tests pass, tsc --noEmit clean. The "leakage to non-CLL" concern checks out — I traced every consumer of cllChangeStatusColors (LineageEdge, LineageCanvas minimap, LineageNode, LineageColumnNode, lineage.ts ancestry) and they all either accept a flag prop, gate on newCllExperience, or live in the parallel new component tree (the OSS app uses GraphEdgeOss / GraphNodeOss / GraphColumnNodeOss via config/nodeTypes.ts, not the new Lineage* tree). The schema CSS variable overrides are correctly scoped to .cll-experience, which SchemaView only adds when serverFlags?.new_cll_experience is true.

Findings

[Nit] box-shadow in .row-impacted references CLL-only variable from base rule

File: js/packages/ui/src/components/schema/style.css:168-170
Issue: .row-impacted { box-shadow: inset 3px 0 0 var(--schema-color-impacted-accent); } is in the base rule, but --schema-color-impacted-accent is only defined inside .cll-experience. Today this is invisible because getRowClass only assigns row-impacted when row.isImpacted is set, which itself only happens under the flag — so the var is always defined when the rule fires. But the contract is fragile: if someone later wires isImpacted outside the flag, the box-shadow silently degrades instead of the OSS view continuing unchanged.
Suggestion: Move the box-shadow into a .cll-experience .row-impacted { ... } block alongside the other CLL overrides, so the base .row-impacted rule stays exactly as it was on main.

[Nit] getIconForChangeStatus silently returns undefined for "impacted" + default palette

File: js/packages/ui/src/components/lineage/styles.tsx (signature accepting ChangeStatus | "impacted")
Issue: The signature accepts "impacted", but changeStatusColors (default palette) has no "impacted" key. A call like getIconForChangeStatus("impacted") (no palette arg) would set color/backgroundColor to undefined. Today only getStyleForImpacted calls it with "impacted" and hardcodes "cll", so it's safe — but the type allows the dangerous combination.
Suggestion: Either narrow the type so "impacted" requires palette: "cll", or fall back to the cll palette when status === "impacted". Lowest effort: leave as-is and add a code comment.

[Nit] DRY/YAGNI — two ways to pass newCllExperience to LineageColumnNode

File: js/packages/ui/src/components/lineage/columns/LineageColumnNode.tsx:88-95, 294
Issue: The flag is accepted both as a top-level prop and as data.newCllExperience, with prop winning. The data path appears to exist only for the storybook standalone case; production callers go through GraphColumnNodeOss which sets the prop. Two channels for one boolean is a small foot-gun.
Suggestion: Pick one. If the prop is enough for production, drop the data.newCllExperience field (and its declaration in LineageColumnNodeData).

[Note — not a blocker] Hardcoded dark-mode hex fallbacks bypass MUI theme

File: js/packages/ui/src/components/lineage/columns/LineageColumnNode.tsx (defaultBg/hoverBg/selectedBg/textColor)
Issue: Raw hex strings (#262626, #333333, #404040, #ffffff) are used when newCllExperience && isDark. The inline comment explains MUI's background.paper falls through to light values in this context. Acceptable — flagging only because future theme work will need to remember these exist.

[Note] Acknowledged palette duplication between TS and CSS

File: js/packages/ui/src/components/lineage/styles.tsx (cllChangeStatus*) and js/packages/ui/src/components/schema/style.css (.cll-experience block)
The same brown/yellow values live in both places with no build-time check. The PR header comments call this out. Reasonable trade-off given the scope; worth a follow-up if the palette grows.

Verdict

Approve with minor nits. Scoping is correct, tests/type-check pass, OSS path is genuinely untouched. Worth tightening the .row-impacted box-shadow scope before merge; the rest are nits.

@danyelf danyelf requested a review from gcko April 15, 2026 06:03
@danyelf danyelf marked this pull request as ready for review April 15, 2026 06:03
@even-wei
Copy link
Copy Markdown
Contributor

Code Review: PR #1312

Files reviewed: 17
Categories: Frontend (lineage components, schema, styles, storybook)
Passes run: A, B, C, D, E, F, G, H

Validation Results

Pass A: Correctness & Logic — PASS

  • paletteFor() correctly dispatches between default/CLL palettes based on PaletteKey
  • getStyleForImpacted() correctly delegates to CLL palette — impacted is a CLL-only concept
  • LineageNode applies impacted styling only when isUnchanged is true, correctly preventing double-styling on already-changed nodes
  • LineageLegend filters out "impacted" entry when newCllExperience=false — OSS users see the same legend as before
  • LineageColumnNode correctly resolves newCllExperience from prop → data fallback → false
  • SchemaView.getRowClass gates row-impacted class on row.isImpacted, which is only populated when the flag is on

Pass B: Security — PASS

No auth, secrets, injection, or PII changes. Pure presentation code.

Pass C: Cross-Reference Consistency — PASS

  • PaletteKey type ("default" | "cll") is consistent across all call sites
  • cllChangeStatusColors type includes "impacted" key — all consumers accessing it are type-safe
  • CSS .cll-experience overrides mirror JS cllChangeStatusColors values:
    • --schema-color-changed-accent: rgb(212 133 11) matches cllChangeStatusColors.modified
    • --schema-color-impacted-accent: rgb(252 211 77) matches cllChangeStatusColors.impacted (light mode)
  • Dark-mode CSS uses rgb(180 83 9) for --schema-color-impacted-accent (darker amber for visual contrast in dark theme) — intentional divergence from the JS value which is for graph nodes, not sidebar accents
  • impactedColumnIds flows correctly: context → SchemaViewtoSchemaDataGridrow.isImpactedColumnNameCell
  • OSS GraphEdge uses getIconForChangeStatus() with default palette (unchanged)
  • CLL LineageEdge uses cllChangeStatusColors directly (correct scoping)
  • Storybook imports use @datarecce/ui/primitives — no internal path imports

Pass D: Error Handling & Edge Cases — PASS

  • data.newCllExperience ?? false fallback pattern used consistently
  • impactedStyle computed only when isImpacted && !changeStatus — prevents conflict
  • CSS variables --schema-badge-impacted-bg and --schema-color-impacted-accent only exist inside .cll-experience scope, but they're only referenced by classes that are only applied when CLL is on

Pass E: Test Coverage & Quality — PASS

  • styles.test.ts covers both palettes with exact color value assertions
  • New getStyleForImpacted tests verify accent color, background (light/dark), and icon
  • CLL palette tests verify modified=brown, impacted=yellow distinction
  • LineageEdge.test.tsx tests all EdgeChangeStatus values against CLL palette
  • All 403 lineage tests pass

Pass F: Diff-Specific Checks — PASS

  • LineageLegend gained newCllExperience prop — LineageViewOss.tsx passes it through
  • LineageColumnNode gained newCllExperience prop — GraphColumnNodeOss.tsx passes it from context
  • CllExperience.stories.tsx expanded with NodeWithColumns demo showing node+column color interaction
  • LineageLegend.stories.tsx added ChangeStatusCll variant for visual verification
  • SchemaView className conditionally adds cll-experience — CSS scoping is correct

Pass G: Performance — PASS

No N+1 queries, unbounded results, or sync-in-async patterns. Color lookups are O(1) record access.

Pass H: Async/Concurrency — PASS

No async code changes in this PR.

Verification Results

  • TypeScript (pnpm type:check): One pre-existing error in excel.test.ts (not in this PR's diff)
  • Lint (pnpm lint:fix): Clean — 602 files, no fixes
  • Tests (pnpm vitest run packages/ui/src/components/lineage): 403/403 pass

Verdict: GO

All validation passes clean. No critical issues found.

Notes

  1. CSS variable scoping (style.css): --schema-badge-impacted-bg, --schema-badge-impacted-fg, and --schema-color-impacted-accent are only defined inside .cll-experience scope. If .schema-change-badge-impacted or .row-impacted were ever applied without a .cll-experience ancestor, the variables would be unresolved (transparent/no box-shadow). Currently safe because isImpacted is only populated when CLL is on, and CLL adds .cll-experience — but a defensive :root fallback for --schema-badge-impacted-* could prevent a future surprise.
  2. Sync comment (styles.tsx:666-667): The "keep both sides in sync" comment between cllChangeStatusColors (JS) and .cll-experience (CSS) is good documentation. Dark-mode --schema-color-impacted-accent intentionally diverges (rgb(180 83 9) vs rgb(252 211 77)) for visual reasons — worth a brief inline note in the CSS explaining the dark-mode accent is deliberately different from the graph-side impacted color.

Copy link
Copy Markdown
Contributor

@even-wei even-wei left a comment

Choose a reason for hiding this comment

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

Claude Code Review: No critical issues found. Clean implementation with well-scoped feature flag gating.

@danyelf danyelf merged commit fb74a40 into main Apr 15, 2026
7 checks passed
@danyelf danyelf deleted the fix/cll-color-scheme branch April 15, 2026 07:13
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.

2 participants