From 2bc72649dc04bc41ef2bb04bddd2ccd3c97eba7f Mon Sep 17 00:00:00 2001 From: debabsah Date: Sun, 4 Oct 2026 22:31:44 -0700 Subject: [PATCH] fix: decompile names per-chart settings it dropped without a note The fact-check of the 0.5.0 docs found about twenty chart settings decompile discarded silently (pie radius and labels, step or smooth lines, a hidden trendline, funnel and treemap labels, heatmap sort, margins, intervals and bounds, histogram normalize, currency format). Each now has its untouched-chart default in _STORED_DEFAULTS, read from the plugin source at 4.1.4, 5.0.0 and 6.1.0; the compile round trip still keeps any value the spec writes back from being reported. --- CHANGELOG.md | 2 ++ chartwright/decompile.py | 26 +++++++++++++++++++++++--- docs/VERIFICATION.md | 2 +- tests/test_decompile.py | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30b2a95..193e8a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ while the major version is 0, minor bumps may include breaking changes and say s ### Fixed +- `decompile` (and so `adopt`) names more chart settings it can't carry when they differ from Superset's defaults: big-number font sizes and a hidden trendline, a smooth or step line, horizontal bars on a time axis, a pie's radius and hidden labels, a funnel's labels, tooltip and percentage calculation, treemap labels, a heatmap's axis sort, legend, margins, label intervals and value bounds, a histogram's normalize setting, and any chart's currency format. Before, they were dropped without a note. + - `standards apply` no longer adds a header or footer row that the body already holds at its edge (the last rows for a footer, the first for a header). A dashboard built in the UI has no header or footer, so `decompile` and `adopt` read its legal line as a body row, and the standard's footer was then added below it, showing it twice. `--claim` moves such a row into the header or footer and records it; `explain` and the apply summary name it. ## 0.5.0 (2026-10-04) diff --git a/chartwright/decompile.py b/chartwright/decompile.py index 0099cb6..444edcd 100644 --- a/chartwright/decompile.py +++ b/chartwright/decompile.py @@ -83,7 +83,25 @@ "sort_by_metric": (True,), "start_y_axis_at_zero": (True,), "time_format": ("smart_date",), "date_format": ("smart_date",), "table_timestamp_format": ("smart_date",), + # Per chart type (plugin-chart-echarts at the three tags): BigNumber controls and + # trendline toggle; Timeseries/constants.ts DEFAULT_FORM_DATA (seriesType line, + # orientation vertical); Pie/types.ts (radii 30 and 70, labels shown); Funnel/types.ts + # and controlPanel.tsx (labels shown, tooltip label KeyValuePercent = 5, percentages + # from the first step); Treemap/types.ts (labels shown); Heatmap/controlPanel.tsx + # (axis sort alpha_asc at 4.1.4, unset at 5.0.0 and 6.1.0; margins auto; label + # intervals -1; unbounded values; continuous legend); Histogram (normalize off). + "header_font_size": (0.4,), "subheader_font_size": (0.15,), "show_trend_line": (True,), + "seriesType": ("line", "echarts_timeseries_line"), "orientation": ("vertical",), + "innerRadius": (30,), "outerRadius": (70,), "show_labels": (True,), + "show_tooltip_labels": (True,), "tooltip_label_type": (5,), + "percent_calculation_type": ("first_step",), "sort_x_axis": (None, "alpha_asc"), + "sort_y_axis": (None, "alpha_asc"), "legend_type": ("continuous",), + "bottom_margin": ("auto",), "left_margin": ("auto",), "xscale_interval": (-1,), + "yscale_interval": (-1,), "value_bounds": ([None, None], [], None), + "normalize": (False,), } +# A currency format has no default: unset reads as {} or every part empty. +_UNSET_WHEN_EMPTY = {"currency_format"} # Keys that matter only when another is set: a changed value with no effect is no loss. _STORED_DEFAULT_WHEN = { "only_total": lambda p: bool(p.get("show_value")) and bool(p.get("stack")), @@ -91,6 +109,7 @@ "x_axis_title_margin": lambda p: bool(p.get("x_axis_title")), "y_axis_title_margin": lambda p: bool(p.get("y_axis_title")), "y_axis_title_position": lambda p: bool(p.get("y_axis_title")), + "innerRadius": lambda p: bool(p.get("donut")), } # Spec chart types with a color_scheme field (spec._ColorSchemeMixin). @@ -949,9 +968,10 @@ def keep_row_limit() -> None: if unmapped: losses.append(Loss(name, f"params not preserved: {unmapped}")) changed = {k: p[k] for k in sorted(p) - if k in _STORED_DEFAULTS and k not in mapped_here - and p[k] not in _STORED_DEFAULTS[k] - and _STORED_DEFAULT_WHEN.get(k, lambda _: True)(p)} + if k not in mapped_here and _STORED_DEFAULT_WHEN.get(k, lambda _: True)(p) + and ((k in _STORED_DEFAULTS and p[k] not in _STORED_DEFAULTS[k]) + or (k in _UNSET_WHEN_EMPTY and p[k] + and not (isinstance(p[k], dict) and not any(p[k].values()))))} if changed: if changed_defaults is None: losses.append(_defaults_loss(name, changed)) diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index d3561f1..e22865b 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -35,7 +35,7 @@ injection. | Layer | Proves | Where it runs | |---|---|---| -| Offline suite (1552 tests, 67 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | +| Offline suite (1564 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 | diff --git a/tests/test_decompile.py b/tests/test_decompile.py index 7745b20..53283f8 100644 --- a/tests/test_decompile.py +++ b/tests/test_decompile.py @@ -257,3 +257,40 @@ def text_row(path, doc): result = decompile_bundle(blob, _stub_lookup_for(spec)) assert "rows" in result.spec["layout"] assert any("COLUMN" in l.what for l in result.losses) + + +@pytest.mark.parametrize("viz, key, value", [ + ("pie", "outerRadius", 60), ("pie", "show_labels", False), + ("funnel", "tooltip_label_type", 2), ("funnel", "show_labels", False), + ("treemap_v2", "show_labels", False), + ("heatmap_v2", "legend_type", "piecewise"), ("heatmap_v2", "xscale_interval", 2), + ("histogram_v2", "normalize", True), ("big_number", "show_trend_line", False), + ("echarts_timeseries_line", "seriesType", "smooth"), + ("echarts_timeseries_line", "currency_format", {"symbol": "USD", "symbolPosition": "prefix"}), +]) +def test_per_chart_settings_changed_in_the_ui_are_named(viz, key, value): + """The fact-check of the 0.5.0 docs found these dropped with no loss; each is now + checked against the value an untouched chart of that type stores.""" + from chartwright.testing import edit_bundle + + spec = load_spec(json.loads((FIXTURES / "kitchen_sink.json").read_text())) + bundle = compile_bundle(spec, stub_resolution(spec)) + + def change(path, doc): + if "/charts/" in path and doc.get("viz_type") == viz: + doc["params"][key] = value + losses = decompile_bundle(edit_bundle(bundle, change), _stub_lookup_for(spec)).losses + assert any(key in l.what and "settings not preserved" in l.what for l in losses), key + + +def test_an_empty_currency_format_is_no_loss(): + from chartwright.testing import edit_bundle + + spec = load_spec(json.loads((FIXTURES / "kitchen_sink.json").read_text())) + + def empty(path, doc): + if "/charts/" in path: + doc["params"]["currency_format"] = {"symbol": None, "symbolPosition": None} + blob = edit_bundle(compile_bundle(spec, stub_resolution(spec)), empty) + assert not [l for l in decompile_bundle(blob, _stub_lookup_for(spec)).losses + if "currency_format" in l.what]