From b8936b61cd9ff6b6bd184068021c5eb5241ee7ed Mon Sep 17 00:00:00 2001 From: debabsah Date: Mon, 5 Oct 2026 11:28:11 -0700 Subject: [PATCH] feat: fitted trendline KPIs, no table chrome over rows that show, named empty IN values Gaps a design review found while building a showcase dashboard, each checked in plugin source at 4.1.4, 5.0.0 and 6.1.0: - Trendline KPI: `y_axis_truncate` unticks "Start y-axis at 0" (start_y_axis_at_zero, BigNumberWithTrendline/controlPanel.tsx:96 at 4.1.4 and 5.0.0, :107 at 6.1.0), which the trendline reads as yAxis.scale: !startYAxisAtZero (transformProps.ts:219, :326 at 6.1.0). A trailing-12-month total that moved a few percent was a flat line over a solid block; fitted, it shows the movement. Omitted, it compiles to true as before, so the goldens are unchanged; decompile reads false back as the field and plan reports a change made in the UI. All three releases declare it: no version gate. - Design brain 10: no page-size picker, pager or search box on a table whose rows all show in its panel. Any page_length above 0 draws the picker, on one page too (hasPagination, DataTable.tsx:123, :173 at 6.1.0). default.search-box now fills only a raw table that pages, where the box sits in the picker's bar, and --fix removes a box it filled over rows that all show; size.table-chrome, a new info rule, names an author's page_length or search_box there. - Apply's data check names the values a chart's IN filter lists that return no rows, on a column the chart groups or draws by (a line chart listing four zones where two have data). A time column and a series-limited groupby are left alone, and a query that reached its row limit says so. No spec field where the plugin has no control, documented instead (spec descriptions, CONTRACTS.md, the chart-choice guide): - A mixed chart draws every line solid at one width: transformSeries takes a line style only from its caller, and MixedTimeseries passes none for either query at any of the three releases. 6.1.0's ECharts Options replace the series list whole and a theme override styles every series, so neither reaches one series. A reference series goes in its own query with a pale label_colors colour. - The area chart declares a fill opacity (already `opacity`) but no line width; on 6.1.0 a theme's echartsOptionsOverridesByChartType can thin every area chart's edges (read from source, not tried live). - Line, bar, area and scatter charts have one number format for the value axis, the value labels and the tooltip; a `~` format such as ",.1~f" keeps the axis in whole numbers. A mixed chart writes query B's value labels in number_format_secondary whichever axis B sits on. Verified live on 4.1.4, 5.0.0 and 6.1.0 with throwaway dashboards: apply, re-apply with stable ids, plan clean, decompile without losses, screenshots of each look and the data check naming an empty deal size. The display-controls live check passes on 6.1.0 with the fixture's new field. --- CHANGELOG.md | 10 +- chartwright/compiler.py | 6 +- chartwright/decompile.py | 6 +- chartwright/design/defaults.py | 29 +++--- chartwright/design/guidelines/chart-choice.md | 10 +- chartwright/design/model.py | 6 +- chartwright/design/rules.py | 40 ++++++++ chartwright/smoke.py | 59 ++++++++++-- chartwright/spec.py | 48 ++++++++-- docs/CONTRACTS.md | 58 ++++++++++++ docs/DESIGN-BRAIN.md | 21 ++++- docs/FEATURES.md | 4 +- docs/VERIFICATION.md | 6 +- schema/dashboard_spec.schema.json | 24 +++-- tests/fixtures/display_controls.json | 3 +- tests/fixtures/live_display_controls.json | 3 +- tests/test_design_defaults.py | 84 +++++++++++++++-- tests/test_display_controls.py | 8 +- tests/test_smoke_fit.py | 94 +++++++++++++++++++ tests/test_standards.py | 4 +- tests/test_standards_content.py | 2 +- 21 files changed, 461 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbc0afd..f6454b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ while the major version is 0, minor bumps may include breaking changes and say s ### Upgrading from 0.5 -- A spec with a mixed chart's `kind: "area"`, a trendline's `rolling_type`, a big number's `date_format` or a heatmap's `x_order` or `y_order` is refused by chartwright 0.5.0 and earlier (`literal_error` or `extra_forbidden`); upgrade CI before committing one. -- The design brain is version 9. `data.rolling-window-span` is a new warn-level rule, so `--design strict` can newly block a rolling trendline KPI whose time range can't hold its window. `advise --fix` fills `compare_suffix` as "vs prior 12 months" where a 12-step rolling window compares with the 12 steps before, and, with column types (`--profile`), `date_format` on a big number of a date column. +- A spec with a mixed chart's `kind: "area"`, a trendline's `rolling_type` or `y_axis_truncate`, a big number's `date_format` or a heatmap's `x_order` or `y_order` is refused by chartwright 0.5.0 and earlier (`literal_error` or `extra_forbidden`); upgrade CI before committing one. +- The design brain is version 10. `data.rolling-window-span` is a new warn-level rule, so `--design strict` can newly block a rolling trendline KPI whose time range can't hold its window. `advise --fix` fills `compare_suffix` as "vs prior 12 months" where a 12-step rolling window compares with the 12 steps before, and, with column types (`--profile`), `date_format` on a big number of a date column. It also removes a search box it filled on a table whose rows all show (below), so a spec it fixed before shows that change in its diff. ### Added @@ -17,6 +17,12 @@ while the major version is 0, minor bumps may include breaking changes and say s - `advise` warns when the chart's time range, or a defaulted dashboard time filter it loads with, is too short to hold the window and the comparison (`data.rolling-window-span`), and apply's data check warns from the time buckets the chart really has. - A big number shown as a date: `"date_format": "%a %-d %b %Y"` on a `big_number_total` shows `MAX(updated_at)` as "Sat 3 Oct 2026", for a tile that says how fresh the data is. It works on a date or timestamp metric and on a number of epoch milliseconds, and replaces `number_format`. With column types (`advise --profile`), `advise --fix` fills it on a big number of a date column's `MIN` or `MAX`, which Superset shows by default as its day alone ("Tue 31"). - Heatmap axis order: `x_order` (left to right) and `y_order` (top to bottom), each `a_to_z`, `z_to_a`, `value_asc` or `value_desc`. `"y_order": "a_to_z"` puts the first label on top, such as a cohort triangle's oldest cohort; omitted, the labels keep running A to Z from the bottom up. On Superset 6.1.0 a value order ranks each label by its total; older releases order by the cells, so `check` warns there. `decompile` reads the order back. +- A trendline KPI fitted to its values: `"y_axis_truncate": true` draws the trendline from its lowest value to its highest instead of up from zero, so a trailing-12-month total that moves a few percent shows the movement rather than a flat line over a solid block. `decompile` reads Superset's unticked "Start y-axis at 0" back as it. +- Apply's data check names the values a chart's `IN` filter lists that return no rows, on a column the chart groups or draws by: a line chart that lists four zones where two have data now says which two it draws nothing for. A series limit that drops values on purpose, or a time column, is left alone. + +### Changed + +- The design brain adds no page-size picker, pager or search box to a table whose rows all show in its panel. `advise --fix` now fills a search box only on a raw table that pages, where the box sits in the bar the page-size picker already draws, and removes one it filled on a table whose rows all show. `advise` names a `page_length` or `search_box` you wrote on such a table (`size.table-chrome`, info), since Superset draws a page-size picker for any page size, even over one page; set `page_length` to 0 or leave it out to show every row with no picker. ### Fixed diff --git a/chartwright/compiler.py b/chartwright/compiler.py index 824fa54..b79fed5 100644 --- a/chartwright/compiler.py +++ b/chartwright/compiler.py @@ -236,7 +236,11 @@ def metric(m: str): p["x_axis"] = chart.time_column p["time_grain_sqla"] = chart.time_grain or DEFAULT_TIME_GRAIN p["show_trend_line"] = True - p["start_y_axis_at_zero"] = True + # "Start y-axis at 0" (BigNumberWithTrendline/controlPanel.tsx:96 at 4.1.4 and 5.0.0, + # :107 at 6.1.0, default true), drawn as the trendline's yAxis scale: + # !startYAxisAtZero (transformProps.ts:219 at 4.1.4 and 5.0.0, :326 at 6.1.0), so + # false fits the line to its values. Always written, true unless y_axis_truncate. + p["start_y_axis_at_zero"] = not chart.y_axis_truncate # The Advanced Analytics rolling window (BigNumberWithTrendline controlPanel.tsx # :179-228 at 4.1.4 and 5.0.0, :252-301 at 6.1.0), applied to the trendline by # rollingWindowOperator in buildQuery, so transformProps reads the number and diff --git a/chartwright/decompile.py b/chartwright/decompile.py index dbcc73a..20c0752 100644 --- a/chartwright/decompile.py +++ b/chartwright/decompile.py @@ -668,6 +668,9 @@ def keep_row_limit() -> None: colour = _rgb_to_spec(p.get("color_picker")) if colour: out["trend_color"] = colour + # "Start y-axis at 0" unticked (an unset key is the control's default, true). + if p.get("start_y_axis_at_zero") is False: + out["y_axis_truncate"] = True _rolling_to_spec(p, out, losses, name) elif spec_type in ("timeseries_line", "timeseries_bar", "timeseries_area", "timeseries_scatter"): ms = [metric_one(m) for m in (p.get("metrics") or [])] @@ -1019,7 +1022,8 @@ def keep_row_limit() -> None: mapped_here = mapped_here | _legend_to_spec(p, out, spec_type) if spec_type == "big_number_trend": mapped_here = mapped_here | {"compare_lag", "compare_suffix", "subtitle", "color_picker", - "rolling_type", "rolling_periods", "min_periods"} + "rolling_type", "rolling_periods", "min_periods", + "start_y_axis_at_zero"} if spec_type == "big_number_total": mapped_here = mapped_here | {"subtitle", "time_format", "force_timestamp_formatting"} if spec_type == "table": diff --git a/chartwright/design/defaults.py b/chartwright/design/defaults.py index 7afdcb1..2fbbf54 100644 --- a/chartwright/design/defaults.py +++ b/chartwright/design/defaults.py @@ -346,8 +346,8 @@ def _page_length(ctx: RuleContext, c): @_fill("default.search-box", "search_box", {"table"}, - "a raw table of more than ~20 rows gets a search box, when its rows still fit " - "beside it (the 20 is judgement)", + "a raw table of more than ~20 rows that outgrow its panel, and so page, gets a " + "search box; one whose rows all show gets none (the 20 is judgement)", superset=False, superset_text="no search box", override="write search_box: false") def _search_box(ctx: RuleContext, c): @@ -358,16 +358,21 @@ def _search_box(ctx: RuleContext, c): n = ctx.params.search_min_rows if c.row_limit <= n: return None, f"row_limit {c.row_limit} is {n} rows or fewer" - if not c.page_length: - # A paged table already draws the bar the search box sits in. On one page, the - # bar takes room from the rows, and must not push any behind the scrollbar. - h = ctx.height(c.name) - fits = math.floor(grid_rows_visible(h, table_header_units(controls=True))) - if c.row_limit > fits: - return None, (f"{c.row_limit} rows on one page at height {h:g}: a search bar " - f"would leave room for {fits}; page the table or raise the height") - return True, (f"up to {c.row_limit} raw rows: searching beats scrolling " - f"(the {n}-row threshold is judgement)") + h = ctx.height(c.name) + # The rows the panel shows with nothing above them. A table that shows every row + # takes no DataTables chrome (size.table-chrome): a search bar over rows the reader + # already sees only pushes the last ones behind the scrollbar. + shown = math.floor(grid_rows_visible(h, table_header_units())) + if c.row_limit <= shown: + return None, f"all {c.row_limit} rows show at height {h:g}" + if not c.page_length or c.page_length >= c.row_limit: + # The box sits in the bar a paged table already draws (its page-size picker), so + # it costs no row there. A table on one page that outgrows its panel is either + # still to be paged (default.page-length) or too short to page. + return None, (f"{c.row_limit} rows on one page outgrow height {h:g}; the box comes " + f"with paging") + return True, (f"up to {c.row_limit} raw rows over pages of {c.page_length}: searching " + f"beats paging (the {n}-row threshold is judgement)") # -- legends and labels ------------------------------------------------------------- diff --git a/chartwright/design/guidelines/chart-choice.md b/chartwright/design/guidelines/chart-choice.md index 9677b43..4da627b 100644 --- a/chartwright/design/guidelines/chart-choice.md +++ b/chartwright/design/guidelines/chart-choice.md @@ -33,10 +33,16 @@ - A rolling KPI (`rolling_type`) shows a trailing window, not the latest period: name the window in its title or subtitle ("Revenue, trailing 12 months"), and give it a range that holds the window plus `compare_lag` - (24 months for a trailing year against the year before). + (24 months for a trailing year against the year before). Its trendline, + drawn up from zero, barely moves; when asked to show the movement, set + `y_axis_truncate`. - A paged table (`page_length`) needs room for one page and its pager, not - for every row; a timeseries with many groups keeps the top few with + for every row; a table whose rows all show takes no `page_length` or + `search_box`. A timeseries with many groups keeps the top few with `series_limit`. +- Superset draws every line of a mixed chart solid at one width; draw a + reference series lighter with a pale `label_colors` colour in its own + query. - One dominant category flattening its siblings is the data talking: note it or filter it, don't hide it. - Every chart with a time axis should tolerate the dashboard time filter; diff --git a/chartwright/design/model.py b/chartwright/design/model.py index 11085d9..7c67c63 100644 --- a/chartwright/design/model.py +++ b/chartwright/design/model.py @@ -32,7 +32,11 @@ # ordered by value alone; default.date-tile fills date_format on a big number of a date # column (data-aware), and a big number shown as a date gets no number format from # narrative.big-number-format, default.count-format or a standard. -DESIGN_BRAIN_VERSION = "9" +# "10" = no DataTables chrome on a table whose rows all show: default.search-box fills only +# a paged table whose rows outgrow its panel (and --fix removes a box it filled on one +# whose rows all show), and size.table-chrome, a new info rule, names an author's page +# size or search box there. +DESIGN_BRAIN_VERSION = "10" KPI_TYPES = {"big_number_total", "big_number_trend"} TIMESERIES_TYPES = {"timeseries_line", "timeseries_bar", "timeseries_area", "timeseries_scatter"} diff --git a/chartwright/design/rules.py b/chartwright/design/rules.py index cc926dc..6a43945 100644 --- a/chartwright/design/rules.py +++ b/chartwright/design/rules.py @@ -27,6 +27,7 @@ grid_rows_visible, grid_units_for_rows, row_items, + table_header_units, without_superset_defaults, ) from .model import AXIS_TYPES, KPI_TYPES, TIMESERIES_TYPES, Finding, RuleContext, rule @@ -201,6 +202,45 @@ def table_window(ctx: RuleContext): ) +@rule("size.table-chrome", "info", + "a table whose rows all show in its panel draws no page-size picker, pager or search " + "box", since="10") +def table_chrome(ctx: RuleContext): + """DataTables chrome over rows the reader already sees. Any page_length above 0 draws + the page-size picker, on one page too (hasPagination, plugin-chart-table + DataTable/DataTable.tsx:123 at 4.1.4 and 5.0.0, :173 at 6.1.0), and the pager joins + it from a second page (:407 at 4.1.4 and 5.0.0, :616 at 6.1.0); search_box + (include_search) draws the search bar (:383 at 4.1.4 and 5.0.0, :593 at 6.1.0). The + brain's own fills never add them to such a table (default.page-length, + default.search-box), so this names the author's.""" + for c in ctx.spec.charts: + if c.type != "table" or c.row_limit is None: + continue + h = ctx.height(c.name) + # The rows the panel shows with nothing above or below them (spec.py's grid model). + shown = math.floor(grid_rows_visible(h, table_header_units())) + if c.row_limit > shown: + continue # the rows outgrow the panel: paging and search earn their room + chrome, fields = [], [] + if c.page_length and not ctx.brain_owns(c, "page_length"): + pages = math.ceil(c.row_limit / c.page_length) + chrome.append("a page-size picker" + (f" and a pager over {pages} pages" + if pages > 1 else "")) + fields.append(f"page_length {c.page_length}") + if c.search_box and not ctx.brain_owns(c, "search_box"): + chrome.append("a search box") + fields.append("search_box") + if not chrome: + continue + yield Finding( + "size.table-chrome", "info", c.name, ctx.where(c.name), + f"all {c.row_limit} rows show at height {h:g}, yet {' and '.join(fields)} " + f"draw{'s' if len(fields) == 1 else ''} {' and '.join(chrome)} over them; " + f"remove {'it' if len(fields) == 1 else 'them'} so the table reads whole " + f"(page_length 0 shows every row on one page with no picker)", + ) + + @rule("size.hbar-window", "warn", "horizontal bars need ~0.5 units of height per bar", fixable=True) def hbar_window(ctx: RuleContext): for c in ctx.spec.charts: diff --git a/chartwright/smoke.py b/chartwright/smoke.py index d9b0ae9..b826c1a 100644 --- a/chartwright/smoke.py +++ b/chartwright/smoke.py @@ -8,7 +8,8 @@ stays the renderer's job. Pass = HTTP 200 AND non-empty result payload. Empty = warning naming the -chart, not a silent pass. +chart, not a silent pass; so are values an IN filter lists on a column the +chart groups or draws by that return no rows (they would draw nothing). """ from __future__ import annotations @@ -79,6 +80,50 @@ def _window_warning(chart, rows: int) -> str | None: + (" and no change" if lag else "") + "; widen the chart's time range") +def _value_key(v) -> str: + """A filter value or a returned one, compared as text: a list of [2, 4] matches the 2 + and 4 a numeric column returns, and 2.0 reads as 2.""" + if isinstance(v, bool): + return str(v).lower() + if isinstance(v, float) and v.is_integer(): + return str(int(v)) + return str(v) + + +def _series_limited(chart, column: str) -> bool: + """The chart keeps only its top N series by `column`, so listed values it drops are + the limit's doing, not missing data.""" + holders = (chart.a, chart.b) if chart.type == "mixed" else (chart,) + return any(getattr(h, "series_limit", None) and getattr(h, "groupby", None) == column + for h in holders) + + +def _empty_values_warning(chart, queries: list[dict], result: list, ds) -> str | None: + """Values a chart lists with IN on a column it groups or draws by, which come back + with no rows: a line chart that names four zones where two have data draws two lines + and a legend of two, and nothing on the dashboard says the other two are missing. + Only a column the query returns can be checked, since the data says which values + came back; a time column comes back as timestamps, so it is left alone.""" + notes = [] + for f in chart.filters: + if f.op != "IN" or f.sql is not None or not isinstance(f.value, list): + continue + column = f.column + pairs = [(q, r) for q, r in zip(queries, result) if column in q.get("columns", [])] + if not pairs or ds.is_temporal(column) or _series_limited(chart, column): + continue + back = {_value_key(row.get(column)) for _, r in pairs for row in r.get("data") or []} + missing = [v for v in f.value if _value_key(v) not in back] + if not missing: + continue + capped = any(len(r.get("data") or []) >= q.get("row_limit", math.inf) for q, r in pairs) + names = ", ".join(repr(v) for v in missing) + notes.append(f"{column} IN lists {names}, which return{'s' if len(missing) == 1 else ''}" + f" no rows" + (" within the row limit" if capped else "") + + f", so the chart draws nothing for {'it' if len(missing) == 1 else 'them'}") + return "; ".join(notes) or None + + @dataclass class SmokeResult: chart: str @@ -209,10 +254,11 @@ def _mixed_queries(chart, spec: DashboardSpec, ds) -> list[dict]: def smoke_chart(chart, spec: DashboardSpec, resolution: Resolution, client: SupersetClient) -> SmokeResult: ds = resolution.for_chart(chart.dataset) + queries = [_with_time_range(q, chart, ds) for q in ( + _mixed_queries(chart, spec, ds) if chart.type == "mixed" else [_query_for(chart, spec)])] ctx = { "datasource": {"id": ds.id, "type": "table"}, - "queries": [_with_time_range(q, chart, ds) for q in ( - _mixed_queries(chart, spec, ds) if chart.type == "mixed" else [_query_for(chart, spec)])], + "queries": queries, "result_format": "json", "result_type": "full", } @@ -226,9 +272,10 @@ def smoke_chart(chart, spec: DashboardSpec, resolution: Resolution, client: Supe return SmokeResult(chart.name, False, False, f"unparseable chart/data response: {e}") if rows == 0: return SmokeResult(chart.name, True, True, "query succeeded but returned 0 rows") - fit = _fit_warning(chart, spec, result) or _window_warning(chart, rows) - if fit: - return SmokeResult(chart.name, True, True, f"{rows} rows; {fit}") + notes = [w for w in (_fit_warning(chart, spec, result) or _window_warning(chart, rows), + _empty_values_warning(chart, queries, result, ds)) if w] + if notes: + return SmokeResult(chart.name, True, True, f"{rows} rows; " + "; ".join(notes)) return SmokeResult(chart.name, True, False, f"{rows} rows") diff --git a/chartwright/spec.py b/chartwright/spec.py index 3e254a9..3bb9ab3 100644 --- a/chartwright/spec.py +++ b/chartwright/spec.py @@ -441,6 +441,14 @@ class BigNumberTrendChart(_ChartBase): "point (and the number) is a whole trailing window; 0 also draws the " "partial windows of the first steps, as Superset's own default does", ) + y_axis_truncate: bool = Field( + default=False, + description="Fit the trendline to its values instead of drawing it up from zero " + "(Superset's Start y-axis at 0, unticked). A trailing-12-month total " + "that moves a few percent is a flat line over a solid block from zero; " + "fitted, the line shows the movement. Say so where the reader expects a " + "zero baseline", + ) @field_validator("trend_color", mode="before") @classmethod @@ -603,13 +611,22 @@ def json_value(v) -> str: return "true" if v is True else "false" if v is False else f'"{v}"' +# The line, bar, area and scatter panels have one number format, "Axis Format" +# (y_axis_format), for the value axis, the values on the marks and the tooltip +# (Timeseries/transformProps.ts at 4.1.4, 5.0.0 and 6.1.0); 6.1.0's x_axis_number_format +# formats a numeric x axis only. +_VALUE_FORMAT = ("d3 format for the value axis, e.g. {example}. Superset uses the same " + "format for the values show_value writes and the tooltip; a '~' drops " + "trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5") + + class _TimeseriesBase(_SeriesDisplay, _Legend, _AxisChart, _ColorSchemeMixin): metrics: list[str] = Field(min_length=1) time_column: str time_grain: str | None = Field(default=None, description="ISO 8601 duration, e.g. P1D, P1W, P1M") groupby: str | None = Field(default=None, description="At most one dimension column") row_limit: int | None = Field(default=None, ge=1) - number_format: str | None = Field(default=None, description="d3 format for the value axis, e.g. '.1%'") + number_format: str | None = Field(default=None, description=_VALUE_FORMAT.format(example="'.1%'")) STACKS: ClassVar[tuple] = (False, True, "stream") @@ -652,7 +669,9 @@ class TimeseriesAreaChart(_TimeseriesBase): markers: bool = Field(default=False, description="A marker at each point") marker_size: int | None = _marker_size() opacity: float | None = Field( - default=None, ge=0, le=1, description="Opacity of the area fill, 0-1 (default 0.2)") + default=None, ge=0, le=1, + description="Opacity of the area fill, 0-1 (default 0.2). Each area's edge stays " + "a full-strength line: Superset's area chart has no line width") STACKS: ClassVar[tuple] = (False, True, "stream", "expand") @model_validator(mode="after") @@ -681,7 +700,7 @@ class BarChart(_SeriesDisplay, _Legend, _AxisChart, _ColorSchemeMixin): groupby: str | None = None row_limit: int | None = Field(default=None, ge=1) orientation: Literal["vertical", "horizontal"] = "vertical" - number_format: str | None = Field(default=None, description="d3 format for the value axis, e.g. ',.0f'") + number_format: str | None = Field(default=None, description=_VALUE_FORMAT.format(example="',.0f'")) category_sort: Literal["asc", "desc"] | None = Field( default=None, description="Order the bars by their category (x_column) instead of by the first " @@ -1042,7 +1061,11 @@ class TreemapChart(_ChartBase, _ColorSchemeMixin): class MixedSeries(BaseModel): """One of a mixed chart's two queries: its metrics, drawn as bars, a line or a - filled area, on the primary (left) or secondary (right) value axis.""" + filled area, on the primary (left) or secondary (right) value axis. Superset draws + every line of a mixed chart solid at one width: it has no line width or dash for a + query or a metric. To draw a reference series lighter, such as last year under this + year, put it in its own query and give it a pale colour with the dashboard's + label_colors, or make it an area with a low opacity.""" model_config = ConfigDict(extra="forbid") @@ -1110,8 +1133,21 @@ class MixedChart(_Legend, _AxisChart, _ColorSchemeMixin): a: MixedSeries b: MixedSeries row_limit: int | None = Field(default=None, ge=1) - number_format: str | None = Field(default=None, description="d3 format for the primary axis") - number_format_secondary: str | None = Field(default=None, description="d3 format for the secondary axis") + # MixedTimeseries/transformProps.ts formats each axis, and the tooltip values of the + # series on it, with that axis's format (:268-282 and :605-631 at 4.1.4, :348-357 and + # :800-827 at 6.1.0), but query A's value labels with y_axis_format and query B's with + # y_axis_format_secondary, whichever axis each sits on (:374-380 and :421-427 at 4.1.4, + # :449-455 and :522-528 at 6.1.0). + number_format: str | None = Field( + default=None, + description="d3 format for the primary axis and the tooltip values of its series; " + "Superset also writes query a's show_value labels in it, whichever axis " + "a sits on") + number_format_secondary: str | None = Field( + default=None, + description="d3 format for the secondary axis and the tooltip values of its series; " + "Superset also writes query b's show_value labels in it, whichever axis " + "b sits on") y_axis_title_secondary: str | None = Field(default=None, description="Title of the secondary axis") y_axis_min_secondary: float | None = Field(default=None, description="Bottom of the secondary axis") y_axis_max_secondary: float | None = Field(default=None, description="Top of the secondary axis") diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index 3de4f22..cdd3902 100644 --- a/docs/CONTRACTS.md +++ b/docs/CONTRACTS.md @@ -393,6 +393,12 @@ running the tool against real instances of all three releases. `:699`, 6.1.0 `:789`). The tool never turns server pagination on, so it writes no `server_page_length` (0.3.0 and earlier wrote Superset's stored default, 10, which changed nothing). `page_length` in the spec sets the page. + Any `page_length` above 0 draws the page-size picker, on a table of one page + too, and the pager joins it from a second page (`hasPagination`, + `DataTable/DataTable.tsx:123` and `:407` at 4.1.4 and 5.0.0, `:173` and + `:616` at 6.1.0); `search_box` adds the search bar beside the picker (`:383` + at 4.1.4 and 5.0.0, `:593` at 6.1.0). The design brain adds neither to a + table whose rows all show in its panel (DESIGN-BRAIN.md §16). - **A bar sorts its categories two ways.** With one series a post-processing sort orders the returned rows on the x column (`operators/sortOperator.ts`, which skips any chart with a groupby, 4.1.4 and 5.0.0 `:46`, 6.1.0 `:45`); @@ -462,6 +468,58 @@ running the tool against real instances of all three releases. including the older `echarts_timeseries_line` and `echarts_timeseries_bar` names, draws a straight line (`Timeseries/transformers.ts:237-243` at 4.1.4, `:306-312` at 6.1.0), so decompile reads it as `kind: "line"`. +- **A mixed chart draws every line solid, at one width.** `transformSeries` + takes a series' line style only from its caller + (`Timeseries/transformers.ts:273-276` at 4.1.4, `:287-290` at 5.0.0, + `:356-359` at 6.1.0), and the Mixed Chart hands it none, for either query + (`MixedTimeseries/transformProps.ts:382-411` and `:429-460` at 4.1.4, `:384` + and `:432` at 5.0.0, `:457-493` and `:530-567` at 6.1.0). Its panel has no + width or dash for a query or a metric, so the spec offers none. 6.1.0's + ECharts Options can't single out a series either: they replace the chart's + list of series whole (`utils/mergeCustomEChartOptions.ts:64-73`), and a + theme's `echartsOptionsOverridesByChartType` merges one style into every + series (`components/Echart.tsx:254-273`, `utils/themeOverrides.ts:69-89`). + To draw a reference series lighter, such as list price under price paid, + put it in its own query and give it a pale colour with the dashboard's + `label_colors`, or make it an area with a low `opacity`. Seen live on 4.1.4, + 5.0.0 and 6.1.0: a pale grey list price beside a blue price paid, both + 2 px solid. Superset dashes a line only + for a line chart's time comparison (`time_compare`: `Timeseries/transformProps.ts:285-297` + at 4.1.4, `:288-295` at 5.0.0, and `:401-423` at 6.1.0 while "Match time + shift color" is ticked, its default), which the spec doesn't write yet; + until it does, a second metric in its own query is the way to draw a + comparison. +- **An area chart's edges have no width.** The Area panel declares the fill's + `opacity` (`Timeseries/Area/controlPanel.tsx:92` at 4.1.4, `:93` at 5.0.0, + `:97` at 6.1.0), which the spec writes as `opacity`, but no line width, so + each area keeps a full-strength edge. On 6.1.0 a theme can thin the edges of + every area chart on the dashboards that use it, with + `"echartsOptionsOverridesByChartType": {"echarts_area": {"series": + {"lineStyle": {"width": 1}}}}` at the top of the theme's JSON + (`superset-core/src/theme/Theme.tsx:119-137`); that was read from the + source, not tried live. +- **A line, bar, area or scatter chart has one number format for its axis + and its values.** The panel's "Axis Format" (`y_axis_format`, + `Timeseries/Regular/Bar/controlPanel.tsx:192-195` at 4.1.4 and 5.0.0, + `:228-231` at 6.1.0) formats the value axis, the values on the marks and + the tooltip (`Timeseries/transformProps.ts` 4.1.4 `:268-270`, `:318-324` + and `:515-521`; 5.0.0 `:271-273`, `:316-322` and `:515-521`; 6.1.0 + `:365-370`, `:490-496` and `:961-967`). 6.1.0's `x_axis_number_format` + formats a numeric x axis only (`:189-196`). So `number_format` sets all + three, and a format with decimals puts them on the axis too. A `~` drops + trailing zeros: seen live on 4.1.4, 5.0.0 and 6.1.0, `,.1~f` labels the + axis in whole numbers (0, 20, 40) and a bar 47.2, where `.1f` labels the + axis 0.0, 20.0, 40.0; a value that is a whole number then shows without its + decimal too. +- **A trendline KPI's line starts at zero unless `y_axis_truncate`.** The + panel's "Start y-axis at 0" (`start_y_axis_at_zero`, + `BigNumberWithTrendline/controlPanel.tsx:96` at 4.1.4 and 5.0.0, `:107` at + 6.1.0, default true) becomes the trendline's `yAxis.scale: + !startYAxisAtZero` (`transformProps.ts:219` at 4.1.4 and 5.0.0, `:326` at + 6.1.0), and the tool always wrote true. `y_axis_truncate: true` writes + false, so the line spans the card from its lowest value to its highest. Seen + live on 4.1.4, 5.0.0 and 6.1.0: a trailing-12-month total of $5.20M, a flat + line over a solid block from zero, rises across the card fitted. - **A heatmap's y axis runs from the bottom up.** Both axes are ECharts category axes, which put the first label at the left and at the bottom (`Heatmap/transformProps.ts:227-240` at 4.1.4, `:431-447` at 6.1.0). The diff --git a/docs/DESIGN-BRAIN.md b/docs/DESIGN-BRAIN.md index 0e9bd79..a91a08c 100644 --- a/docs/DESIGN-BRAIN.md +++ b/docs/DESIGN-BRAIN.md @@ -1,8 +1,10 @@ # The Design Brain -> **Status: SHIPPED, design brain 7** (4 added `narrative.color-scheme`; 5 the +> **Status: SHIPPED, design brain 10** (4 added `narrative.color-scheme`; 5 the > design defaults of §16; 6 the `standard.*` rules of §18's content; 7 fills keeping -> a null record after an edit, and `default.stale-record`). This page is both the design and the +> a null record after an edit, and `default.stale-record`; 9 rolling trendline +> windows and date tiles; 10 no page-size picker, pager or search box on a table +> whose rows all show, and `size.table-chrome`). This page is both the design and the > reference for the implementation in `chartwright/design/`. The decision log > at the bottom records every judgment call made without a review gate; §15 > records where the implementation deliberately deviates from the design @@ -330,7 +332,7 @@ fails when it drifts. | `default.count-format` | info | ✔ | - | 5 | counts read as whole numbers with thousands separators (',.0f') | | `default.date-tile` | info | ✔ | - | 9 | a big number of a date column's MIN or MAX reads as a whole date ('Sat 3 Oct 2026'); needs the column's type (--profile) | | `default.page-length` | info | ✔ | - | 5 | a table whose row_limit outgrows its panel pages by the rows that fit beside its page controls | -| `default.search-box` | info | ✔ | - | 5 | a raw table of more than ~20 rows gets a search box, when its rows still fit beside it (the 20 is judgement) | +| `default.search-box` | info | ✔ | - | 5 | a raw table of more than ~20 rows that outgrow its panel, and so page, gets a search box; one whose rows all show gets none (the 20 is judgement) | | `default.single-series-legend` | info | ✔ | - | 5 | a single series named by the chart or y-axis title needs no legend | | `default.stale-record` | info | ✔ | - | 7 | design.filled names only charts and fields the spec has | | `default.value-labels` | info | ✔ | - | 5 | few bars carry their values: <= 12 bars, on a panel >= 6/12 wide (vertical) or tall enough to space the labels (horizontal) | @@ -364,6 +366,7 @@ fails when it drifts. | `size.pie-geometry` | warn | ✔ | - | 1 | pies need >= 5/12 width and 8 height or the ring shrinks and the legend crowds | | `size.pivot-window` | warn | ✔ | - | 2 | a pivot's height should show a meaningful share of its row_limit | | `size.row-harmony` | warn | ✔ | - | 1 | charts sharing a row should share a height (Superset sizes the row to its tallest child) | +| `size.table-chrome` | info | - | - | 10 | a table whose rows all show in its panel draws no page-size picker, pager or search box | | `size.table-window` | warn | ✔ | - | 1 | a table's height should show a meaningful share of its row_limit | | `standard.classification` | error | - | - | 6 | the dashboard's classification is one the standard lists | | `standard.content-locked` | error | - | - | 6 | content a standard locks is in the spec as the standard has it (standards apply writes it; a change goes through the standard's file) | @@ -1077,13 +1080,23 @@ thresholds; those marked judgement are usability choices, not pixel facts. | `default.count-format` | `number_format` | `,.0f` | every metric the chart shows is `COUNT` or `COUNT_DISTINCT`, on a chart with one `number_format` (not a table or a mixed chart); not with `contribution`, a 100 % stack, or a pivot aggregation that leaves fractions | | `default.cell-bars` | `cell_bars` | `false` | a raw-mode table with a column named `id`, `code`, `year`, `zip`, `zipcode` or `postcode` as a whole trailing token (`order_id`, `fiscal_year`; not `uuid` or `zip_count`). With column types (`--profile`), only a numeric one counts. Aggregate tables draw bars on metrics only, so they never need it | | `default.page-length` | `page_length` | the whole rows that fit beside the page-size bar and the pager | a table with an explicit `row_limit` larger than the rows that fit on one page, and a page of at least `page_min_rows` (3, judgement). The one grid model `size.table-window` reads, so the fill can never make that rule ask for more height | -| `default.search-box` | `search_box` | `true` | a raw-mode table with an explicit `row_limit` above `search_min_rows` (20, judgement), when the bar the box sits in hides no row: a paged table already draws it, and a table on one page must still fit every row beside it | +| `default.search-box` | `search_box` | `true` | a raw-mode table with an explicit `row_limit` above `search_min_rows` (20, judgement) that outgrows its panel and pages (`page_length` above 0 and below `row_limit`), so the box sits in the bar the page-size picker already draws and hides no row. Never on a table whose rows all show (brain 10; brain 5 to 9 also filled one there when every row still fit beside it) | | `default.single-series-legend` | `show_legend` | `false` | a timeseries chart or categorical bar with one metric, no groupby, no series limit, no goal lines and no legend placement written, whose shown title or `y_axis_title` contains the metric's label, and that label is at least 3 characters long. Never a heatmap, whose legend is the colour scale | | `default.value-labels` | `show_value` | `true` | a categorical bar with one metric, no groupby, no `contribution`, an explicit `row_limit` of at most `value_label_max_bars` (12). A vertical bar also needs a width of at least `value_label_min_width` (6/12); a horizontal bar needs the height to space its labels, 4.5 units plus 0.4125 a bar (§17) | `narrative.big-number-format` stands down where `default.count-format` offers the same remedy with a fix: one remedy, one finding. +A table whose rows all show in its panel gets no DataTables chrome from the +brain: no page-size picker, which any `page_length` above 0 draws even on one +page (`hasPagination`, `plugin-chart-table/src/DataTable/DataTable.tsx:123` at +4.1.4 and 5.0.0, `:173` at 6.1.0), no pager and no search box. The two table +fills read the same grid model, so neither adds one there, and a fill an +earlier brain left is removed by the next `--fix`. `size.table-chrome` (info, +no fix) names an author's `page_length` or `search_box` on such a table; +"rows" there is `row_limit`, the most the table can show, so a table with no +`row_limit` is left alone. + The decision record's suggestion-only list stays without fills: category sort (`chart.ordinal-order` reports ordinal names), y-axis truncation, `compare_lag`, a trendline's rolling window (`data.rolling-window-span` reports diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 77b52d2..2c25b48 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -34,6 +34,7 @@ into one. Everything below works from that one file. - A time range per chart, such as a "Last 30 days" KPI on a dashboard that shows all time. - A trendline KPI's change against an earlier period ("+4% vs last month"), its line colour, and on Superset 6.0+ a subtitle. - A trendline KPI over a rolling window: a trailing-12-month total compared with the 12 months before ("+30% vs prior 12 months"), a 3-month average, or a running total. + - A trendline fitted to its values, so a trailing total that moves a few percent shows the movement. - Filled areas on a mixed chart, such as solar output under a net-load line. - A big number shown as a date, such as the latest order date as "Sat 3 Oct 2026". - Tables: page size, a totals row, a search box, column alignment and widths, and on Superset 6.0+ header names. @@ -73,7 +74,7 @@ into one. Everything below works from that one file. - **The Critic**: `chartwright advise` reviews a finished spec: readable minimum sizes, layout composition (KPIs first, fold budgets, row density), chart-choice limits, narrative polish. `--fix` applies the safe geometry subset; `--profile` adds data-aware checks (a time axis on a non-temporal column, a pie hiding 40 slices); `--chart` looks at one chart. - **Design Defaults in the Spec**: Leave the small display choices unset and `advise --fix` writes sensible values into the spec, where the diff shows them; `chartwright explain` says where each value came from and how to change it. - Monthly time axes labelled `Sep 2026`, counts shown as `12,345`, "vs previous month" after a trendline KPI's change. - - Tables that page by what fits their panel, a search box on long raw tables, no cell bars behind id, code, year or zip columns. + - Tables that page by what fits their panel, a search box on long raw tables that page, no cell bars behind id, code, year or zip columns. A table whose rows all fit its panel shows just its rows, and `advise` points out a page size or search box you wrote there. - No legend on a single series the title already names, values written on a few bars. - A field you write is never touched. A filled value is kept up to date as the chart changes until you edit or delete it; then it is yours, and a deleted one stays deleted. To keep Superset's default from the start, add the rule to `design.ignore`. - The bundle depends on the spec alone: compile, `plan` and decompile never add a value of their own. @@ -153,6 +154,7 @@ into one. Everything below works from that one file. - **Self-Healing Applies**: When an apply fails while preparing, importing or updating charts, the previous state is restored automatically, whatever the error. A failure after that (linkage, filter scopes, chart queries) leaves the new version live, and the report gives the backup to restore. - **Stale-Tab Protection**: An old browser tab writing back stale state is detected by `plan` and repaired by `apply`. - **Verified at Every Step**: References are checked before anything is written, the finished dashboard is compared chart-by-chart against the spec, and every chart's query is run once: an error fails the apply, and a chart that returns no rows is named. Any failure says what went wrong and where. + - The data check also names each value a chart's `IN` filter lists that has no rows, such as the two empty zones of a line chart that lists four. - Your spec is held to the instance's Superset release too: a setting the release can't take (tags or a theme on 4.1.4 or 5.0.0, chart timestamps before 6.1) stops the apply before anything is written and names the field to remove; one it would ignore, such as a trendline subtitle before 6.0, comes back as a warning. Offline, `chartwright compile --superset-version 5.0.0` runs the same check. - **Ownership Guard**: The tool overwrites only dashboards it created or that you adopted with `chartwright adopt`; to manage any other dashboard, adopt it first. diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 315db08..ba8d02d 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -35,7 +35,7 @@ injection. | Layer | Proves | Where it runs | |---|---|---| -| Offline suite (1624 tests, 67 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | +| Offline suite (1727 tests, 67 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | | Chart-option contract | Every emitted chart option is declared by each version's plugin source | every PR and push to main | | Live guarantee check | Three specs applied (every chart type, every display control, every dashboard and filter control), per-chart data check, ids stable across re-apply | every PR and push to main, all 3 versions | | Standards content round trip | `standards apply` writes an org footer, a team header and two CSS blocks into a spec; it applies, `plan` stays clean, decompile reads the CSS markers and every managed row back, `--claim` rebuilds the record, re-apply stays clean | every PR and push to main, all 3 versions | @@ -217,8 +217,8 @@ filter bar with two value pickers and a time range, plus markdown and tabs. second-writer and fault-injection runs.) The same check then runs on two more specs, `tests/fixtures/live_display_controls.json` (every chart display control: legends, axis titles and bounds, stacking, labels, table and pivot -options, a trendline KPI's rolling window, a mixed chart's filled areas and a -heatmap's axis order) and `tests/fixtures/live_dashboard_controls.json` (dashboard +options, a trendline KPI's rolling window and fitted axis, a mixed chart's +filled areas and a heatmap's axis order) and `tests/fixtures/live_dashboard_controls.json` (dashboard settings and owners, colour schemes, goal lines, a header and footer outside the tabs, header rows inside them, cascading and pre-filtered native filters, time grain and time column filters). They are diff --git a/schema/dashboard_spec.schema.json b/schema/dashboard_spec.schema.json index 49e49c2..930df2b 100644 --- a/schema/dashboard_spec.schema.json +++ b/schema/dashboard_spec.schema.json @@ -576,7 +576,7 @@ } ], "default": null, - "description": "d3 format for the value axis, e.g. ',.0f'", + "description": "d3 format for the value axis, e.g. ',.0f'. Superset uses the same format for the values show_value writes and the tooltip; a '~' drops trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5", "title": "Number Format" }, "category_sort": { @@ -1106,6 +1106,12 @@ "default": null, "description": "Steps a window needs before the trendline shows it. Omitted, it is rolling_periods: the line starts at the first full window, so every point (and the number) is a whole trailing window; 0 also draws the partial windows of the first steps, as Superset's own default does", "title": "Rolling Min Periods" + }, + "y_axis_truncate": { + "default": false, + "description": "Fit the trendline to its values instead of drawing it up from zero (Superset's Start y-axis at 0, unticked). A trailing-12-month total that moves a few percent is a flat line over a solid block from zero; fitted, the line shows the movement. Say so where the reader expects a zero baseline", + "title": "Y Axis Truncate", + "type": "boolean" } }, "required": [ @@ -3207,7 +3213,7 @@ } ], "default": null, - "description": "d3 format for the primary axis", + "description": "d3 format for the primary axis and the tooltip values of its series; Superset also writes query a's show_value labels in it, whichever axis a sits on", "title": "Number Format" }, "number_format_secondary": { @@ -3220,7 +3226,7 @@ } ], "default": null, - "description": "d3 format for the secondary axis", + "description": "d3 format for the secondary axis and the tooltip values of its series; Superset also writes query b's show_value labels in it, whichever axis b sits on", "title": "Number Format Secondary" }, "y_axis_title_secondary": { @@ -3282,7 +3288,7 @@ }, "MixedSeries": { "additionalProperties": false, - "description": "One of a mixed chart's two queries: its metrics, drawn as bars, a line or a\nfilled area, on the primary (left) or secondary (right) value axis.", + "description": "One of a mixed chart's two queries: its metrics, drawn as bars, a line or a\nfilled area, on the primary (left) or secondary (right) value axis. Superset draws\nevery line of a mixed chart solid at one width: it has no line width or dash for a\nquery or a metric. To draw a reference series lighter, such as last year under this\nyear, put it in its own query and give it a pale colour with the dashboard's\nlabel_colors, or make it an area with a low opacity.", "properties": { "metrics": { "items": { @@ -5508,7 +5514,7 @@ } ], "default": null, - "description": "d3 format for the value axis, e.g. '.1%'", + "description": "d3 format for the value axis, e.g. '.1%'. Superset uses the same format for the values show_value writes and the tooltip; a '~' drops trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5", "title": "Number Format" }, "type": { @@ -5549,7 +5555,7 @@ } ], "default": null, - "description": "Opacity of the area fill, 0-1 (default 0.2)", + "description": "Opacity of the area fill, 0-1 (default 0.2). Each area's edge stays a full-strength line: Superset's area chart has no line width", "title": "Opacity" } }, @@ -6013,7 +6019,7 @@ } ], "default": null, - "description": "d3 format for the value axis, e.g. '.1%'", + "description": "d3 format for the value axis, e.g. '.1%'. Superset uses the same format for the values show_value writes and the tooltip; a '~' drops trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5", "title": "Number Format" }, "type": { @@ -6482,7 +6488,7 @@ } ], "default": null, - "description": "d3 format for the value axis, e.g. '.1%'", + "description": "d3 format for the value axis, e.g. '.1%'. Superset uses the same format for the values show_value writes and the tooltip; a '~' drops trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5", "title": "Number Format" }, "type": { @@ -6993,7 +6999,7 @@ } ], "default": null, - "description": "d3 format for the value axis, e.g. '.1%'", + "description": "d3 format for the value axis, e.g. '.1%'. Superset uses the same format for the values show_value writes and the tooltip; a '~' drops trailing zeros, so ',.1~f' labels the axis 0, 5, 10 and a value 12.5", "title": "Number Format" }, "type": { diff --git a/tests/fixtures/display_controls.json b/tests/fixtures/display_controls.json index 84f2e51..e5591a9 100644 --- a/tests/fixtures/display_controls.json +++ b/tests/fixtures/display_controls.json @@ -26,7 +26,8 @@ "rolling_periods": 12, "rolling_min_periods": 6, "compare_lag": 12, - "subtitle": "Trailing 12 months" + "subtitle": "Trailing 12 months", + "y_axis_truncate": true }, { "name": "Orders This Quarter", diff --git a/tests/fixtures/live_display_controls.json b/tests/fixtures/live_display_controls.json index 0406633..abf15f0 100644 --- a/tests/fixtures/live_display_controls.json +++ b/tests/fixtures/live_display_controls.json @@ -26,7 +26,8 @@ "rolling_periods": 12, "rolling_min_periods": 6, "compare_lag": 12, - "subtitle": "Trailing 12 months" + "subtitle": "Trailing 12 months", + "y_axis_truncate": true }, { "name": "Orders This Quarter", diff --git a/tests/test_design_defaults.py b/tests/test_design_defaults.py index e0ddec4..54ce045 100644 --- a/tests/test_design_defaults.py +++ b/tests/test_design_defaults.py @@ -375,17 +375,81 @@ def test_search_threshold_comes_from_the_audience_params(): assert fills(data, "default.search-box", overlay=Overlay(params={"search_min_rows": 100})) == {} -def test_a_search_box_on_one_page_needs_room_for_its_bar(): - """The search bar sits above the rows (41.1 px at 6.1.0): on a table that shows - every row on one page it takes room from them. 25 rows fit at height 21 without - it and 23 with it, so the fill stands down there, and fills at height 23.""" - assert fills(mk([raw_table(row_limit=25, height=21, page_length=0)]), - "default.search-box") == {} - assert "T" in fills(mk([raw_table(row_limit=25, height=23, page_length=0)]), - "default.search-box") - # A paged table already draws the bar (its page-size picker): no room to find. +def test_no_search_box_on_a_table_whose_rows_all_show(): + """DataTables chrome over rows the reader already sees is noise (brain 10): 25 rows + show at heights 21 and 23, so neither gets a box, paged or not. A table whose rows + outgrow its panel gets one once it pages, in the bar its page-size picker already + draws (41.1 px at 6.1.0), so the box costs no row.""" + for height in (21, 23): + for page in (0, 10): + assert fills(mk([raw_table(row_limit=25, height=height, page_length=page)]), + "default.search-box") == {}, (height, page) assert "T" in fills(mk([raw_table(row_limit=400, height=8, page_length=4)]), "default.search-box") + # On one long page (0, or a page that holds every row) it waits for paging. + for page in (0, 400): + assert fills(mk([raw_table(row_limit=400, height=8, page_length=page)]), + "default.search-box") == {}, page + + +def test_a_search_box_an_older_brain_filled_over_rows_that_all_show_is_removed(): + """Brain 9 filled a box on a table that showed all its rows when they still fit + beside it; --fix now removes that fill and its record.""" + data = mk([raw_table(row_limit=25, height=23, search_box=True)], + design={"filled": {"T": {"search_box": True}}}) + found = fills(data, "default.search-box")["T"] + assert found.fix["unset"] == ["search_box"] and "all 25 rows show" in found.detail + fixed, _ = fix(data) + assert "search_box" not in fixed["charts"][0] + assert "search_box" not in (fixed.get("design") or {}).get("filled", {}).get("T", {}) + + +@pytest.mark.parametrize("row_limit", [10, 21, 25, 30, 50, 400]) +@pytest.mark.parametrize("height", range(6, 21)) +def test_the_fills_add_no_chrome_to_a_table_whose_rows_all_show(row_limit, height): + """No page-size picker, pager or search box over rows that all show in the panel, + whatever height the fix loop settles on (a picker shows on one page too: + DataTable.tsx hasPagination).""" + fixed, rep = fix(mk([raw_table(row_limit=row_limit, height=height)])) + t = fixed["charts"][0] + if row_limit <= math.floor(grid_rows_visible(t["height"], table_header_units())): + assert "page_length" not in t and "search_box" not in t, t + else: + assert t.get("search_box") is (True if row_limit > 20 and t.get("page_length") + else None), t + assert not [f for f in rep.findings if f.rule == "size.table-chrome"] + + +def _chrome(data): + return [f for f in advise(load_spec(data), overlay=EMPTY).findings + if f.rule == "size.table-chrome"] + + +def test_table_chrome_names_an_authors_page_size_or_search_box_over_rows_that_all_show(): + """8 rows show at height 9 with nothing above them: a page-size picker (drawn on one + page too), a pager or a search box over them is chrome. Reported, never fixed: the + fields are the author's.""" + (f,) = _chrome(mk([raw_table(row_limit=8, height=9, page_length=10)])) + assert f.severity == "info" and not f.fix and f.chart == "T" + assert "all 8 rows show at height 9, yet page_length 10 draws a page-size picker" in f.detail + (f,) = _chrome(mk([raw_table(row_limit=8, height=9, page_length=4, search_box=True)])) + assert ("page_length 4 and search_box draw a page-size picker and a pager over 2 pages " + "and a search box") in f.detail + for quiet in ({"row_limit": 8}, {"row_limit": 8, "page_length": 0}, + {"row_limit": 400, "page_length": 6, "search_box": True}, # rows outgrow it + {"page_length": 10}): # rows unknown + assert _chrome(mk([raw_table(height=9, **quiet)])) == [], quiet + assert _chrome(mk([{"type": "table", "name": "Agg", "dataset": DS, "groupby": ["r"], + "metrics": ["SUM(x)"], "row_limit": 5, "search_box": True}])) != [] + + +def test_table_chrome_leaves_the_brains_own_page_to_its_fill(): + """A page the brain filled follows the panel: default.page-length removes it once the + rows all show, so size.table-chrome doesn't name it too (one remedy, one finding).""" + data = mk([raw_table(row_limit=8, height=9, page_length=5)], + design={"filled": {"T": {"page_length": 5}}}) + assert _chrome(data) == [] + assert fills(data, "default.page-length")["T"].fix["unset"] == ["page_length"] @pytest.mark.parametrize("row_limit", [21, 24, 30, 50, 400]) @@ -818,7 +882,7 @@ def test_explain_shows_every_governed_field_and_its_source(monkeypatch, tmp_path code, out = _cli(["explain", str(spec), "--json", "--chart", "A"], monkeypatch, tmp_path, capsys) assert code == 0 and json.loads(out)["charts"][0]["chart"] == "A" code, out = _cli(["explain", str(spec)], monkeypatch, tmp_path, capsys) - assert code == 0 and out.startswith("Design defaults (design brain 9") + assert code == 0 and out.startswith("Design defaults (design brain 10") code, out = _cli(["explain", str(spec), "--chart", "Z"], monkeypatch, tmp_path, capsys) assert code == 1 and "unknown_chart" in out diff --git a/tests/test_display_controls.py b/tests/test_display_controls.py index 3f158e4..7db3b33 100644 --- a/tests/test_display_controls.py +++ b/tests/test_display_controls.py @@ -126,7 +126,8 @@ def _display_params() -> dict[str, dict]: # x by value, largest first; y A to Z from the TOP (bottom-up axis) "sort_x_axis": "value_desc", "sort_y_axis": "alpha_desc"}), ("Trailing Revenue", {"rolling_type": "sum", "rolling_periods": 12, "min_periods": 6, - "compare_lag": 12}), + "compare_lag": 12, "start_y_axis_at_zero": False}), + ("Revenue KPI", {"start_y_axis_at_zero": True}), # the trendline starts at zero unless truncated ("Sales and Price Areas", {"seriesType": "line", "seriesTypeB": "line", "area": True, "areaB": True, "opacity": 1, "opacityB": 0.4, "yAxisIndexB": 1}), ("Price Spread", {"x_axis_title": "Unit price", "y_axis_title": "Orders"}), @@ -334,6 +335,9 @@ def edit(path, doc): ("Revenue KPI", {"color_picker": {"r": 0x1B, "g": 0x7F, "b": 0x3B, "a": 1}}, {"trend_color": "green"}), ("Revenue KPI", {"compare_lag": "2"}, {"compare_lag": 2}), ("Revenue KPI", {"rolling_type": "None"}, {"rolling_type": None}), + # "Start y-axis at 0" unticked fits the trendline; unset is the control's default, true + ("Revenue KPI", {"start_y_axis_at_zero": False}, {"y_axis_truncate": True}), + ("Trailing Revenue", {"start_y_axis_at_zero": None}, {"y_axis_truncate": None}), # rollingWindowOperator: a missing window is 1, missing min periods 0 (ensureIsInt) ("Trailing Revenue", {"min_periods": 12}, {"rolling_min_periods": None}), ("Trailing Revenue", {"min_periods": None}, {"rolling_min_periods": 0}), @@ -450,6 +454,8 @@ def test_plan_is_clean_when_nothing_changed(monkeypatch): ("Trailing Revenue", {"rolling_periods": 6}), ("Trailing Revenue", {"min_periods": 0}), ("Trailing Revenue", {"rolling_type": "mean"}), + ("Trailing Revenue", {"start_y_axis_at_zero": True}), + ("Revenue KPI", {"start_y_axis_at_zero": False}), ("Line by Deal Size", {"sort_y_axis": "alpha_asc"}), ("Line by Deal Size", {"sort_x_axis": "value_asc"}), ("Sales and Price Areas", {"opacityB": 0.9}), diff --git a/tests/test_smoke_fit.py b/tests/test_smoke_fit.py index 2af496d..3aa38be 100644 --- a/tests/test_smoke_fit.py +++ b/tests/test_smoke_fit.py @@ -114,3 +114,97 @@ def test_a_rolling_kpi_warns_when_its_data_holds_too_few_buckets(): assert _window_warning(spec_with({**ttm, "compare_lag": None}).charts[0], 13) is None cum = {**ttm, "rolling_type": "cumsum", "rolling_periods": None} assert _window_warning(spec_with(cum).charts[0], 2) is None + + +# -- values an IN filter lists that return no rows --------------------------------------- + +ZONES = ["North", "South", "East", "West"] + + +class _ChartData: + """A client whose chart_data answers each query with rows(query).""" + + def __init__(self, rows): + self.rows = rows + + def chart_data(self, ctx): + result = [{"data": self.rows(q)} for q in ctx["queries"]] + + class Response: + status_code = 200 + + def json(self): + return {"result": result} + + return Response() + + +def _smoke(chart, rows, temporal=()): + from chartwright.smoke import smoke_chart + from chartwright.testing import stub_resolution + + spec = spec_with(chart) + res = stub_resolution(spec) + for ds in res.datasets.values(): + ds.temporal_columns = set(temporal) + return smoke_chart(spec.charts[0], spec, res, _ChartData(rows)) + + +def test_a_listed_value_with_no_rows_is_named(): + """The analytics QA's line chart named four zones and drew two: its lines and legend + said nothing of the other two.""" + line = {"type": "timeseries_line", "name": "By zone", "dataset": DS, "metrics": ["SUM(x)"], + "time_column": "ts", "groupby": "zone", + "filters": [{"column": "zone", "op": "IN", "value": ZONES}]} + out = _smoke(line, lambda q: [{"ts": 1, "zone": z, "SUM(x)": 1} for z in ("North", "South")]) + assert out.ok and out.warning + assert out.detail == ("2 rows; zone IN lists 'East', 'West', which return no rows, so the " + "chart draws nothing for them") + every = _smoke(line, lambda q: [{"ts": 1, "zone": z} for z in ZONES]) + assert not every.warning and every.detail == "4 rows" + + +def test_listed_values_are_compared_as_text_and_named_one_at_a_time(): + bar = {"type": "bar", "name": "B", "dataset": DS, "x_column": "hour", "metrics": ["COUNT(*)"], + "filters": [{"column": "hour", "op": "IN", "value": [1, 2, 3]}]} + out = _smoke(bar, lambda q: [{"hour": "1"}, {"hour": 2.0}]) + assert out.detail == ("2 rows; hour IN lists 3, which returns no rows, so the chart draws " + "nothing for it") + + +def test_only_a_column_the_query_returns_is_checked(): + """A filter on a column the chart neither groups nor draws by: the rows don't say + which values came back, so there is nothing to name; nor on a time column (it comes + back as timestamps), nor values a series limit drops on purpose.""" + pie = {"type": "pie", "name": "P", "dataset": DS, "metric": "COUNT(*)", "groupby": "status", + "filters": [{"column": "zone", "op": "IN", "value": ["North", "East"]}]} + assert not _smoke(pie, lambda q: [{"status": "open"}]).warning + table = {"type": "table", "name": "T", "dataset": DS, "columns": ["day", "x"], + "filters": [{"column": "day", "op": "IN", "value": ["2026-01-01", "2026-01-02"]}]} + assert not _smoke(table, lambda q: [{"day": 1767225600000, "x": 1}], temporal=["day"]).warning + assert _smoke(table, lambda q: [{"day": "2026-01-01", "x": 1}]).warning + top = {"type": "timeseries_line", "name": "L", "dataset": DS, "metrics": ["SUM(x)"], + "time_column": "ts", "groupby": "zone", "series_limit": 2, + "filters": [{"column": "zone", "op": "IN", "value": ZONES}]} + assert not _smoke(top, lambda q: [{"ts": 1, "zone": "North"}]).warning + other = {**top, "series_limit": None, + "filters": [{"column": "zone", "op": "NOT IN", "value": ["North"]}]} + assert not _smoke(other, lambda q: [{"ts": 1, "zone": "South"}]).warning + + +def test_a_mixed_chart_reads_both_queries_and_a_capped_query_says_so(): + mixed = {"type": "mixed", "name": "M", "dataset": DS, "x_column": "zone", "row_limit": 2, + "filters": [{"column": "zone", "op": "IN", "value": ["North", "South", "East"]}], + "a": {"metrics": ["SUM(x)"]}, "b": {"metrics": ["SUM(y)"], "kind": "line"}} + out = _smoke(mixed, lambda q: [{"zone": "North"}, {"zone": "South"}]) + assert out.detail == ("4 rows; zone IN lists 'East', which returns no rows within the row " + "limit, so the chart draws nothing for it") + answers = iter([[{"zone": "North"}], [{"zone": "South"}, {"zone": "East"}]]) + assert not _smoke({**mixed, "row_limit": 10}, lambda q: next(answers)).warning + + +def test_a_height_warning_and_empty_values_are_both_reported(): + table = {"type": "table", "name": "T", "dataset": DS, "columns": ["zone"], "height": 4, + "row_limit": 50, "filters": [{"column": "zone", "op": "IN", "value": ["a", "z"]}]} + out = _smoke(table, lambda q: [{"zone": "a"}] * 10) + assert "inner scrollbar" in out.detail and "zone IN lists 'z'" in out.detail diff --git a/tests/test_standards.py b/tests/test_standards.py index 0f97441..f5cba06 100644 --- a/tests/test_standards.py +++ b/tests/test_standards.py @@ -497,7 +497,7 @@ def test_the_standards_disable_list_is_reported_as_ignored(repo, capsys): def test_explain_names_the_standard_chain(repo, capsys): spec = write_spec(repo / "specs" / "s.json", standard="finance") text = run_ok(capsys, "explain", str(spec)) - assert text.splitlines()[0] == ("Design defaults (design brain 9, audience analytical, " + assert text.splitlines()[0] == ("Design defaults (design brain 10, audience analytical, " "standard org -> finance)") payload = run_ok(capsys, "explain", str(spec), "--json") assert payload["standard"] == {"name": "finance", "chain": ["org", "finance"], @@ -889,7 +889,7 @@ def test_no_standards_directory_changes_nothing(tmp_path, capsys): assert all("layer" not in f and "locked" not in f for f in payload["findings"]) assert payload == advise(load_spec(DATA)).payload() text = run_ok(capsys, "explain", str(spec)) - assert text.splitlines()[0] == "Design defaults (design brain 9, audience analytical)" + assert text.splitlines()[0] == "Design defaults (design brain 10, audience analytical)" def _bundle_hash(data: dict) -> str: diff --git a/tests/test_standards_content.py b/tests/test_standards_content.py index f074d9f..3b5d7a1 100644 --- a/tests/test_standards_content.py +++ b/tests/test_standards_content.py @@ -1450,7 +1450,7 @@ def test_explain_has_a_dashboard_section(repo, capsys): "released", "bnbColors", "supersetColors") assert scheme["override"].startswith("it is yours") text = run_ok(capsys, "explain", str(path)) - assert text.splitlines()[0].startswith("Design defaults (design brain 9") + assert text.splitlines()[0].startswith("Design defaults (design brain 10") assert "Dashboard content (standard org -> finance, via design.standard)" in text # A chart field the standard wrote says so in the chart section too. k = next(c for c in payload["charts"] if c["chart"] == "K")