Review burn-down tiers 1-2: severity truth, a real rule-table generator, and the unwindowed-history gap

.github/workflows/ci.yml

@@ -6,8 +6,10 @@ on:
   # Branches must clear the same bar as main BEFORE they merge: the offline
   # matrix (Linux + Windows, 3.11) and the live matrix (three Superset
   # versions) are exactly the checks a feature branch is most likely to break.
+  # Deliberately unfiltered by base branch: a stacked PR (one that targets
+  # another feature branch rather than main) would otherwise run no checks at
+  # all, which is the case this trigger exists to prevent.
   pull_request:
-    branches: [main]
   workflow_dispatch:
 
 jobs:

.gitignore

@@ -8,3 +8,6 @@ profiles.toml
 
 .mcp.json
 .DS_Store
+
+# Local working notes (roadmaps, scratch plans). Never committed.
+*.local.md

CHANGELOG.md

@@ -0,0 +1,71 @@
+# Changelog
+
+Notable changes per release. Versions follow [semantic versioning](https://semver.org);
+while the major version is 0, minor bumps may include breaking changes and say so here.
+
+## 0.2.0 (unreleased)
+
+The design brain, plus the fixes found reviewing it.
+
+### Added
+
+- **The design brain**, an optional layer that checks whether a dashboard
+  reads well, not just whether it imports. Off with `--design off`, which
+  restores byte-identical output.
+  - `chartwright brief` prints design guidance to read before writing a spec,
+    tuned to an audience preset (`executive`, `analytical`, `operational`).
+  - `chartwright advise` reviews a finished spec against 49 rules with stable
+    ids. `--fix` applies the safe geometry repairs; `--profile` adds checks
+    that need live metadata, such as a time axis on a non-temporal column.
+  - `chartwright redesign` decompiles a live dashboard, audits it, applies the
+    safe fixes, and writes the repaired spec.
+  - `chartwright calibrate` proposes recommended heights from the sizes you
+    have polished by hand and absorbed.
+  - `check` and `apply` gain `--design off|warn|strict`, defaulting to `warn`.
+  - An optional `design` block in the spec sets the audience and suppresses
+    individual rules per dashboard or per chart.
+  - `~/.config/chartwright/design.yaml` tunes thresholds, disables rules, and
+    appends house guidance for a whole deployment.
+  - Four MCP tools covering the same ground, taking the server from six to ten.
+- `data.unwindowed-history` warns when nothing bounds a timeseries dashboard's
+  date range, so every load queries the dataset's full history.
+- Apply-time warning when a table or pivot renders more rows than its
+  configured height can show, which otherwise hides them behind an inner
+  scrollbar with everything else looking healthy.
+
+### Fixed
+
+- `sort_by` on table charts had no effect. It compiled to a sort direction
+  with no sort key, so a `row_limit` returned arbitrary rows rather than a
+  ranking. It now compiles correctly in both aggregate and raw mode, is
+  validated against the dataset like every other reference, and survives a
+  decompile, so `chartwright plan` no longer reports a sorted table as
+  permanently changed.
+- Rows whose widths were left implicit could sum to more than the twelve
+  column grid when a row held repeated markdown blocks.
+- `chartwright absorb` now writes its report before touching the spec file, so
+  a reporting failure cannot follow a silent mutation.
+- `chartwright decompile` says so when its dataset index is truncated, instead
+  of reporting a dataset it never looked at as unresolvable.
+- Duplicate tab titles are rejected at validation rather than producing a
+  dashboard whose tabs cannot be told apart.
+- Layout sketches are parsed once per holder rather than once per lookup.
+
+### Changed
+
+- A design finding that is withheld because a chart carries a hand-polished
+  height is now reported rather than passing silently.
+- `--design strict` blocks when the advice cannot be evaluated at all, for
+  example because `design.yaml` is malformed. It previously reported zero
+  findings and allowed the run.
+- Tables and pivots are expected to show at least half the rows their
+  `row_limit` requests, up from a quarter. Specs with large explicit row
+  limits will see more findings than before.
+
+## 0.1.0
+
+Initial release. Compile a spec into an Apache Superset dashboard, verify
+every dataset, column, and metric before building, and check every chart
+returns data afterwards. Decompile an existing dashboard back into a spec,
+diff a spec against what is live with `chartwright plan`, and restore a
+dashboard from an automatic backup.

CONTRIBUTING.md

@@ -26,8 +26,14 @@ is also worth fixing.
   staying decompilable and every supported Superset version staying in
   contract (docs/CONTRACTS.md).
 - Every pull request runs the same gates as every push: the offline suite
-  (`python -m pytest tests/ -q`) and the params drift check
+  (`pytest tests/ -q`) and the params drift check
   (`python tools/params_drift.py --all`) must both pass.
+- Run the suite exactly as written above, from the repo root. `python -m
+  pytest` also works, but it puts the current directory on `sys.path`, which
+  CI does not: a test module importing through the `tests.` package passes
+  that way and is uncollectable in CI. That gap once let a whole test module
+  sit unrun; `tests/test_docs.py` now guards the specific case, but the habit
+  is what keeps the two honest.
 
 By contributing, you agree that your contributions are licensed under the
 Apache License 2.0, the same license as the project.

README.md

@@ -155,11 +155,11 @@ space under `K` deliberately empty. Every rule, drawn and explained:
 ## Testing and evidence
 
 Every push and every pull request runs the full offline suite on Linux and
-Windows, plus the full pipeline (apply,
-lifecycle soak, stale-tab adversary, fault injection) against real Superset
-4.1.4, 5.0.0, and 6.1.0 containers. Chart options are checked against
-Superset's own source for every supported version, so a Superset change is
-caught in our tests before it reaches your dashboards.
+Windows, plus the full pipeline (apply, lifecycle soak, stale-tab adversary,
+fault injection) against real Superset 4.1.4, 5.0.0, and 6.1.0 containers.
+Chart options are checked against Superset's own source for every supported
+version, so a Superset change is caught in our tests before it reaches your
+dashboards.
 
 Full evidence: [docs/VERIFICATION.md](https://github.com/debabsah/chartwright/blob/main/docs/VERIFICATION.md). Source citations
 for every Superset behavior the tool relies on:

chartwright/cli.py

@@ -277,6 +277,18 @@ def _main(argv: list[str] | None = None) -> None:
             payload["written"] = written
         if resolution is not None and resolution.errors:
             payload["resolution_errors"] = [e.as_dict() for e in resolution.errors]
+        if report.gate(args.strict):
+            # `ok` stays error-driven by contract (§10), so the exit code was
+            # the ONLY signal that --strict blocked. Name the cause the way
+            # check/apply do, or a caller sees exit 1 with nothing to read.
+            payload.setdefault("errors", []).append({
+                "code": "design_gate",
+                "detail": "warn-severity findings block under --strict; fix them, run "
+                          "`chartwright advise --fix`, or record deliberate exceptions "
+                          "in the spec's design.ignore"
+                          if report.counts["error"] == 0 else
+                          "error-severity findings block; fix them or record deliberate "
+                          "exceptions in the spec's design.ignore"})
         print(json.dumps(payload, indent=2))
         sys.exit(1 if report.gate(args.strict) else 0)
 

chartwright/decompile.py

@@ -611,11 +611,27 @@ def width_of(item) -> int:
         spec["filters"] = filters
     if not ordered:
         losses.append(Loss("dashboard", "no representable charts; spec is not valid for apply"))
+    truncated = getattr(lookup, "truncated", 0)
+    if truncated:
+        losses.append(Loss(
+            "dashboard",
+            f"the dataset index stopped at {truncated} datasets (page cap); any "
+            f"'dataset uuid not resolvable' loss above may be a dataset past the cap "
+            f"rather than a missing one -- re-check those charts before trusting this spec"))
     return DecompileResult(spec=spec, losses=losses, dataset_uuids=dataset_uuids)
 
 
+PAGE_CAP = 200  # 20,000 datasets; a runaway guard, not an expected ceiling
+
+
 def live_dataset_lookup(client) -> DatasetLookup:
-    """uuid -> triple, resolved lazily against the live instance."""
+    """uuid -> triple, resolved lazily against the live instance.
+
+    Sets `lookup.truncated` when the runaway guard trips, so decompile can SAY
+    the index is incomplete. Without that, a dataset past the cap silently
+    became "uuid not resolvable" and its chart was dropped -- a wrong answer
+    dressed as an honest loss, which is the one failure this decompiler must
+    never produce."""
     cache: dict[str, dict] | None = None
 
     def lookup(u: str) -> dict | None:
@@ -638,10 +654,12 @@ def lookup(u: str) -> dict | None:
                         "table": d["table_name"],
                     }
                 page += 1
-                if page > 200:
+                if page > PAGE_CAP:
+                    lookup.truncated = len(cache)
                     break
         return cache.get(u)
 
+    lookup.truncated = 0
     return lookup
 
 

chartwright/design/model.py

@@ -12,7 +12,12 @@
 from ..resolver import ResolvedDataset, Resolution
 from ..spec import DEFAULT_HEIGHT, DashboardSpec, MarkdownBlock
 
-DESIGN_BRAIN_VERSION = "2"
+# "3" = the post-review batch: the reconciled grid model and stricter
+# table_visible_ratio, `polished` provenance in the payload, and the
+# data.unwindowed-history rule. Bumped because all three change what a spec
+# is told -- a new warn-severity rule can newly block a `--design strict`
+# pipeline, so consumers keying on this get an honest signal.
+DESIGN_BRAIN_VERSION = "3"
 
 KPI_TYPES = {"big_number_total", "big_number_trend"}
 TIMESERIES_TYPES = {"timeseries_line", "timeseries_bar", "timeseries_area", "timeseries_scatter"}
@@ -211,27 +216,44 @@ def fix_height(self, chart, floor: float) -> dict:
 # -- registry -----------------------------------------------------------------
 
 
+SEVERITY_RANK = {"error": 0, "warn": 1, "info": 2}
+
+
 @dataclass
 class Rule:
     id: str
-    severity: str            # the rule's DEFAULT level; individual findings may
-    #                          vary it (e.g. row-density escalates to error when
-    #                          a chart is starved), and the overlay's `severity`
+    severity: str            # the rule's DEFAULT level; the overlay's `severity`
     #                          map overrides it per deployment.
     doc: str                 # one line; the brief prints these
     fixable: bool
     data_aware: bool         # needs a live resolution (skipped offline)
     fn: Callable[[RuleContext], Iterator[Finding]]
     since: str = "1"         # design_brain version that introduced the rule
+    # Every level this rule can actually emit, default first. Four rules vary
+    # it per finding (row-density escalates to error when a chart is starved;
+    # row-fill and format-bands soften to info), and `ok` is error-driven --
+    # so a rule that can produce an error while advertising `warn` understates
+    # exactly the case a reader most needs to know about. Declared, printed in
+    # the generated table, and checked against the rule's source by a test.
+    severities: tuple[str, ...] = ()
+
+    def __post_init__(self) -> None:
+        self.severities = self.severities or (self.severity,)
+
+    @property
+    def severity_label(self) -> str:
+        """'warn', or 'warn/error' when the rule varies it per finding."""
+        rest = sorted(set(self.severities) - {self.severity}, key=SEVERITY_RANK.get)
+        return "/".join([self.severity, *rest])
 
 
 RULES: dict[str, Rule] = {}
 
 
 def rule(id: str, severity: str, doc: str, fixable: bool = False, data_aware: bool = False,
-         since: str = "1"):
+         since: str = "1", severities: tuple[str, ...] = ()):
     def deco(fn):
