diff --git a/pkg-py/CHANGELOG.md b/pkg-py/CHANGELOG.md index 59095ab16..914a27b9f 100644 --- a/pkg-py/CHANGELOG.md +++ b/pkg-py/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Breaking changes + +* `DataDict` now follows the current [data-dict spec](https://data-dict.tidyverse.org/spec.html): `tables` is a list of tables each with a `name` field (previously a mapping of name to definition), column `range` is a two-element `[min, max]` list, `values` may be a value-to-label map, and `constraints` may include assertion objects. Files written in the older mapping format, or with a `$version` that is invalid or predates the first spec version, fail with a descriptive error explaining how to update. Files declaring a `$version` newer than the spec version querychat understands are still read on a best-effort basis (unknown fields are ignored), with a warning. (#316) + ### New features * **Faster iterative visualization, and support for layer-level data sources.** Visualization queries now execute as a whole through `ggsql.CachingReader`: the underlying SQL runs against your data source once and the result is memoized for the chat session, so re-rendering a chart with a different encoding no longer re-queries the source (a win for remote databases especially). Visualization layers can also draw from other relations in the query — e.g. raw points from the final `SELECT` with a summary line from a CTE — via `DRAW ... MAPPING ... FROM `. Caching can be tuned or disabled with the `GGSQL_CACHE_TTL`, `GGSQL_CACHE_MAX_BYTES`, and `GGSQL_CACHE_DISABLED` environment variables. (#321) diff --git a/pkg-py/docs/context.qmd b/pkg-py/docs/context.qmd index 123335dc0..bd0603d34 100644 --- a/pkg-py/docs/context.qmd +++ b/pkg-py/docs/context.qmd @@ -30,13 +30,13 @@ qc.add_table(customers, "customers") ### Format -A data dictionary has three top-level sections: `tables`, `relationships`, and `glossary`. +The file format is defined by the [data-dict spec](https://data-dict.tidyverse.org/spec.html). A data dictionary has three top-level sections: `tables`, `relationships`, and `glossary`. The optional `$version` field declares the spec version the file conforms to. A `$version` that is invalid or predates the first spec version fails with a descriptive error; a file declaring a version newer than querychat understands is still read on a best-effort basis (unknown fields are ignored), with a warning. ```{.yaml filename="data-dict.yaml"} -version: "0.2.0" +$version: "0.1.0" tables: - orders: + - name: orders description: One row per customer order. columns: - name: order_id @@ -55,7 +55,7 @@ tables: values: [pending, shipped, delivered, cancelled] description: Current order status. - customers: + - name: customers description: One row per customer. columns: - name: id @@ -81,15 +81,16 @@ glossary: #### Tables -Each entry under `tables` describes one table. The key must match the table name you pass to `QueryChat` or `.add_table()`. +Each entry under `tables` describes one table. Its `name` must match the table name you pass to `QueryChat` or `.add_table()`. - **`description`**: What this table represents (one sentence is usually enough). - **`columns`**: A list of column annotations. Each column can have: - `name`: Column name (must match the actual column) - `type`: Semantic type hint — `string`, `number`, `number(id)`, `number(quantity)`, `date`, `enum` - - `constraints`: Optional list — `primary_key`, `foreign_key` + - `constraints`: Optional list — barewords like `primary_key`/`foreign_key`, or assertion objects (`assert` with an optional `language` and `description`) - `description`: What this column means in plain English - - `values`: For `enum` columns, the list of possible values + - `values`: For `enum` columns, the possible values — a list, or a value-to-label map (e.g. `{M: Male, F: Female}`) + - `range`: Optional `[min, max]` bounds. Either end may be left open with the `-.inf`/`.inf` sentinels (or `null`) — if you leave a bound open, describe the range in prose in the column's `description`. Replaces live min/max queries for the column Columns listed in the data dictionary are excluded from the auto-generated schema (since your description supersedes the auto-detected metadata). Columns not listed are still auto-detected as usual. diff --git a/pkg-py/examples/11-multi-table-nutrition/nutrition-data-dict.yaml b/pkg-py/examples/11-multi-table-nutrition/nutrition-data-dict.yaml index a8455e6fc..6cd8426e4 100644 --- a/pkg-py/examples/11-multi-table-nutrition/nutrition-data-dict.yaml +++ b/pkg-py/examples/11-multi-table-nutrition/nutrition-data-dict.yaml @@ -1,7 +1,7 @@ -version: "0.2.0" +$version: "0.1.0" tables: - foods: + - name: foods description: > One row per USDA Foundation Food item. Nutrient values are not stored here — see food_nutrients. All nutrient amounts in food_nutrients are per 100g of the food. @@ -21,7 +21,7 @@ tables: type: date description: Date the food record was published by USDA. - food_categories: + - name: food_categories description: Lookup table of USDA food categories. One row per category. columns: - name: id @@ -35,7 +35,7 @@ tables: type: string description: Human-readable category name (e.g., "Vegetables and Vegetable Products"). - nutrients: + - name: nutrients description: > Registry of the 12 key nutritional compounds tracked in this dataset. One row per nutrient type. See the glossary for the full list of nutrient IDs. @@ -54,7 +54,7 @@ tables: values: [G, KCAL, MG] description: Unit of measurement. G = grams, KCAL = kilocalories, MG = milligrams. - food_nutrients: + - name: food_nutrients description: > Nutrient content per 100g of food. One row per food–nutrient combination. Only the 12 key nutrients are included (see nutrients table and glossary). @@ -76,7 +76,7 @@ tables: nutrients.unit_name. May be null if the nutrient was not measured for this food. - food_portions: + - name: food_portions description: > Common serving sizes for foods. One row per portion definition. A single food may have multiple portions (e.g., "1 cup" and "1 oz"). @@ -107,7 +107,7 @@ tables: Optional preparation note for the portion (e.g., "chopped", "drained", "cooked"). Null when no modifier applies. - measure_units: + - name: measure_units description: > Lookup table of measurement unit names used in food_portions. Only units that appear in food_portions are included (~32 of the 123 total). diff --git a/pkg-py/src/querychat/_data_dict.py b/pkg-py/src/querychat/_data_dict.py index 0baa589e4..90c0bcc48 100644 --- a/pkg-py/src/querychat/_data_dict.py +++ b/pkg-py/src/querychat/_data_dict.py @@ -1,38 +1,70 @@ from __future__ import annotations +import math +import warnings from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Literal -from pydantic import BaseModel +from packaging.version import InvalidVersion, Version +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator if TYPE_CHECKING: from ._datasource import ColumnMeta from ._query_executor import QueryExecutor -class ColumnRange(BaseModel): - """Inclusive numeric range for a column, used instead of live min/max queries.""" +class ConstraintAssertion(BaseModel): + """ + A column constraint expressed as a boolean expression. + + In the data-dict spec, entries in a column's ``constraints`` list are either + barewords (``primary_key``, ``foreign_key``, ``required``, ``unique``) or + assertion objects with an ``assert`` expression. + + Parameters + ---------- + assert_ + The boolean expression (e.g. ``"amount >= 0"``). Populated from the + YAML key ``assert``. + language + The language ``assert`` is written in (``sql``, ``r``, or ``python``). + description + Human-readable explanation of the assertion. + + """ + + # populate_by_name allows construction with `assert_=` and round-trips + # through model_dump() (which emits the field name, not the YAML alias) + model_config = ConfigDict(populate_by_name=True) + + assert_: str = Field(alias="assert") + language: Literal["sql", "r", "python"] | None = None + description: str | None = None - min: Any = None - max: Any = None + def __str__(self) -> str: + if self.description: + return f"{self.assert_} ({self.description})" + return self.assert_ class ColumnSpec(BaseModel): """ Per-column metadata entry in a :class:`DataDict`. - All fields are optional. Only ``name`` is required, and is used to match - this spec against columns returned by the data source. + Follows the column format of the `data-dict spec + `_. Only ``name`` is required; + it is used to match this spec against columns returned by the data source. Parameters ---------- name Column name as it appears in the data source. type - Human-readable type override (e.g. ``"date"``, ``"currency"``). When + Type override (e.g. ``"date"``, ``"number(quantity)"``). When supplied, this replaces the inferred SQL type in the LLM schema view. constraints - Free-text constraints conveyed to the LLM (e.g. ``"non-negative"``). + Bareword constraints (``primary_key``, ``required``, ...) and/or + :class:`ConstraintAssertion` objects, conveyed to the LLM. description Short description forwarded verbatim to the LLM's schema view. details @@ -41,12 +73,14 @@ class ColumnSpec(BaseModel): units Unit label (e.g. ``"kg"``, ``"USD"``), included in the schema view. values - Exhaustive list of valid values. Replaces categorical inference for - this column — querychat will not query the data source for distinct - values when this is set. + Valid values for the column, either a list (``[M, F, U]``) or a + value-to-label map (``{M: Male, F: Female}``). Replaces categorical + inference for this column — querychat will not query the data source + for distinct values when this is set. For the map form, the values + (keys) are used. range - Inclusive min/max bounds. Replaces live min/max statistics queries - when set. + Inclusive ``[min, max]`` bounds; either end may be ``None`` for an + open range. Replaces live min/max statistics queries when set. examples Representative sample values shown to the LLM as context. @@ -54,14 +88,42 @@ class ColumnSpec(BaseModel): name: str type: str | None = None - constraints: list[str] = [] + constraints: list[str | ConstraintAssertion] = [] description: str | None = None details: str | None = None units: str | None = None - values: list[Any] | None = None - range: ColumnRange | None = None + values: list[Any] | dict[str, str] | None = None + range: list[Any] | None = None examples: list[Any] | None = None + @field_validator("values", mode="before") + @classmethod + def _values_map_keys_are_strings(cls, v: Any) -> Any: + # YAML parses numeric map keys (e.g. {0: Control, 1: Treatment}) as + # ints; the spec allows them, so coerce keys to strings. + if isinstance(v, dict): + return {str(k): val for k, val in v.items()} + return v + + @field_validator("range", mode="before") + @classmethod + def _range_open_bound_sentinels(cls, v: Any) -> Any: + # The spec's open-bound sentinels (-.inf/.inf) parse as float + # infinities; normalize them to None for an open end. + if isinstance(v, list): + return [None if isinstance(x, float) and math.isinf(x) else x for x in v] + return v + + @field_validator("range") + @classmethod + def _range_is_two_elements(cls, v: list[Any] | None) -> list[Any] | None: + if v is not None and len(v) != 2: # noqa: PLR2004 + raise ValueError( + "range must be a two-element list [min, max]; " + "use null (or the spec's -.inf/.inf sentinels) for an open end" + ) + return v + class TableSpec(BaseModel): """ @@ -69,6 +131,8 @@ class TableSpec(BaseModel): Parameters ---------- + name + Table name. Must match a table registered with ``QueryChat``. description Short description of the table, forwarded to the LLM's schema view. details @@ -80,6 +144,7 @@ class TableSpec(BaseModel): """ + name: str description: str | None = None details: str | None = None columns: list[ColumnSpec] = [] @@ -105,13 +170,80 @@ class RelationshipSpec(BaseModel): join: str +_LEGACY_TABLES_MESSAGE = """\ +This data dictionary uses an outdated format: `tables` must be a list of +tables, each with a `name` field — not a mapping of name to definition. + + # before + tables: + orders: + columns: [...] + + # after + tables: + - name: orders + columns: [...] + +Also changed in the current spec: column `range` is a two-element list +[min, max], and `values` may be a value-to-label map ({M: Male, F: Female}). + +Run `data-dict validate-spec ` to check the file against the spec: +https://data-dict.tidyverse.org/spec.html +""" + +# The first data-dict spec version ever released. A `$version` below this +# is not a real spec version. +_FIRST_SPEC_VERSION = Version("0.1.0") + +# The newest data-dict spec version this version of querychat fully +# understands. Files declaring a newer `$version` are still read, on a +# best-effort basis (unknown fields are ignored), with a warning. Bump this +# opportunistically to keep the warning calibrated — it is not a hard gate. +_KNOWN_SPEC_VERSION = Version("0.1.0") + + +def _check_spec_version(version: Any) -> None: + if version is None: + # `$version:` with no value means the same as omitting it + return + try: + parsed = Version(str(version)) + except InvalidVersion: + parsed = None + # Old or unreadable files fail hard: we can't establish they're readable, + # and there is a clear migration path (unlike newer files, which may only + # use additive features and are read on a best-effort basis below) + if parsed is None or parsed < _FIRST_SPEC_VERSION: + raise ValueError( + f"This data dictionary declares `$version: {version!r}`, which is " + "not a data-dict spec version this version of querychat can read " + f"(the first spec version is {_FIRST_SPEC_VERSION}). Update the " + "file to the current spec — `data-dict validate-spec ` " + "checks a file against the spec: " + "https://data-dict.tidyverse.org/spec.html" + ) + if parsed > _KNOWN_SPEC_VERSION: + warnings.warn( + f'This data dictionary declares `$version: "{version}"`, which is ' + "newer than the newest data-dict spec version this version of " + f"querychat fully understands ({_KNOWN_SPEC_VERSION}). The file is " + "read on a best-effort basis — spec fields querychat doesn't know " + "about are ignored. If anything looks off, upgrade querychat, or " + "check the file against the spec with " + "`data-dict validate-spec `: " + "https://data-dict.tidyverse.org/spec.html", + stacklevel=3, + ) + + class DataDict(BaseModel): """ A data dictionary providing rich per-table and per-column metadata. Pass a ``DataDict`` to ``QueryChat`` (or load one from YAML via :meth:`from_yaml`) to give the LLM better context about your data without - querying the data source for statistics at startup. + querying the data source for statistics at startup. Files must follow the + current `data-dict spec `_. For columns listed in a ``DataDict``: @@ -132,9 +264,8 @@ class DataDict(BaseModel): One-line summary of the domain, shown alongside ``name`` in the system prompt. tables - Per-table metadata, keyed by table name. Each value is a - :class:`TableSpec` with optional description and column specs. - Table names must match those registered with ``QueryChat``. + Per-table metadata. Each :class:`TableSpec` has a required ``name`` + that must match a table registered with ``QueryChat``. relationships Cross-table relationship declarations. Useful context for multi-table apps where the LLM needs to know how tables join. @@ -149,23 +280,42 @@ class DataDict(BaseModel): ```python from querychat import QueryChat, DataDict - qc = QueryChat(df, "sales", data_dict=DataDict.from_yaml("data_dict.yaml")) + qc = QueryChat(df, "sales", data_dict=DataDict.from_yaml("data-dict.yaml")) ``` Or pass a path directly and let QueryChat load it: ```python - qc = QueryChat(df, "sales", data_dict="data_dict.yaml") + qc = QueryChat(df, "sales", data_dict="data-dict.yaml") ``` """ name: str | None = None description: str | None = None - tables: dict[str, TableSpec] = {} + tables: list[TableSpec] = [] relationships: list[RelationshipSpec] = [] glossary: dict[str, str] = {} + @model_validator(mode="before") + @classmethod + def _reject_legacy_tables(cls, data: Any) -> Any: + if isinstance(data, dict) and isinstance(data.get("tables"), dict): + # ValueError (not TypeError) so pydantic wraps it in ValidationError + raise ValueError(_LEGACY_TABLES_MESSAGE) # noqa: TRY004 + return data + + @model_validator(mode="before") + @classmethod + def _reject_unsupported_spec_version(cls, data: Any) -> Any: + if isinstance(data, dict) and "$version" in data: + _check_spec_version(data["$version"]) + return data + + def get_table(self, table_name: str) -> TableSpec | None: + """Return the :class:`TableSpec` for ``table_name``, or ``None``.""" + return next((t for t in self.tables if t.name == table_name), None) + def to_prompt_dict(self) -> dict[str, Any]: """Return a filtered dict for the system prompt (excludes per-column details).""" result: dict[str, Any] = {} @@ -175,8 +325,8 @@ def to_prompt_dict(self) -> dict[str, Any]: result["description"] = self.description if self.tables: result["tables"] = { - name: ({"description": spec.description} if spec.description else None) - for name, spec in self.tables.items() + t.name: ({"description": t.description} if t.description else None) + for t in self.tables } if self.relationships: result["relationships"] = [ @@ -197,7 +347,7 @@ def get_table_schema( metas: list[ColumnMeta] = executor.get_column_metas(table_name) # Build lookup from data_dict for this table - table_spec = self.tables.get(table_name) + table_spec = self.get_table(table_name) documented: dict[str, ColumnSpec] = ( {col.name: col for col in table_spec.columns} if table_spec else {} ) @@ -209,16 +359,21 @@ def get_table_schema( if spec.type is not None: meta.sql_type = spec.type if spec.range is not None: - meta.min_val = spec.range.min - meta.max_val = spec.range.max + meta.min_val = spec.range[0] + meta.max_val = spec.range[1] if spec.values is not None: - meta.categories = [str(v) for v in spec.values] + vals = ( + list(spec.values) + if isinstance(spec.values, dict) + else spec.values + ) + meta.categories = [str(v) for v in vals] if spec.description is not None: meta.description = spec.description if spec.units is not None: meta.units = spec.units if spec.constraints: - meta.constraints = list(spec.constraints) + meta.constraints = [str(c) for c in spec.constraints] else: undocumented.append(meta) diff --git a/pkg-py/src/querychat/_datasource.py b/pkg-py/src/querychat/_datasource.py index 5dee42c6a..05fb716dc 100644 --- a/pkg-py/src/querychat/_datasource.py +++ b/pkg-py/src/querychat/_datasource.py @@ -75,12 +75,13 @@ def format_schema(table_name: str, columns: list[ColumnMeta]) -> str: lines.append(f" Description: {col.description}") if col.constraints: lines.append(f" Constraints: {', '.join(col.constraints)}") - if ( - col.kind in ("numeric", "date") - and col.min_val is not None - and col.max_val is not None + if col.kind in ("numeric", "date") and ( + col.min_val is not None or col.max_val is not None ): - lines.append(f" Range: {col.min_val} to {col.max_val}") + # Open-ended bounds (allowed by the data-dict spec) render as '?' + lo = col.min_val if col.min_val is not None else "?" + hi = col.max_val if col.max_val is not None else "?" + lines.append(f" Range: {lo} to {hi}") elif col.categories: cats = ", ".join(f"'{v}'" for v in col.categories) lines.append(f" Categorical values: {cats}") diff --git a/pkg-py/src/querychat/_system_prompt.py b/pkg-py/src/querychat/_system_prompt.py index b53626827..1e9f1b607 100644 --- a/pkg-py/src/querychat/_system_prompt.py +++ b/pkg-py/src/querychat/_system_prompt.py @@ -50,7 +50,7 @@ def __init__( self._data_dicts = [ dd for dd in self._data_dicts - if any(n in resolved for n in dd.tables) + if any(t.name in resolved for t in dd.tables) or (not dd.tables and dd.description) ] update: dict[str, object] = {} @@ -119,7 +119,7 @@ def escape_attr(val: str) -> str: d.pop("name", None) d.pop("description", None) - claimed = {n for n in self._data_sources if n in dd.tables} + claimed = {t.name for t in dd.tables if t.name in self._data_sources} all_claimed.update(claimed) if "tables" in d: d["tables"] = { diff --git a/pkg-py/src/querychat/tools.py b/pkg-py/src/querychat/tools.py index a371e5e8b..21de714ad 100644 --- a/pkg-py/src/querychat/tools.py +++ b/pkg-py/src/querychat/tools.py @@ -61,7 +61,7 @@ def get_schema(table_name: str) -> ContentToolResult: error = f"Table '{table_name}' not found. Available: {available}" return ContentToolResult(value=error, error=Exception(error)) - dd = next((d for d in data_dicts if table_name in d.tables), None) + dd = next((d for d in data_dicts if d.get_table(table_name) is not None), None) if dd is not None: columns = dd.get_table_schema(table_name, executor, categorical_threshold) else: @@ -139,12 +139,13 @@ def _schema_table(columns: list[ColumnMeta]) -> Tag: ) range_values = "" - if ( - column.kind in ("numeric", "date") - and column.min_val is not None - and column.max_val is not None + if column.kind in ("numeric", "date") and ( + column.min_val is not None or column.max_val is not None ): - range_values = f"{column.min_val} to {column.max_val}" + # Open-ended bounds (allowed by the data-dict spec) render as '?' + lo = column.min_val if column.min_val is not None else "?" + hi = column.max_val if column.max_val is not None else "?" + range_values = f"{lo} to {hi}" elif column.categories: range_values = ", ".join(f"'{value}'" for value in column.categories) diff --git a/pkg-py/tests/test_base.py b/pkg-py/tests/test_base.py index 11925a6f0..96d7f4a3c 100644 --- a/pkg-py/tests/test_base.py +++ b/pkg-py/tests/test_base.py @@ -282,27 +282,27 @@ def test_cleanup(self, sample_df): class TestDataDict: def test_data_dict_path_accepted(self, tmp_path: Path) -> None: f = tmp_path / "spec.yaml" - f.write_text('version: "0.1.0"\ntables:\n t:\n columns: []\n') + f.write_text('$version: "0.1.0"\ntables:\n - name: t\n columns: []\n') df = pl.DataFrame({"x": [1]}) qc = QueryChat(df, table_name="t", data_dict=str(f)) assert len(qc._data_dicts) == 1 - assert "t" in qc._data_dicts[0].tables + assert qc._data_dicts[0].get_table("t") is not None def test_data_dict_instance_accepted(self) -> None: - dd = DataDict(tables={"t": TableSpec(columns=[])}) + dd = DataDict(tables=[TableSpec(name="t")]) df = pl.DataFrame({"x": [1]}) qc = QueryChat(df, table_name="t", data_dict=dd) assert qc._data_dicts[0] is dd def test_data_dict_table_not_in_dict_is_allowed(self) -> None: - dd = DataDict(tables={"other": TableSpec(columns=[])}) + dd = DataDict(tables=[TableSpec(name="other")]) df = pl.DataFrame({"x": [1]}) qc = QueryChat(df, table_name="t", data_dict=dd) assert "t" in qc._data_sources def test_add_table_not_in_data_dict_is_allowed(self) -> None: dd = DataDict( - tables={"t1": TableSpec(columns=[])}, + tables=[TableSpec(name="t1")], ) df = pl.DataFrame({"x": [1]}) qc = QueryChat(df, table_name="t1", data_dict=dd) @@ -575,9 +575,7 @@ def test_remove_after_sessions_started_raises(self, sample_df): qc.remove_table("other") assert qc.table_names() == ["users", "other"] - def test_add_tables_replace_after_sessions_started_raises( - self, multi_table_engine - ): + def test_add_tables_replace_after_sessions_started_raises(self, multi_table_engine): qc = QueryChatBase() qc.add_tables(multi_table_engine) qc._sessions_started = True diff --git a/pkg-py/tests/test_data_dict.py b/pkg-py/tests/test_data_dict.py index 0cf62f007..e8a827bef 100644 --- a/pkg-py/tests/test_data_dict.py +++ b/pkg-py/tests/test_data_dict.py @@ -1,17 +1,20 @@ +import warnings from pathlib import Path import narwhals.stable.v1 as nw import polars as pl -from querychat._data_dict import ColumnRange, ColumnSpec, DataDict, TableSpec +import pytest +from pydantic import ValidationError +from querychat._data_dict import ColumnSpec, DataDict, TableSpec from querychat._datasource import DataFrameSource from querychat._query_executor import DataSourceExecutor def test_from_yaml_full_spec(tmp_path: Path) -> None: yaml_content = """\ -version: "0.1.0" +$version: "0.1.0" tables: - orders: + - name: orders description: One row per order. columns: - name: order_id @@ -21,12 +24,19 @@ def test_from_yaml_full_spec(tmp_path: Path) -> None: examples: [1, 2, 3] - name: amount type: number(quantity) - range: - min: 0 - max: 10000 + range: [0, 10000] - name: status type: enum values: [pending, shipped, delivered] + - name: channel + type: enum + values: {W: Web, R: Retail} + - name: discount + type: number(quantity) + constraints: + - assert: discount >= 0 + language: sql + description: Non-negative discount relationships: - description: Order placed by customer. cardinality: many-to-one @@ -38,23 +48,25 @@ def test_from_yaml_full_spec(tmp_path: Path) -> None: f.write_text(yaml_content) dd = DataDict.from_yaml(f) - assert "orders" in dd.tables - t = dd.tables["orders"] + assert [t.name for t in dd.tables] == ["orders"] + t = dd.tables[0] assert t.description == "One row per order." - assert len(t.columns) == 3 + assert len(t.columns) == 5 col = t.columns[0] assert col.name == "order_id" assert col.description == "Unique order identifier." assert col.constraints == ["primary_key"] - col = t.columns[1] - assert col.range is not None - assert col.range.min == 0 - assert col.range.max == 10000 + assert t.columns[1].range == [0, 10000] + assert t.columns[2].values == ["pending", "shipped", "delivered"] + assert t.columns[3].values == {"W": "Web", "R": "Retail"} - col = t.columns[2] - assert col.values == ["pending", "shipped", "delivered"] + constraint = t.columns[4].constraints[0] + assert not isinstance(constraint, str) + assert constraint.assert_ == "discount >= 0" + assert constraint.language == "sql" + assert constraint.description == "Non-negative discount" assert len(dd.relationships) == 1 assert dd.relationships[0].join == "orders.customer_id = customers.id" @@ -62,20 +74,81 @@ def test_from_yaml_full_spec(tmp_path: Path) -> None: assert dd.glossary["churn"] == "Customer with no orders in 90+ days." +def test_from_yaml_legacy_dict_tables_error(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text("tables:\n orders:\n columns: []\n") + with pytest.raises(ValidationError, match="outdated format"): + DataDict.from_yaml(f) + + +def test_newer_spec_version_warns_but_is_read(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text('$version: "0.2.0"\ntables: []\n') + with pytest.warns(UserWarning, match="best-effort basis"): + dd = DataDict.from_yaml(f) + assert dd.tables == [] + + +def test_spec_version_predating_first_spec_errors() -> None: + with pytest.raises(ValidationError, match="not a data-dict spec version"): + DataDict.model_validate({"$version": "0.0.9"}) + + +def test_invalid_spec_version_errors(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text('$version: "not-a-version"\n') + with pytest.raises(ValidationError, match="not a data-dict spec version"): + DataDict.from_yaml(f) + + +def test_null_spec_version_means_absent(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text("$version:\ntables: []\n") + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert DataDict.from_yaml(f).tables == [] + + +def test_known_spec_version_ok(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text('$version: "0.1.0"\ntables: []\n') + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert DataDict.from_yaml(f).tables == [] + + # Short forms are zero-padded ("0.1" == "0.1.0") + f.write_text('$version: "0.1"\ntables: []\n') + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert DataDict.from_yaml(f).tables == [] + + +def test_table_requires_name(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text("tables:\n - description: No name here\n columns: []\n") + with pytest.raises(ValidationError): + DataDict.from_yaml(f) + + +def test_range_must_have_two_elements() -> None: + with pytest.raises(ValidationError, match="two-element"): + ColumnSpec(name="x", range=[0, 1, 2]) + + def test_from_yaml_partial_spec(tmp_path: Path) -> None: f = tmp_path / "spec.yaml" - f.write_text('version: "0.1.0"\ntables:\n orders:\n columns: []\n') + f.write_text("tables:\n - name: orders\n columns: []\n") dd = DataDict.from_yaml(f) - assert dd.tables["orders"].description is None + assert dd.tables[0].description is None assert dd.relationships == [] assert dd.glossary == {} def test_from_yaml_str_path(tmp_path: Path) -> None: f = tmp_path / "spec.yaml" - f.write_text('version: "0.1.0"\n') + f.write_text('$version: "0.1.0"\n') dd = DataDict.from_yaml(str(f)) - assert dd.tables == {} + assert dd.tables == [] def test_name_and_description_are_optional() -> None: @@ -86,7 +159,7 @@ def test_name_and_description_are_optional() -> None: def test_from_yaml_derives_name_from_file_stem(tmp_path: Path) -> None: f = tmp_path / "my_schema.yaml" - f.write_text('version: "0.1.0"\n') + f.write_text('$version: "0.1.0"\n') dd = DataDict.from_yaml(f) assert dd.name == "my_schema" @@ -107,7 +180,7 @@ def test_from_yaml_loads_description(tmp_path: Path) -> None: def test_from_yaml_description_defaults_to_none(tmp_path: Path) -> None: f = tmp_path / "spec.yaml" - f.write_text('version: "0.1.0"\n') + f.write_text('$version: "0.1.0"\n') dd = DataDict.from_yaml(f) assert dd.description is None @@ -119,14 +192,15 @@ def _make_executor(df: pl.DataFrame, table_name: str) -> DataSourceExecutor: def test_get_table_schema_all_documented() -> None: dd = DataDict( - tables={ - "orders": TableSpec( + tables=[ + TableSpec( + name="orders", description="Order records.", columns=[ ColumnSpec( name="amount", description="Order total in USD.", - range=ColumnRange(min=0, max=500), + range=[0, 500], ), ColumnSpec( name="status", @@ -134,7 +208,7 @@ def test_get_table_schema_all_documented() -> None: ), ], ) - }, + ], ) df = pl.DataFrame({"amount": [10, 20], "status": ["pending", "shipped"]}) executor = _make_executor(df, "orders") @@ -149,7 +223,7 @@ def test_get_table_schema_all_documented() -> None: def test_get_table_schema_no_documentation() -> None: - dd = DataDict(tables={"orders": TableSpec(columns=[])}) + dd = DataDict(tables=[TableSpec(name="orders", columns=[])]) df = pl.DataFrame({"amount": [10, 20, 30], "status": ["a", "b", "a"]}) executor = _make_executor(df, "orders") cols = dd.get_table_schema("orders", executor, categorical_threshold=10) @@ -161,13 +235,14 @@ def test_get_table_schema_no_documentation() -> None: def test_get_table_schema_mixed_coverage() -> None: dd = DataDict( - tables={ - "orders": TableSpec( + tables=[ + TableSpec( + name="orders", columns=[ - ColumnSpec(name="amount", range=ColumnRange(min=0, max=999)), - ] + ColumnSpec(name="amount", range=[0, 999]), + ], ) - }, + ], ) df = pl.DataFrame({"amount": [10, 20], "status": ["a", "b"]}) executor = _make_executor(df, "orders") @@ -178,28 +253,83 @@ def test_get_table_schema_mixed_coverage() -> None: assert "status" in col_map # from SQL fallback +def test_get_table_schema_open_ended_range() -> None: + dd = DataDict( + tables=[ + TableSpec( + name="orders", + columns=[ColumnSpec(name="amount", range=[0, None])], + ) + ], + ) + df = pl.DataFrame({"amount": [10, 20]}) + executor = _make_executor(df, "orders") + cols = dd.get_table_schema("orders", executor, categorical_threshold=10) + assert cols[0].min_val == 0 + assert cols[0].max_val is None + + +def test_get_table_schema_values_map_uses_keys() -> None: + dd = DataDict( + tables=[ + TableSpec( + name="orders", + columns=[ColumnSpec(name="status", values={"W": "Web", "R": "Retail"})], + ) + ], + ) + df = pl.DataFrame({"status": ["W", "R"]}) + executor = _make_executor(df, "orders") + cols = dd.get_table_schema("orders", executor, categorical_threshold=10) + assert cols[0].categories == ["W", "R"] + + +def test_get_table_schema_assertion_constraints() -> None: + dd = DataDict( + tables=[ + TableSpec( + name="orders", + columns=[ + ColumnSpec( + name="amount", + constraints=[ + {"assert": "amount >= 0", "description": "Non-negative"}, + "required", + ], + ) + ], + ) + ], + ) + df = pl.DataFrame({"amount": [1, 2]}) + executor = _make_executor(df, "orders") + cols = dd.get_table_schema("orders", executor, categorical_threshold=10) + assert cols[0].constraints == ["amount >= 0 (Non-negative)", "required"] + + def test_to_prompt_dict_excludes_column_specs() -> None: dd = DataDict( name="sales", - tables={ - "orders": TableSpec( + tables=[ + TableSpec( + name="orders", description="Order records.", - columns=[ColumnSpec(name="amount", range=ColumnRange(min=0, max=100))], + columns=[ColumnSpec(name="amount", range=[0, 100])], ) - }, + ], ) d = dd.to_prompt_dict() assert "columns" not in (d.get("tables", {}).get("orders") or {}) def test_to_prompt_dict_includes_table_description() -> None: - dd = DataDict(tables={"orders": TableSpec(description="Order records.")}) + dd = DataDict(tables=[TableSpec(name="orders", description="Order records.")]) d = dd.to_prompt_dict() assert d["tables"]["orders"]["description"] == "Order records." def test_to_prompt_dict_table_with_no_description_is_null() -> None: - dd = DataDict(tables={"orders": TableSpec()}) + dd = DataDict(tables=[TableSpec(name="orders")]) d = dd.to_prompt_dict() assert d["tables"]["orders"] is None @@ -231,3 +361,45 @@ def test_to_prompt_dict_includes_glossary() -> None: dd = DataDict(glossary={"ARR": "Annual Recurring Revenue"}) d = dd.to_prompt_dict() assert d["glossary"]["ARR"] == "Annual Recurring Revenue" + + +def test_constraint_assertion_constructible_by_name_and_round_trips() -> None: + from querychat._data_dict import ConstraintAssertion + + ca = ConstraintAssertion(assert_="amount >= 0", description="Non-negative") + assert ca.assert_ == "amount >= 0" + dd = DataDict( + tables=[ + TableSpec( + name="orders", + columns=[ColumnSpec(name="amount", constraints=[ca])], + ) + ], + ) + dd2 = DataDict.model_validate(dd.model_dump()) + constraint = dd2.tables[0].columns[0].constraints[0] + assert not isinstance(constraint, str) + assert constraint.assert_ == "amount >= 0" + assert constraint.description == "Non-negative" + + +def test_values_map_accepts_numeric_keys(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text( + "tables:\n - name: orders\n columns:\n" + " - name: arm\n values: {0: Control, 1: Treatment}\n" + ) + dd = DataDict.from_yaml(f) + assert dd.tables[0].columns[0].values == {"0": "Control", "1": "Treatment"} + + +def test_range_infinity_sentinels_mean_open(tmp_path: Path) -> None: + f = tmp_path / "spec.yaml" + f.write_text( + "tables:\n - name: orders\n columns:\n" + " - name: amount\n range: [0, .inf]\n" + " - name: score\n range: [-.inf, 100]\n" + ) + dd = DataDict.from_yaml(f) + assert dd.tables[0].columns[0].range == [0, None] + assert dd.tables[0].columns[1].range == [None, 100] diff --git a/pkg-py/tests/test_datasource.py b/pkg-py/tests/test_datasource.py index 1f17389fb..12b555a73 100644 --- a/pkg-py/tests/test_datasource.py +++ b/pkg-py/tests/test_datasource.py @@ -584,3 +584,13 @@ def test_sqlalchemy_cleanup_does_not_dispose_caller_owned_engine( dispose.assert_not_called() assert len(source.get_data()) > 0 + + +def test_format_schema_open_ended_range() -> None: + cols = [ + ColumnMeta(name="a", sql_type="INTEGER", kind="numeric", min_val=0), + ColumnMeta(name="b", sql_type="INTEGER", kind="numeric", max_val=100), + ] + text = format_schema("t", cols) + assert " Range: 0 to ?" in text + assert " Range: ? to 100" in text diff --git a/pkg-py/tests/test_multi_table.py b/pkg-py/tests/test_multi_table.py index 709f65e71..f82eb17ea 100644 --- a/pkg-py/tests/test_multi_table.py +++ b/pkg-py/tests/test_multi_table.py @@ -1122,9 +1122,11 @@ class TestDataDictListInput: def test_accepts_list_of_data_dicts(self, orders_df, customers_df) -> None: from querychat._data_dict import DataDict, TableSpec - dd1 = DataDict(name="sales", tables={"orders": TableSpec(description="Orders")}) + dd1 = DataDict( + name="sales", tables=[TableSpec(name="orders", description="Orders")] + ) dd2 = DataDict( - name="people", tables={"customers": TableSpec(description="Customers")} + name="people", tables=[TableSpec(name="customers", description="Customers")] ) qc = QueryChat(orders_df, "orders", data_dict=[dd1, dd2]) qc.add_table(customers_df, "customers") @@ -1133,8 +1135,10 @@ def test_accepts_list_of_data_dicts(self, orders_df, customers_df) -> None: def test_accepts_list_of_paths(self, orders_df, customers_df, tmp_path) -> None: f1 = tmp_path / "orders_dict.yaml" f2 = tmp_path / "customers_dict.yaml" - f1.write_text('tables:\n orders:\n description: "Order records."\n') - f2.write_text('tables:\n customers:\n description: "Customer records."\n') + f1.write_text('tables:\n - name: orders\n description: "Order records."\n') + f2.write_text( + 'tables:\n - name: customers\n description: "Customer records."\n' + ) qc = QueryChat(orders_df, "orders", data_dict=[f1, f2]) qc.add_table(customers_df, "customers") assert qc is not None @@ -1142,16 +1146,20 @@ def test_accepts_list_of_paths(self, orders_df, customers_df, tmp_path) -> None: def test_single_data_dict_still_works(self, orders_df) -> None: from querychat._data_dict import DataDict, TableSpec - dd = DataDict(name="sales", tables={"orders": TableSpec(description="Orders")}) + dd = DataDict( + name="sales", tables=[TableSpec(name="orders", description="Orders")] + ) qc = QueryChat(orders_df, "orders", data_dict=dd) assert qc is not None def test_list_dicts_appear_in_system_prompt(self, orders_df, customers_df) -> None: from querychat._data_dict import DataDict, TableSpec - dd1 = DataDict(name="sales", tables={"orders": TableSpec(description="Orders")}) + dd1 = DataDict( + name="sales", tables=[TableSpec(name="orders", description="Orders")] + ) dd2 = DataDict( - name="people", tables={"customers": TableSpec(description="Customers")} + name="people", tables=[TableSpec(name="customers", description="Customers")] ) qc = QueryChat(orders_df, "orders", data_dict=[dd1, dd2]) qc.add_table(customers_df, "customers") diff --git a/pkg-py/tests/test_querychat.py b/pkg-py/tests/test_querychat.py index 03b26bd42..fa1331804 100644 --- a/pkg-py/tests/test_querychat.py +++ b/pkg-py/tests/test_querychat.py @@ -325,12 +325,12 @@ def test_greeting_prompt_omits_dicts_for_excluded_tables(sqlite_engine, tmp_path orders_yaml = tmp_path / "orders.yaml" orders_yaml.write_text( "name: orders_dict\ndescription: ORDERS_DICT_DESC\n" - "tables:\n orders:\n description: Orders info\n" + "tables:\n - name: orders\n description: Orders info\n" ) customers_yaml = tmp_path / "customers.yaml" customers_yaml.write_text( "name: customers_dict\ndescription: CUSTOMERS_DICT_DESC\n" - "tables:\n customers:\n description: Customers info\n" + "tables:\n - name: customers\n description: Customers info\n" ) qc = QueryChat(data_dict=[str(orders_yaml), str(customers_yaml)]) @@ -353,7 +353,7 @@ def test_greeting_prompt_keeps_global_dict_desc_drops_global_fields( orders_yaml = tmp_path / "orders.yaml" orders_yaml.write_text( "name: orders_dict\ndescription: ORDERS_DICT_DESC\n" - "tables:\n orders:\n description: Orders info\n" + "tables:\n - name: orders\n description: Orders info\n" "relationships:\n - join: orders.id = customers.id\n description: REL_DESC\n" ) diff --git a/pkg-py/tests/test_system_prompt.py b/pkg-py/tests/test_system_prompt.py index 38f99b819..8b8d91be0 100644 --- a/pkg-py/tests/test_system_prompt.py +++ b/pkg-py/tests/test_system_prompt.py @@ -70,7 +70,7 @@ def test_system_prompt_includes_table_description() -> None: df = nw.from_native(pl.DataFrame({"x": [1]})) source = DataFrameSource(df, "orders") dd = DataDict( - tables={"orders": TableSpec(description="Order records.")}, + tables=[TableSpec(name="orders", description="Order records.")], ) sp = _make_system_prompt({"orders": source}, data_dicts=[dd]) rendered = sp.render({"query"}) @@ -81,7 +81,7 @@ def test_system_prompt_includes_glossary() -> None: df = nw.from_native(pl.DataFrame({"x": [1]})) source = DataFrameSource(df, "orders") dd = DataDict( - tables={"orders": TableSpec(columns=[])}, + tables=[TableSpec(name="orders")], glossary={"churn": "No orders in 90 days."}, ) sp = _make_system_prompt({"orders": source}, data_dicts=[dd]) @@ -94,7 +94,7 @@ def test_system_prompt_includes_relationships() -> None: df = nw.from_native(pl.DataFrame({"x": [1]})) source = DataFrameSource(df, "orders") dd = DataDict( - tables={"orders": TableSpec(columns=[])}, + tables=[TableSpec(name="orders")], relationships=[ RelationshipSpec( join="orders.customer_id = customers.id", @@ -442,7 +442,9 @@ def test_no_dict_renders_flat_tables_block(self) -> None: assert " None: - dd = DataDict(name="sales", tables={"orders": TableSpec(description="Orders.")}) + dd = DataDict( + name="sales", tables=[TableSpec(name="orders", description="Orders.")] + ) sp = _make_system_prompt( {"orders": self._make_source("orders")}, data_dicts=[dd] ) @@ -451,7 +453,7 @@ def test_single_dict_renders_xml_tag_with_name(self) -> None: assert "Orders." in rendered def test_xml_tag_includes_description_attribute(self) -> None: - dd = DataDict(name="sales", description="Sales domain data", tables={}) + dd = DataDict(name="sales", description="Sales domain data", tables=[]) sp = _make_system_prompt( {"orders": self._make_source("orders")}, data_dicts=[dd] ) @@ -459,7 +461,7 @@ def test_xml_tag_includes_description_attribute(self) -> None: assert 'description="Sales domain data"' in rendered def test_xml_tag_omits_description_when_absent(self) -> None: - dd = DataDict(name="sales", tables={}) + dd = DataDict(name="sales", tables=[]) sp = _make_system_prompt( {"orders": self._make_source("orders")}, data_dicts=[dd] ) @@ -467,7 +469,7 @@ def test_xml_tag_omits_description_when_absent(self) -> None: assert "description=" not in rendered def test_yaml_body_excludes_name_and_description(self) -> None: - dd = DataDict(name="sales", description="Sales domain data", tables={}) + dd = DataDict(name="sales", description="Sales domain data", tables=[]) sp = _make_system_prompt( {"orders": self._make_source("orders")}, data_dicts=[dd] ) @@ -479,7 +481,7 @@ def test_yaml_body_excludes_name_and_description(self) -> None: def test_yaml_body_includes_tables_relationships_glossary(self) -> None: dd = DataDict( name="sales", - tables={"orders": TableSpec(description="Order records.")}, + tables=[TableSpec(name="orders", description="Order records.")], relationships=[RelationshipSpec(join="orders.customer_id = customers.id")], glossary={"churn": "No orders in 90 days."}, ) @@ -492,9 +494,11 @@ def test_yaml_body_includes_tables_relationships_glossary(self) -> None: assert "Order records." in rendered def test_multiple_dicts_render_sibling_xml_tags(self) -> None: - dd1 = DataDict(name="sales", tables={"orders": TableSpec(description="Orders")}) + dd1 = DataDict( + name="sales", tables=[TableSpec(name="orders", description="Orders")] + ) dd2 = DataDict( - name="catalog", tables={"products": TableSpec(description="Products")} + name="catalog", tables=[TableSpec(name="products", description="Products")] ) sp = _make_system_prompt( { diff --git a/pkg-py/tests/test_tools.py b/pkg-py/tests/test_tools.py index 0b0c64a4d..b1305dc21 100644 --- a/pkg-py/tests/test_tools.py +++ b/pkg-py/tests/test_tools.py @@ -8,7 +8,7 @@ import polars as pl import pytest from htmltools import Tag -from querychat._data_dict import ColumnRange, ColumnSpec, DataDict, TableSpec +from querychat._data_dict import ColumnSpec, DataDict, TableSpec from querychat._datasource import ColumnMeta, DataFrameSource from querychat._query_executor import DataSourceExecutor from querychat._utils import querychat_tool_starts_open @@ -193,23 +193,24 @@ def _make_executor_and_table( def test_get_schema_tool_preserves_metadata_for_model_and_display() -> None: dd = DataDict( - tables={ - "orders": TableSpec( + tables=[ + TableSpec( + name="orders", columns=[ ColumnSpec( name="amount", description="Gross & tax", units="USD", constraints=[">= 0", "required"], - range=ColumnRange(min=0, max=100), + range=[0, 100], ), ColumnSpec( name="status", values=["pending", "shipped & paid"], ), - ] + ], ) - }, + ], ) df = pl.DataFrame( { @@ -248,8 +249,7 @@ def test_get_schema_tool_preserves_metadata_for_model_and_display() -> None: "Constraints", ) header_positions = [ - rendered.index(f'{header}') - for header in expected_headers + rendered.index(f'{header}') for header in expected_headers ] amount_cell_positions = [ rendered.index(content) @@ -329,3 +329,15 @@ def test_get_schema_tool_uses_singular_column_preview() -> None: result = tool.func(table_name="orders") assert result.extra["display"].value_preview == "1 column" + + +def test_schema_card_shows_open_ended_range() -> None: + from querychat.tools import _schema_table + + cols = [ + ColumnMeta(name="a", sql_type="INTEGER", kind="numeric", min_val=0), + ColumnMeta(name="b", sql_type="INTEGER", kind="numeric", max_val=100), + ] + html = str(_schema_table(cols)) + assert "0 to ?" in html + assert "? to 100" in html diff --git a/pkg-r/NEWS.md b/pkg-r/NEWS.md index a9b27b7a4..2b3dc65d1 100644 --- a/pkg-r/NEWS.md +++ b/pkg-r/NEWS.md @@ -1,5 +1,9 @@ # querychat (development version) +## Breaking changes + +* Data dictionaries now follow the current [data-dict spec](https://data-dict.tidyverse.org/spec.html): `tables` must be a list of tables each with a `name` field (previously a mapping of name to definition), column `range` is a two-element `[min, max]` list, and `values` may be a value-to-label map. Update older dictionary files accordingly; `data-dict validate-spec` (from the data-dict CLI) checks a file against the spec. Files in the older mapping format, or with a `$version` that is invalid or predates the first spec version, fail with a descriptive error; files declaring a newer `$version` warn at load time but are still read on a best-effort basis. (#316) + ## Bug fixes * JSON pins now fall back to `pins::pin_read()` when DuckDB's JSON extension cannot be installed or loaded, including on builds that disable extension loading. Native DuckDB JSON reading is retained when the extension is available. diff --git a/pkg-r/R/DBISource.R b/pkg-r/R/DBISource.R index 723434d40..f78fa5386 100644 --- a/pkg-r/R/DBISource.R +++ b/pkg-r/R/DBISource.R @@ -372,15 +372,38 @@ build_column_details_impl <- function( max_val <- NA_character_ categories <- list() constraints <- if (length(spec[["constraints"]]) > 0) { - as.list(spec[["constraints"]]) + lapply(spec[["constraints"]], function(c) { + if (is.list(c)) { + # Assertion object: {assert, language?, description?} + txt <- c[["assert"]] %||% "" + if (!is.null(c[["description"]])) { + txt <- paste0(txt, " (", c[["description"]], ")") + } + txt + } else { + as.character(c) + } + }) } else { list() } if (col %in% numeric_columns) { if (!is.null(spec[["range"]])) { - min_val <- as.character(spec[["range"]][["min"]] %||% "?") - max_val <- as.character(spec[["range"]][["max"]] %||% "?") + # Spec form: two-element list [min, max]. Either end may be open: + # null, or the spec's -.inf/.inf sentinels (which parse as -Inf/Inf). + # (`[` indexing pads short lists with NULL instead of erroring.) + rng <- unname(spec[["range"]]) + lo <- rng[1][[1]] + hi <- rng[2][[1]] + if (is.numeric(lo) && is.infinite(lo)) { + lo <- NULL + } + if (is.numeric(hi) && is.infinite(hi)) { + hi <- NULL + } + min_val <- as.character(lo %||% "?") + max_val <- as.character(hi %||% "?") } else { min_key <- paste0(col, "__min") max_key <- paste0(col, "__max") @@ -397,7 +420,12 @@ build_column_details_impl <- function( } } else if (col %in% text_columns) { if (!is.null(spec[["values"]])) { - categories <- as.list(as.character(spec[["values"]])) + vals <- spec[["values"]] + # Map form (value -> label): the values are the names + if (!is.null(names(vals))) { + vals <- as.list(names(vals)) + } + categories <- as.list(as.character(vals)) } else if (col %in% names(categorical_values)) { vals <- categorical_values[[col]] if (length(vals) > 0) categories <- as.list(as.character(vals)) diff --git a/pkg-r/R/DataDict.R b/pkg-r/R/DataDict.R index 04b59651c..ea047b5ee 100644 --- a/pkg-r/R/DataDict.R +++ b/pkg-r/R/DataDict.R @@ -8,6 +8,11 @@ #' #' If `name` is absent from the YAML file, it defaults to the file stem. #' +#' Files written for older formats (a mapping-form `tables`, table entries +#' without a `name`, or a `$version` that is invalid or predates the first +#' spec version) fail with a descriptive error. Files declaring a newer spec +#' `$version` warn and are read on a best-effort basis. +#' #' @param path Path to the YAML file. #' #' @return A named list with the structure of the YAML file. @@ -17,6 +22,8 @@ read_data_dict <- function(path) { dd <- yaml::read_yaml(path) %||% list() + dd_check_spec_compat(dd) + if (is.null(dd[["name"]])) { dd[["name"]] <- tools::file_path_sans_ext(basename(path)) } @@ -24,6 +31,102 @@ read_data_dict <- function(path) { dd } +# The first data-dict spec version ever released. A `$version` below this +# is not a real spec version. +.dd_first_spec_version <- numeric_version("0.1.0") + +# The newest data-dict spec version this version of querychat fully +# understands. Files declaring a newer `$version` are still read, on a +# best-effort basis (unknown fields are ignored), with a warning. Bump this +# opportunistically to keep the warning calibrated -- it is not a hard gate. +.dd_known_spec_version <- numeric_version("0.1.0") + +#' Check a data dict for spec compatibility problems. +#' +#' Old or malformed files fail hard -- there is a clear migration path, and +#' reading on would silently produce a degraded dictionary. Files declaring +#' a newer spec `$version` are only warned about: they may use purely +#' additive features, so they are read on a best-effort basis. +#' +#' @param dd A data dict list (from [yaml::read_yaml()]). +#' @noRd +dd_check_spec_compat <- function(dd) { + version <- dd[["$version"]] + if (!is.null(version)) { + v <- tryCatch( + numeric_version(as.character(version)), + error = function(e) NULL + ) + if (is.null(v) || v < .dd_first_spec_version) { + cli::cli_abort(c( + "Data dictionary declares {.field $version} {.val {version}}, which is not a data-dict spec version this version of {.pkg querychat} can read.", + "i" = "The first spec version is {.val {as.character(.dd_first_spec_version)}}; update the file to the current spec ({.url https://data-dict.tidyverse.org/spec.html}).", + "i" = "{.code data-dict validate-spec } checks a file against the spec." + )) + } + if (v > .dd_known_spec_version) { + cli::cli_warn(c( + "!" = "Data dictionary declares {.field $version} {.val {version}}, which is newer than the newest data-dict spec version this version of {.pkg querychat} fully understands ({.val {as.character(.dd_known_spec_version)}}).", + "i" = "The file is read on a best-effort basis \u2014 spec fields querychat doesn't know about are ignored.", + "i" = "If anything looks off, consider upgrading {.pkg querychat} or running {.code data-dict validate-spec }." + )) + } + } + + tables <- dd[["tables"]] + if (length(tables) == 0) { + return(invisible(dd)) + } + + table_names <- names(tables) + if (!is.null(table_names) && any(nzchar(table_names))) { + cli::cli_abort(c( + "Data dictionary {.field tables} uses an outdated format.", + "i" = "{.field tables} must be a list of tables, each with a {.field name} field \u2014 not a mapping of name to definition.", + "i" = "Update the file to the current spec ({.url https://data-dict.tidyverse.org/spec.html}); {.code data-dict validate-spec } checks a file against the spec." + )) + } + + unnamed <- vapply( + tables, + function(tbl) { + nm <- if (is.list(tbl)) tbl[["name"]] else NULL + is.null(nm) || length(nm) != 1 || !nzchar(nm) + }, + logical(1) + ) + if (any(unnamed)) { + cli::cli_abort(c( + "{sum(unnamed)} table entr{?y/ies} in the data dictionary ha{?s/ve} no {.field name}.", + "i" = "Each entry in {.field tables} needs a {.field name}; see {.url https://data-dict.tidyverse.org/spec.html}." + )) + } + + invisible(dd) +} + +#' Key a data dict's table list by table name. +#' +#' In the data-dict spec, `tables` is a list of table entries each with a +#' `name` field. Returns a named list mapping table name to table entry, for +#' lookup-style access. Entries without a `name` are skipped. On duplicate +#' names the first entry wins (matching pkg-py's `DataDict.get_table()`). +#' +#' @param dd A data dict list (from [read_data_dict()]). +#' @return A named list of table entries. +#' @noRd +dd_tables_by_name <- function(dd) { + tables <- dd[["tables"]] %||% list() + out <- list() + for (tbl in tables) { + nm <- tbl[["name"]] + if (!is.null(nm) && nzchar(nm) && is.null(out[[nm]])) { + out[[nm]] <- tbl + } + } + out +} + #' Convert a data dict list to a filtered list for system prompt rendering. #' #' Keeps table descriptions (for LLM context), strips per-column details. @@ -41,8 +144,9 @@ data_dict_to_prompt_list <- function(dd) { if (!is.null(dd[["description"]])) { result[["description"]] <- dd[["description"]] } - if (length(dd[["tables"]]) > 0) { - result[["tables"]] <- lapply(dd[["tables"]], function(ts) { + tables <- dd_tables_by_name(dd) + if (length(tables) > 0) { + result[["tables"]] <- lapply(tables, function(ts) { if (is.null(ts[["description"]])) { NULL } else { diff --git a/pkg-r/R/QueryChatSystemPrompt.R b/pkg-r/R/QueryChatSystemPrompt.R index 712f521dc..67cd06902 100644 --- a/pkg-r/R/QueryChatSystemPrompt.R +++ b/pkg-r/R/QueryChatSystemPrompt.R @@ -85,8 +85,9 @@ QueryChatSystemPrompt <- R6::R6Class( # (table-less) dict carrying a dict-level description. self$data_dicts <- Filter( function(dd) { - length(intersect(names(dd$tables), resolved)) > 0 || - (length(dd$tables) == 0 && !is.null(dd$description)) + tbl_names <- names(dd_tables_by_name(dd)) + length(intersect(tbl_names, resolved)) > 0 || + (length(tbl_names) == 0 && !is.null(dd$description)) }, self$data_dicts ) @@ -217,7 +218,10 @@ QueryChatSystemPrompt <- R6::R6Class( d$name <- NULL d$description <- NULL - claimed <- intersect(names(self$data_sources), names(dd$tables)) + claimed <- intersect( + names(self$data_sources), + names(dd_tables_by_name(dd)) + ) all_claimed <- c(all_claimed, claimed) if (!is.null(d$tables)) { d$tables <- d$tables[names(d$tables) %in% names(self$data_sources)] diff --git a/pkg-r/R/querychat_tools.R b/pkg-r/R/querychat_tools.R index 6b653a042..b7e0b0754 100644 --- a/pkg-r/R/querychat_tools.R +++ b/pkg-r/R/querychat_tools.R @@ -23,8 +23,9 @@ tool_get_schema <- function( } table_spec <- NULL for (dd in data_dicts) { - if (!is.null(dd[["tables"]][[table_name]])) { - table_spec <- dd[["tables"]][[table_name]] + tbl <- dd_tables_by_name(dd)[[table_name]] + if (!is.null(tbl)) { + table_spec <- tbl break } } diff --git a/pkg-r/inst/examples-shiny/11-multi-table-nutrition/nutrition-data-dict.yaml b/pkg-r/inst/examples-shiny/11-multi-table-nutrition/nutrition-data-dict.yaml index a8455e6fc..6cd8426e4 100644 --- a/pkg-r/inst/examples-shiny/11-multi-table-nutrition/nutrition-data-dict.yaml +++ b/pkg-r/inst/examples-shiny/11-multi-table-nutrition/nutrition-data-dict.yaml @@ -1,7 +1,7 @@ -version: "0.2.0" +$version: "0.1.0" tables: - foods: + - name: foods description: > One row per USDA Foundation Food item. Nutrient values are not stored here — see food_nutrients. All nutrient amounts in food_nutrients are per 100g of the food. @@ -21,7 +21,7 @@ tables: type: date description: Date the food record was published by USDA. - food_categories: + - name: food_categories description: Lookup table of USDA food categories. One row per category. columns: - name: id @@ -35,7 +35,7 @@ tables: type: string description: Human-readable category name (e.g., "Vegetables and Vegetable Products"). - nutrients: + - name: nutrients description: > Registry of the 12 key nutritional compounds tracked in this dataset. One row per nutrient type. See the glossary for the full list of nutrient IDs. @@ -54,7 +54,7 @@ tables: values: [G, KCAL, MG] description: Unit of measurement. G = grams, KCAL = kilocalories, MG = milligrams. - food_nutrients: + - name: food_nutrients description: > Nutrient content per 100g of food. One row per food–nutrient combination. Only the 12 key nutrients are included (see nutrients table and glossary). @@ -76,7 +76,7 @@ tables: nutrients.unit_name. May be null if the nutrient was not measured for this food. - food_portions: + - name: food_portions description: > Common serving sizes for foods. One row per portion definition. A single food may have multiple portions (e.g., "1 cup" and "1 oz"). @@ -107,7 +107,7 @@ tables: Optional preparation note for the portion (e.g., "chopped", "drained", "cooked"). Null when no modifier applies. - measure_units: + - name: measure_units description: > Lookup table of measurement unit names used in food_portions. Only units that appear in food_portions are included (~32 of the 123 total). diff --git a/pkg-r/man/read_data_dict.Rd b/pkg-r/man/read_data_dict.Rd index 25c43271d..4deccba9c 100644 --- a/pkg-r/man/read_data_dict.Rd +++ b/pkg-r/man/read_data_dict.Rd @@ -19,4 +19,9 @@ returned as a plain list and can be passed directly to \link{QueryChat} via the \code{data_dict} argument. If \code{name} is absent from the YAML file, it defaults to the file stem. + +Files written for older formats (a mapping-form \code{tables}, table entries +without a \code{name}, or a \verb{$version} that is invalid or predates the first +spec version) fail with a descriptive error. Files declaring a newer spec +\verb{$version} warn and are read on a best-effort basis. } diff --git a/pkg-r/tests/testthat/test-DataDict.R b/pkg-r/tests/testthat/test-DataDict.R index f10aab403..5d62162be 100644 --- a/pkg-r/tests/testthat/test-DataDict.R +++ b/pkg-r/tests/testthat/test-DataDict.R @@ -3,10 +3,11 @@ describe("read_data_dict()", { yaml_file <- withr::local_tempfile(fileext = ".yaml") writeLines( c( + "$version: \"0.1.0\"", "name: test", "description: Test domain", "tables:", - " orders:", + " - name: orders", " description: Orders table", " columns:", " - name: id", @@ -26,10 +27,11 @@ describe("read_data_dict()", { expect_true(is.list(dd)) expect_equal(dd[["name"]], "test") expect_equal(dd[["description"]], "Test domain") - expect_true(is.list(dd[["tables"]][["orders"]])) - expect_equal(dd[["tables"]][["orders"]][["description"]], "Orders table") - expect_length(dd[["tables"]][["orders"]][["columns"]], 1) - expect_equal(dd[["tables"]][["orders"]][["columns"]][[1]][["name"]], "id") + orders <- dd[["tables"]][[1]] + expect_equal(orders[["name"]], "orders") + expect_equal(orders[["description"]], "Orders table") + expect_length(orders[["columns"]], 1) + expect_equal(orders[["columns"]][[1]][["name"]], "id") expect_length(dd[["relationships"]], 1) expect_equal( dd[["relationships"]][[1]][["join"]], @@ -51,12 +53,10 @@ describe("read_data_dict()", { writeLines( c( "tables:", - " products:", + " - name: products", " columns:", " - name: price", - " range:", - " min: 0", - " max: 999", + " range: [0, 999]", " - name: category", " values: [A, B, C]" ), @@ -64,16 +64,96 @@ describe("read_data_dict()", { ) dd <- read_data_dict(yaml_file) - cols <- dd[["tables"]][["products"]][["columns"]] + cols <- dd[["tables"]][[1]][["columns"]] price_col <- cols[[1]] cat_col <- cols[[2]] - expect_equal(price_col[["range"]][["min"]], 0) - expect_equal(price_col[["range"]][["max"]], 999) + expect_equal(price_col[["range"]][[1]], 0) + expect_equal(price_col[["range"]][[2]], 999) expect_equal(cat_col[["values"]], c("A", "B", "C")) }) }) +describe("dd_tables_by_name()", { + it("keys the table list by name", { + dd <- list( + tables = list( + list(name = "orders", description = "Orders"), + list(name = "customers", description = "Customers") + ) + ) + tables <- dd_tables_by_name(dd) + expect_equal(names(tables), c("orders", "customers")) + expect_equal(tables[["orders"]][["description"]], "Orders") + }) + + it("returns an empty list when there are no tables", { + expect_equal(dd_tables_by_name(list()), list()) + expect_equal(dd_tables_by_name(list(tables = list())), list()) + }) + + it("skips table entries without a name", { + dd <- list(tables = list(list(description = "No name"))) + expect_equal(dd_tables_by_name(dd), list()) + }) + + it("keeps the first entry on duplicate names", { + dd <- list( + tables = list( + list(name = "orders", description = "First"), + list(name = "orders", description = "Second") + ) + ) + tables <- dd_tables_by_name(dd) + expect_equal(tables[["orders"]][["description"]], "First") + }) +}) + +describe("dd_check_spec_compat()", { + it("accepts the supported $version without warning", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines('$version: "0.1.0"', yaml_file) + expect_no_warning(read_data_dict(yaml_file)) + }) + + it("treats a null $version as absent", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines(c("$version:", "tables: []"), yaml_file) + expect_no_warning(read_data_dict(yaml_file)) + }) + + it("warns when $version is newer, but still reads the file", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines('$version: "99.0.0"', yaml_file) + expect_warning(dd <- read_data_dict(yaml_file), "best-effort basis") + expect_equal(dd[["$version"]], "99.0.0") + }) + + it("errors when $version predates the first spec version", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines('$version: "0.0.9"', yaml_file) + expect_error(read_data_dict(yaml_file), "can read") + }) + + it("errors when $version is invalid", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines('$version: "not-a-version"', yaml_file) + expect_error(read_data_dict(yaml_file), "can read") + }) + + it("errors when tables uses the legacy mapping format", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines(c("tables:", " orders:", " columns: []"), yaml_file) + expect_error(read_data_dict(yaml_file), "outdated format") + }) + + it("errors when a table entry has no name", { + yaml_file <- withr::local_tempfile(fileext = ".yaml") + writeLines(c("tables:", " - description: No name"), yaml_file) + expect_error(read_data_dict(yaml_file), "has no") + }) +}) + describe("data_dict_to_prompt_list()", { it("returns list with name and description", { dd <- list(name = "sales", description = "Sales domain") @@ -92,7 +172,8 @@ describe("data_dict_to_prompt_list()", { it("includes table descriptions but strips column details", { dd <- list( tables = list( - orders = list( + list( + name = "orders", description = "Orders table", details = "Long details that should not appear", columns = list( @@ -146,7 +227,7 @@ describe("data_dict_to_prompt_list()", { }) it("includes table entry as NULL when table has no description", { - dd <- list(tables = list(no_desc = list())) + dd <- list(tables = list(list(name = "no_desc"))) result <- data_dict_to_prompt_list(dd) expect_true("tables" %in% names(result)) expect_null(result[["tables"]][["no_desc"]]) diff --git a/pkg-r/tests/testthat/test-QueryChat.R b/pkg-r/tests/testthat/test-QueryChat.R index c2b1ddb22..604a010e7 100644 --- a/pkg-r/tests/testthat/test-QueryChat.R +++ b/pkg-r/tests/testthat/test-QueryChat.R @@ -1783,7 +1783,7 @@ describe("QueryChatGreeter", { "name: orders_dict", "description: ORDERS_DICT_DESC", "tables:", - " orders:", + " - name: orders", " description: Orders info" ), orders_yaml @@ -1794,7 +1794,7 @@ describe("QueryChatGreeter", { "name: customers_dict", "description: CUSTOMERS_DICT_DESC", "tables:", - " customers:", + " - name: customers", " description: Customers info" ), customers_yaml @@ -1831,7 +1831,7 @@ describe("QueryChatGreeter", { "name: orders_dict", "description: ORDERS_DICT_DESC", "tables:", - " orders:", + " - name: orders", " description: Orders info", "relationships:", " - join: orders.id = customers.id", diff --git a/pkg-r/tests/testthat/test-querychat_tools.R b/pkg-r/tests/testthat/test-querychat_tools.R index 46fbbc0f7..a27ab5107 100644 --- a/pkg-r/tests/testthat/test-querychat_tools.R +++ b/pkg-r/tests/testthat/test-querychat_tools.R @@ -517,21 +517,65 @@ describe("tool_update_dashboard_impl()", { describe("tool_get_schema()", { skip_if_no_dataframe_engine() + it("renders spec range, value keys, and assertion constraints", { + df <- data.frame( + id = 1:5, + score = 1:5, + grp = c("A", "B", "A", "B", "A"), + stringsAsFactors = FALSE + ) + df_source <- local_data_frame_source(df) + executor <- local_executor(df_source) + table_spec <- list( + name = "test_table", + columns = list( + list(name = "id", range = list(1, NULL)), + # The spec's -.inf/.inf sentinels also mean an open bound + list(name = "score", range = list(-Inf, 5)), + list( + name = "grp", + values = list(A = "Alpha", B = "Beta"), + constraints = list( + list(assert = "grp <> ''", description = "Non-empty"), + "required" + ) + ) + ) + ) + schema_result <- executor$get_schema_result( + "test_table", + 20, + table_spec = table_spec + ) + text <- schema_result$text + + # Open-ended ranges keep their known bound + expect_match(text, "Range: 1 to ?", fixed = TRUE) + expect_match(text, "Range: ? to 5", fixed = TRUE) + # Map-form values render the values (keys), not the labels + expect_match(text, "Categorical values: 'A', 'B'", fixed = TRUE) + expect_false(grepl("Alpha", text, fixed = TRUE)) + # Assertion objects render as 'assert (description)' alongside barewords + expect_match(text, "grp <> '' (Non-empty)", fixed = TRUE) + expect_match(text, "required", fixed = TRUE) + }) + it("returns schema text with a native rich display", { df_source <- local_data_frame_source(new_test_df()) executor <- local_executor(df_source) table_spec <- list( + name = "test_table", columns = list( list( name = "id", description = "Primary key", units = "rows", constraints = c("positive", "required"), - range = list(min = 1, max = 5) + range = list(1, 5) ) ) ) - data_dicts <- list(list(tables = list(test_table = table_spec))) + data_dicts <- list(list(tables = list(table_spec))) expected <- executor$get_schema_result( "test_table", 20, diff --git a/pkg-r/vignettes/context.Rmd b/pkg-r/vignettes/context.Rmd index e66413cf5..1f4237e89 100644 --- a/pkg-r/vignettes/context.Rmd +++ b/pkg-r/vignettes/context.Rmd @@ -45,14 +45,14 @@ qc$add_table(customers, "customers") ### Format -A data dictionary has three top-level sections: `tables`, `relationships`, and `glossary`. +The file format is defined by the [data-dict spec](https://data-dict.tidyverse.org/spec.html). A data dictionary has three top-level sections: `tables` (a list of tables, each with a `name`), `relationships`, and `glossary`. The optional `$version` field declares the spec version the file conforms to. A `$version` that is invalid or predates the first spec version fails with a descriptive error; a file declaring a version newer than querychat understands warns at load time but is still read on a best-effort basis. ```yaml # data-dict.yaml -version: "0.2.0" +$version: "0.1.0" tables: - orders: + - name: orders description: One row per customer order. columns: - name: order_id @@ -71,7 +71,7 @@ tables: values: [pending, shipped, delivered, cancelled] description: Current order status. - customers: + - name: customers description: One row per customer. columns: - name: id @@ -97,15 +97,16 @@ glossary: #### Tables -Each entry under `tables` describes one table. The key must match the table name you pass to `QueryChat$new()` or `$add_table()`. +Each entry under `tables` describes one table. Its `name` must match the table name you pass to `QueryChat$new()` or `$add_table()`. - **`description`**: What this table represents (one sentence is usually enough). - **`columns`**: A list of column annotations. Each column can have: - `name`: Column name (must match the actual column) - `type`: Semantic type hint — `string`, `number`, `number(id)`, `number(quantity)`, `date`, `enum` - - `constraints`: Optional list — `primary_key`, `foreign_key` + - `constraints`: Optional list — barewords like `primary_key`/`foreign_key`, or assertion objects (`assert` with an optional `language` and `description`) - `description`: What this column means in plain English - - `values`: For `enum` columns, the list of possible values + - `values`: For `enum` columns, the possible values — a list, or a value-to-label map (e.g. `{M: Male, F: Female}`) + - `range`: Optional `[min, max]` bounds. Either end may be left open with the `-.inf`/`.inf` sentinels (or `null`) — if you leave a bound open, describe the range in prose in the column's `description`. Replaces live min/max queries for the column Columns listed in the data dictionary are excluded from the auto-generated schema (since your description supersedes the auto-detected metadata). Columns not listed are still auto-detected as usual. diff --git a/pyproject.toml b/pyproject.toml index d8bea98de..4fd04f744 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ dependencies = [ "great-tables>=0.16.0", "pyyaml", "nbformat>=5.10.4", + "packaging", ] classifiers = [ "Programming Language :: Python",