diff --git a/CHANGELOG.md b/CHANGELOG.md index f7a086d..d361da7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ The design brain, plus the fixes found reviewing it. ### Added +- **A missing column names the real ones.** The error suggests up to three close matches + (a case-only difference first, then by spelling) and lists the dataset's columns, up to + 50, where it used to give only their count. The JSON error carries the suggestions in a + new `candidates` field, so an AI agent can correct the spec in one round; the MCP + tools return the same. - **`layout.footer`**: rows below the rows, tabs, or sketch, outside any tab, so a tabbed dashboard shows them under every tab. Round-trips through decompile and `plan`; rows below a dashboard's tabs, including ones placed diff --git a/chartwright/resolver.py b/chartwright/resolver.py index 6993811..b94b848 100644 --- a/chartwright/resolver.py +++ b/chartwright/resolver.py @@ -43,6 +43,9 @@ class ResolutionError: chart: str | None ref: str detail: str + # column_not_found only: the dataset's closest column names, best first, so + # an agent can correct the spec without parsing `detail` or asking Superset. + candidates: list[str] = field(default_factory=list) def as_dict(self) -> dict: return asdict(self) @@ -147,18 +150,41 @@ def _check_metric(metric: str, chart_name: str, ds: ResolvedDataset, res: Resolu return col = adhoc["column"] if col != "*" and col not in ds.columns: - res.errors.append(ResolutionError( - "column_not_found", chart_name, col, - f"ad-hoc metric {metric!r}: column {col!r} not on dataset {ds.table!r}", - )) + _missing_column(col, chart_name, ds, res, f"ad-hoc metric {metric!r}: column {col!r}") + + +SUGGESTIONS = 3 # close matches named per missing column +LISTED_COLUMNS = 50 # the dataset's columns listed in the message, then "and N more" + + +def close_columns(col: str, columns: list[str]) -> list[str]: + """Up to SUGGESTIONS column names closest to `col`, best first. A match + that differs only in case always comes first; the rest are by spelling + similarity (difflib, ignoring case).""" + import difflib + + exact = [c for c in columns if c.lower() == col.lower()] + lowered = {c.lower(): c for c in columns} + near = difflib.get_close_matches(col.lower(), list(lowered), n=SUGGESTIONS, cutoff=0.6) + out = exact + [lowered[n] for n in near if lowered[n] not in exact] + return out[:SUGGESTIONS] + + +def _missing_column(col: str, chart_name: str, ds: ResolvedDataset, res: Resolution, lead: str) -> None: + candidates = close_columns(col, ds.columns) + hint = f"; did you mean {', '.join(repr(c) for c in candidates)}?" if candidates else "." + shown = ds.columns[:LISTED_COLUMNS] + more = f", and {len(ds.columns) - LISTED_COLUMNS} more" if len(ds.columns) > LISTED_COLUMNS else "" + res.errors.append(ResolutionError( + "column_not_found", chart_name, col, + f"{lead} not on dataset {ds.table!r}{hint} Columns: {', '.join(shown)}{more}", + candidates, + )) def _check_column(col: str, chart_name: str, ds: ResolvedDataset, res: Resolution, what: str = "column") -> None: if col not in ds.columns: - res.errors.append(ResolutionError( - "column_not_found", chart_name, col, - f"{what} {col!r} not on dataset {ds.table!r} (has {len(ds.columns)} columns)", - )) + _missing_column(col, chart_name, ds, res, f"{what} {col!r}") def _check_chart_fields(chart, ds: ResolvedDataset, res: Resolution) -> None: diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index e0ac653..ff1dcca 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -35,7 +35,7 @@ injection. | Layer | Proves | Where it runs | |---|---|---| -| Offline suite (347 tests, 36 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | +| Offline suite (354 tests, 37 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 | 15-chart apply, per-chart data check, ids stable across re-apply | every PR and push to main, all 3 versions | | Lifecycle soak | 500 randomized edit cycles with invariants held | 500 cycles on 6.1.0 and 4.1.4 before release; 25 cycles per version on every PR and push to main | @@ -106,7 +106,9 @@ a runtime surprise. ## 3. Live guarantee check (`tools/ci_live_check.py`) Runs against a real instance, start to finish. First, pre-flight -resolution, which collects every bad reference into typed errors. Then an +resolution, which collects every bad reference into typed errors, and a +deliberately misspelled column, which must come back with the real column +as its first suggestion. Then an apply of the complete example spec (`tests/fixtures/kitchen_sink.json`), which exercises all 15 chart types, per-chart WHERE filters, a native filter bar with two value pickers and a time range, plus markdown and tabs. diff --git a/tests/test_column_suggestions.py b/tests/test_column_suggestions.py new file mode 100644 index 0000000..bad73c8 --- /dev/null +++ b/tests/test_column_suggestions.py @@ -0,0 +1,64 @@ +"""A missing column names the dataset's real columns: close matches first (in +`candidates` and in the message), then the column list, capped. The count +alone left agents and people guessing.""" + +from chartwright.resolver import ( + LISTED_COLUMNS, Resolution, ResolvedDataset, _check_column, _check_metric, close_columns, +) + + +def _ds(columns): + return ResolvedDataset(id=1, uuid="u", table="orders", schema=None, + database_name="warehouse", columns=columns, metrics=["revenue"]) + + +COLS = ["order_id", "ordered_at", "region", "amount", "status"] + + +def _err(col, columns=COLS, what="groupby"): + res = Resolution() + _check_column(col, "Revenue by Region", _ds(columns), res, what) + assert len(res.errors) == 1 + return res.errors[0] + + +def test_a_case_only_mismatch_is_the_first_suggestion(): + e = _err("Region") + assert e.code == "column_not_found" + assert e.candidates[0] == "region" + assert "did you mean 'region'" in e.detail + + +def test_a_typo_suggests_the_column(): + assert "amount" in _err("amont").candidates + + +def test_no_close_match_still_lists_the_columns(): + e = _err("zzzz") + assert e.candidates == [] + assert "did you mean" not in e.detail + assert e.detail.endswith("Columns: order_id, ordered_at, region, amount, status") + + +def test_a_wide_dataset_lists_the_first_columns_and_counts_the_rest(): + wide = [f"col_{i:03d}" for i in range(200)] + e = _err("missing", wide) + assert f"col_{LISTED_COLUMNS - 1:03d}" in e.detail + assert f"col_{LISTED_COLUMNS:03d}" not in e.detail + assert f"and {200 - LISTED_COLUMNS} more" in e.detail + + +def test_the_column_inside_an_aggregate_gets_suggestions_too(): + res = Resolution() + _check_metric("SUM(amont)", "Revenue", _ds(COLS), res) + assert [e.code for e in res.errors] == ["column_not_found"] + assert "amount" in res.errors[0].candidates + + +def test_candidates_reach_the_json_error(): + assert _err("Region").as_dict()["candidates"][0] == "region" + + +def test_at_most_three_suggestions(): + cols = ["amount", "amount_1", "amount_2", "amount_3", "amounts"] + assert len(close_columns("amont", cols)) <= 3 diff --git a/tools/ci_live_check.py b/tools/ci_live_check.py index fb02247..fbc5362 100644 --- a/tools/ci_live_check.py +++ b/tools/ci_live_check.py @@ -46,6 +46,20 @@ def main(argv: list[str] | None = None) -> int: print(json.dumps([e.as_dict() for e in res.errors], indent=2)) return 2 + # A misspelled column must come back with the real one suggested, from + # the column list this version's dataset API returns. + raw = json.loads(Path(args.spec).read_text(encoding="utf-8")) + target = next((c for c in raw["charts"] if isinstance(c.get("x_column"), str)), None) + if target is not None: + real_col = target["x_column"] + target["x_column"] = real_col.upper() if real_col != real_col.upper() else real_col.lower() + bad = [e for e in resolve(load_spec(raw), client).errors if e.code == "column_not_found"] + if not bad or bad[0].candidates[:1] != [real_col]: + print(f"FAIL: misspelled column {target['x_column']!r} did not suggest {real_col!r}: " + f"{[e.as_dict() for e in bad]}") + return 1 + print(f"column suggestion OK: {target['x_column']!r} -> {real_col!r}") + r1 = run_apply(spec, client, "ci") print(r1.to_json()) if not r1.ok: