diff --git a/pkg-py/src/commons/_data_dictionary.py b/pkg-py/src/commons/_data_dictionary.py index 974476d0..de1402a4 100644 --- a/pkg-py/src/commons/_data_dictionary.py +++ b/pkg-py/src/commons/_data_dictionary.py @@ -99,6 +99,10 @@ class Table(_Permissive): details: str | None = None columns: dict[str, Column] = {} definitions: dict[str, Definition] = {} + # Attached by _definitions at data-source construction; empty until then, + # so the registry can be exercised without the compiler. Elements are + # ExportRecord, typed Any so this model need not import _definitions. + compiled_definitions: list[Any] = [] @model_validator(mode="before") @classmethod @@ -202,16 +206,17 @@ def entry_parts( entry = self.tables.get(table) if entry is None: return [] - # Governed definitions belong between the columns and the - # relationships, and are added once the compiler can supply them. - # They render as compiled SQL, never as the authored expression, so - # there is nothing correct to show before compilation happens. + # Imported here because _definitions does not import this module and + # this keeps it that way. + from ._definitions import entry_text as definitions_entry_text + parts = [ part for part in ( entry.description, entry.details, columns_text, + definitions_entry_text(entry.compiled_definitions), self._relationships_text(table), ) if part @@ -288,8 +293,10 @@ def context_chunks(self) -> list[str]: if prose: chunks.append(f"Table `{name}`: {prose}") chunks.extend(f"{term}: {body}" for term, body in self.glossary.items()) - # One chunk per governed definition joins these once the compiler can - # supply them, for the same reason as the first-touch entry. + from ._definitions import context_chunks as definitions_context_chunks + + for entry in self.tables.values(): + chunks.extend(definitions_context_chunks(entry.compiled_definitions)) return [chunk for chunk in chunks if chunk] diff --git a/pkg-py/src/commons/_definitions/__init__.py b/pkg-py/src/commons/_definitions/__init__.py new file mode 100644 index 00000000..b30e236e --- /dev/null +++ b/pkg-py/src/commons/_definitions/__init__.py @@ -0,0 +1,37 @@ +"""Governed definitions: the registry, and the compiler that will feed it. + +Definitions are authored in data-dict's expression language, not in the SQL +dialect of the attached source, so they are type-checked against the +dictionary and lowered to the source's dialect before anything runs. That work +sits behind one export-record seam, pinned to data-dict commit +d950c5ac90d0ab939d330600f3a5ee1bfde0f604, so it can be replaced by a shared +data-dict interface later. + +Runtime code consumes `ExportRecord` and never an expression parser or a typed +IR. Cross-implementation conformance against the data-dict binary is the +authority, not this code. +""" + +from ._registry import ( + ExportRecord, + Registry, + applied_text, + build_registry, + context_chunks, + entry_text, + expand_tokens, + index_overflows, + index_text, +) + +__all__ = [ + "ExportRecord", + "Registry", + "applied_text", + "build_registry", + "context_chunks", + "entry_text", + "expand_tokens", + "index_overflows", + "index_text", +] diff --git a/pkg-py/src/commons/_definitions/_registry.py b/pkg-py/src/commons/_definitions/_registry.py new file mode 100644 index 00000000..1434803f --- /dev/null +++ b/pkg-py/src/commons/_definitions/_registry.py @@ -0,0 +1,308 @@ +"""The registry surface commons owns, whatever data-dict eventually provides. + +Everything here consumes export records. Nothing here may reach into an +expression parser or a typed IR: that rule is what keeps the seam real, and +what makes an eventual swap to a shared data-dict interface a deletion rather +than a rewrite. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass, field, replace +from typing import Any + +__all__ = [ + "ExportRecord", + "Registry", + "applied_text", + "build_registry", + "context_chunks", + "entry_text", + "expand_tokens", + "index_overflows", + "index_text", +] + +INDEX_CAP_CHARS = 4000 + +_TOKEN = re.compile(r"\{\{\s*([^{}]+?)\s*\}\}") +_LEGACY_DOTTED = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)+$") +_GROUPS = {"filter": "filters", "derived": "derived", "metric": "metrics"} + + +@dataclass(frozen=True) +class ExportRecord: + """One governed definition, compiled for a source and ready to expand. + + `mixed_grain` is the one field data-dict's export does not carry. It is + derived from the typed IR, and `call_metrics` needs it to refuse a mix of + row and aggregate definitions in one call. + """ + + name: str + table: str + source: str + kind: str + type: str | None + expression: str + label: str | None + description: str | None + details: str | None + columns: list[str] + definitions: list[str] + sql: str + target: str + notes: list[str] + mixed_grain: bool + + +@dataclass +class Registry: + """Every source's definitions in one place.""" + + records: list[ExportRecord] = field(default_factory=list) + + def for_source(self, name: str | None = None) -> list[ExportRecord]: + if name is None: + return list(self.records) + return [record for record in self.records if record.source == name] + + +def build_registry(sources: dict[str, Any]) -> Registry: + """Collect every source's compiled definitions into one registry. + + Takes `DataSource` values, typed loosely because `_data_source` imports + this module and the reverse import would close the cycle. + + A definition on a table the source does not expose is a construction + error: its token would resolve at conversation time against nothing. + """ + records: list[ExportRecord] = [] + for label, source in sources.items(): + dictionary = getattr(source, "dictionary", None) + if dictionary is None: + continue + exposed = set(getattr(source, "tables", None) or []) + for table, entry in dictionary.tables.items(): + compiled = getattr(entry, "compiled_definitions", None) or [] + if not compiled: + continue + if table not in exposed: + raise ValueError( + f"The data dictionary declares definitions on table {table!r}, " + "which the data source does not expose. Exposed tables: " + f"{', '.join(sorted(exposed))}." + ) + for record in compiled: + records.append(replace(record, table=table, source=label)) + return Registry(records) + + +# ---- token expansion ----------------------------------------------------- + + +def expand_tokens( + sql: str, records: list[ExportRecord] +) -> tuple[str, list[ExportRecord]]: + """Replace `{{name}}` and `{{table::name}}` with compiled SQL. + + Runs before the read-only guard, so the guard sees the SQL that will + actually execute. Failures are tool errors the model can recover from + in-conversation, so they name the alternatives. + """ + tokens: list[str] = [] + for match in _TOKEN.finditer(sql): + token = match.group(1) + if token not in tokens: + tokens.append(token) + + # Every token resolves against the query as written. Compiled SQL can + # name a table the query does not, and resolving against already-expanded + # text would let that count as the table being present, which is the check + # that keeps a token bound to its own table. + resolved = [(token, _resolve_token(token, sql, records)) for token in tokens] + + applied: list[ExportRecord] = [] + for token, record in resolved: + pattern = re.compile(r"\{\{\s*" + re.escape(token) + r"\s*\}\}") + # A lambda, not a replacement string: the SQL is literal and a + # backslash in it would otherwise be read as an escape. + sql = pattern.sub(lambda _match, found=record: f"({found.sql})", sql) + applied.append(record) + return sql, applied + + +def _word_pattern(word: str) -> re.Pattern[str]: + return re.compile(rf"(? bool: + # Tokens are stripped first so a token's own text cannot bring its table + # into scope. A word match rather than a parse, which is the same + # heuristic that picks entries for first touch. + return bool(_word_pattern(table).search(_TOKEN.sub("", sql))) + + +def _resolve_token(token: str, sql: str, records: list[ExportRecord]) -> ExportRecord: + if "::" in token: + table, _, name = token.partition("::") + return _resolve_qualified(table, name, token, sql, records) + + named = [record for record in records if record.name == token] + if not named: + if not _LEGACY_DOTTED.match(token): + _abort_unknown(token, records) + table, _, name = token.rpartition(".") + return _resolve_qualified(table, name, token, sql, records) + + in_scope = [record for record in named if _table_in_query(record.table, sql)] + if len(in_scope) == 1: + return in_scope[0] + if not in_scope: + tables = ", ".join(sorted({record.table for record in named})) + raise ValueError( + f"{{{{{token}}}}} is defined on table {tables}, which does not " + "appear in this query." + ) + qualified = " or ".join(f"{{{{{record.table}::{token}}}}}" for record in in_scope) + raise ValueError( + f"{{{{{token}}}}} is ambiguous here: it is defined on several tables in " + f"this query. Qualify the token: {qualified}." + ) + + +def _resolve_qualified( + table: str, name: str, token: str, sql: str, records: list[ExportRecord] +) -> ExportRecord: + hits = [ + record for record in records if record.table == table and record.name == name + ] + if not hits: + _abort_unknown(token, records) + if not _table_in_query(table, sql): + raise ValueError( + f"{{{{{token}}}}} is defined on table {table}, which does not appear " + "in this query." + ) + return hits[0] + + +def _abort_unknown(token: str, records: list[ExportRecord]) -> None: + if not records: + detail = "This source has no governed definitions." + else: + available = ", ".join(f"{{{{{r.name}}}}} ({r.table})" for r in records) + detail = f"Available definitions: {available}." + raise ValueError(f"No governed definition matches {{{{{token}}}}}. {detail}") + + +# ---- rendering ----------------------------------------------------------- + + +def _flatten_inline(text: str) -> str: + # Collapse each newline and the whitespace around it; other whitespace + # stays as authored. tests/shared/definition-rendering.json pins this. + return re.sub(r"\s*\n\s*", " ", text).strip() + + +def _index_lines(registry: Registry) -> list[str]: + records = registry.records + if not records: + return [] + multi_source = len({record.source for record in records}) > 1 + + def scope(record: ExportRecord) -> str: + return f"{record.source}.{record.table}" if multi_source else record.table + + lines: list[str] = [] + for one in dict.fromkeys(scope(record) for record in records): + here = [record for record in records if scope(record) == one] + parts = [] + for kind, plural in _GROUPS.items(): + items = [ + f"`{{{{{record.name}}}}}` ({_flatten_inline(record.label)})" + if record.label + else f"`{{{{{record.name}}}}}`" + for record in here + if record.kind == kind + ] + if items: + parts.append(f"{plural} {', '.join(items)}") + lines.append(f"- {one}: {'; '.join(parts)}") + return lines + + +def index_text(registry: Registry, cap_chars: int = INDEX_CAP_CHARS) -> str: + """The kind index for the system prompt, truncated to the cap.""" + kept: list[str] = [] + for line in _index_lines(registry): + # Measured as joined, so the newlines between lines count against the + # cap rather than pushing the result past it. + if len("\n".join([*kept, line])) > cap_chars: + break + kept.append(line) + return "\n".join(kept) + + +def index_overflows(registry: Registry, cap_chars: int = INDEX_CAP_CHARS) -> bool: + """Whether the index did not fit, so the model is told to search instead.""" + return len("\n".join(_index_lines(registry))) > cap_chars + + +def _gist(record: ExportRecord) -> str: + detail = _flatten_inline( + " ".join(part for part in (record.description, record.details) if part) + ) + # An absent type is left out rather than printed: data-dict omits it when + # no single type is inferred, and the language's null spelling would + # otherwise reach the model as a word. + scope = f"{record.kind}, {record.type}" if record.type else record.kind + parts = [f"({scope})"] + if detail: + parts.append(detail) + parts.append(f"Selected {record.target}: `({_flatten_inline(record.sql)})`.") + if record.notes: + parts.append(f"Translation notes: {' '.join(record.notes)}") + return " ".join(parts) + + +def entry_text(records: list[ExportRecord]) -> str | None: + """A table's governed definitions, delivered at first touch. + + Shows the compiled SQL, never the authored expression: the expression is + in data-dict's language, and the model writes SQL. + """ + if not records: + return None + header = ( + "Governed definitions (write as `{{name}}` tokens in SQL; use " + "`{{table::name}}` to qualify):\n\n" + ) + lines = [f"- `{{{{{record.name}}}}}` {_gist(record)}" for record in records] + return header + "\n".join(lines) + + +def context_chunks(records: list[ExportRecord]) -> list[str]: + """One retrievable chunk per definition.""" + return [ + f"Governed definition `{{{{{record.name}}}}}` on table `{record.table}` " + f"{_gist(record)}" + for record in records + ] + + +def applied_text(applied: list[ExportRecord]) -> str | None: + """What each expanded token became, reported alongside a query's results.""" + if not applied: + return None + lines = [] + for record in applied: + line = ( + f"- {{{{{record.name}}}}} ({record.table}): {record.target} " + f"`({_flatten_inline(record.sql)})`" + ) + if record.notes: + line += f"\n Translation notes: {' '.join(record.notes)}" + lines.append(line) + return "Applied governed definitions:\n\n" + "\n".join(lines) diff --git a/pkg-py/tests/test_data_dictionary.py b/pkg-py/tests/test_data_dictionary.py index 5abf86ce..9c7ad067 100644 --- a/pkg-py/tests/test_data_dictionary.py +++ b/pkg-py/tests/test_data_dictionary.py @@ -347,3 +347,62 @@ def test_a_frame_named_dictionary_fails_loudly_rather_than_silently() -> None: with pytest.raises(TypeError, match="DataFrame"): data_source(dictionary=pd.DataFrame({"revenue": [1.0]})) + + +def test_a_tables_entry_lists_its_governed_definitions(retail: DataDictionary) -> None: + from commons._definitions import ExportRecord + + retail.tables["sales"].compiled_definitions = [ + ExportRecord( + name="net_revenue", + table="sales", + source="", + kind="metric", + type="number", + expression="SUM(revenue)", + label=None, + description=None, + details=None, + columns=["revenue"], + definitions=[], + sql="sum(revenue)", + target="SQL(duckdb)", + notes=[], + mixed_grain=False, + ) + ] + + text = retail.entry_text("sales") + + assert text is not None + assert "Governed definitions" in text + assert text.index("Documented columns:") < text.index("Governed definitions") + assert text.index("Governed definitions") < text.index("Relationships:") + + +def test_context_chunks_include_governed_definitions(retail: DataDictionary) -> None: + from commons._definitions import ExportRecord + + retail.tables["sales"].compiled_definitions = [ + ExportRecord( + name="net_revenue", + table="sales", + source="", + kind="metric", + type="number", + expression="SUM(revenue)", + label=None, + description=None, + details=None, + columns=["revenue"], + definitions=[], + sql="sum(revenue)", + target="SQL(duckdb)", + notes=[], + mixed_grain=False, + ) + ] + + chunks = retail.context_chunks() + + assert any(chunk.startswith("Governed definition `{{net_revenue}}`") for chunk in chunks) diff --git a/pkg-py/tests/test_definition_rendering.py b/pkg-py/tests/test_definition_rendering.py new file mode 100644 index 00000000..35f89fd8 --- /dev/null +++ b/pkg-py/tests/test_definition_rendering.py @@ -0,0 +1,92 @@ +"""The definition-rendering contract both packages consume. + +The R suite runs the same cases from the same file. See tests/shared/README.md. +""" + +import pytest + +from commons._definitions import ( + ExportRecord, + Registry, + expand_tokens, + index_overflows, + index_text, +) +from commons._definitions._registry import _gist + +from ._shared import load_shared_fixture + +SPEC = load_shared_fixture("definition-rendering") +RECORDS = SPEC["records"]["values"] + +# The fixture names why a query must be refused; the wording is this package's. +REFUSAL = { + "table_not_in_query": "does not appear in this query", + "unknown_token": "No governed definition matches", + "ambiguous_token": "is ambiguous here", +} + + +def record(key: str) -> ExportRecord: + return ExportRecord(**RECORDS[key]) + + +def cases(section: str) -> list[dict]: + # An empty section would collect zero cases and the runner would pass. + found = SPEC[section]["cases"] + assert found, section + return found + + +@pytest.mark.parametrize("case", cases("expand_tokens"), ids=lambda c: c["name"]) +def test_expansion_matches_the_shared_contract(case: dict) -> None: + records = [record(key) for key in case["records"]] + + if case["expanded"] is None: + with pytest.raises(ValueError, match=REFUSAL[case["reason"]]): + expand_tokens(case["sql"], records) + return + + sql, applied = expand_tokens(case["sql"], records) + + assert sql == case["expanded"] + assert [found.name for found in applied] == case["applied"] + + +@pytest.mark.parametrize("case", cases("gist"), ids=lambda c: c["name"]) +def test_the_gist_matches_the_shared_contract(case: dict) -> None: + assert _gist(record(case["record"])) == case["expected"] + + +def test_every_record_in_the_bank_is_used() -> None: + used: set[str] = set() + for section in ("expand_tokens", "index"): + for case in cases(section): + used.update(case["records"]) + used.update(case["record"] for case in cases("gist")) + + assert used == set(RECORDS) + + +def test_the_index_cases_pin_both_sides_of_the_cap() -> None: + # A fixture where nothing overflows would pass against an implementation + # that never reports overflow. + overflows = {case["overflows"] for case in cases("index")} + + assert overflows == {True, False} + + +def test_the_gist_cases_cover_a_typeless_definition() -> None: + # The defect this section exists for: an absent type must not take the + # rest of the gist with it. + typeless = [key for key, found in RECORDS.items() if found["type"] is None] + + assert any(case["record"] in typeless for case in cases("gist")) + + +@pytest.mark.parametrize("case", cases("index"), ids=lambda c: c["name"]) +def test_the_index_matches_the_shared_contract(case: dict) -> None: + registry = Registry([record(key) for key in case["records"]]) + + assert index_text(registry, case["cap_chars"]) == case["text"] + assert index_overflows(registry, case["cap_chars"]) is case["overflows"] diff --git a/pkg-py/tests/test_definition_rendering_fixture.py b/pkg-py/tests/test_definition_rendering_fixture.py deleted file mode 100644 index 9c1ad1ef..00000000 --- a/pkg-py/tests/test_definition_rendering_fixture.py +++ /dev/null @@ -1,63 +0,0 @@ -"""The definition-rendering contract both packages consume. - -This checks the fixture's own integrity: that every section has cases, that -each case names records the bank defines, and that a refusal says why. Running -the cases against an implementation comes with the registry. -""" - -from typing import Any - -from ._shared import load_shared_fixture - -SPEC = load_shared_fixture("definition-rendering") -RECORDS: dict[str, dict[str, Any]] = SPEC["records"]["values"] -SECTIONS = ("expand_tokens", "gist", "index") - - -def test_every_section_has_cases() -> None: - # An empty section would collect zero cases and every runner would pass. - for section in SECTIONS: - assert SPEC[section]["cases"], section - - -def test_every_record_is_used() -> None: - used: set[str] = set() - for section in ("expand_tokens", "index"): - for case in SPEC[section]["cases"]: - used.update(case["records"]) - used.update(case["record"] for case in SPEC["gist"]["cases"]) - - assert used == set(RECORDS) - - -def test_every_named_record_resolves() -> None: - for section in ("expand_tokens", "index"): - for case in SPEC[section]["cases"]: - for key in case["records"]: - assert key in RECORDS, f"{section}: {case['name']}: {key}" - - -def test_refusals_say_why_and_expansions_do_not() -> None: - for case in SPEC["expand_tokens"]["cases"]: - if case["expanded"] is None: - assert case["reason"], case["name"] - assert case["applied"] == [], case["name"] - else: - assert "reason" not in case, case["name"] - - -def test_the_index_cases_pin_both_sides_of_the_cap() -> None: - # A fixture where nothing overflows would pass against an implementation - # that never reports overflow. - overflows = {case["overflows"] for case in SPEC["index"]["cases"]} - - assert overflows == {True, False} - - -def test_the_gist_cases_cover_a_typeless_definition() -> None: - # The defect this section exists for: an absent type must not take the - # rest of the gist with it. - typeless = [key for key, record in RECORDS.items() if record["type"] is None] - - assert typeless - assert any(case["record"] in typeless for case in SPEC["gist"]["cases"]) diff --git a/pkg-py/tests/test_definitions_fixture.py b/pkg-py/tests/test_definitions_fixture.py index 5da0b1ee..42cc0ce9 100644 --- a/pkg-py/tests/test_definitions_fixture.py +++ b/pkg-py/tests/test_definitions_fixture.py @@ -1,8 +1,9 @@ """The definitions contract both packages consume. This checks the fixture's own integrity: that it covers the corpus, that its -sections agree with each other, and that it is not empty. Running the cases -against an implementation comes with the registry and the compiler. +sections agree with each other, and that it is not empty. +test_definitions_registry.py runs the export records against the registry; +running them against the compiler's own output comes with the compiler. """ from typing import Any diff --git a/pkg-py/tests/test_definitions_registry.py b/pkg-py/tests/test_definitions_registry.py new file mode 100644 index 00000000..ab02a87e --- /dev/null +++ b/pkg-py/tests/test_definitions_registry.py @@ -0,0 +1,497 @@ +"""Token expansion, the kind index, and grain metadata. + +The registry consumes export records. These tests build records directly +rather than compiling them, which is the seam working as intended: the +registry is usable before the compiler exists and keeps working after it is +replaced by whatever data-dict eventually provides. +""" + +import pytest + +from commons._definitions import ( + ExportRecord, + Registry, + applied_text, + context_chunks, + entry_text, + expand_tokens, + index_overflows, + index_text, +) +from commons._definitions._registry import _GROUPS as _GROUPS_FOR_TEST + + +def record( + name: str, + table: str = "sales", + *, + source: str = "sales_db", + kind: str = "metric", + sql: str = "sum(revenue)", + label: str | None = None, + description: str | None = None, + mixed_grain: bool = False, + definitions: list[str] | None = None, + notes: list[str] | None = None, +) -> ExportRecord: + return ExportRecord( + name=name, + table=table, + source=source, + kind=kind, + type="number", + expression="SUM(revenue)", + label=label, + description=description, + details=None, + columns=["revenue"], + definitions=definitions or [], + sql=sql, + target="SQL(duckdb)", + notes=notes or [], + mixed_grain=mixed_grain, + ) + + +# ---- token expansion ----------------------------------------------------- + + +def test_a_bare_token_expands_to_parenthesized_sql() -> None: + sql, applied = expand_tokens( + "SELECT {{net_revenue}} FROM sales", [record("net_revenue")] + ) + + assert sql == "SELECT (sum(revenue)) FROM sales" + assert [item.name for item in applied] == ["net_revenue"] + + +def test_whitespace_inside_a_token_is_tolerated() -> None: + sql, _ = expand_tokens( + "SELECT {{ net_revenue }} FROM sales", [record("net_revenue")] + ) + + assert sql == "SELECT (sum(revenue)) FROM sales" + + +def test_sql_without_tokens_passes_through_untouched() -> None: + sql, applied = expand_tokens("SELECT 1 FROM sales", [record("net_revenue")]) + + assert sql == "SELECT 1 FROM sales" + assert applied == [] + + +def test_a_qualified_token_selects_the_named_table() -> None: + records = [ + record("total", table="sales", sql="sum(a)"), + record("total", table="returns", sql="sum(b)"), + ] + + sql, _ = expand_tokens("SELECT {{sales::total}} FROM sales JOIN returns", records) + + assert sql == "SELECT (sum(a)) FROM sales JOIN returns" + + +def test_a_bare_token_scopes_to_the_table_the_query_names() -> None: + records = [ + record("total", table="sales", sql="sum(a)"), + record("total", table="returns", sql="sum(b)"), + ] + + sql, _ = expand_tokens("SELECT {{total}} FROM returns", records) + + assert sql == "SELECT (sum(b)) FROM returns" + + +def test_an_ambiguous_bare_token_says_how_to_qualify_it() -> None: + records = [record("total", table="sales"), record("total", table="returns")] + + with pytest.raises(ValueError) as caught: + expand_tokens("SELECT {{total}} FROM sales JOIN returns", records) + + assert "{{sales::total}}" in str(caught.value) + assert "{{returns::total}}" in str(caught.value) + + +def test_a_token_whose_table_is_absent_from_the_query_errors() -> None: + with pytest.raises(ValueError, match="does not appear in this query"): + expand_tokens("SELECT {{net_revenue}} FROM other", [record("net_revenue")]) + + +def test_an_unknown_token_lists_what_is_available() -> None: + with pytest.raises(ValueError) as caught: + expand_tokens("SELECT {{nope}} FROM sales", [record("net_revenue")]) + + assert "{{net_revenue}} (sales)" in str(caught.value) + + +def test_an_unknown_token_against_no_definitions_says_so() -> None: + with pytest.raises(ValueError, match="no governed definitions"): + expand_tokens("SELECT {{nope}} FROM sales", []) + + +def test_a_dotted_token_is_read_as_table_qualified() -> None: + sql, _ = expand_tokens( + "SELECT {{sales.net_revenue}} FROM sales", [record("net_revenue")] + ) + + assert sql == "SELECT (sum(revenue)) FROM sales" + + +def test_a_qualified_token_whose_table_is_absent_from_the_query_errors() -> None: + with pytest.raises(ValueError, match="does not appear in this query"): + expand_tokens( + "SELECT {{returns::total}} FROM sales", [record("total", table="returns")] + ) + + +def test_an_unknown_qualified_token_lists_what_is_available() -> None: + with pytest.raises(ValueError, match="No governed definition matches"): + expand_tokens("SELECT {{sales::nope}} FROM sales", [record("total")]) + + +def test_an_unknown_dotted_token_lists_what_is_available() -> None: + with pytest.raises(ValueError, match="No governed definition matches"): + expand_tokens("SELECT {{sales.nope}} FROM sales", [record("total")]) + + +def test_a_dotted_token_whose_table_is_absent_from_the_query_errors() -> None: + with pytest.raises(ValueError, match="does not appear in this query"): + expand_tokens( + "SELECT {{other.total}} FROM sales", [record("total", table="other")] + ) + + +def test_a_definition_name_may_contain_spaces() -> None: + sql, _ = expand_tokens( + "SELECT {{net revenue}} FROM sales", [record("net revenue", sql="sum(x)")] + ) + + assert sql == "SELECT (sum(x)) FROM sales" + + +def test_backslashes_in_expanded_sql_survive() -> None: + escaped = record("pattern", sql=r"regexp_matches(x, '\d+')") + + sql, _ = expand_tokens("SELECT {{pattern}} FROM sales", [escaped]) + + assert sql == r"SELECT (regexp_matches(x, '\d+')) FROM sales" + + +def test_the_same_token_twice_expands_both_occurrences() -> None: + sql, applied = expand_tokens( + "SELECT {{total}}, {{total}} FROM sales", [record("total", sql="sum(a)")] + ) + + assert sql == "SELECT (sum(a)), (sum(a)) FROM sales" + assert len(applied) == 1 + + +# ---- the prompt index ---------------------------------------------------- + + +def test_the_index_groups_definitions_by_table_and_kind() -> None: + registry = Registry( + [ + record("net_revenue", kind="metric"), + record("is_emea", kind="filter"), + record("list_price", kind="derived"), + ] + ) + + assert index_text(registry) == ( + "- sales: filters `{{is_emea}}`; derived `{{list_price}}`; " + "metrics `{{net_revenue}}`" + ) + + +def test_a_label_is_the_index_hint() -> None: + registry = Registry([record("is_emea", kind="filter", label="EMEA segment")]) + + assert index_text(registry) == "- sales: filters `{{is_emea}}` (EMEA segment)" + + +def test_the_index_scopes_by_source_when_there_are_several() -> None: + registry = Registry( + [record("a", source="one"), record("b", source="two", table="other")] + ) + + text = index_text(registry) + + assert "- one.sales:" in text + assert "- two.other:" in text + + +def test_the_index_is_capped_and_reports_its_overflow() -> None: + registry = Registry([record(f"filter_{i:03d}", kind="filter") for i in range(400)]) + + assert index_overflows(registry) + # One line per table, so a roster this size is a single long line: the cap + # drops it entirely rather than truncating mid-token. + assert index_text(registry) == "" + assert not index_overflows(registry, cap_chars=100_000) + assert "filter_399" in index_text(registry, cap_chars=100_000) + + +def test_an_empty_registry_has_an_empty_index() -> None: + assert index_text(Registry([])) == "" + assert not index_overflows(Registry([])) + + +# ---- rendering ----------------------------------------------------------- + + +def test_first_touch_shows_compiled_sql_and_not_the_expression() -> None: + text = entry_text([record("net_revenue", sql="sum(revenue)")]) + + assert text is not None + assert "Governed definitions" in text + assert "`{{net_revenue}}`" in text + assert "Selected SQL(duckdb)" in text + assert "SUM(revenue)" not in text + + +def test_first_touch_carries_translation_notes() -> None: + text = entry_text([record("remainder", notes=["integer modulus by zero differs"])]) + + assert text is not None + assert "Translation notes: integer modulus by zero differs" in text + + +def test_first_touch_is_none_without_definitions() -> None: + assert entry_text([]) is None + + +def test_definitions_are_indexed_as_context_chunks() -> None: + # A retrieved chunk has to say what the token expands to, or retrieval + # surfaces a name the model cannot use. + chunks = context_chunks([record("net_revenue", sql="sum(revenue)")]) + + assert len(chunks) == 1 + assert chunks[0].startswith( + "Governed definition `{{net_revenue}}` on table `sales` (metric, number)" + ) + assert "Selected SQL(duckdb): `(sum(revenue))`." in chunks[0] + + +def test_a_context_chunk_shows_compiled_sql_and_not_the_expression() -> None: + chunks = context_chunks([record("net_revenue", sql="sum(revenue)")]) + + assert "SUM(revenue)" not in chunks[0] + + +def test_a_context_chunk_carries_translation_notes() -> None: + chunks = context_chunks([record("remainder", notes=["modulus by zero differs"])]) + + assert "Translation notes: modulus by zero differs" in chunks[0] + + +def test_applied_text_reports_the_sql_each_token_became() -> None: + text = applied_text([record("net_revenue", sql="sum(revenue)")]) + + assert text is not None + assert "- {{net_revenue}} (sales): SQL(duckdb) `(sum(revenue))`" in text + + +def test_applied_text_is_none_when_nothing_expanded() -> None: + assert applied_text([]) is None + + +# ---- grain and construction ---------------------------------------------- + + +def test_grain_metadata_rides_on_the_record() -> None: + # call_metrics' mixed-grain guard reads this and nothing else; the shape + # that produced it stays behind the seam. + assert record("net_revenue", mixed_grain=True).mixed_grain is True + + +def test_definitions_on_an_unexposed_table_fail_construction() -> None: + from commons import _duckdb + from commons._backends import DuckDBBackend + from commons._data_dictionary import DataDictionary, Table + from commons._data_source import DataSource + from commons._definitions import build_registry + + dictionary = DataDictionary(tables={"ghost": Table()}) + dictionary.tables["ghost"].compiled_definitions = [record("total", table="ghost")] + source = DataSource( + backend=DuckDBBackend(_duckdb.connect()), + tables=["sales"], + dictionary=dictionary, + ) + + with pytest.raises(ValueError, match="ghost"): + build_registry({"sales_db": source}) + + +def test_the_registry_labels_records_with_their_source() -> None: + from commons import _duckdb + from commons._backends import DuckDBBackend + from commons._data_dictionary import DataDictionary, Table + from commons._data_source import DataSource + from commons._definitions import build_registry + + dictionary = DataDictionary(tables={"sales": Table()}) + dictionary.tables["sales"].compiled_definitions = [record("total", source="")] + source = DataSource( + backend=DuckDBBackend(_duckdb.connect()), + tables=["sales"], + dictionary=dictionary, + ) + + registry = build_registry({"sales_db": source}) + + assert [item.source for item in registry.records] == ["sales_db"] + assert registry.for_source("nope") == [] + + +def test_an_empty_source_set_builds_an_empty_registry() -> None: + from commons._definitions import build_registry + + assert build_registry({}).records == [] + + +def test_the_registry_merges_sources_and_for_source_filters_by_label() -> None: + from commons import _duckdb + from commons._backends import DuckDBBackend + from commons._data_dictionary import DataDictionary, Table + from commons._data_source import DataSource + from commons._definitions import build_registry + + def source_with(definition_name: str) -> DataSource: + dictionary = DataDictionary(tables={"sales": Table()}) + dictionary.tables["sales"].compiled_definitions = [ + record(definition_name, source="") + ] + return DataSource( + backend=DuckDBBackend(_duckdb.connect()), + tables=["sales"], + dictionary=dictionary, + ) + + registry = build_registry( + {"one": source_with("total"), "two": source_with("count_all")} + ) + + assert sorted(item.source for item in registry.records) == ["one", "two"] + assert [item.name for item in registry.for_source("one")] == ["total"] + assert [item.name for item in registry.for_source("two")] == ["count_all"] + assert len(registry.for_source()) == 2 + + +# ---- against the shared contract ----------------------------------------- +# +# The first Python code to consume the definitions contract. The compiler does +# not exist yet, so these read records data-dict itself produced, which is +# what the registry will consume either way. + + +def fixture_records() -> list[ExportRecord]: + from ._shared import load_shared_fixture + + spec = load_shared_fixture("definitions") + records = [] + for name, cases in spec["export_records"].items(): + for key, payload in cases.items(): + table, _, definition = key.partition("::") + translation = payload["translation"] + records.append( + ExportRecord( + name=definition, + table=table, + source=name, + kind=payload["kind"], + type=payload["type"], + expression=payload["expression"], + label=None, + description=None, + details=None, + columns=list(payload["columns"]), + definitions=list(payload["definitions"]), + sql=translation["code"], + target=translation["target"], + notes=list(translation["notes"]), + mixed_grain=spec["mixed_grain"][name][key], + ) + ) + return records + + +def test_the_fixture_yields_records() -> None: + assert len(fixture_records()) > 0 + + +def test_every_fixture_definition_expands_to_its_compiled_sql() -> None: + for item in fixture_records(): + sql, applied = expand_tokens( + f"SELECT {{{{{item.table}::{item.name}}}}} FROM {item.table}", [item] + ) + + assert sql == f"SELECT ({item.sql}) FROM {item.table}", item.name + assert applied == [item] + + +def test_every_fixture_kind_is_one_the_index_groups() -> None: + # An unrecognised kind would silently vanish from the prompt index. + for item in fixture_records(): + assert item.kind in _GROUPS_FOR_TEST, item.name + + +def test_the_index_renders_every_fixture_definition() -> None: + registry = Registry(fixture_records()) + + text = index_text(registry, cap_chars=1_000_000) + + for item in registry.records: + assert f"`{{{{{item.name}}}}}`" in text, item.name + + +def test_first_touch_renders_every_fixture_definition() -> None: + records = fixture_records() + text = entry_text(records) + + assert text is not None + for item in records: + assert f"`{{{{{item.name}}}}}`" in text, item.name + + +def test_a_typeless_definition_omits_the_type_rather_than_naming_it() -> None: + # data-dict omits `type` when no single one is inferred. Printing the + # language's null spelling would put "None" in the prompt, so the type is + # left out and the rest of the gist still reaches the model. + typeless = [item for item in fixture_records() if item.type is None] + assert len(typeless) == 1 + + chunk = context_chunks(typeless)[0] + + assert "None" not in chunk + assert "(derived)" in chunk + assert "Selected SQL(duckdb)" in chunk + + +def test_grain_metadata_reaches_the_records() -> None: + mixed = [item for item in fixture_records() if item.mixed_grain] + + assert mixed + assert not all(item.mixed_grain for item in fixture_records()) + + +def test_an_expansion_cannot_widen_scope_for_a_later_token() -> None: + # Compiled SQL can name a table the query does not. Resolving later tokens + # against already-expanded text would let that count as the table being + # present, which is the check that keeps a token bound to its own table. + records = [ + record("total", table="sales", sql="sum(returns.x)"), + record("other", table="returns", sql="count(*)"), + ] + + with pytest.raises(ValueError, match="does not appear in this query"): + expand_tokens("SELECT {{sales::total}}, {{other}} FROM sales", records) + + +def test_the_index_cap_counts_the_lines_it_joins() -> None: + registry = Registry( + [record(f"d{i}", table=f"t{i}", kind="metric") for i in range(30)] + ) + + assert len(index_text(registry, cap_chars=300)) <= 300 diff --git a/pkg-r/tests/testthat/fixtures/shared/definition-rendering.json b/pkg-r/tests/testthat/fixtures/shared/definition-rendering.json index 7df1d35a..2d5fe68d 100644 --- a/pkg-r/tests/testthat/fixtures/shared/definition-rendering.json +++ b/pkg-r/tests/testthat/fixtures/shared/definition-rendering.json @@ -1,5 +1,5 @@ { - "description": "How both packages expand governed-definition tokens and render definitions into the prompt. The source is tests/shared/definition-rendering.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. The R suite runs these cases against the implementation; the Python suite checks the fixture's integrity until its registry lands, then runs them too.", + "description": "How both packages expand governed-definition tokens and render definitions into the prompt. The source is tests/shared/definition-rendering.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Both suites run these cases against their own implementation.", "records": { "description": "Export records keyed by a fixture-local id, in the shape both packages hydrate: pkg-py builds ExportRecord, pkg-r builds a row of the registry data frame. A null scalar is a field data-dict omitted.", "values": { @@ -116,6 +116,26 @@ "target": "SQL(duckdb)", "notes": [], "mixed_grain": false + }, + "sales_prose": { + "name": "region_list", + "table": "sales", + "source": "warehouse", + "kind": "derived", + "type": "string", + "expression": "CONCAT(region, country)", + "label": null, + "description": "Region and country.\n Kept verbatim.", + "details": null, + "columns": [ + "region", + "country" + ], + "definitions": [], + "sql": "concat(\"region\", country)", + "target": "SQL(duckdb)", + "notes": [], + "mixed_grain": false } } }, @@ -178,6 +198,26 @@ "applied": [], "reason": "unknown_token" }, + { + "name": "a qualified token whose table is absent is refused", + "records": [ + "returns_other" + ], + "sql": "SELECT {{returns::other}} FROM sales", + "expanded": null, + "applied": [], + "reason": "table_not_in_query" + }, + { + "name": "a qualified token no definition matches is refused", + "records": [ + "sales_total" + ], + "sql": "SELECT {{sales::nope}} FROM sales", + "expanded": null, + "applied": [], + "reason": "unknown_token" + }, { "name": "a bare token defined on two tables in the query is refused", "records": [ @@ -205,7 +245,7 @@ }, "gist": { "description": "The one-line summary rendered for a definition, used in the first-touch entry and in every retrieval chunk. Shows the compiled SQL, never the authored expression.", - "rule": "`(kind, type)` when data-dict inferred a type, `(kind)` alone when it did not. The rest of the gist is unaffected by an absent type.", + "rule": "`(kind, type)` when data-dict inferred a type, `(kind)` alone when it did not. The rest of the gist is unaffected by an absent type. Multi-line prose flattens onto one line by collapsing each newline and the whitespace around it; other whitespace stays as authored.", "cases": [ { "name": "kind and type, with a description", @@ -221,6 +261,11 @@ "name": "an absent type is left out and the rest of the gist survives", "record": "survey_mixed_temporal", "expected": "(derived) When the response was captured. Selected SQL(duckdb): `(CASE WHEN \"done\" THEN \"taken_on\" ELSE \"taken_at\" END)`." + }, + { + "name": "newlines flatten but authored whitespace survives", + "record": "sales_prose", + "expected": "(derived, string) Region and country. Kept verbatim. Selected SQL(duckdb): `(concat(\"region\", country))`." } ] }, diff --git a/tests/shared/README.md b/tests/shared/README.md index f54deb26..44b1d522 100644 --- a/tests/shared/README.md +++ b/tests/shared/README.md @@ -20,12 +20,18 @@ Most Posit R/Python pairs (ellmer/chatlas, ragnar/raghilda, shiny/py-shiny) shar The Python suite reads this directory directly. The R suite cannot. `testthat` needs its fixtures inside the package, and an installed R package cannot reach files outside its own directory. The R suite reads a copy synced into `pkg-r/tests/testthat/fixtures/shared/`. That copy is committed, `scripts/sync-shared-fixtures.sh` generates it, and a CI job re-runs the script and fails when the copy is stale. -## What belongs here +## What sort of fixtures are here - **Span names and attributes.** `commons_conversation_turn`, `commons_agent_create`, `commons_data_source_create`, and friends. Also `gen_ai.conversation.id`, `commons.provenance.tag`, and the exact JSON shape of `commons.citation.candidates`. This contract lets the R trajectory reviewer read Python traces. Write it so that it survives the conversation-id ownership moving upstream to shinychat. - **The provenance and citation behavior.** The `derive_provenance_tag()` truth table, `normalize_citation()` input/output pairs, `match_citation()` verdicts including both guards (10-character minimum, only-the-quote-verifies), `parse_commons_citation()` well-formed and malformed bodies. Also the chunk-invariance cases of the streaming scanner. The scanner is a pure chunks-in/string-out function, so it is ideal fixture material. - **The citation dialect and display copy.** The `` grammar and the `PROVENANCE_DISPLAY` strings, so that both UIs say the same words. -- **The definitions interface.** `definitions.json` pins the export-record contract both packages consume, the grain metadata `call_metrics` needs for its mixed-grain guard, and the data-dict problem code each invalid fixture must produce. `definition-export/` is the conformance corpus itself, read by both suites rather than copied into either. `export_records` is generated from the data-dict binary at the pinned commit by `scripts/generate-definitions-fixture.sh`, which refuses to run against a binary built from anything else; `mixed_grain` and `invalid` are hand-maintained and the generator preserves them. This fixture does not replace the conformance harness: the harness compares against a real binary, while this pins what both packages agree to consume. +- **The definitions interface.** `definition-export/` holds the shared fixtures: 14 data dictionaries in data-dict's YAML format, each declaring table-level `definitions` whose expressions use data-dict's expression language — 3 valid files (42 definitions) and 11 invalid ones — read by both commons implementations. `definitions.json` pins what both packages agree to produce from them, in three sections: + - `export_records` — the expected export for each valid definition: its SQL translation, its inferred kind and type, and the columns and definitions it references. Generated from the data-dict binary at the pinned commit by `scripts/generate-definitions-fixture.sh`, which refuses to run against a binary built from anything else. Never hand-edit. + - `mixed_grain` — a per-definition boolean for `call_metrics`' mixed-grain guard: true when a definition's exported shape is `row` but its expression contains an aggregate, directly or through a definition it references. Absent from data-dict's export (it exists only in the compiler's internal parse tree), so it is hand-maintained and the generator preserves it. + - `invalid` — the data-dict problem code each invalid fixture must produce (e.g. `cycle.yaml` must fail with the cycle error, not a generic parse failure). Hand-maintained; the generator preserves it. + + This fixture does not replace the conformance harness: the harness compares against a real binary, while this pins what both packages agree to consume. +- **Definition expansion and rendering.** `definition-rendering.json` pins what happens to a governed definition after the compiler is done with it: which `{{token}}` queries expand and to what, the one-line gist shown at first touch and in retrieval, and the kind index under a character cap. It carries a bank of export records that each package hydrates into its own shape. A refused query pins the refusal and a reason slug rather than the message, because the wording belongs to each language. - **Trace file naming.** `trace(-[0-9]+)?\.jsonl`, one OTLP envelope per line. ## Conventions diff --git a/tests/shared/definition-rendering.json b/tests/shared/definition-rendering.json index 7df1d35a..2d5fe68d 100644 --- a/tests/shared/definition-rendering.json +++ b/tests/shared/definition-rendering.json @@ -1,5 +1,5 @@ { - "description": "How both packages expand governed-definition tokens and render definitions into the prompt. The source is tests/shared/definition-rendering.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. The R suite runs these cases against the implementation; the Python suite checks the fixture's integrity until its registry lands, then runs them too.", + "description": "How both packages expand governed-definition tokens and render definitions into the prompt. The source is tests/shared/definition-rendering.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Both suites run these cases against their own implementation.", "records": { "description": "Export records keyed by a fixture-local id, in the shape both packages hydrate: pkg-py builds ExportRecord, pkg-r builds a row of the registry data frame. A null scalar is a field data-dict omitted.", "values": { @@ -116,6 +116,26 @@ "target": "SQL(duckdb)", "notes": [], "mixed_grain": false + }, + "sales_prose": { + "name": "region_list", + "table": "sales", + "source": "warehouse", + "kind": "derived", + "type": "string", + "expression": "CONCAT(region, country)", + "label": null, + "description": "Region and country.\n Kept verbatim.", + "details": null, + "columns": [ + "region", + "country" + ], + "definitions": [], + "sql": "concat(\"region\", country)", + "target": "SQL(duckdb)", + "notes": [], + "mixed_grain": false } } }, @@ -178,6 +198,26 @@ "applied": [], "reason": "unknown_token" }, + { + "name": "a qualified token whose table is absent is refused", + "records": [ + "returns_other" + ], + "sql": "SELECT {{returns::other}} FROM sales", + "expanded": null, + "applied": [], + "reason": "table_not_in_query" + }, + { + "name": "a qualified token no definition matches is refused", + "records": [ + "sales_total" + ], + "sql": "SELECT {{sales::nope}} FROM sales", + "expanded": null, + "applied": [], + "reason": "unknown_token" + }, { "name": "a bare token defined on two tables in the query is refused", "records": [ @@ -205,7 +245,7 @@ }, "gist": { "description": "The one-line summary rendered for a definition, used in the first-touch entry and in every retrieval chunk. Shows the compiled SQL, never the authored expression.", - "rule": "`(kind, type)` when data-dict inferred a type, `(kind)` alone when it did not. The rest of the gist is unaffected by an absent type.", + "rule": "`(kind, type)` when data-dict inferred a type, `(kind)` alone when it did not. The rest of the gist is unaffected by an absent type. Multi-line prose flattens onto one line by collapsing each newline and the whitespace around it; other whitespace stays as authored.", "cases": [ { "name": "kind and type, with a description", @@ -221,6 +261,11 @@ "name": "an absent type is left out and the rest of the gist survives", "record": "survey_mixed_temporal", "expected": "(derived) When the response was captured. Selected SQL(duckdb): `(CASE WHEN \"done\" THEN \"taken_on\" ELSE \"taken_at\" END)`." + }, + { + "name": "newlines flatten but authored whitespace survives", + "record": "sales_prose", + "expected": "(derived, string) Region and country. Kept verbatim. Selected SQL(duckdb): `(concat(\"region\", country))`." } ] },