Skip to content

feat(diff-views): per-bar histogram colors, sparkline rendering, color_static parity - #958

Open
paddymul wants to merge 1 commit into
mainfrom
feat/diff-view-pinned-plots
Open

feat(diff-views): per-bar histogram colors, sparkline rendering, color_static parity#958
paddymul wants to merge 1 commit into
mainfrom
feat/diff-view-pinned-plots

Conversation

@paddymul

@paddymul paddymul commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Renderer and type changes needed by diff views (surfaced by the tallyman diff-view prototyping; the sparkline fixes currently live downstream as CSS overrides on the bundled grid, which this PR obsoletes).

What

  • HistogramCell — a HistogramBar may carry an optional color; the population Bar renders one recharts Cell per datum so the color lands on that individual bar. Bars without a color keep the scheme-default fill. This lets a diff view ship a change-distribution histogram whose bars share the exact color key of the cells it summarizes (colors ride the data, so it works in pinned rows where cellRendererParams are not plumbed).
  • ChartCell — every line series draws at strokeWidth={1} with dot={false} (the 100×24 chart is CSS-scaled to fill its grid cell, which turns the recharts defaults into a fat blobby squiggle), and a hidden YAxis with 2px top/bottom padding keeps a series whose values sit exactly at dataMin/dataMax — e.g. the collapsed "after" side of a diff — off the plot border where the clip rect swallows half the stroke.
  • styling_core — Python catches up to the TS types: DIVERGING_BLUE_WHITE_RED joins ColorMap, ColorStaticRules joins ColorMappingConfig, and ChartDisplayerA grows the colors field the JS already consumes.
  • compare.col_join_dfs — join-key columns emit color_static directly, replacing the categorical-map-of-four-identical-colors workaround whose comment ("once the JS is rebuilt") has been stale since color_static landed in the compiled JS.

Why

Diff views color cells by a per-column change key. The summary row needs to tell the same story: a histogram of the change distribution colored by the same key, and before/after sparklines where the flattened "after" series stays visible. At 24px tall, dots plus scaled strokes make lines unreadable, and a floor-hugging series is half-clipped by the SVG border — both currently patched downstream with CSS attribute-selector overrides that reach into recharts internals.

Tests

TDD structure: the first commit carries only the failing tests so CI shows them red; the implementation lands in the follow-up commit on this PR.

  • HistogramCell.colors.test.tsx — per-datum Cell fills on the population bar, colored and default
  • ChartCell.render.test.tsx — all line series 1px/dotless; hidden padded YAxis present
  • compare_test.py::test_join_key_columns_use_color_static — join-key override emits color_static

Deliberately out of scope, noted for follow-up: plumbing displayer_args through getCellRendererSelector for pinned rows (blocks Python-configured chart colors in pinned rows), configurable pinned-row height, and a real per-series color/z-order API to replace the fixed lineRed/lineGray slots.

🤖 Generated with Claude Code

…ne rendering, color_static join keys

Three rendering contracts needed by downstream diff views (tallyman),
committed red per TDD:

- HistogramCell: a HistogramBar may carry an optional color, applied
  per-datum to the population bar via recharts Cell — lets a diff
  histogram share the color key of the cells it summarizes
- ChartCell: line series draw 1px with dots disabled (CSS cell-scaling
  turns the recharts defaults into a blobby squiggle), and a hidden
  padded YAxis keeps floor-hugging series off the clip border
- compare.col_join_dfs: join-key columns emit color_static directly
  instead of the categorical-map-of-identical-colors workaround

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99f90b97c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

_m_df, overrides, _eqs = col_join_dfs(df1, df2, join_columns=["id"], how="outer")

cfg = overrides["id"]["color_map_config"]
assert cfg["color_rule"] == "color_static"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep the new tests green before merging

When this commit is reviewed by itself, this assertion makes the suite fail: col_join_dfs still returns {"color_rule": "color_categorical", ...} for join keys in buckaroo/compare.py, and the same commit also adds JS assertions for ChartCell/HistogramCell behavior that those components still do not render. Because only tests changed here, CI will stay red for every run that includes these tests; include the corresponding implementation in this commit or keep these red tests out of the mergeable change.

Useful? React with 👍 / 👎.

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