-        RULES[id] = Rule(id, severity, doc, fixable, data_aware, fn, since)
+        RULES[id] = Rule(id, severity, doc, fixable, data_aware, fn, since, severities)
         return fn
     return deco
 

chartwright/design/rules.py

@@ -28,7 +28,8 @@
 # -- size: minimum readable geometry ------------------------------------------
 
 
-@rule("size.min-width", "warn", "below 3/12 width a chart is unreadable; KPIs need 2/12", since="2")
+@rule("size.min-width", "warn", "below 3/12 width a chart is unreadable; KPIs need 2/12",
+      since="2", severities=("warn", "error"))
 def min_width(ctx: RuleContext):
     for c in ctx.spec.charts:
         if c.type in ("pie", "heatmap"):
@@ -328,7 +329,8 @@ def kpi_band(ctx: RuleContext):
                     )
 
 
-@rule("layout.row-density", "warn", "too many axis charts side by side starves each of width")
+@rule("layout.row-density", "warn", "too many axis charts side by side starves each of width",
+      severities=("warn", "error"))
 def row_density(ctx: RuleContext):
     # Horizontal SLOTS, not flattened charts: a stack of three charts occupies
     # one slot's width, so it counts once (the user already split vertically).
@@ -348,7 +350,8 @@ def row_density(ctx: RuleContext):
             )
 
 
