feat(diff-views): per-bar histogram colors, sparkline rendering, color_static parity - #958
feat(diff-views): per-bar histogram colors, sparkline rendering, color_static parity#958paddymul wants to merge 1 commit into
Conversation
…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>
There was a problem hiding this comment.
💡 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" |
There was a problem hiding this comment.
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 👍 / 👎.
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— aHistogramBarmay carry an optionalcolor; thepopulationBar renders one rechartsCellper 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 wherecellRendererParamsare not plumbed).ChartCell— every line series draws atstrokeWidth={1}withdot={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 hiddenYAxiswith 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_REDjoinsColorMap,ColorStaticRulesjoinsColorMappingConfig, andChartDisplayerAgrows thecolorsfield the JS already consumes.compare.col_join_dfs— join-key columns emitcolor_staticdirectly, replacing the categorical-map-of-four-identical-colors workaround whose comment ("once the JS is rebuilt") has been stale sincecolor_staticlanded 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-datumCellfills on the population bar, colored and defaultChartCell.render.test.tsx— all line series 1px/dotless; hidden padded YAxis presentcompare_test.py::test_join_key_columns_use_color_static— join-key override emitscolor_staticDeliberately out of scope, noted for follow-up: plumbing
displayer_argsthroughgetCellRendererSelectorfor 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 fixedlineRed/lineGrayslots.🤖 Generated with Claude Code