From db658b28cc4a30532fd86ced5e6d044446142a5b Mon Sep 17 00:00:00 2001 From: Sebastien Henry Date: Thu, 25 Jun 2026 15:49:50 -0500 Subject: [PATCH 01/26] feat(ingest): auto-sniff UTF-16/TSV + coerce display-formatted measures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unblocks the 'Super Sample Superstore' migrated-data file (UTF-16 LE, TAB- separated, measures stored as '$16'/'20%'/'($5)' strings): - file_to_dataframe gains optional encoding/sep (plumbed through FileRequest → FileArgs); when omitted it auto-sniffs the BOM (utf-16/utf-8-sig) and the header delimiter (tab vs comma). - _coerce_formatted_numerics converts currency/percent/accounting-negative string columns to numeric, but only when >=90% of a column parses as a number so genuine text dimensions (names, IDs, categories) are untouched. Verified on the real file: 9994x68 loads; Sales/Profit/CP/PP/Difference become numeric (Sales sums to $2,297,354), Region/State/Order ID stay text. +13 tests. Co-Authored-By: Claude Opus 4.8 --- sidecar/hyper_builder.py | 104 +++++++++++++++++++- sidecar/server.py | 5 + sidecar/tests/test_hyper_builder_formats.py | 94 ++++++++++++++++++ src/sidecar.ts | 9 ++ 4 files changed, 208 insertions(+), 4 deletions(-) diff --git a/sidecar/hyper_builder.py b/sidecar/hyper_builder.py index 8360771..8784f65 100644 --- a/sidecar/hyper_builder.py +++ b/sidecar/hyper_builder.py @@ -146,6 +146,88 @@ def hyper_table_info(hyper_path: Path) -> dict[str, Any]: return {"row_count": int(row_count), "columns": columns} +_FORMATTED_NUM_RE = re.compile(r"^-?\$?-?[\d,]+(?:\.\d+)?%?$") +_ACCT_NEG_RE = re.compile(r"^\(\s*\$?[\d,]+(?:\.\d+)?%?\s*\)$") +_NULL_TOKENS = {"", "null", "nan", "none", "-", "n/a", "na"} + + +def _parse_formatted_number(value: object) -> float | None: + """Parse a display-formatted number ('$1,234', '20%', '($5)') to float, else None. + + Currency ($) and thousands (,) separators are stripped; a trailing percent is + converted to a ratio (20% -> 0.20); accounting negatives '($5)' -> -5.0. + """ + s = str(value).strip() + if s.lower() in _NULL_TOKENS: + return None + negative = False + if _ACCT_NEG_RE.match(s): + negative = True + s = s[1:-1].strip() + compact = s.replace(" ", "") + if not _FORMATTED_NUM_RE.match(compact): + return None + is_pct = compact.endswith("%") + cleaned = compact.replace("$", "").replace(",", "").replace("%", "") + try: + num = float(cleaned) + except ValueError: + return None + if is_pct: + num /= 100.0 + return -num if negative else num + + +def _coerce_formatted_numerics(df: pd.DataFrame) -> pd.DataFrame: + """Convert display-formatted string columns ('$16', '20%') to numeric in place. + + A column is converted only when >= 90% of its non-blank values parse as a + formatted number, so genuine text dimensions (names, IDs, categories) are left + untouched. Real-world exports (e.g. Tableau "migrated data") carry measures as + display strings that would otherwise import as un-aggregatable text. + """ + for col in df.columns: + if df[col].dtype != object: + continue + series = df[col] + text = series.astype(str).str.strip() + nonblank = text[~text.str.lower().isin(_NULL_TOKENS)] + if len(nonblank) == 0: + continue + parsed_sample = nonblank.map(_parse_formatted_number) + if parsed_sample.notna().mean() >= 0.9: + df[col] = series.map(_parse_formatted_number) + return df + + +def _sniff_csv_dialect(p: Path) -> tuple[str, str]: + """Sniff ``(encoding, delimiter)`` for a delimited text file from its first bytes. + + Detects a UTF-16/UTF-8 byte-order mark and whether the header row is tab- or + comma-separated. Used only when the caller does not pass ``encoding``/``sep``. + Many real-world exports (e.g. Tableau's "migrated data" CSVs) are UTF-16 LE and + TAB-separated, which the default ``pd.read_csv`` (UTF-8 + comma) cannot parse. + + Returns a best-effort guess; falls back to ``("utf-8", ",")``. + """ + with p.open("rb") as fh: + head = fh.read(65536) + if head[:2] in (b"\xff\xfe", b"\xfe\xff"): + encoding = "utf-16" + elif head[:3] == b"\xef\xbb\xbf": + encoding = "utf-8-sig" + else: + encoding = "utf-8" + try: + text = head.decode(encoding, errors="replace") + except LookupError: + text = head.decode("utf-8", errors="replace") + lines = text.splitlines() + first_line = lines[0] if lines else "" + delimiter = "\t" if first_line.count("\t") > first_line.count(",") else "," + return encoding, delimiter + + def file_to_dataframe( file_type: str, path: str, @@ -153,6 +235,8 @@ def file_to_dataframe( json_path: str | None = None, max_rows: int = DEFAULT_MAX_ROWS, max_bytes: int = MAX_FILE_BYTES, + encoding: str | None = None, + sep: str | None = None, ) -> pd.DataFrame: """Read a local file into a DataFrame, with pre-read size and post-read row caps. @@ -162,6 +246,11 @@ def file_to_dataframe( ``json_path`` supports a single-level selector of the form ``$.`` (e.g. ``$.data``). Anything deeper is rejected with a clear error rather than silently mis-parsing. + ``encoding`` / ``sep`` (csv only): explicit overrides for the text encoding and the + column delimiter. When either is omitted, the dialect is auto-sniffed from the file's + first bytes (BOM → encoding; tab-vs-comma in the header → delimiter), so UTF-16 / TSV + exports load without the caller having to know the encoding up front. + Safety caps (PA-1): - ``max_bytes``: rejects files larger than this limit before any parsing begins. - ``max_rows``: clamps the returned DataFrame to at most this many rows. For csv/json/jsonl @@ -183,20 +272,27 @@ def file_to_dataframe( ) if ftype == "csv": - return pd.read_csv(p, nrows=max_rows) + enc, delim = encoding, sep + if enc is None or delim is None: + sniffed_enc, sniffed_delim = _sniff_csv_dialect(p) + enc = enc or sniffed_enc + delim = delim or sniffed_delim + return _coerce_formatted_numerics( + pd.read_csv(p, nrows=max_rows, encoding=enc, sep=delim) + ) if ftype in {"xlsx", "xls"}: sheet: str | int = excel_sheet if excel_sheet is not None else 0 df_excel = pd.read_excel(p, sheet_name=sheet, engine="openpyxl") if len(df_excel) > max_rows: df_excel = df_excel.head(max_rows) - return df_excel + return _coerce_formatted_numerics(df_excel) if ftype == "parquet": df_parquet = pd.read_parquet(p) if len(df_parquet) > max_rows: df_parquet = df_parquet.head(max_rows) - return df_parquet + return _coerce_formatted_numerics(df_parquet) if ftype in {"json", "jsonl"}: if json_path is not None: @@ -241,7 +337,7 @@ def file_to_dataframe( if len(df_json) > max_rows: df_json = df_json.head(max_rows) - return df_json + return _coerce_formatted_numerics(df_json) raise ValueError( f"Unsupported file_type: {file_type!r}. " diff --git a/sidecar/server.py b/sidecar/server.py index ba71797..c869a04 100644 --- a/sidecar/server.py +++ b/sidecar/server.py @@ -108,6 +108,9 @@ class FileRequest(BaseModel): excel_sheet: str | int | None = Field(default=None, alias="excelSheet") json_path: str | None = Field(default=None, alias="jsonPath") max_rows: int = Field(default=hyper_builder.DEFAULT_MAX_ROWS, alias="maxRows") + # csv only; auto-sniffed from the file's BOM/header when omitted. + encoding: str | None = Field(default=None, alias="encoding") + delimiter: str | None = Field(default=None, alias="delimiter") @app.get("/health") @@ -264,6 +267,8 @@ def datasource_from_file(req: FileRequest) -> FileResult: excel_sheet=req.excel_sheet, json_path=req.json_path, max_rows=req.max_rows, + encoding=req.encoding, + sep=req.delimiter, ) except ValueError as exc: raise HTTPException(status_code=400, detail=str(exc)) from exc diff --git a/sidecar/tests/test_hyper_builder_formats.py b/sidecar/tests/test_hyper_builder_formats.py index 016627e..9b64740 100644 --- a/sidecar/tests/test_hyper_builder_formats.py +++ b/sidecar/tests/test_hyper_builder_formats.py @@ -23,6 +23,100 @@ def test_file_to_dataframe_csv(tmp_path: Path) -> None: assert len(df) == 2 +# --------------------------------------------------------------------------- +# Encoding/delimiter auto-sniff (UTF-16 / TSV) + explicit override +# --------------------------------------------------------------------------- + + +def _write_utf16_tsv(p: Path) -> None: + rows = ["region\tcustomer\trevenue", "West\tAcme\t$1,234", "East\tBeta\t$5"] + p.write_bytes(("\n".join(rows) + "\n").encode("utf-16")) + + +def test_file_to_dataframe_csv_autosniffs_utf16_tsv(tmp_path: Path) -> None: + p = tmp_path / "migrated.csv" + _write_utf16_tsv(p) + df = hyper_builder.file_to_dataframe("csv", str(p)) # no encoding/sep passed + assert list(df.columns) == ["region", "customer", "revenue"] + assert len(df) == 2 + assert df["region"].tolist() == ["West", "East"] + + +def test_file_to_dataframe_csv_explicit_encoding_sep(tmp_path: Path) -> None: + p = tmp_path / "migrated.csv" + _write_utf16_tsv(p) + df = hyper_builder.file_to_dataframe("csv", str(p), encoding="utf-16", sep="\t") + assert list(df.columns) == ["region", "customer", "revenue"] + assert len(df) == 2 + + +def test_sniff_csv_dialect_detects_utf16_tab(tmp_path: Path) -> None: + p = tmp_path / "m.csv" + _write_utf16_tsv(p) + enc, delim = hyper_builder._sniff_csv_dialect(p) + assert enc == "utf-16" + assert delim == "\t" + + +def test_sniff_csv_dialect_defaults_utf8_comma(tmp_path: Path) -> None: + p = tmp_path / "plain.csv" + p.write_text("a,b,c\n1,2,3\n") + enc, delim = hyper_builder._sniff_csv_dialect(p) + assert enc == "utf-8" + assert delim == "," + + +# --------------------------------------------------------------------------- +# Numeric coercion of display-formatted measures ($, %, accounting negatives) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("$16", 16.0), + ("-$5", -5.0), + ("($5)", -5.0), + ("20%", 0.20), + ("$1,234.5", 1234.5), + ("$0", 0.0), + ("4.0", 4.0), + ("", None), + ("CA-2011-103800", None), + ("Texas", None), + ], +) +def test_parse_formatted_number(raw: str, expected: float | None) -> None: + assert hyper_builder._parse_formatted_number(raw) == expected + + +def test_file_to_dataframe_coerces_formatted_measures(tmp_path: Path) -> None: + """Currency/percent string columns become numeric; text dimensions stay text.""" + csv = tmp_path / "f.csv" + csv.write_text( + "Region,Sales,Discount,OrderID\n" + "West,$1234,20%,CA-001\n" + "East,$56,10%,CA-002\n" + "Central,($7),0%,CA-003\n" + ) + df = hyper_builder.file_to_dataframe("csv", str(csv)) + assert str(df["Sales"].dtype).startswith(("float", "int")) + assert str(df["Discount"].dtype).startswith("float") + assert df["Sales"].tolist() == [1234.0, 56.0, -7.0] + assert df["Discount"].tolist() == [0.20, 0.10, 0.0] + assert df["Region"].tolist() == ["West", "East", "Central"] + assert df["OrderID"].dtype == object + + +def test_coercion_leaves_mostly_text_columns_alone(tmp_path: Path) -> None: + """A column with <90% numeric-looking values is NOT coerced.""" + csv = tmp_path / "g.csv" + csv.write_text("code\n$5\nABC\nDEF\nGHI\nJKL\n") # 1/5 numeric -> below 90% + df = hyper_builder.file_to_dataframe("csv", str(csv)) + assert df["code"].dtype == object + assert df["code"].tolist() == ["$5", "ABC", "DEF", "GHI", "JKL"] + + def test_file_to_dataframe_json(tmp_path: Path) -> None: data = [{"id": 1, "val": "a"}, {"id": 2, "val": "b"}] p = tmp_path / "data.json" diff --git a/src/sidecar.ts b/src/sidecar.ts index 543eb4c..e27b179 100644 --- a/src/sidecar.ts +++ b/src/sidecar.ts @@ -74,6 +74,13 @@ export interface FileArgs { * document (e.g. `$.data`). Only a single-level key is supported. */ jsonPath?: string; + /** + * CSV only: explicit text encoding (e.g. `utf-16`) and column delimiter + * (e.g. `"\t"`). When omitted, the sidecar auto-sniffs both from the file's + * BOM and header row, so UTF-16/TSV exports load without specifying them. + */ + encoding?: string; + delimiter?: string; } export interface WorkbookArgs { @@ -342,6 +349,8 @@ export class AuthoringSidecar { if (args.fileType) payload["fileType"] = args.fileType; if (args.excelSheet !== undefined) payload["excelSheet"] = args.excelSheet; if (args.jsonPath) payload["jsonPath"] = args.jsonPath; + if (args.encoding) payload["encoding"] = args.encoding; + if (args.delimiter) payload["delimiter"] = args.delimiter; const result = await this.post("/datasource/from-file", payload); return { tdsxPath: result.path, columns: result.columns, hyperPath: result.hyperPath }; From 8d4d01a5dca0827f618c22a1f2c6334bbc6a6942 Mon Sep 17 00:00:00 2001 From: Sebastien Henry Date: Fri, 26 Jun 2026 13:42:38 -0500 Subject: [PATCH 02/26] feat(schema): grow planner schema + sidecar plumbing for rich dashboards Slice 2 of exec-dashboards. All additions OPTIONAL/additive (schemaVersion stays 1; bump deferred to when required-by-kind validation lands): - MarkTypeEnum += scatter, map_filled; new SheetKindEnum (chart|kpi_tile) - SheetSpec += kind/color/kpi/scatter/geo (the rich-encoding fields) - DashboardPlan += dashboardTitle/subtitle/textZones/layoutGrammar - DatasourceSpec += encoding/delimiter (the sidecar already honors these) - DashboardProposalSchema (the propose->confirm contract) + isDashboardProposal - SheetModel/DashboardWorkbookRequest + sidecar.ts carry the new fields end-to-end Slice 3 makes the builder EMIT them; Slice 4 makes the planner PRODUCE them. Backward-compat verified: a plain {title,markType,rows,cols,measures} sheet and a minimal plan still validate. Gate: 128 TS + 149 Python = 277 tests. Co-Authored-By: Claude Opus 4.8 --- eslint.config.js | 1 + sidecar/server.py | 87 +++++ sidecar/tests/test_schema_growth.py | 347 +++++++++++++++++ src/planner/schema.ts | 251 ++++++++++++- src/sidecar.ts | 83 +++++ tests/schema-growth.test.ts | 560 ++++++++++++++++++++++++++++ 6 files changed, 1311 insertions(+), 18 deletions(-) create mode 100644 sidecar/tests/test_schema_growth.py create mode 100644 tests/schema-growth.test.ts diff --git a/eslint.config.js b/eslint.config.js index b8487cf..e74756d 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -10,6 +10,7 @@ export default tseslint.config( "sidecar/**", "coverage/**", ".cursor/**", + ".remember/**", ], }, eslint.configs.recommended, diff --git a/sidecar/server.py b/sidecar/server.py index c869a04..9f0e885 100644 --- a/sidecar/server.py +++ b/sidecar/server.py @@ -61,6 +61,56 @@ class TableRequest(BaseModel): records: list[dict[str, Any]] | None = None +# --------------------------------------------------------------------------- +# Phase-1 optional encoding sub-models (Slice 2: carry end-to-end). +# These just need to PARSE and survive model_dump(). +# Slice 3 will consume them in the builder. +# --------------------------------------------------------------------------- + + +class SheetColorModel(BaseModel): + """Color encoding for a worksheet (mirrors schema.ts SheetColor).""" + + model_config = ConfigDict(populate_by_name=True) + + field: str + kind: str # "dimension" | "measure_names" | "measure" + + +class SheetKpiModel(BaseModel): + """KPI tile configuration (mirrors schema.ts SheetKpi).""" + + model_config = ConfigDict(populate_by_name=True) + + primary_measure: str = Field(alias="primaryMeasure") + comparison_measure: str | None = Field(default=None, alias="comparisonMeasure") + delta_measure: str | None = Field(default=None, alias="deltaMeasure") + delta_is_positive_good: bool | None = Field(default=None, alias="deltaIsPositiveGood") + sparkline_field: str | None = Field(default=None, alias="sparklineField") + value_prefix: str | None = Field(default=None, alias="valuePrefix") + value_suffix: str | None = Field(default=None, alias="valueSuffix") + + +class SheetScatterModel(BaseModel): + """Scatter-plot axis binding (mirrors schema.ts SheetScatter).""" + + model_config = ConfigDict(populate_by_name=True) + + x: str + y: str + breakdown: str | None = None + + +class SheetGeoModel(BaseModel): + """Geographic / filled-map encoding (mirrors schema.ts SheetGeo).""" + + model_config = ConfigDict(populate_by_name=True) + + geo_field: str = Field(alias="geoField") + geo_role: str = Field(alias="geoRole") # "state" | "country" | "city" | "zipcode" + color_measure: str | None = Field(default=None, alias="colorMeasure") + + class SheetModel(BaseModel): model_config = ConfigDict(populate_by_name=True) @@ -69,6 +119,13 @@ class SheetModel(BaseModel): rows: list[str] = [] cols: list[str] = [] measures: list[str] = [] + # Phase-1 optional encoding fields (Slice 2: carry end-to-end). + # Consumed by the builder in Slice 3. + kind: str | None = None # "chart" | "kpi_tile" + color: SheetColorModel | None = None + kpi: SheetKpiModel | None = None + scatter: SheetScatterModel | None = None + geo: SheetGeoModel | None = None class WorkbookRequest(BaseModel): @@ -81,6 +138,30 @@ class WorkbookRequest(BaseModel): sheets: list[SheetModel] +# --------------------------------------------------------------------------- +# Dashboard-level text-zone and layout-grammar sub-models +# --------------------------------------------------------------------------- + + +class TextZoneModel(BaseModel): + """A text zone to render in the dashboard (mirrors schema.ts TextZone).""" + + model_config = ConfigDict(populate_by_name=True) + + text: str + position: str # "header" | "footer" + + +class LayoutGrammarModel(BaseModel): + """Layout grammar for the dashboard canvas (mirrors schema.ts LayoutGrammar).""" + + model_config = ConfigDict(populate_by_name=True) + + kind: str # "kpi_band_over_charts" | "tiled_vertical" | "tiled_horizontal" + kpi_tile_titles: list[str] | None = Field(default=None, alias="kpiTileTitles") + chart_titles: list[str] | None = Field(default=None, alias="chartTitles") + + class DashboardWorkbookRequest(BaseModel): model_config = ConfigDict(populate_by_name=True) @@ -97,6 +178,12 @@ class DashboardWorkbookRequest(BaseModel): # This is the self-contained path that renders on Tableau Cloud without a # prior publish_datasource step. hyper_path: str | None = Field(default=None, alias="hyperPath") + # Phase-1 optional dashboard-level fields (Slice 2: carry end-to-end). + # Consumed by the builder in Slice 3. + dashboard_title: str | None = Field(default=None, alias="dashboardTitle") + dashboard_subtitle: str | None = Field(default=None, alias="dashboardSubtitle") + text_zones: list[TextZoneModel] | None = Field(default=None, alias="textZones") + layout_grammar: LayoutGrammarModel | None = Field(default=None, alias="layoutGrammar") class FileRequest(BaseModel): diff --git a/sidecar/tests/test_schema_growth.py b/sidecar/tests/test_schema_growth.py new file mode 100644 index 0000000..1f11d3e --- /dev/null +++ b/sidecar/tests/test_schema_growth.py @@ -0,0 +1,347 @@ +"""Tests for Slice 2 — schema growth: SheetModel / DashboardWorkbookRequest new optional fields. + +Verifies that: +1. A minimal old-style payload (title/markType/rows/cols/measures) still parses + (backward-compatibility guard). +2. All new SheetModel optional fields (kind / color / kpi / scatter / geo) parse + and round-trip through model_dump(). +3. DashboardWorkbookRequest accepts the new optional dashboard-level fields + (dashboard_title / dashboard_subtitle / text_zones / layout_grammar) and they + survive model_dump(). +4. Old minimal DashboardWorkbookRequest payloads still parse (backward-compat). +""" + +from __future__ import annotations + +from server import ( + DashboardWorkbookRequest, + SheetModel, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +MINIMAL_SHEET_PAYLOAD: dict[str, object] = { + "title": "Revenue by Region", + "markType": "bar", + "rows": ["Region"], + "cols": [], + "measures": ["Sales"], +} + +MINIMAL_DASHBOARD_PAYLOAD: dict[str, object] = { + "datasourceName": "Superstore", + "datasourceContentUrl": "superstore", + "site": "", + "sheets": [MINIMAL_SHEET_PAYLOAD], + "dashboardLayout": "tiled_vertical", + "canvasWidth": 1000, + "canvasHeight": 800, +} + + +# =========================================================================== +# SheetModel — backward-compatibility +# =========================================================================== + + +def test_sheet_model_minimal_parses() -> None: + """A minimal old-style sheet payload must parse without new fields.""" + sheet = SheetModel.model_validate(MINIMAL_SHEET_PAYLOAD) + assert sheet.title == "Revenue by Region" + assert sheet.mark_type == "bar" + assert sheet.rows == ["Region"] + assert sheet.kind is None + assert sheet.color is None + assert sheet.kpi is None + assert sheet.scatter is None + assert sheet.geo is None + + +def test_sheet_model_minimal_round_trips() -> None: + """model_dump() on a minimal sheet must not include None optional fields as non-None.""" + sheet = SheetModel.model_validate(MINIMAL_SHEET_PAYLOAD) + dumped = sheet.model_dump() + assert dumped["title"] == "Revenue by Region" + assert dumped["kind"] is None + assert dumped["color"] is None + assert dumped["kpi"] is None + assert dumped["scatter"] is None + assert dumped["geo"] is None + + +# =========================================================================== +# SheetModel — optional kind field +# =========================================================================== + + +def test_sheet_model_kind_kpi_tile() -> None: + """kind='kpi_tile' must parse and survive round-trip.""" + sheet = SheetModel.model_validate({**MINIMAL_SHEET_PAYLOAD, "kind": "kpi_tile"}) + assert sheet.kind == "kpi_tile" + assert sheet.model_dump()["kind"] == "kpi_tile" + + +def test_sheet_model_kind_chart() -> None: + """kind='chart' must parse.""" + sheet = SheetModel.model_validate({**MINIMAL_SHEET_PAYLOAD, "kind": "chart"}) + assert sheet.kind == "chart" + + +# =========================================================================== +# SheetModel — optional color field +# =========================================================================== + + +def test_sheet_model_color_dimension() -> None: + """color block with kind='dimension' must parse and round-trip.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "color": {"field": "Category", "kind": "dimension"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.color is not None + assert sheet.color.field == "Category" + assert sheet.color.kind == "dimension" + dumped = sheet.model_dump() + assert dumped["color"]["field"] == "Category" + + +def test_sheet_model_color_measure() -> None: + """color block with kind='measure' must parse.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "markType": "map_filled", + "color": {"field": "Profit", "kind": "measure"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.color is not None + assert sheet.color.kind == "measure" + + +def test_sheet_model_color_measure_names() -> None: + """color block with kind='measure_names' must parse.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "color": {"field": "Measure Names", "kind": "measure_names"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.color is not None + assert sheet.color.kind == "measure_names" + + +# =========================================================================== +# SheetModel — optional kpi field +# =========================================================================== + + +def test_sheet_model_kpi_full() -> None: + """A fully-specified kpi block must parse and round-trip.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "kind": "kpi_tile", + "kpi": { + "primaryMeasure": "CP Sales", + "comparisonMeasure": "PP Sales", + "deltaMeasure": "Sales Difference", + "deltaIsPositiveGood": True, + "sparklineField": "Order Date", + "valuePrefix": "$", + "valueSuffix": "K", + }, + } + sheet = SheetModel.model_validate(payload) + assert sheet.kpi is not None + assert sheet.kpi.primary_measure == "CP Sales" + assert sheet.kpi.comparison_measure == "PP Sales" + assert sheet.kpi.delta_is_positive_good is True + assert sheet.kpi.value_prefix == "$" + dumped = sheet.model_dump() + assert dumped["kpi"]["primary_measure"] == "CP Sales" + + +def test_sheet_model_kpi_minimal() -> None: + """A kpi block with only primaryMeasure must parse.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "kind": "kpi_tile", + "kpi": {"primaryMeasure": "Profit"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.kpi is not None + assert sheet.kpi.primary_measure == "Profit" + assert sheet.kpi.comparison_measure is None + assert sheet.kpi.delta_measure is None + + +# =========================================================================== +# SheetModel — optional scatter field +# =========================================================================== + + +def test_sheet_model_scatter_with_breakdown() -> None: + """scatter block with x, y, and breakdown must parse and round-trip.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "markType": "scatter", + "scatter": {"x": "Sales", "y": "Profit", "breakdown": "Category"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.scatter is not None + assert sheet.scatter.x == "Sales" + assert sheet.scatter.y == "Profit" + assert sheet.scatter.breakdown == "Category" + dumped = sheet.model_dump() + assert dumped["scatter"]["x"] == "Sales" + + +def test_sheet_model_scatter_without_breakdown() -> None: + """scatter block without breakdown must parse (breakdown is optional).""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "markType": "scatter", + "scatter": {"x": "Quantity", "y": "Discount"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.scatter is not None + assert sheet.scatter.breakdown is None + + +# =========================================================================== +# SheetModel — optional geo field +# =========================================================================== + + +def test_sheet_model_geo_state_with_color() -> None: + """geo block with geoRole='state' and colorMeasure must parse and round-trip.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "markType": "map_filled", + "geo": {"geoField": "State", "geoRole": "state", "colorMeasure": "Profit"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.geo is not None + assert sheet.geo.geo_field == "State" + assert sheet.geo.geo_role == "state" + assert sheet.geo.color_measure == "Profit" + dumped = sheet.model_dump() + assert dumped["geo"]["geo_field"] == "State" + + +def test_sheet_model_geo_country_no_color() -> None: + """geo block with geoRole='country' and no colorMeasure must parse.""" + payload = { + **MINIMAL_SHEET_PAYLOAD, + "markType": "map_filled", + "geo": {"geoField": "Country", "geoRole": "country"}, + } + sheet = SheetModel.model_validate(payload) + assert sheet.geo is not None + assert sheet.geo.color_measure is None + + +# =========================================================================== +# DashboardWorkbookRequest — backward-compatibility +# =========================================================================== + + +def test_dashboard_request_minimal_parses() -> None: + """A minimal old-style request must parse without new dashboard-level fields.""" + req = DashboardWorkbookRequest.model_validate(MINIMAL_DASHBOARD_PAYLOAD) + assert req.datasource_name == "Superstore" + assert req.dashboard_title is None + assert req.dashboard_subtitle is None + assert req.text_zones is None + assert req.layout_grammar is None + + +# =========================================================================== +# DashboardWorkbookRequest — new optional dashboard-level fields +# =========================================================================== + + +def test_dashboard_request_with_title_and_subtitle() -> None: + """dashboard_title and dashboard_subtitle must parse and survive round-trip.""" + payload = { + **MINIMAL_DASHBOARD_PAYLOAD, + "dashboardTitle": "Executive Overview", + "dashboardSubtitle": "Q1 2024 Performance", + } + req = DashboardWorkbookRequest.model_validate(payload) + assert req.dashboard_title == "Executive Overview" + assert req.dashboard_subtitle == "Q1 2024 Performance" + dumped = req.model_dump() + assert dumped["dashboard_title"] == "Executive Overview" + assert dumped["dashboard_subtitle"] == "Q1 2024 Performance" + + +def test_dashboard_request_with_text_zones() -> None: + """text_zones list must parse and survive round-trip.""" + payload = { + **MINIMAL_DASHBOARD_PAYLOAD, + "textZones": [ + {"text": "Q1 2024 Executive Dashboard", "position": "header"}, + {"text": "Confidential", "position": "footer"}, + ], + } + req = DashboardWorkbookRequest.model_validate(payload) + assert req.text_zones is not None + assert len(req.text_zones) == 2 + assert req.text_zones[0].text == "Q1 2024 Executive Dashboard" + assert req.text_zones[0].position == "header" + assert req.text_zones[1].position == "footer" + dumped = req.model_dump() + assert len(dumped["text_zones"]) == 2 + + +def test_dashboard_request_with_layout_grammar_kpi_band() -> None: + """layoutGrammar with kind='kpi_band_over_charts' must parse and survive round-trip.""" + payload = { + **MINIMAL_DASHBOARD_PAYLOAD, + "layoutGrammar": { + "kind": "kpi_band_over_charts", + "kpiTileTitles": ["Sales KPI", "Profit KPI"], + "chartTitles": ["Revenue by Region", "Sales by Category"], + }, + } + req = DashboardWorkbookRequest.model_validate(payload) + assert req.layout_grammar is not None + assert req.layout_grammar.kind == "kpi_band_over_charts" + assert req.layout_grammar.kpi_tile_titles == ["Sales KPI", "Profit KPI"] + assert req.layout_grammar.chart_titles == ["Revenue by Region", "Sales by Category"] + dumped = req.model_dump() + assert dumped["layout_grammar"]["kind"] == "kpi_band_over_charts" + assert dumped["layout_grammar"]["kpi_tile_titles"] == ["Sales KPI", "Profit KPI"] + + +def test_dashboard_request_with_all_new_fields() -> None: + """All four new dashboard-level fields together must parse cleanly.""" + payload = { + **MINIMAL_DASHBOARD_PAYLOAD, + "dashboardTitle": "Executive Overview", + "dashboardSubtitle": "Q1 2024", + "textZones": [{"text": "Header Text", "position": "header"}], + "layoutGrammar": {"kind": "tiled_vertical"}, + "sheets": [ + { + **MINIMAL_SHEET_PAYLOAD, + "kind": "kpi_tile", + "kpi": {"primaryMeasure": "Sales"}, + "color": {"field": "Category", "kind": "dimension"}, + "scatter": {"x": "Sales", "y": "Profit"}, + "geo": {"geoField": "State", "geoRole": "state"}, + } + ], + } + req = DashboardWorkbookRequest.model_validate(payload) + assert req.dashboard_title == "Executive Overview" + assert req.layout_grammar is not None + assert req.layout_grammar.kind == "tiled_vertical" + assert len(req.sheets) == 1 + sheet = req.sheets[0] + assert sheet.kind == "kpi_tile" + assert sheet.kpi is not None + assert sheet.kpi.primary_measure == "Sales" + assert sheet.geo is not None + assert sheet.geo.geo_role == "state" diff --git a/src/planner/schema.ts b/src/planner/schema.ts index f65f6dc..0f6ad71 100644 --- a/src/planner/schema.ts +++ b/src/planner/schema.ts @@ -1,11 +1,17 @@ /** - * Shared DashboardPlan / ClarifyingQuestions Zod schema (BI_DESIGN §0/§8.1). + * Shared DashboardPlan / ClarifyingQuestions / DashboardProposal Zod schema + * (BI_DESIGN §0/§8.1). * * This module is the single source of truth for the contract between * `design_dashboard` (producer) and `build_from_plan` (consumer). * Both tools import from here; the sidecar mirrors the shape as a Pydantic model. * * The `schemaVersion` literal `1` is enforced at parse time — any other value throws. + * + * NOTE — schemaVersion bump to 2: deferred until required-by-kind validation + * (asserting e.g. that a kpi_tile sheet always carries a kpi block) lands in a + * later phase. All additions below are OPTIONAL so existing callers and tests + * remain valid against schemaVersion 1. */ import { z } from "zod"; @@ -37,12 +43,21 @@ export function assertSchemaVersion(version: unknown): void { export const AudienceEnum = z.enum(["exec", "analyst", "operational", "mixed"]); export type Audience = z.infer; -export const MarkTypeEnum = z.enum(["bar", "line", "text", "map"]); +/** Extended with Phase-1 mark types: scatter (Circle mark) and map_filled (filled-map). */ +export const MarkTypeEnum = z.enum(["bar", "line", "text", "map", "scatter", "map_filled"]); export type MarkType = z.infer; export const DashboardLayoutEnum = z.enum(["tiled_vertical", "tiled_horizontal"]); export type DashboardLayout = z.infer; +/** + * Sheet kind — distinguishes a standard chart from a KPI tile. + * Defaults to "chart" so plain {title, markType, rows, cols, measures} sheets + * are backward-compatible without explicitly setting this field. + */ +export const SheetKindEnum = z.enum(["chart", "kpi_tile"]); +export type SheetKind = z.infer; + // --------------------------------------------------------------------------- // FieldHint schema // --------------------------------------------------------------------------- @@ -53,6 +68,61 @@ export const FieldHintSchema = z.object({ dataType: z.enum(["string", "number", "date", "boolean"]).optional(), }); +// --------------------------------------------------------------------------- +// SheetSpec sub-schemas (new optional encoding blocks) +// --------------------------------------------------------------------------- + +/** + * Color encoding for a worksheet. + * - kind "dimension": color by a categorical field (e.g. Category). + * - kind "measure_names": color by Measure Names (used in multi-measure lines/bars). + * - kind "measure": color by a quantitative measure (e.g. Profit on a filled map). + */ +export const SheetColorSchema = z.object({ + field: z.string().min(1), + kind: z.enum(["dimension", "measure_names", "measure"]), +}); +export type SheetColor = z.infer; + +/** + * KPI tile encoding. + * primaryMeasure is always shown; comparison/delta/sparkline are optional. + * deltaIsPositiveGood controls the coloring of the delta arrow (green-up vs red-up). + */ +export const SheetKpiSchema = z.object({ + primaryMeasure: z.string().min(1), + comparisonMeasure: z.string().optional(), + deltaMeasure: z.string().optional(), + deltaIsPositiveGood: z.boolean().optional(), + sparklineField: z.string().optional(), + valuePrefix: z.string().optional(), + valueSuffix: z.string().optional(), +}); +export type SheetKpi = z.infer; + +/** + * Scatter-plot encoding. + * x / y are measure fields; breakdown is an optional dimension for color/shape. + */ +export const SheetScatterSchema = z.object({ + x: z.string().min(1), + y: z.string().min(1), + breakdown: z.string().optional(), +}); +export type SheetScatter = z.infer; + +/** + * Filled-map / geographic encoding. + * geoField is the dimension column; geoRole defines how Tableau geocodes it. + * colorMeasure is the optional KPI painted on the map. + */ +export const SheetGeoSchema = z.object({ + geoField: z.string().min(1), + geoRole: z.enum(["state", "country", "city", "zipcode"]), + colorMeasure: z.string().optional(), +}); +export type SheetGeo = z.infer; + // --------------------------------------------------------------------------- // SheetSpec — superset of the existing sidecar.ts SheetSpec // --------------------------------------------------------------------------- @@ -64,6 +134,17 @@ export const SheetSpecSchema = z.object({ cols: z.array(z.string()).default([]), measures: z.array(z.string()).default([]), rationale: z.string().optional(), + // --- Phase-1 optional encoding blocks (Slice 2: carry end-to-end) --- + /** Sheet kind: "chart" (default) or "kpi_tile". */ + kind: SheetKindEnum.optional(), + /** Color encoding for dimension, measure-names, or quantitative coloring. */ + color: SheetColorSchema.optional(), + /** KPI tile configuration (required when kind = "kpi_tile"; validated in a later phase). */ + kpi: SheetKpiSchema.optional(), + /** Scatter-plot axis binding (for markType = "scatter"). */ + scatter: SheetScatterSchema.optional(), + /** Geographic encoding (for markType = "map_filled"). */ + geo: SheetGeoSchema.optional(), }); export type SheetSpec = z.infer; @@ -81,6 +162,17 @@ export const DatasourceSpecSchema = z jsonPath: z.string().optional(), sql: z.string().optional(), connection: z.record(z.unknown()).optional(), + /** + * Explicit text encoding for CSV files (e.g. "utf-16"). + * When absent the sidecar auto-sniffs from the BOM. + * Threaded end-to-end in Slice 1; schema field added here for completeness. + */ + encoding: z.string().optional(), + /** + * Column delimiter for CSV files (e.g. "\t"). + * When absent the sidecar auto-sniffs from the decoded header row. + */ + delimiter: z.string().optional(), }) .refine( (d) => (d.filePath !== undefined) !== (d.sql !== undefined) || d.filePath !== undefined, @@ -91,24 +183,65 @@ export const DatasourceSpecSchema = z export type DatasourceSpec = z.infer; +// --------------------------------------------------------------------------- +// DashboardPlan layout sub-schemas +// --------------------------------------------------------------------------- + +/** + * Layout grammar for the dashboard canvas. + * + * - "kpi_band_over_charts": a horizontal KPI strip at the top with chart tiles + * below (Phase-1 target for exec dashboards). + * - "tiled_vertical" / "tiled_horizontal": simple 1-D tiling (existing behavior). + */ +export const LayoutGrammarKindEnum = z.enum([ + "kpi_band_over_charts", + "tiled_vertical", + "tiled_horizontal", +]); +export type LayoutGrammarKind = z.infer; + +export const LayoutGrammarSchema = z.object({ + kind: LayoutGrammarKindEnum, + /** Titles for the KPI tiles in the top band (used by kpi_band_over_charts). */ + kpiTileTitles: z.array(z.string()).optional(), + /** Titles for the chart tiles in the main area. */ + chartTitles: z.array(z.string()).optional(), +}); +export type LayoutGrammar = z.infer; + +export const TextZoneSchema = z.object({ + text: z.string().min(1), + position: z.enum(["header", "footer"]), +}); +export type TextZone = z.infer; + // --------------------------------------------------------------------------- // DashboardPlan // --------------------------------------------------------------------------- -export const DashboardPlanSchema = z - .object({ - schemaVersion: z.literal(SCHEMA_VERSION), - kind: z.literal("plan"), - workbookName: z.string().min(1), - datasourceLuid: z.string().min(1), - datasourceName: z.string().min(1), - projectName: z.string().min(1), - audience: AudienceEnum, - rationale: z.string().min(1), - dashboardLayout: DashboardLayoutEnum, - sheets: z.array(SheetSpecSchema).min(1), - datasourceSpec: DatasourceSpecSchema.optional(), - }); +export const DashboardPlanSchema = z.object({ + schemaVersion: z.literal(SCHEMA_VERSION), + kind: z.literal("plan"), + workbookName: z.string().min(1), + datasourceLuid: z.string().min(1), + datasourceName: z.string().min(1), + projectName: z.string().min(1), + audience: AudienceEnum, + rationale: z.string().min(1), + dashboardLayout: DashboardLayoutEnum, + sheets: z.array(SheetSpecSchema).min(1), + datasourceSpec: DatasourceSpecSchema.optional(), + // --- Phase-1 optional dashboard-level fields (Slice 2: carry end-to-end) --- + /** Human-readable dashboard title rendered in the title text zone. */ + dashboardTitle: z.string().optional(), + /** Human-readable dashboard subtitle rendered below the title. */ + dashboardSubtitle: z.string().optional(), + /** Explicit text zones (header / footer) additional to any auto-derived title zone. */ + textZones: z.array(TextZoneSchema).optional(), + /** Structured layout grammar used by the builder to emit multi-zone XML. */ + layoutGrammar: LayoutGrammarSchema.optional(), +}); export type DashboardPlan = z.infer; @@ -128,13 +261,86 @@ export const ClarifyingQuestionsSchema = z.object({ }), ) .min(3) - .max(7), + .max(10), }); export type ClarifyingQuestions = z.infer; // --------------------------------------------------------------------------- -// Type guard +// DashboardProposal — the propose→confirm contract (BI_DESIGN §2.6) +// +// Returned by `design_dashboard` once enough context is known. +// The agent presents this to the user; on "confirm" the agent calls +// `build_from_plan` with `proposal.plan` verbatim (re-validated server-side). +// The `design_dashboard` tool NEVER builds; building is always a separate step. +// --------------------------------------------------------------------------- + +export const KpiStripItemSchema = z.object({ + /** Display label for this KPI tile. */ + label: z.string().min(1), + /** Primary measure field name. */ + primaryMeasure: z.string().min(1), + /** Comparison-period measure field name (optional). */ + comparisonMeasure: z.string().optional(), + /** Delta / difference measure field name (optional). */ + deltaMeasure: z.string().optional(), + /** + * Directional interpretation of the delta for coloring: + * - "up_good": positive delta → green arrow. + * - "down_good": negative delta → green arrow (e.g. cost, returns). + * - "neutral": no coloring applied. + */ + direction: z.enum(["up_good", "down_good", "neutral"]), +}); +export type KpiStripItem = z.infer; + +export const ProposedViewSchema = z.object({ + /** Worksheet title. */ + title: z.string().min(1), + /** Mark type / chart type identifier (e.g. "bar", "scatter", "map_filled"). */ + chartType: z.string().min(1), + /** Human-readable encoding summary (e.g. "Sales by Category, colored by Segment"). */ + encodingSummary: z.string().min(1), + /** Field names referenced by this view. */ + fields: z.array(z.string()).min(1), +}); +export type ProposedView = z.infer; + +export const DashboardProposalSchema = z.object({ + schemaVersion: z.literal(SCHEMA_VERSION), + kind: z.literal("proposal"), + workbookName: z.string().min(1), + audience: AudienceEnum, + datasourceName: z.string().min(1), + projectName: z.string().min(1), + /** One-paragraph natural-language summary of the proposal for the user. */ + summary: z.string().min(1), + /** Proposed dashboard title (optional; mirrors DashboardPlan.dashboardTitle). */ + dashboardTitle: z.string().optional(), + /** Proposed dashboard subtitle (optional). */ + dashboardSubtitle: z.string().optional(), + /** KPI strip items; empty array means no KPI band. */ + kpiStrip: z.array(KpiStripItemSchema), + /** Proposed worksheet views. */ + views: z.array(ProposedViewSchema).min(1), + /** Natural-language description of the layout for the user. */ + layoutSummary: z.string().min(1), + /** + * Open questions the agent needs answered before producing a full plan. + * Present only when the proposal is preliminary. + */ + openQuestions: z.array(z.string()).optional(), + /** + * The ready-to-execute plan embedded in the proposal. + * The agent passes this verbatim to `build_from_plan` on user confirmation. + */ + plan: DashboardPlanSchema, +}); + +export type DashboardProposal = z.infer; + +// --------------------------------------------------------------------------- +// Type guards // --------------------------------------------------------------------------- /** Returns true when the payload is a DashboardPlan (discriminated by `kind`). */ @@ -145,3 +351,12 @@ export function isDashboardPlan(payload: unknown): payload is DashboardPlan { const result = DashboardPlanSchema.safeParse(payload); return result.success; } + +/** Returns true when the payload is a DashboardProposal (discriminated by `kind`). */ +export function isDashboardProposal(payload: unknown): payload is DashboardProposal { + if (typeof payload !== "object" || payload === null) return false; + const p = payload as Record; + if (p["kind"] !== "proposal") return false; + const result = DashboardProposalSchema.safeParse(payload); + return result.success; +} diff --git a/src/sidecar.ts b/src/sidecar.ts index e27b179..59b2004 100644 --- a/src/sidecar.ts +++ b/src/sidecar.ts @@ -47,12 +47,58 @@ export interface TableArgs { records?: Array>; } +// --------------------------------------------------------------------------- +// SheetSpec sub-types (mirror planner/schema.ts — kept in sync manually) +// --------------------------------------------------------------------------- + +/** Color encoding for a worksheet (dimension / measure-names / quantitative). */ +export interface SheetColor { + field: string; + kind: "dimension" | "measure_names" | "measure"; +} + +/** KPI tile encoding: primary + optional comparison/delta/sparkline. */ +export interface SheetKpi { + primaryMeasure: string; + comparisonMeasure?: string; + deltaMeasure?: string; + deltaIsPositiveGood?: boolean; + sparklineField?: string; + valuePrefix?: string; + valueSuffix?: string; +} + +/** Scatter-plot axis binding (x / y measures, optional breakdown dimension). */ +export interface SheetScatter { + x: string; + y: string; + breakdown?: string; +} + +/** Geographic / filled-map encoding. */ +export interface SheetGeo { + geoField: string; + geoRole: "state" | "country" | "city" | "zipcode"; + colorMeasure?: string; +} + export interface SheetSpec { title: string; markType: string; rows: string[]; cols: string[]; measures: string[]; + // --- Phase-1 optional encoding fields (Slice 2: carry end-to-end) --- + /** Sheet kind: "chart" (default) or "kpi_tile". */ + kind?: "chart" | "kpi_tile"; + /** Color encoding block. */ + color?: SheetColor; + /** KPI tile configuration. */ + kpi?: SheetKpi; + /** Scatter-plot axis binding. */ + scatter?: SheetScatter; + /** Geographic encoding. */ + geo?: SheetGeo; } export interface FileArgs { @@ -95,6 +141,19 @@ export interface WorkbookArgs { sheets: SheetSpec[]; } +/** Text zone for dashboard header / footer (mirrors schema.ts TextZone). */ +export interface TextZone { + text: string; + position: "header" | "footer"; +} + +/** Layout grammar for the dashboard canvas (mirrors schema.ts LayoutGrammar). */ +export interface LayoutGrammar { + kind: "kpi_band_over_charts" | "tiled_vertical" | "tiled_horizontal"; + kpiTileTitles?: string[]; + chartTitles?: string[]; +} + export interface DashboardWorkbookArgs extends WorkbookArgs { /** Sheet titles to include in the dashboard (subset or all of sheets[].title). */ dashboardSheetTitles: string[]; @@ -114,6 +173,15 @@ export interface DashboardWorkbookArgs extends WorkbookArgs { * Omit only when falling back to the legacy sqlproxy reference path. */ hyperPath?: string; + // --- Phase-1 optional dashboard-level fields (Slice 2: carry end-to-end) --- + /** Human-readable dashboard title rendered in the title text zone. */ + dashboardTitle?: string; + /** Human-readable dashboard subtitle rendered below the title. */ + dashboardSubtitle?: string; + /** Explicit text zones (header / footer). */ + textZones?: TextZone[]; + /** Structured layout grammar used by the builder to emit multi-zone XML. */ + layoutGrammar?: LayoutGrammar; } const HEALTH_TIMEOUT_MS = 30_000; @@ -393,6 +461,21 @@ export class AuthoringSidecar { if (args.hyperPath) { payload["hyperPath"] = args.hyperPath; } + // Phase-1 optional dashboard-level fields (Slice 2: carry end-to-end). + // Slice 3 will consume these in the builder; here we just forward them + // so the sidecar Pydantic models can parse and round-trip them. + if (args.dashboardTitle !== undefined) { + payload["dashboardTitle"] = args.dashboardTitle; + } + if (args.dashboardSubtitle !== undefined) { + payload["dashboardSubtitle"] = args.dashboardSubtitle; + } + if (args.textZones !== undefined) { + payload["textZones"] = args.textZones; + } + if (args.layoutGrammar !== undefined) { + payload["layoutGrammar"] = args.layoutGrammar; + } const { path } = await this.post("/workbook/dashboard", payload); return { twbxPath: path }; } diff --git a/tests/schema-growth.test.ts b/tests/schema-growth.test.ts new file mode 100644 index 0000000..1fa1218 --- /dev/null +++ b/tests/schema-growth.test.ts @@ -0,0 +1,560 @@ +/** + * Unit tests for Slice 2 — schema growth (Phase 1 §2). + * + * Verifies that: + * 1. Existing minimal {title, markType, rows, cols, measures} sheets still parse + * (backward-compatibility guard). + * 2. New optional SheetSpec fields (kind / color / kpi / scatter / geo) parse + * correctly. + * 3. New optional DashboardPlan fields (dashboardTitle / dashboardSubtitle / + * textZones / layoutGrammar) parse correctly. + * 4. DashboardProposalSchema parses a complete proposal payload. + * 5. isDashboardProposal type-guard correctly discriminates. + * 6. isDashboardPlan type-guard is unaffected. + * 7. New MarkTypeEnum values ("scatter", "map_filled") are accepted. + * 8. ClarifyingQuestions max raised to 10. + * + * All tests are purely deterministic — no LLM calls, no network. + */ + +import { describe, it, expect } from "vitest"; +import { + SheetSpecSchema, + DashboardPlanSchema, + DashboardProposalSchema, + ClarifyingQuestionsSchema, + MarkTypeEnum, + SheetKindEnum, + isDashboardPlan, + isDashboardProposal, + SCHEMA_VERSION, +} from "../src/planner/schema.js"; + +// --------------------------------------------------------------------------- +// Shared fixtures +// --------------------------------------------------------------------------- + +/** Minimal old-style sheet — must still parse unchanged (backward-compat). */ +const MINIMAL_SHEET = { + title: "Revenue by Region", + markType: "bar", + rows: ["Region"], + cols: [], + measures: ["Sales"], +}; + +/** Minimal valid DashboardPlan (old-style, no new fields). */ +const MINIMAL_PLAN = { + schemaVersion: SCHEMA_VERSION, + kind: "plan" as const, + workbookName: "My Workbook", + datasourceLuid: "abc-123", + datasourceName: "Superstore", + projectName: "Finance", + audience: "exec" as const, + rationale: "Show KPIs for Q1", + dashboardLayout: "tiled_vertical" as const, + sheets: [MINIMAL_SHEET], +}; + +// --------------------------------------------------------------------------- +// §1 — Backward-compatibility: minimal sheet still parses +// --------------------------------------------------------------------------- + +describe("backward-compatibility — minimal sheet", () => { + it("parses a plain {title, markType, rows, cols, measures} sheet unchanged", () => { + const result = SheetSpecSchema.safeParse(MINIMAL_SHEET); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.title).toBe("Revenue by Region"); + expect(result.data.markType).toBe("bar"); + expect(result.data.kind).toBeUndefined(); + expect(result.data.color).toBeUndefined(); + expect(result.data.kpi).toBeUndefined(); + expect(result.data.scatter).toBeUndefined(); + expect(result.data.geo).toBeUndefined(); + } + }); + + it("parses a minimal DashboardPlan without any new fields", () => { + const result = DashboardPlanSchema.safeParse(MINIMAL_PLAN); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.dashboardTitle).toBeUndefined(); + expect(result.data.dashboardSubtitle).toBeUndefined(); + expect(result.data.textZones).toBeUndefined(); + expect(result.data.layoutGrammar).toBeUndefined(); + } + }); +}); + +// --------------------------------------------------------------------------- +// §2 — New MarkTypeEnum values +// --------------------------------------------------------------------------- + +describe("MarkTypeEnum extensions", () => { + it('accepts "scatter"', () => { + const result = MarkTypeEnum.safeParse("scatter"); + expect(result.success).toBe(true); + }); + + it('accepts "map_filled"', () => { + const result = MarkTypeEnum.safeParse("map_filled"); + expect(result.success).toBe(true); + }); + + it('still accepts legacy values "bar", "line", "text", "map"', () => { + for (const v of ["bar", "line", "text", "map"] as const) { + expect(MarkTypeEnum.safeParse(v).success).toBe(true); + } + }); + + it('rejects unknown "heatmap"', () => { + expect(MarkTypeEnum.safeParse("heatmap").success).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// §3 — SheetKindEnum +// --------------------------------------------------------------------------- + +describe("SheetKindEnum", () => { + it('accepts "chart" and "kpi_tile"', () => { + expect(SheetKindEnum.safeParse("chart").success).toBe(true); + expect(SheetKindEnum.safeParse("kpi_tile").success).toBe(true); + }); + + it('rejects unknown "sparkline"', () => { + expect(SheetKindEnum.safeParse("sparkline").success).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// §4 — SheetSpec with optional encoding blocks +// --------------------------------------------------------------------------- + +describe("SheetSpec — optional color block", () => { + it("parses a sheet with a dimension color encoding", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + color: { field: "Category", kind: "dimension" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.color?.field).toBe("Category"); + expect(result.data.color?.kind).toBe("dimension"); + } + }); + + it("parses a sheet with a measure color encoding", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + markType: "map_filled", + color: { field: "Profit", kind: "measure" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.color?.kind).toBe("measure"); + } + }); + + it("parses a sheet with measure_names color kind", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + color: { field: "Measure Names", kind: "measure_names" }, + }); + expect(result.success).toBe(true); + }); + + it("rejects color with an invalid kind", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + color: { field: "Category", kind: "categorical" }, + }); + expect(result.success).toBe(false); + }); +}); + +describe("SheetSpec — optional kpi block", () => { + it("parses a kpi_tile sheet with a full kpi block", () => { + const result = SheetSpecSchema.safeParse({ + title: "Sales KPI", + markType: "text", + rows: [], + cols: [], + measures: ["CP Sales"], + kind: "kpi_tile", + kpi: { + primaryMeasure: "CP Sales", + comparisonMeasure: "PP Sales", + deltaMeasure: "Sales Difference", + deltaIsPositiveGood: true, + sparklineField: "Order Date", + valuePrefix: "$", + valueSuffix: "K", + }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.kind).toBe("kpi_tile"); + expect(result.data.kpi?.primaryMeasure).toBe("CP Sales"); + expect(result.data.kpi?.comparisonMeasure).toBe("PP Sales"); + expect(result.data.kpi?.deltaIsPositiveGood).toBe(true); + expect(result.data.kpi?.valuePrefix).toBe("$"); + } + }); + + it("parses a kpi_tile sheet with only primaryMeasure", () => { + const result = SheetSpecSchema.safeParse({ + title: "Profit KPI", + markType: "text", + rows: [], + cols: [], + measures: ["Profit"], + kind: "kpi_tile", + kpi: { primaryMeasure: "Profit" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.kpi?.comparisonMeasure).toBeUndefined(); + } + }); +}); + +describe("SheetSpec — optional scatter block", () => { + it("parses a scatter sheet with x, y, and breakdown", () => { + const result = SheetSpecSchema.safeParse({ + title: "Sales vs Profit", + markType: "scatter", + rows: ["Sales"], + cols: ["Profit"], + measures: ["Sales", "Profit"], + scatter: { x: "Sales", y: "Profit", breakdown: "Category" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.scatter?.x).toBe("Sales"); + expect(result.data.scatter?.breakdown).toBe("Category"); + } + }); + + it("parses a scatter sheet with only x and y (no breakdown)", () => { + const result = SheetSpecSchema.safeParse({ + title: "Qty vs Discount", + markType: "scatter", + rows: ["Quantity"], + cols: ["Discount"], + measures: ["Quantity", "Discount"], + scatter: { x: "Quantity", y: "Discount" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.scatter?.breakdown).toBeUndefined(); + } + }); + + it("rejects scatter block missing y", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + markType: "scatter", + scatter: { x: "Sales" }, + }); + expect(result.success).toBe(false); + }); +}); + +describe("SheetSpec — optional geo block", () => { + it("parses a map_filled sheet with a geo block", () => { + const result = SheetSpecSchema.safeParse({ + title: "Profit by State", + markType: "map_filled", + rows: [], + cols: ["State"], + measures: ["Profit"], + geo: { geoField: "State", geoRole: "state", colorMeasure: "Profit" }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.geo?.geoRole).toBe("state"); + expect(result.data.geo?.colorMeasure).toBe("Profit"); + } + }); + + it("parses geo with geoRole country and no colorMeasure", () => { + const result = SheetSpecSchema.safeParse({ + title: "Global Map", + markType: "map_filled", + rows: [], + cols: ["Country"], + measures: [], + geo: { geoField: "Country", geoRole: "country" }, + }); + expect(result.success).toBe(true); + }); + + it("rejects geo with an invalid geoRole", () => { + const result = SheetSpecSchema.safeParse({ + ...MINIMAL_SHEET, + markType: "map_filled", + geo: { geoField: "State", geoRole: "province" }, + }); + expect(result.success).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// §5 — DashboardPlan with new optional dashboard-level fields +// --------------------------------------------------------------------------- + +describe("DashboardPlan — new optional fields", () => { + it("parses a plan with dashboardTitle and dashboardSubtitle", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + dashboardTitle: "Executive Overview", + dashboardSubtitle: "Q1 2024 Performance", + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.dashboardTitle).toBe("Executive Overview"); + expect(result.data.dashboardSubtitle).toBe("Q1 2024 Performance"); + } + }); + + it("parses a plan with textZones array", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + textZones: [ + { text: "Q1 2024 Executive Dashboard", position: "header" }, + { text: "Confidential — Internal use only", position: "footer" }, + ], + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.textZones).toHaveLength(2); + expect(result.data.textZones?.[0]?.position).toBe("header"); + } + }); + + it("parses a plan with layoutGrammar kpi_band_over_charts", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + layoutGrammar: { + kind: "kpi_band_over_charts", + kpiTileTitles: ["Sales KPI", "Profit KPI"], + chartTitles: ["Revenue by Region", "Sales by Category"], + }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.layoutGrammar?.kind).toBe("kpi_band_over_charts"); + expect(result.data.layoutGrammar?.kpiTileTitles).toHaveLength(2); + } + }); + + it("rejects textZones with an invalid position value", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + textZones: [{ text: "Title", position: "sidebar" }], + }); + expect(result.success).toBe(false); + }); + + it("rejects layoutGrammar with an unknown kind", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + layoutGrammar: { kind: "floating_grid" }, + }); + expect(result.success).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// §6 — DashboardPlan DatasourceSpec with optional encoding/delimiter +// --------------------------------------------------------------------------- + +describe("DatasourceSpec — optional encoding and delimiter", () => { + it("parses a datasourceSpec with explicit encoding and delimiter", () => { + const result = DashboardPlanSchema.safeParse({ + ...MINIMAL_PLAN, + datasourceSpec: { + datasourceName: "Superstore", + filePath: "/data/superstore.csv", + fileType: "csv", + encoding: "utf-16", + delimiter: "\t", + }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.datasourceSpec?.encoding).toBe("utf-16"); + expect(result.data.datasourceSpec?.delimiter).toBe("\t"); + } + }); +}); + +// --------------------------------------------------------------------------- +// §7 — DashboardProposal schema and isDashboardProposal guard +// --------------------------------------------------------------------------- + +/** A complete valid proposal payload. */ +const VALID_PROPOSAL = { + schemaVersion: SCHEMA_VERSION, + kind: "proposal" as const, + workbookName: "Superstore Executive", + audience: "exec" as const, + datasourceName: "Superstore", + projectName: "Finance", + summary: "A KPI-strip dashboard showing Sales, Profit, and Orders for Q1 2024.", + dashboardTitle: "Executive Overview", + dashboardSubtitle: "Q1 2024 Performance", + kpiStrip: [ + { + label: "Sales", + primaryMeasure: "CP Sales", + comparisonMeasure: "PP Sales", + deltaMeasure: "Sales Difference", + direction: "up_good" as const, + }, + { + label: "Profit", + primaryMeasure: "CP Profit", + direction: "up_good" as const, + }, + ], + views: [ + { + title: "Sales by Category", + chartType: "bar", + encodingSummary: "Sales by Category, colored by Sub-Category", + fields: ["Category", "Sales"], + }, + { + title: "Profit by State", + chartType: "map_filled", + encodingSummary: "Profit colored by state", + fields: ["State", "Profit"], + }, + ], + layoutSummary: "KPI strip at top; category bar and filled map below.", + openQuestions: ["Should discount be included?"], + plan: MINIMAL_PLAN, +}; + +describe("DashboardProposalSchema", () => { + it("parses a complete proposal payload", () => { + const result = DashboardProposalSchema.safeParse(VALID_PROPOSAL); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.kind).toBe("proposal"); + expect(result.data.kpiStrip).toHaveLength(2); + expect(result.data.views).toHaveLength(2); + expect(result.data.openQuestions).toHaveLength(1); + expect(result.data.plan.kind).toBe("plan"); + } + }); + + it("parses a proposal without optional fields (openQuestions, dashboardSubtitle)", () => { + const { openQuestions: _openQuestions, dashboardSubtitle: _dashboardSubtitle, ...minimal } = VALID_PROPOSAL; + const result = DashboardProposalSchema.safeParse(minimal); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.openQuestions).toBeUndefined(); + expect(result.data.dashboardSubtitle).toBeUndefined(); + } + }); + + it("rejects a proposal missing required views array", () => { + const { views: _views, ...bad } = VALID_PROPOSAL; + const result = DashboardProposalSchema.safeParse(bad); + expect(result.success).toBe(false); + }); + + it('rejects a proposal with an invalid direction value in kpiStrip', () => { + const bad = { + ...VALID_PROPOSAL, + kpiStrip: [ + { label: "Sales", primaryMeasure: "Sales", direction: "always_good" }, + ], + }; + const result = DashboardProposalSchema.safeParse(bad); + expect(result.success).toBe(false); + }); + + it("rejects a proposal with an empty views array", () => { + const result = DashboardProposalSchema.safeParse({ + ...VALID_PROPOSAL, + views: [], + }); + expect(result.success).toBe(false); + }); +}); + +describe("isDashboardProposal type-guard", () => { + it("returns true for a valid proposal", () => { + expect(isDashboardProposal(VALID_PROPOSAL)).toBe(true); + }); + + it("returns false for a DashboardPlan (kind mismatch)", () => { + expect(isDashboardProposal(MINIMAL_PLAN)).toBe(false); + }); + + it("returns false for null", () => { + expect(isDashboardProposal(null)).toBe(false); + }); + + it("returns false for an object with wrong kind", () => { + expect(isDashboardProposal({ kind: "report", schemaVersion: 1 })).toBe(false); + }); + + it("returns false for a structurally invalid proposal", () => { + expect(isDashboardProposal({ kind: "proposal", schemaVersion: 1 })).toBe(false); + }); +}); + +describe("isDashboardPlan type-guard (regression)", () => { + it("still returns true for a valid minimal plan", () => { + expect(isDashboardPlan(MINIMAL_PLAN)).toBe(true); + }); + + it("returns false for a proposal (kind mismatch)", () => { + expect(isDashboardPlan(VALID_PROPOSAL)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// §8 — ClarifyingQuestions max raised to 10 +// --------------------------------------------------------------------------- + +describe("ClarifyingQuestions max raised to 10", () => { + const makeQuestion = (id: string) => ({ + id, + question: `Question ${id}?`, + }); + + it("accepts exactly 10 questions", () => { + const result = ClarifyingQuestionsSchema.safeParse({ + schemaVersion: SCHEMA_VERSION, + kind: "questions", + questions: Array.from({ length: 10 }, (_, i) => makeQuestion(`q${i + 1}`)), + }); + expect(result.success).toBe(true); + }); + + it("rejects 11 questions", () => { + const result = ClarifyingQuestionsSchema.safeParse({ + schemaVersion: SCHEMA_VERSION, + kind: "questions", + questions: Array.from({ length: 11 }, (_, i) => makeQuestion(`q${i + 1}`)), + }); + expect(result.success).toBe(false); + }); + + it("still accepts 7 questions (within old and new limits)", () => { + const result = ClarifyingQuestionsSchema.safeParse({ + schemaVersion: SCHEMA_VERSION, + kind: "questions", + questions: Array.from({ length: 7 }, (_, i) => makeQuestion(`q${i + 1}`)), + }); + expect(result.success).toBe(true); + }); +}); From daef18a38846c65eff06bf08e111b3f5ffcee478 Mon Sep 17 00:00:00 2001 From: Sebastien Henry Date: Fri, 26 Jun 2026 13:55:04 -0500 Subject: [PATCH 03/26] feat(builder): color encoding, scatter (Circle), multi-measure KPI tiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 3A of exec-dashboards. twb_builder._build_worksheet now emits, mirroring wb1, only when the new optional SheetSpec fields are present: - color encoding: (G-01) - scatter: with x-measure on cols, y-measure on rows, optional breakdown on color (G-02) - KPI tile (kind=kpi_tile): Automatic mark with primary+comparison+delta each on a encoding (value + delta) Each element XSD-validated (official gate) + structure-matched to wb1, across both the sqlproxy and embedded build paths, with plain-sheet regression guards (a plain bar/line/text sheet emits no — byte-unchanged). +48 tests. Gate: 128 TS + 197 Python = 325 tests. Co-Authored-By: Claude Opus 4.8 --- sidecar/tests/test_twb_encodings.py | 308 ++++++++++++++++++++++++++ sidecar/tests/test_twb_kpi_tile.py | 326 ++++++++++++++++++++++++++++ sidecar/tests/test_twb_scatter.py | 310 ++++++++++++++++++++++++++ sidecar/twb_builder.py | 173 ++++++++++++++- 4 files changed, 1105 insertions(+), 12 deletions(-) create mode 100644 sidecar/tests/test_twb_encodings.py create mode 100644 sidecar/tests/test_twb_kpi_tile.py create mode 100644 sidecar/tests/test_twb_scatter.py diff --git a/sidecar/tests/test_twb_encodings.py b/sidecar/tests/test_twb_encodings.py new file mode 100644 index 0000000..2c578b5 --- /dev/null +++ b/sidecar/tests/test_twb_encodings.py @@ -0,0 +1,308 @@ +"""Tests for color encoding (G-01) in _build_worksheet. + +Covers the three ``color.kind`` variants: +- ``measure_names`` → ``[:Measure Names]`` (wb1 ~3538-3540) +- ``dimension`` → ``_dim_instance(field)`` +- ``measure`` → ``_measure_instance(field)`` + +Each test asserts: +1. XSD validity via the vendored twb_2026.1.0.xsd gate. +2. ```` is present in the pane. +3. The exact column format matches the wb1 reference. + +Both build paths are covered: +- ``build_twb_xml`` (sqlproxy) +- ``build_embedded_twb_xml`` (federated) +""" + +from __future__ import annotations + +import xml.etree.ElementTree as ET +from pathlib import Path + +import pandas as pd +import pytest +from lxml import etree + +import hyper_builder +import twb_builder + +# --------------------------------------------------------------------------- +# Shared helpers +# --------------------------------------------------------------------------- + +SCHEMAS_DIR = Path(__file__).parent / "schemas" +XSD_PATH = SCHEMAS_DIR / "twb_2026.1.0.xsd" + + +def _safe_parser() -> etree.XMLParser: + return etree.XMLParser( + resolve_entities=False, + no_network=True, + load_dtd=False, + huge_tree=False, + ) + + +def _load_schema() -> etree.XMLSchema: + parser = _safe_parser() + xsd_doc = etree.parse(str(XSD_PATH), parser) + return etree.XMLSchema(xsd_doc) + + +def _assert_xsd_valid(xml_str: str, label: str) -> None: + schema = _load_schema() + parser = _safe_parser() + doc = etree.fromstring(xml_str.encode(), parser) + valid = schema.validate(doc) + errors = "\n".join(f" line {e.line}: {e.message}" for e in schema.error_log) + assert valid, f"XSD validation FAILED [{label}]:\n{errors}" + + +@pytest.fixture() +def hyper_file(tmp_path: Path) -> Path: + """Tiny .hyper with Region (string) + Revenue (real) + Category (string).""" + df = pd.DataFrame( + { + "Region": ["West", "East"], + "Category": ["Furniture", "Tech"], + "Revenue": [100.0, 200.0], + } + ) + out = tmp_path / "extract.hyper" + hyper_builder.dataframe_to_hyper(df, out) + return out + + +# --------------------------------------------------------------------------- +# Fixtures: sheet specs with color encoding +# --------------------------------------------------------------------------- + +SHEET_COLOR_MEASURE_NAMES = { + "title": "Revenue by Category", + "mark_type": "bar", + "cols": ["Category"], + "rows": [], + "measures": ["Revenue"], + "color": {"field": "Measure Names", "kind": "measure_names"}, +} + +SHEET_COLOR_DIMENSION = { + "title": "Revenue by Region", + "mark_type": "bar", + "cols": ["Region"], + "rows": [], + "measures": ["Revenue"], + "color": {"field": "Region", "kind": "dimension"}, +} + +SHEET_COLOR_MEASURE = { + "title": "Profit vs Revenue", + "mark_type": "bar", + "cols": ["Region"], + "rows": [], + "measures": ["Revenue"], + "color": {"field": "Revenue", "kind": "measure"}, +} + +SHEET_NO_COLOR = { + "title": "Plain Bar", + "mark_type": "bar", + "cols": ["Region"], + "rows": [], + "measures": ["Revenue"], +} + + +# --------------------------------------------------------------------------- +# Helper: find the encoding element in a parsed workbook +# --------------------------------------------------------------------------- + + +def _find_color_encoding(root: ET.Element) -> ET.Element | None: + return root.find(".//pane/encodings/color") + + +# --------------------------------------------------------------------------- +# A — color.kind == "measure_names" (wb1 ~3538-3540) +# --------------------------------------------------------------------------- + + +def test_color_measure_names_emits_encoding_sqlproxy() -> None: + """bar sheet with color.kind='measure_names' must emit .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_MEASURE_NAMES]) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None, " not found in pane" + col = color_el.get("column", "") + assert col.endswith(".[:Measure Names]"), ( + f"Expected column ending in '.[:Measure Names]', got {col!r} " + "(mirrors wb1 line ~3539: column='[Sample - Superstore].[:Measure Names]')" + ) + + +def test_color_measure_names_xsd_valid_sqlproxy() -> None: + """build_twb_xml with color.kind='measure_names' must be XSD-valid.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_MEASURE_NAMES]) + _assert_xsd_valid(xml, "color measure_names sqlproxy") + + +def test_color_measure_names_emits_encoding_embedded(hyper_file: Path) -> None: + """build_embedded_twb_xml with color.kind='measure_names' must emit encoding.""" + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_MEASURE_NAMES] + ) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None, " not found in embedded pane" + col = color_el.get("column", "") + assert col.endswith(".[:Measure Names]"), ( + f"Expected column ending '.[:Measure Names]', got {col!r}" + ) + + +def test_color_measure_names_xsd_valid_embedded(hyper_file: Path) -> None: + """build_embedded_twb_xml with color.kind='measure_names' must be XSD-valid.""" + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_MEASURE_NAMES] + ) + _assert_xsd_valid(xml, "color measure_names embedded") + + +# --------------------------------------------------------------------------- +# B — color.kind == "dimension" +# --------------------------------------------------------------------------- + + +def test_color_dimension_emits_encoding_sqlproxy() -> None: + """bar sheet with color.kind='dimension' must emit with none:Field:nk instance.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_DIMENSION]) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None, " not found in pane" + col = color_el.get("column", "") + assert "[none:Region:nk]" in col, ( + f"Dimension color encoding must use '[none:Region:nk]', got {col!r}" + ) + + +def test_color_dimension_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_DIMENSION]) + _assert_xsd_valid(xml, "color dimension sqlproxy") + + +def test_color_dimension_emits_encoding_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_DIMENSION] + ) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None + col = color_el.get("column", "") + assert "[none:Region:nk]" in col, ( + f"Dimension color encoding must use '[none:Region:nk]', got {col!r}" + ) + + +def test_color_dimension_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_DIMENSION] + ) + _assert_xsd_valid(xml, "color dimension embedded") + + +# --------------------------------------------------------------------------- +# C — color.kind == "measure" +# --------------------------------------------------------------------------- + + +def test_color_measure_emits_encoding_sqlproxy() -> None: + """bar sheet with color.kind='measure' must emit with sum:Field:qk instance.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_MEASURE]) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None, " not found in pane" + col = color_el.get("column", "") + assert "[sum:Revenue:qk]" in col, ( + f"Measure color encoding must use '[sum:Revenue:qk]', got {col!r}" + ) + + +def test_color_measure_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_MEASURE]) + _assert_xsd_valid(xml, "color measure sqlproxy") + + +def test_color_measure_emits_encoding_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_MEASURE] + ) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is not None + col = color_el.get("column", "") + assert "[sum:Revenue:qk]" in col + + +def test_color_measure_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "Sales DS", hyper_file.name, columns, [SHEET_COLOR_MEASURE] + ) + _assert_xsd_valid(xml, "color measure embedded") + + +# --------------------------------------------------------------------------- +# D — Regression: plain sheet without color is UNCHANGED +# --------------------------------------------------------------------------- + + +def test_no_color_spec_produces_no_color_encoding() -> None: + """A sheet without a 'color' key must NOT emit any encoding element.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_NO_COLOR]) + root = ET.fromstring(xml) + color_el = _find_color_encoding(root) + assert color_el is None, ( + "Plain bar sheet without color spec must not have in " + ) + + +def test_no_color_spec_xsd_valid() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_NO_COLOR]) + _assert_xsd_valid(xml, "no color regression") + + +# --------------------------------------------------------------------------- +# E — color field appears in datasource-dependencies +# --------------------------------------------------------------------------- + + +def test_color_dimension_field_in_dependencies() -> None: + """The color dimension field must appear in .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_DIMENSION]) + root = ET.fromstring(xml) + deps = root.find(".//datasource-dependencies") + assert deps is not None + col_names = {c.get("name") for c in deps.findall("column")} + assert "[Region]" in col_names, ( + f"Color dimension field [Region] must be declared in datasource-dependencies; " + f"got {col_names!r}" + ) + + +def test_color_measure_field_in_dependencies() -> None: + """The color measure field must appear in .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_COLOR_MEASURE]) + root = ET.fromstring(xml) + deps = root.find(".//datasource-dependencies") + assert deps is not None + col_names = {c.get("name") for c in deps.findall("column")} + assert "[Revenue]" in col_names, ( + f"Color measure field [Revenue] must be declared in datasource-dependencies; " + f"got {col_names!r}" + ) diff --git a/sidecar/tests/test_twb_kpi_tile.py b/sidecar/tests/test_twb_kpi_tile.py new file mode 100644 index 0000000..e232875 --- /dev/null +++ b/sidecar/tests/test_twb_kpi_tile.py @@ -0,0 +1,326 @@ +"""Tests for KPI tile (kind='kpi_tile') in _build_worksheet. + +The KPI tile mirrors the wb1 Sales KPI / Customer KPI / Order KPI worksheets: +- Mark class is 'Automatic' (wb1 ~4946, ~3392, ~4107) +- Multiple encodings inside (wb1 ~3394-3397) +- primaryMeasure is required; comparisonMeasure and deltaMeasure are optional +- All declared measures appear in +- Output is XSD-valid + +Regression: +- A plain text sheet (mark_type='text', no kind='kpi_tile') still uses a + single encoding and is structurally unchanged. +- All new fields being absent leaves a bar/line sheet byte-equivalent. + +Both build paths are covered: ``build_twb_xml`` and ``build_embedded_twb_xml``. +""" + +from __future__ import annotations + +import xml.etree.ElementTree as ET +from pathlib import Path + +import pandas as pd +import pytest +from lxml import etree + +import hyper_builder +import twb_builder + +# --------------------------------------------------------------------------- +# Shared helpers +# --------------------------------------------------------------------------- + +SCHEMAS_DIR = Path(__file__).parent / "schemas" +XSD_PATH = SCHEMAS_DIR / "twb_2026.1.0.xsd" + + +def _safe_parser() -> etree.XMLParser: + return etree.XMLParser( + resolve_entities=False, + no_network=True, + load_dtd=False, + huge_tree=False, + ) + + +def _load_schema() -> etree.XMLSchema: + parser = _safe_parser() + xsd_doc = etree.parse(str(XSD_PATH), parser) + return etree.XMLSchema(xsd_doc) + + +def _assert_xsd_valid(xml_str: str, label: str) -> None: + schema = _load_schema() + parser = _safe_parser() + doc = etree.fromstring(xml_str.encode(), parser) + valid = schema.validate(doc) + errors = "\n".join(f" line {e.line}: {e.message}" for e in schema.error_log) + assert valid, f"XSD validation FAILED [{label}]:\n{errors}" + + +@pytest.fixture() +def hyper_file(tmp_path: Path) -> Path: + """Tiny .hyper with three measure columns matching the KPI spec.""" + df = pd.DataFrame( + { + "Current Sales": [500_000.0], + "Previous Sales": [450_000.0], + "Sales Delta": [50_000.0], + } + ) + out = tmp_path / "extract.hyper" + hyper_builder.dataframe_to_hyper(df, out) + return out + + +# --------------------------------------------------------------------------- +# Sheet specs +# --------------------------------------------------------------------------- + +SHEET_KPI_PRIMARY_ONLY = { + "title": "Sales KPI", + "kind": "kpi_tile", + "mark_type": "text", + "cols": [], + "rows": [], + "measures": [], + "kpi": {"primaryMeasure": "Current Sales"}, +} + +SHEET_KPI_FULL = { + "title": "Sales KPI Full", + "kind": "kpi_tile", + "mark_type": "text", + "cols": [], + "rows": [], + "measures": [], + "kpi": { + "primaryMeasure": "Current Sales", + "comparisonMeasure": "Previous Sales", + "deltaMeasure": "Sales Delta", + }, +} + +SHEET_PLAIN_TEXT = { + "title": "Top Customers", + "mark_type": "text", + "cols": [], + "rows": [], + "measures": ["Revenue"], +} + +SHEET_PLAIN_BAR = { + "title": "Revenue by Region", + "mark_type": "bar", + "cols": ["Region"], + "rows": [], + "measures": ["Revenue"], +} + + +# --------------------------------------------------------------------------- +# A — Mark class is 'Automatic' (mirrors wb1 KPI sheets ~4946, ~3392, ~4107) +# --------------------------------------------------------------------------- + + +def test_kpi_tile_emits_automatic_mark_sqlproxy() -> None: + """kind='kpi_tile' must emit (mirrors wb1 ~4946).""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_PRIMARY_ONLY]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None, " not found in pane" + assert mark.get("class") == "Automatic", ( + f"KPI tile must use class='Automatic' (mirrors wb1 ~4946); " + f"got {mark.get('class')!r}" + ) + + +def test_kpi_tile_emits_automatic_mark_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_KPI_PRIMARY_ONLY] + ) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Automatic" + + +# --------------------------------------------------------------------------- +# B — Primary measure text encoding +# --------------------------------------------------------------------------- + + +def test_kpi_tile_primary_measure_in_text_encoding() -> None: + """primaryMeasure must appear as a inside .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_PRIMARY_ONLY]) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None, " must be present for kpi_tile" + text_cols = [el.get("column", "") for el in encodings.findall("text")] + assert any("[sum:Current Sales:qk]" in c for c in text_cols), ( + f"primaryMeasure 'Current Sales' must appear as [sum:Current Sales:qk] " + f"in a encoding; text cols: {text_cols!r}" + ) + + +def test_kpi_tile_primary_measure_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_KPI_PRIMARY_ONLY] + ) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None + text_cols = [el.get("column", "") for el in encodings.findall("text")] + assert any("[sum:Current Sales:qk]" in c for c in text_cols) + + +# --------------------------------------------------------------------------- +# C — Full KPI: three encodings (primary + comparison + delta) +# --------------------------------------------------------------------------- + + +def test_kpi_tile_full_has_three_text_encodings() -> None: + """Full KPI spec must emit three encoding elements (wb1 ~3394-3397).""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_FULL]) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None + text_els = encodings.findall("text") + assert len(text_els) == 3, ( + f"Full KPI tile must have 3 encodings " + f"(primary + comparison + delta, mirrors wb1 ~3394-3397); " + f"got {len(text_els)}" + ) + + +def test_kpi_tile_full_text_columns() -> None: + """All three KPI measures must appear as .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_FULL]) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None + text_cols = [el.get("column", "") for el in encodings.findall("text")] + assert any("[sum:Current Sales:qk]" in c for c in text_cols), ( + f"primaryMeasure must be in text encodings; got {text_cols!r}" + ) + assert any("[sum:Previous Sales:qk]" in c for c in text_cols), ( + f"comparisonMeasure must be in text encodings; got {text_cols!r}" + ) + assert any("[sum:Sales Delta:qk]" in c for c in text_cols), ( + f"deltaMeasure must be in text encodings; got {text_cols!r}" + ) + + +def test_kpi_tile_full_text_columns_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_KPI_FULL] + ) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None + text_cols = [el.get("column", "") for el in encodings.findall("text")] + assert any("[sum:Current Sales:qk]" in c for c in text_cols) + assert any("[sum:Previous Sales:qk]" in c for c in text_cols) + assert any("[sum:Sales Delta:qk]" in c for c in text_cols) + + +# --------------------------------------------------------------------------- +# D — All KPI measures in datasource-dependencies +# --------------------------------------------------------------------------- + + +def test_kpi_tile_full_measures_in_dependencies() -> None: + """All KPI measures must appear in .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_FULL]) + root = ET.fromstring(xml) + deps = root.find(".//datasource-dependencies") + assert deps is not None + col_names = {c.get("name") for c in deps.findall("column")} + for field in ("[Current Sales]", "[Previous Sales]", "[Sales Delta]"): + assert field in col_names, ( + f"KPI measure {field} must be declared in datasource-dependencies; " + f"got {col_names!r}" + ) + + +# --------------------------------------------------------------------------- +# E — XSD validity +# --------------------------------------------------------------------------- + + +def test_kpi_tile_primary_only_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_PRIMARY_ONLY]) + _assert_xsd_valid(xml, "kpi_tile primary-only sqlproxy") + + +def test_kpi_tile_full_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_FULL]) + _assert_xsd_valid(xml, "kpi_tile full sqlproxy") + + +def test_kpi_tile_primary_only_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_KPI_PRIMARY_ONLY] + ) + _assert_xsd_valid(xml, "kpi_tile primary-only embedded") + + +def test_kpi_tile_full_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_KPI_FULL] + ) + _assert_xsd_valid(xml, "kpi_tile full embedded") + + +# --------------------------------------------------------------------------- +# F — Regression: plain text mark path is UNCHANGED +# --------------------------------------------------------------------------- + + +def test_plain_text_mark_still_single_encoding() -> None: + """A plain 'text' mark sheet (no kind='kpi_tile') must keep its single encoding.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_PLAIN_TEXT]) + root = ET.fromstring(xml) + encodings = root.find(".//pane/encodings") + assert encodings is not None, "Plain text mark must still have " + text_els = encodings.findall("text") + assert len(text_els) == 1, ( + f"Plain text mark must have exactly 1 encoding; got {len(text_els)}" + ) + + +def test_plain_text_mark_xsd_valid() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_PLAIN_TEXT]) + _assert_xsd_valid(xml, "plain text mark regression") + + +def test_plain_bar_unaffected_by_kpi_fields() -> None: + """A plain bar sheet with no 'kind' or 'kpi' key must emit unchanged.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_PLAIN_BAR]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Bar", ( + f"Plain bar sheet must not be affected by kpi_tile path; got {mark.get('class')!r}" + ) + # No element on a plain bar + encodings = root.find(".//pane/encodings") + assert encodings is None, "Plain bar sheet must not have " + + +def test_kpi_tile_mark_type_automatic_not_text() -> None: + """KPI tile mark class must be 'Automatic', not 'Text', even though mark_type='text'.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_KPI_PRIMARY_ONLY]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Automatic", ( + f"KPI tile (kind='kpi_tile') must override mark_type='text' to 'Automatic'; " + f"got {mark.get('class')!r}" + ) diff --git a/sidecar/tests/test_twb_scatter.py b/sidecar/tests/test_twb_scatter.py new file mode 100644 index 0000000..66f28b7 --- /dev/null +++ b/sidecar/tests/test_twb_scatter.py @@ -0,0 +1,310 @@ +"""Tests for scatter (Circle mark / G-02) in _build_worksheet. + +Verifies: +- ``mark_type="scatter"`` emits ```` (wb1 ~3815) +- ``scatter.x`` measure appears on ```` +- ``scatter.y`` measure appears on ```` +- Both measures appear in ```` +- Optional ``scatter.breakdown`` dimension appears as a ```` encoding + and in ```` +- Output is XSD-valid via the vendored twb_2026.1.0.xsd gate +- Both build paths covered: ``build_twb_xml`` and ``build_embedded_twb_xml`` +- Regression: a plain bar sheet is byte-structurally unchanged +""" + +from __future__ import annotations + +import xml.etree.ElementTree as ET +from pathlib import Path + +import pandas as pd +import pytest +from lxml import etree + +import hyper_builder +import twb_builder + +# --------------------------------------------------------------------------- +# Shared helpers +# --------------------------------------------------------------------------- + +SCHEMAS_DIR = Path(__file__).parent / "schemas" +XSD_PATH = SCHEMAS_DIR / "twb_2026.1.0.xsd" + + +def _safe_parser() -> etree.XMLParser: + return etree.XMLParser( + resolve_entities=False, + no_network=True, + load_dtd=False, + huge_tree=False, + ) + + +def _load_schema() -> etree.XMLSchema: + parser = _safe_parser() + xsd_doc = etree.parse(str(XSD_PATH), parser) + return etree.XMLSchema(xsd_doc) + + +def _assert_xsd_valid(xml_str: str, label: str) -> None: + schema = _load_schema() + parser = _safe_parser() + doc = etree.fromstring(xml_str.encode(), parser) + valid = schema.validate(doc) + errors = "\n".join(f" line {e.line}: {e.message}" for e in schema.error_log) + assert valid, f"XSD validation FAILED [{label}]:\n{errors}" + + +@pytest.fixture() +def hyper_file(tmp_path: Path) -> Path: + """Tiny .hyper with Sales, Profit (real) and Category (string).""" + df = pd.DataFrame( + { + "Category": ["Furniture", "Tech"], + "Sales": [100.0, 200.0], + "Profit": [10.0, 40.0], + } + ) + out = tmp_path / "extract.hyper" + hyper_builder.dataframe_to_hyper(df, out) + return out + + +# --------------------------------------------------------------------------- +# Sheet specs +# --------------------------------------------------------------------------- + +SHEET_SCATTER = { + "title": "Sales vs Profit", + "mark_type": "scatter", + "cols": [], + "rows": [], + "measures": [], + "scatter": {"x": "Sales", "y": "Profit"}, +} + +SHEET_SCATTER_WITH_BREAKDOWN = { + "title": "Sales vs Profit by Category", + "mark_type": "scatter", + "cols": [], + "rows": [], + "measures": [], + "scatter": {"x": "Sales", "y": "Profit", "breakdown": "Category"}, +} + +SHEET_SCATTER_VIA_FIELD = { + # Scatter triggered via the 'scatter' dict, mark_type may be omitted or "bar". + "title": "Sales vs Profit (field trigger)", + "mark_type": "bar", + "cols": [], + "rows": [], + "measures": [], + "scatter": {"x": "Sales", "y": "Profit"}, +} + +SHEET_PLAIN_BAR = { + "title": "Plain Bar", + "mark_type": "bar", + "cols": ["Category"], + "rows": [], + "measures": ["Sales"], +} + + +# --------------------------------------------------------------------------- +# A — Circle mark class (wb1 ~3815) +# --------------------------------------------------------------------------- + + +def test_scatter_emits_circle_mark_sqlproxy() -> None: + """mark_type='scatter' must emit (mirrors wb1 line ~3815).""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None, " element not found in pane" + assert mark.get("class") == "Circle", ( + f"Expected class='Circle', got {mark.get('class')!r} " + "(mirrors wb1 line ~3815: )" + ) + + +def test_scatter_emits_circle_mark_embedded(hyper_file: Path) -> None: + """build_embedded_twb_xml: scatter must emit .""" + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_SCATTER] + ) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Circle", ( + f"Expected class='Circle', got {mark.get('class')!r}" + ) + + +def test_scatter_via_scatter_field_emits_circle() -> None: + """The scatter spec field alone (without mark_type='scatter') triggers Circle mark.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER_VIA_FIELD]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Circle", ( + f"'scatter' dict field must trigger Circle even when mark_type is 'bar'; " + f"got {mark.get('class')!r}" + ) + + +# --------------------------------------------------------------------------- +# B — x measure on , y measure on +# --------------------------------------------------------------------------- + + +def test_scatter_x_on_cols_sqlproxy() -> None: + """scatter.x measure must appear on .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + root = ET.fromstring(xml) + cols_el = root.find(".//table/cols") + assert cols_el is not None + cols_text = cols_el.text or "" + assert "[sum:Sales:qk]" in cols_text, ( + f"scatter.x='Sales' must be on as [sum:Sales:qk]; got {cols_text!r}" + ) + + +def test_scatter_y_on_rows_sqlproxy() -> None: + """scatter.y measure must appear on .""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + root = ET.fromstring(xml) + rows_el = root.find(".//table/rows") + assert rows_el is not None + rows_text = rows_el.text or "" + assert "[sum:Profit:qk]" in rows_text, ( + f"scatter.y='Profit' must be on as [sum:Profit:qk]; got {rows_text!r}" + ) + + +def test_scatter_x_on_cols_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_SCATTER] + ) + root = ET.fromstring(xml) + cols_el = root.find(".//table/cols") + assert cols_el is not None + assert "[sum:Sales:qk]" in (cols_el.text or "") + + +def test_scatter_y_on_rows_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_SCATTER] + ) + root = ET.fromstring(xml) + rows_el = root.find(".//table/rows") + assert rows_el is not None + assert "[sum:Profit:qk]" in (rows_el.text or "") + + +# --------------------------------------------------------------------------- +# C — Both measures in datasource-dependencies +# --------------------------------------------------------------------------- + + +def test_scatter_x_y_in_dependencies_sqlproxy() -> None: + """Both scatter.x and scatter.y measures must appear in datasource-dependencies.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + root = ET.fromstring(xml) + deps = root.find(".//datasource-dependencies") + assert deps is not None + col_names = {c.get("name") for c in deps.findall("column")} + assert "[Sales]" in col_names, f"[Sales] must be in deps; got {col_names!r}" + assert "[Profit]" in col_names, f"[Profit] must be in deps; got {col_names!r}" + + +# --------------------------------------------------------------------------- +# D — Optional breakdown dimension as color encoding +# --------------------------------------------------------------------------- + + +def test_scatter_breakdown_emits_color_encoding() -> None: + """scatter.breakdown must produce encoding with the dimension instance.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER_WITH_BREAKDOWN]) + root = ET.fromstring(xml) + color_el = root.find(".//pane/encodings/color") + assert color_el is not None, ( + "scatter.breakdown must produce in the pane" + ) + col = color_el.get("column", "") + assert "[none:Category:nk]" in col, ( + f"breakdown color encoding must use [none:Category:nk]; got {col!r}" + ) + + +def test_scatter_breakdown_in_dependencies() -> None: + """scatter.breakdown dimension must appear in datasource-dependencies.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER_WITH_BREAKDOWN]) + root = ET.fromstring(xml) + deps = root.find(".//datasource-dependencies") + assert deps is not None + col_names = {c.get("name") for c in deps.findall("column")} + assert "[Category]" in col_names, ( + f"breakdown dimension [Category] must be in deps; got {col_names!r}" + ) + + +def test_scatter_no_breakdown_no_color_encoding() -> None: + """A scatter without breakdown must not emit a encoding.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + root = ET.fromstring(xml) + color_el = root.find(".//pane/encodings/color") + assert color_el is None, ( + "Scatter without breakdown must not produce encoding" + ) + + +# --------------------------------------------------------------------------- +# E — XSD validity +# --------------------------------------------------------------------------- + + +def test_scatter_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER]) + _assert_xsd_valid(xml, "scatter sqlproxy") + + +def test_scatter_with_breakdown_xsd_valid_sqlproxy() -> None: + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_SCATTER_WITH_BREAKDOWN]) + _assert_xsd_valid(xml, "scatter breakdown sqlproxy") + + +def test_scatter_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_SCATTER] + ) + _assert_xsd_valid(xml, "scatter embedded") + + +def test_scatter_with_breakdown_xsd_valid_embedded(hyper_file: Path) -> None: + columns = hyper_builder.read_hyper_columns(hyper_file) + xml = twb_builder.build_embedded_twb_xml( + "DS", hyper_file.name, columns, [SHEET_SCATTER_WITH_BREAKDOWN] + ) + _assert_xsd_valid(xml, "scatter breakdown embedded") + + +# --------------------------------------------------------------------------- +# F — Regression: plain bar sheet unchanged +# --------------------------------------------------------------------------- + + +def test_plain_bar_not_circle() -> None: + """A plain bar sheet must still emit , not Circle.""" + xml = twb_builder.build_twb_xml("DS", "ds", "site", [SHEET_PLAIN_BAR]) + root = ET.fromstring(xml) + mark = root.find(".//pane/mark") + assert mark is not None + assert mark.get("class") == "Bar", ( + f"Plain bar sheet must not be affected by scatter path; got {mark.get('class')!r}" + ) diff --git a/sidecar/twb_builder.py b/sidecar/twb_builder.py index 1a4fe13..63854cf 100644 --- a/sidecar/twb_builder.py +++ b/sidecar/twb_builder.py @@ -57,7 +57,7 @@ "datetime": "135", } -_MARK_CLASS = {"bar": "Bar", "line": "Line", "text": "Text", "map": "Map"} +_MARK_CLASS = {"bar": "Bar", "line": "Line", "text": "Text", "map": "Map", "scatter": "Circle"} def _slug(value: str) -> str: @@ -134,6 +134,27 @@ def _ds_internal_name(content_key: str) -> str: return f"sqlproxy.{safe}" +def _color_column_instance(ds_internal: str, color: dict[str, Any]) -> str: + """Return the fully-qualified column reference for a color encoding. + + Mirrors wb1 ~3538-3540 and ~3776 for three ``kind`` variants: + + - ``"measure_names"`` → ``[:Measure Names]`` (Tableau virtual field) + - ``"dimension"`` → ``_dim_instance(field)`` + - ``"measure"`` → ``_measure_instance(field)`` + """ + field = str(color["field"]) + kind = str(color.get("kind", "dimension")) + ds_ref = f"[{ds_internal}]" + if kind == "measure_names": + instance = "[:Measure Names]" + elif kind == "measure": + instance = _measure_instance(field) + else: # "dimension" + instance = _dim_instance(field) + return f"{ds_ref}.{instance}" + + def _build_worksheet( sheet: dict[str, Any], ds_caption: str, @@ -161,6 +182,27 @@ def _build_worksheet( ← required by XSD + Rich encodings (Slice 3A) + ------------------------- + color + When ``sheet["color"]`` is present (``{field, kind}``), a + ```` element is emitted inside ```` + in the pane. Mirrors wb1 ~3538-3540. + + scatter + When ``mark_type == "scatter"`` (or ``sheet["scatter"]`` is present), + the mark class is ``Circle``; ``scatter.x`` is placed on ```` + and ``scatter.y`` on ```` via ``_measure_instance``. Optional + ``scatter.breakdown`` dimension is added as a color encoding. + Mirrors wb1 ~3815. + + kpi_tile + When ``sheet["kind"] == "kpi_tile"``, a ``Text`` mark (class + ``Automatic`` per wb1 KPI sheets) is emitted with one ```` + encoding per measure listed in ``kpi.primaryMeasure`` (required), + ``kpi.comparisonMeasure`` (optional), and ``kpi.deltaMeasure`` + (optional). Mirrors wb1 ~3388-3398. + Args: sheet: Sheet spec dict. ds_caption: Human-readable datasource caption. @@ -169,24 +211,74 @@ def _build_worksheet( """ title = str(sheet["title"]) mark_type = str(sheet.get("mark_type", "bar")).lower() + sheet_kind = str(sheet.get("kind", "chart")) cols_dims = [str(c) for c in sheet.get("cols", [])] rows_dims = [str(r) for r in sheet.get("rows", [])] measures = [str(m) for m in sheet.get("measures", [])] + # --- Scatter spec ------------------------------------------------------ + scatter = sheet.get("scatter") # optional {x, y, breakdown?} + is_scatter = mark_type == "scatter" or scatter is not None + + # --- Color spec -------------------------------------------------------- + color_spec = sheet.get("color") # optional {field, kind} + + # --- KPI tile spec ----------------------------------------------------- + kpi_spec = sheet.get("kpi") # optional {primaryMeasure, comparisonMeasure?, deltaMeasure?} + is_kpi_tile = sheet_kind == "kpi_tile" + worksheet = ET.Element("worksheet", {"name": title}) table = ET.SubElement(worksheet, "table") # --- ------------------------------------------------------- # Schema sequence: datasources → datasource-dependencies → aggregation view = ET.SubElement(table, "view") - datasources = ET.SubElement(view, "datasources") + datasources_el = ET.SubElement(view, "datasources") ET.SubElement( - datasources, + datasources_el, "datasource", {"caption": ds_caption, "name": ds_internal}, ) deps = ET.SubElement(view, "datasource-dependencies", {"datasource": ds_internal}) - _add_dependency_columns(deps, cols_dims + rows_dims, measures) + + # Collect dimension and measure fields for dependency declarations. + # Scatter: x/y measures on their respective shelves; breakdown (if any) as dimension. + dep_dims = list(cols_dims) + list(rows_dims) + dep_measures = list(measures) + + if is_scatter and scatter: + scatter_x = str(scatter["x"]) + scatter_y = str(scatter["y"]) + scatter_breakdown = scatter.get("breakdown") + if scatter_x not in dep_measures: + dep_measures.append(scatter_x) + if scatter_y not in dep_measures: + dep_measures.append(scatter_y) + if scatter_breakdown and str(scatter_breakdown) not in dep_dims: + dep_dims.append(str(scatter_breakdown)) + else: + scatter_x = "" + scatter_y = "" + scatter_breakdown = None + + # Color field must appear in dependency declarations so Tableau resolves it. + if color_spec: + color_field = str(color_spec["field"]) + color_kind = str(color_spec.get("kind", "dimension")) + if color_kind == "measure" and color_field not in dep_measures: + dep_measures.append(color_field) + elif color_kind == "dimension" and color_field not in dep_dims: + dep_dims.append(color_field) + # measure_names is a Tableau virtual field — no column declaration needed + + # KPI tile: all measures go into dependency declarations. + if is_kpi_tile and kpi_spec: + for kpi_field_key in ("primaryMeasure", "comparisonMeasure", "deltaMeasure"): + kpi_field = kpi_spec.get(kpi_field_key) + if kpi_field and str(kpi_field) not in dep_measures: + dep_measures.append(str(kpi_field)) + + _add_dependency_columns(deps, dep_dims, dep_measures) # is required by the XSD (last mandatory child of ) ET.SubElement(view, "aggregation", {"value": "true"}) @@ -198,18 +290,75 @@ def _build_worksheet( pane = ET.SubElement(panes, "pane") pane_view = ET.SubElement(pane, "view") ET.SubElement(pane_view, "breakdown", {"value": "auto"}) - ET.SubElement(pane, "mark", {"class": _MARK_CLASS.get(mark_type, "Automatic")}) - # For a text/table mark, place the first measure on the Text encoding so it renders. + # Determine mark class. + # KPI tiles use "Automatic" (mirrors wb1 Sales KPI / Customer KPI sheets + # at lines ~4946, ~3392 which have ). + # Scatter maps to "Circle" via _MARK_CLASS. + if is_kpi_tile: + mark_class = "Automatic" + elif is_scatter: + mark_class = "Circle" + else: + mark_class = _MARK_CLASS.get(mark_type, "Automatic") + ET.SubElement(pane, "mark", {"class": mark_class}) + + # --- Encodings ----------------------------------------------------- + # Collect all encoding elements to decide whether to emit . + # Order: color first, then text entries (mirrors wb1 Pie pane at ~3775-3780). ds_ref = f"[{ds_internal}]" - if mark_type == "text" and measures: - encodings = ET.SubElement(pane, "encodings") - ET.SubElement(encodings, "text", {"column": f"{ds_ref}.{_measure_instance(measures[0])}"}) + encoding_elements: list[tuple[str, str]] = [] # (tag, column_value) + + # Color encoding (G-01) + # Emitted for: explicit color_spec, or scatter breakdown as color. + effective_color_spec = color_spec + if is_scatter and scatter_breakdown and not effective_color_spec: + effective_color_spec = {"field": str(scatter_breakdown), "kind": "dimension"} + + if effective_color_spec: + col_val = _color_column_instance(ds_internal, effective_color_spec) + encoding_elements.append(("color", col_val)) + + # Text encoding(s) + if is_kpi_tile and kpi_spec: + # Multi-measure text: primary, comparison, delta (mirrors wb1 ~3394-3397) + for kpi_field_key in ("primaryMeasure", "comparisonMeasure", "deltaMeasure"): + kpi_field = kpi_spec.get(kpi_field_key) + if kpi_field: + encoding_elements.append( + ("text", f"{ds_ref}.{_measure_instance(str(kpi_field))}") + ) + elif mark_type == "text" and measures and not is_kpi_tile: + # Plain text mark: single-measure text encoding (original behaviour) + encoding_elements.append(("text", f"{ds_ref}.{_measure_instance(measures[0])}")) + + if encoding_elements: + encodings_el = ET.SubElement(pane, "encodings") + for tag, col_val in encoding_elements: + ET.SubElement(encodings_el, tag, {"column": col_val}) # --- / (after