-@rule("layout.row-fill", "warn", "a row should fill the 12-column grid")
+@rule("layout.row-fill", "warn", "a row should fill the 12-column grid",
+      severities=("warn", "info"))
 def row_fill(ctx: RuleContext):
     for si, sec in enumerate(ctx.sections):
         if sec.mode != "rows":
@@ -635,7 +638,8 @@ def pivot_columns(ctx: RuleContext):
             )
 
 
-@rule("chart.format-bands", "warn", "conditional-formatting bands must tell one coherent story per metric", since="2")
+@rule("chart.format-bands", "warn", "conditional-formatting bands must tell one coherent story per metric",
+      since="2", severities=("warn", "info"))
 def format_bands(ctx: RuleContext):
     for c in ctx.spec.charts:
         if c.type != "pivot_table" or not c.conditional_formatting:
@@ -831,14 +835,20 @@ def grain_vs_range(ctx: RuleContext):
             )
 
 
+_FINE_GRAINS = (None, "PT1S", "PT1M", "PT1H", "P1D")
+
+
+def _unwindowed(ctx: RuleContext) -> bool:
+    """No defaulted dashboard time filter, so first load spans ALL history."""
+    return not any(f.type == "time_range" and f.default for f in ctx.spec.filters)
+
+
 @rule("chart.trend-grain", "info", "trend tiles at a fine grain over full history draw thousands of points in a small card", since="2")
 def trend_grain(ctx: RuleContext):
-    fine = (None, "PT1S", "PT1M", "PT1H", "P1D")
-    windowed = any(f.type == "time_range" and f.default for f in ctx.spec.filters)
-    if windowed:
+    if not _unwindowed(ctx):
         return
     for c in ctx.spec.charts:
-        if c.type == "big_number_trend" and c.time_grain in fine:
+        if c.type == "big_number_trend" and c.time_grain in _FINE_GRAINS:
             yield Finding(
                 "chart.trend-grain", "info", c.name, ctx.where(c.name),
                 f"sparkline at grain {c.time_grain or 'P1D (default)'} with no defaulted "
@@ -847,6 +857,44 @@ def trend_grain(ctx: RuleContext):
             )
 
 
+@rule("data.unwindowed-history", "warn",
+      "timeseries charts with no way to bound the window draw ALL history at their grain",
+      since="3")
+def unwindowed_history(ctx: RuleContext):
+    """The commonest real-world Superset dashboard failure, and the one the
+    rulebook missed entirely: nothing bounds the time window, so the board
+    queries the full table and draws a point per day on every load.
+
+    Reported ONCE for the dashboard, not once per chart: it is a single
+    property of the dashboard with a single fix, and a six-timeseries board
+    would otherwise emit six warns for it.
+
+    Deliberately silent when a time_range filter EXISTS without a default --
+    `filters.time-default` already names that exact one-line fix, and
+    double-reporting one remedy at two severities is noise. Deployments that
+    want it to bite raise that rule via the overlay's `severity` map, which is
+    the mechanism sec.15.8 chose for precisely this.
+
+    `data.grain-vs-range` is the sibling for when a range IS set: it can count
+    the points. This one cannot, because the span is "however much data
+    exists" -- which is what makes it dangerous. Not autofixable: every remedy
+    changes what data the chart shows (sec.2.2)."""
+    if not _unwindowed(ctx) or any(f.type == "time_range" for f in ctx.spec.filters):
+        return
+    exposed = [c.name for c in ctx.spec.charts
+               if c.type in TIMESERIES_TYPES and not c.time_range
+               and c.time_grain in _FINE_GRAINS]
+    if not exposed:
+        return
+    yield Finding(
+        "data.unwindowed-history", "warn", None, "filters",
+        f"{exposed} have no time_range and the dashboard has no time_range filter at "
+        f"all: at a daily-or-finer grain every load queries and draws the dataset's "
+        f"FULL history. Add a time_range filter WITH a default (e.g. 'Last quarter'), "
+        f"set the charts' time_range, or coarsen the grain",
+    )
+
+
 # -- narrative & filters: polish -------------------------------------------------
 
 _MINOR_WORDS = {"a", "an", "the", "of", "by", "vs", "and", "or", "in", "on",

chartwright/mcp_server.py

@@ -60,7 +60,9 @@ def _advice(spec, resolution=None, audience: str | None = None) -> dict:
     try:
         return advise(spec, audience=audience, resolution=resolution).payload()
     except Exception as e:  # noqa: BLE001
-        return {"stage": "design", "ok": True, "design_brain": "1",
+        from .design import DESIGN_BRAIN_VERSION
+
+        return {"stage": "design", "ok": True, "design_brain": DESIGN_BRAIN_VERSION,
                 "counts": {"error": 0, "warn": 0, "info": 0}, "findings": [],
                 "fixed": [], "ignored": [],
                 "errors": [{"code": "overlay" if isinstance(e, ValueError) else "advice",