From 007562436f068afbf6c382980e131629f63be7e0 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:08:29 +0100 Subject: [PATCH 1/4] feat(native): report macro errors natively without re-running Python --- crates/README.md | 12 +- .../_helpers/corpus/render_failure_cases.py | 49 ++++ .../references/docs/concepts/macros.md | 2 + .../macro_bridge/_helpers/splicing.py | 23 ++ .../macro_bridge/classes/macro_bridge.py | 43 ++-- .../main/run_with_macro_bridge.py | 47 ++-- .../cli/commands/main/compile/_test_types.py | 21 +- .../main/compile/test_native_macro_errors.py | 215 ++++++++++++++++++ .../sqlbuild/compiler/compile/_test_types.py | 15 ++ .../src/sqlbuild/compiler/compile/helpers.py | 200 +++++++++++++++- .../compile/test_native_macro_bridge.py | 44 ++++ .../compiler/macro_bridge/main/_test_types.py | 12 +- .../compiler/macro_bridge/main/helpers.py | 22 +- .../main/test_run_with_macro_bridge.py | 71 ++---- .../src/content/docs/docs/concepts/macros.mdx | 2 + 15 files changed, 652 insertions(+), 126 deletions(-) create mode 100644 src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py diff --git a/crates/README.md b/crates/README.md index 2f50320c3b..414d3707ea 100644 --- a/crates/README.md +++ b/crates/README.md @@ -64,12 +64,12 @@ stages run natively: The shipped tier covers discovery and rendering: declaration files and scopes, model config, reference extraction, the model loop, macro calls and the macro-call store, and attachments. SQL -analysis, contracts, lineage, SQL-test glue and project assembly are still `preview`. When a -native render stage fails, the compile runs the Python stage again so the error is exactly -Python's; native error messages replace that re-run later. Because the macro-call store is -shipped, the [macro determinism contract](../website/src/content/docs/docs/concepts/macros.mdx) -applies to every default compile; `SQLBUILD_COMPILER_ENGINE=python` runs every macro call each -time. +analysis, contracts, lineage, SQL-test glue and project assembly are still `preview`. A failing +render raises its first error directly, as the Python stage would, and runs each macro call at +most once; a divergence the bridge detects is reported as a native stage mismatch. Because the +macro-call store is shipped, the +[macro determinism contract](../website/src/content/docs/docs/concepts/macros.mdx) applies to +every default compile; `SQLBUILD_COMPILER_ENGINE=python` runs every macro call each time. Each native stage declares its tier once, in `NATIVE_STAGE_TIERS` in `src/sqlbuild/compiler/frontier/constants.py`. A stage moves from `preview` to `shipped` by diff --git a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py index 1cfe9d0c6c..aa3cd6717e 100644 --- a/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/render_failure_cases.py @@ -23,6 +23,10 @@ 'def cents(expression: str) -> str:\n """Convert to cents."""\n' ' return f"({expression} * 100)"\n' ) +_EXPLODE_MACRO: str = ( + '\n\ndef explode(expression: str) -> str:\n """Fail while rendering."""\n' + ' raise ValueError(f"cannot render {expression}")\n' +) _ORDERS_FOR: str = ( "FUNCTION (\n" ' description "Orders for one customer",\n' @@ -93,6 +97,51 @@ def _macro_cases() -> tuple[FailureCase, ...]: }, ), ), + failure_case( + name="macro-raises-after-a-call", + expected_code="P001", + expected_message="failed: cannot render status", + files=_staging_columns( + columns='@cents("amount") AS amount, @explode("status") AS status', + files={_STAGING_MACROS: _CENTS_MACRO + _EXPLODE_MACRO}, + ), + ), + failure_case( + name="macro-arguments-do-not-parse", + expected_code="P001", + expected_message="could not be parsed", + files=_staging_columns( + columns='amount, status, @cents("amount",,) AS cents', + files={_STAGING_MACROS: _CENTS_MACRO}, + ), + ), + failure_case( + name="macro-private-helper", + expected_code="P001", + expected_message="Unknown macro '@_scaled'", + files=_staging_columns( + columns='amount, status, @_scaled("amount") AS scaled', + files={ + _STAGING_MACROS: _CENTS_MACRO + + "\n\ndef _scaled(expression: str) -> str:\n return expression\n" + }, + ), + ), + failure_case( + name="macro-first-of-two-failing-models", + expected_code="P001", + expected_message="failed: cannot render total", + files={ + **_staging_columns( + columns='amount, @explode("status") AS status', + files={_STAGING_MACROS: _CENTS_MACRO + _EXPLODE_MACRO}, + ), + "macros/explode.py": _EXPLODE_MACRO.replace("def explode", "def blow_up"), + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + "SUM(amount)", 'SUM(@blow_up("total"))' + ), + }, + ), failure_case( name="macro-output-calls-macro", expected_code="P001", diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md index 0068663e6c..15e69e1124 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros.md @@ -225,6 +225,8 @@ SQLBuild relies on this contract to reuse macro results: - [Compile reuse](../cli/compile.md) replays an unchanged compile without running macros again. - A compile may run each distinct macro call once and reuse its result for identical calls elsewhere in the project. +- A compile that fails stops at its first error. It does not run the project's macros a second + time to report that error. - Macro call results are kept between compiles under `target/cache/` by default, so editing a model does not run its unchanged macro calls again. They are recomputed when macro or other Python files, configuration, declarations, seeds, sources, variables, the target, diff --git a/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py b/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py new file mode 100644 index 0000000000..f160f2f1e8 --- /dev/null +++ b/src/sqlbuild/compiler/macro_bridge/_helpers/splicing.py @@ -0,0 +1,23 @@ +"""Splice macro outputs in Python for text the native splice cannot read.""" + +from __future__ import annotations + + +def splice_text( + *, sql: str, bounds: list[tuple[int, int]], outputs: list[str] +) -> tuple[str, list[tuple[int, int, int, int]]]: + """Replace each `(start, end)` span with its output, as `splice_macro_calls` does.""" + + parts: list[str] = [] + spans: list[tuple[int, int, int, int]] = [] + cursor: int = 0 + length: int = 0 + for (start, end), output in zip(bounds, outputs, strict=True): + parts.append(sql[cursor:start]) + length += start - cursor + parts.append(output) + spans.append((start, end, length, length + len(output))) + length += len(output) + cursor = end + parts.append(sql[cursor:]) + return "".join(parts), spans diff --git a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py index e2d317fa45..d680cf40d8 100644 --- a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py @@ -14,6 +14,7 @@ LoadedMacro, MacroContext, ) +from sqlbuild.compiler.macro_bridge._helpers.splicing import splice_text from sqlbuild.compiler.macro_bridge._helpers.store_environment import ( digest_module_files, module_digests_metadata, @@ -51,7 +52,6 @@ def __init__(self, *, python_version: tuple[int, int], unicode_version: str) -> self._module_digests: dict[str, str] = {} self._observed_modules: dict[str, object] = {} self._project_inputs: tuple[Path, list[str], str] = (Path(), [], "") - self._scanned: bool = False def attach_store(self, *, cache_dir: Path, project_dir: Path, model_paths: list[str]) -> None: """Reuse call results stored by earlier compiles of the same code and project files.""" @@ -134,10 +134,12 @@ def _observe_modules(self) -> bool: def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: """Return the top-level call sites of `sql`, or None when Python must expand it.""" - self._scanned = True - rows: list[tuple[int, int, str, list[str], bool]] | None = _native.scan_macro_call_sites( - sql, self._python_version, self._unicode_version - ) + try: + rows: list[tuple[int, int, str, list[str], bool]] | None = ( + _native.scan_macro_call_sites(sql, self._python_version, self._unicode_version) + ) + except UnicodeEncodeError: + return None if rows is None: return None return tuple( @@ -151,12 +153,6 @@ def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: for start, end, name, tree_names, typed_reference_text in rows ) - @property - def scanned(self) -> bool: - """Whether any authored string has been scanned, so this compile left the Python path.""" - - return self._scanned - def call_class( self, *, @@ -249,11 +245,18 @@ def record( if self._store_trusted and len(sys.modules) != len(self._observed_modules): _ = self._observe_modules() - self._memo.record( - class_id, - call_text, - (sql, [(relation.kind.value, relation.name) for relation in relations], list(events)), - ) + try: + self._memo.record( + class_id, + call_text, + ( + sql, + [(relation.kind.value, relation.name) for relation in relations], + list(events), + ), + ) + except UnicodeEncodeError: + return def splice( self, *, sql: str, sites: tuple[MacroCallSite, ...], outputs: list[str] @@ -262,9 +265,11 @@ def splice( rendered: str spans: list[tuple[int, int, int, int]] - rendered, spans = _native.splice_macro_calls( - sql, [(site.start, site.end) for site in sites], outputs - ) + bounds: list[tuple[int, int]] = [(site.start, site.end) for site in sites] + try: + rendered, spans = _native.splice_macro_calls(sql, bounds, outputs) + except UnicodeEncodeError: + rendered, spans = splice_text(sql=sql, bounds=bounds, outputs=outputs) return rendered, tuple( ExpansionSpan( source_start=source_start, diff --git a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py index c30adf809f..fcaa59d694 100644 --- a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py @@ -1,4 +1,4 @@ -"""Run one render stage through the native macro bridge, re-running Python on its failures.""" +"""Run one render stage through the native macro bridge, storing its calls on success.""" from __future__ import annotations @@ -9,14 +9,12 @@ from contextvars import Token import sqlbuild._native as _native -from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.constants import ACTIVE_MACRO_BRIDGE, STORE_MODULE_PATHS def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: - """Run `stage` with the bridge, storing calls on success; failures it could shape re-run.""" + """Run `stage` once with the bridge; its errors are the Python stage's, raised as they occur.""" python_version: tuple[int, int] = (sys.version_info[0], sys.version_info[1]) if not _native.native_text_supported(python_version, unicodedata.unidata_version): @@ -25,34 +23,21 @@ def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: python_version=python_version, unicode_version=unicodedata.unidata_version ) token: Token[object | None] = ACTIVE_MACRO_BRIDGE.set(bridge) - native_error: Exception | None = None try: result: T = stage() - except Exception as error: - if not bridge.scanned or ( - isinstance(error, CompileInputError) and error.bridge_independent - ): - raise - native_error = error - else: - bridge.save_store() - STORE_MODULE_PATHS.update(bridge.store_module_paths()) - hits, misses, recorded = bridge.stats() - store_hits, store_records = bridge.store_stats() - logging.getLogger(__name__).debug( - "Native macro bridge: %d memo hits, %d store hits, %d misses, %d recorded calls, " - "%d stored calls", - hits, - store_hits, - misses, - recorded, - store_records, - ) - return result finally: ACTIVE_MACRO_BRIDGE.reset(token) - _ = stage() - raise NativeStageMismatchError( - "The native macro bridge failed where the Python render succeeded; " - "run with SQLBUILD_COMPILER_ENGINE=python" - ) from native_error + bridge.save_store() + STORE_MODULE_PATHS.update(bridge.store_module_paths()) + hits, misses, recorded = bridge.stats() + store_hits, store_records = bridge.store_stats() + logging.getLogger(__name__).debug( + "Native macro bridge: %d memo hits, %d store hits, %d misses, %d recorded calls, " + "%d stored calls", + hits, + store_hits, + misses, + recorded, + store_records, + ) + return result diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index a1f174b8ff..2eb571f5e1 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -943,6 +943,25 @@ class ReferenceScanErrorTestCase: expected_error_types: tuple[str, ...] +@dataclass(frozen=True) +class NativeMacroErrorTestCase: + """A failing macro project every engine must report identically, running each call once.""" + + description: str + project_files: dict[str, str] + expected_report_text: str + expected_macro_runs: list[str] + + +@dataclass(frozen=True) +class MacroErrorLifecycleTestCase: + """A failing macro project whose invocation lifecycle every engine reports the same way.""" + + description: str + project_files: dict[str, str] + expected_error_types: tuple[str, ...] + + @dataclass(frozen=True) class NativeConfigErrorTestCase: """A failing project whose model config error both engines must report the same way.""" @@ -956,7 +975,7 @@ class NativeConfigErrorTestCase: @dataclass(frozen=True) class MacroExpandedValidatorErrorTestCase: - """A validator error from macro-expanded SQL that the preview bridge must re-run.""" + """A validator error from macro-expanded SQL that every engine reports, running macros once.""" description: str project_files: dict[str, str] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py new file mode 100644 index 0000000000..eec6cd6f8e --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_macro_errors.py @@ -0,0 +1,215 @@ +"""Every engine reports macro errors as Python does and runs each macro call once.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from scripts.compiler_differential.constants import FAILURE_BASE_FILES, FAILURE_BASE_MART +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + MacroErrorLifecycleTestCase, + NativeMacroErrorTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + CompileReuseRun, + lifecycle_error_type, + report_without_engine, + run_reuse_compile, + stderr_without_durations, + write_project_file, +) + +_ENGINES: tuple[str, ...] = ("python", "native", "native-preview") +_CALL_LOG: str = "macro_calls.log" +_MACROS: str = ( + "from pathlib import Path\n\n\n" + "def _log(name: str) -> None:\n" + f" with (Path(__file__).resolve().parents[1] / {_CALL_LOG!r}).open(\n" + ' "a", encoding="utf-8"\n' + " ) as log:\n" + ' _ = log.write(f"{name}\\n")\n\n\n' + "def counted(expression: str) -> str:\n" + ' """Return the expression, logging each execution."""\n' + ' _log(f"counted {expression}")\n' + " return expression\n\n\n" + "def explode(expression: str) -> str:\n" + ' """Fail after logging the call."""\n' + ' _log(f"explode {expression}")\n' + ' raise ValueError(f"cannot render {expression}")\n\n\n' + "def answer() -> int:\n" + ' """Return a value that is not SQL."""\n' + ' _log("answer")\n' + " return 42\n\n\n" + "def upstream() -> str:\n" + ' """Read a model that does not exist."""\n' + ' _log("upstream")\n' + " return '__ref(\"ghost\")'\n" +) +_MART_MACROS: str = ( + "def mart_only(expression: str) -> str:\n" + ' """Qualify an expression for marts."""\n' + " return expression\n" +) +_CYCLIC_MACROS: str = ( + 'def ping(expression: str) -> str:\n """Call pong."""\n return pong(expression)\n\n\n' + 'def pong(expression: str) -> str:\n """Call ping."""\n return ping(expression)\n' +) +_COUNTED_MART: str = FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@counted('amount'))") +_STAGING_PATH: str = "models/staging/stg_orders.sql" +_MART_PATH: str = "models/marts/customer_totals.sql" +_STAGING_PREFIX: str = ( + 'MODEL (\n description "Staged orders",\n);\n\nSELECT order_id, customer_id, ' +) +_STAGING_SUFFIX: str = ' AS amount, status\nFROM __source("raw_orders")\n' + + +@pytest.mark.parametrize( + "test_case", + [ + NativeMacroErrorTestCase( + description="macro_raises_after_a_side_effect", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX}, + expected_report_text="stg_orders.sql' failed: cannot render amount", + expected_macro_runs=["counted amount", "explode amount"], + ), + NativeMacroErrorTestCase( + description="macro_returns_a_value_that_is_not_sql", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@answer()" + _STAGING_SUFFIX}, + expected_report_text="must return a SQL string when used directly in SQL", + expected_macro_runs=["counted amount", "answer"], + ), + NativeMacroErrorTestCase( + description="macro_arguments_do_not_parse", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@counted('amount',,)" + _STAGING_SUFFIX + }, + expected_report_text="could not be parsed", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="unknown_macro", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@missing('amount')" + _STAGING_SUFFIX}, + expected_report_text="Unknown macro '@missing'", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="private_macro_helper", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@_log('amount')" + _STAGING_SUFFIX}, + expected_report_text="Unknown macro '@_log'", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="macro_in_a_sibling_scope", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@mart_only('amount')" + _STAGING_SUFFIX, + "models/marts/_sqlbuild/macros/local.py": _MART_MACROS, + }, + expected_report_text="stg_orders.sql' is inaccessible", + expected_macro_runs=["counted amount"], + ), + NativeMacroErrorTestCase( + description="macro_call_cycle", + project_files={"macros/cyclic.py": _CYCLIC_MACROS}, + expected_report_text="Macro call cycle in 'macros/cyclic.py': ping -> pong -> ping", + expected_macro_runs=[], + ), + NativeMacroErrorTestCase( + description="first_of_two_failing_models", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX, + _MART_PATH: FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@explode('total'))"), + }, + expected_report_text="customer_totals.sql' failed: cannot render total", + expected_macro_runs=["explode total"], + ), + NativeMacroErrorTestCase( + description="reference_error_in_macro_sql", + project_files={ + _STAGING_PATH: _STAGING_PREFIX + + "amount FROM @upstream() UNION ALL SELECT 1, 2, amount" + + _STAGING_SUFFIX + }, + expected_report_text="references unknown model 'ghost'", + expected_macro_runs=["counted amount", "upstream"], + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_error_when_compiling_with_each_engine_then_error_matches_python_once( + test_case: NativeMacroErrorTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "orders" + runs: list[CompileReuseRun] = [] + macro_runs: list[list[str]] = [] + for engine in _ENGINES: + shutil.rmtree(project_dir, ignore_errors=True) + for relative_path, contents in { + **FAILURE_BASE_FILES, + "macros/orders.py": _MACROS, + _MART_PATH: _COUNTED_MART, + _CALL_LOG: "", + **test_case.project_files, + }.items(): + write_project_file(project_dir, relative_path, contents) + runs.append( + run_reuse_compile(project_dir=project_dir, global_args=("--compiler-engine", engine)) + ) + macro_runs.append((project_dir / _CALL_LOG).read_text(encoding="utf-8").splitlines()) + + assert ( + tuple(run.returncode for run in runs), + {report_without_engine(run) for run in runs} == {report_without_engine(runs[0])}, + {stderr_without_durations(stderr=run.stderr) for run in runs} + == {stderr_without_durations(stderr=runs[0].stderr)}, + test_case.expected_report_text in runs[0].report + runs[0].stderr, + macro_runs, + ) == ( + (1,) * len(_ENGINES), + True, + True, + True, + [test_case.expected_macro_runs] * len(_ENGINES), + ), tuple(run.report + run.stderr for run in runs) + + +@pytest.mark.parametrize( + "test_case", + [ + MacroErrorLifecycleTestCase( + description="macro_raises_after_a_side_effect", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@explode('amount')" + _STAGING_SUFFIX}, + expected_error_types=("CompileInputError",) * 3, + ), + MacroErrorLifecycleTestCase( + description="unknown_macro", + project_files={_STAGING_PATH: _STAGING_PREFIX + "@missing('amount')" + _STAGING_SUFFIX}, + expected_error_types=("CompileInputError",) * 3, + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_error_when_compiling_with_each_engine_then_lifecycle_error_type_matches( + test_case: MacroErrorLifecycleTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + error_types: list[str] = [] + for engine in _ENGINES: + project_dir: Path = tmp_path / engine + for relative_path, contents in { + **FAILURE_BASE_FILES, + "macros/orders.py": _MACROS, + _MART_PATH: _COUNTED_MART, + _CALL_LOG: "", + **test_case.project_files, + }.items(): + write_project_file(project_dir, relative_path, contents) + error_types.append( + lifecycle_error_type(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + + assert tuple(error_types) == test_case.expected_error_types, test_case.description + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/compile/_test_types.py b/tests/integration/src/sqlbuild/compiler/compile/_test_types.py index 2b8f407f13..2267a116aa 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/compile/_test_types.py @@ -40,3 +40,18 @@ class MacroExpansionDifferentialTestCase: samples: int expected_minimum_successes: int expected_mismatches: list[tuple[object, object, object]] + + +@dataclass(frozen=True) +class FailingMacroProjectParityTestCase: + """Seeded projects whose failing macros every engine must report identically, running once.""" + + description: str + seed: int + projects: int + engines: tuple[str, ...] + expected_minimum_failures: int + expected_minimum_successes: int + expected_minimum_errors_after_macro_runs: int + expected_minimum_distinct_errors: int + expected_mismatches: list[tuple[object, object, object]] diff --git a/tests/integration/src/sqlbuild/compiler/compile/helpers.py b/tests/integration/src/sqlbuild/compiler/compile/helpers.py index fe10c48a49..eeb2dbc841 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/compile/helpers.py @@ -6,8 +6,11 @@ import re from collections.abc import Callable from contextvars import Token +from dataclasses import dataclass from functools import partial +from itertools import compress from pathlib import Path +from typing import cast import pytest @@ -39,6 +42,7 @@ from sqlbuild.spec.contracts.main.resolve_effective_collection_rendering import ( resolve_effective_collection_rendering, ) +from tests.integration.src.sqlbuild.compiler.helpers import mismatches MACRO_CALL_LOG_ENV_VAR: str = "SQLBUILD_TEST_MACRO_CALL_LOG" _RUN_ID_PATTERN: re.Pattern[str] = re.compile(r'"run_id": "[^"]*"') @@ -47,7 +51,8 @@ "@n(@mm(3), '@m(4)')", "@tag(__ref('orders'))", "@tag(__seed('rates'))", "@tag(__ref('customers'))", "@cols(__ref('orders'))", "@gen()", "@nope(1)", "@m(x)", "@ab c(1)", "@m('é')", "'@m(1)'", "-- @m(1)\n", "/* @n() */", "$$@m()$$", "`@m()`", "@@v", - "@", " ", "\n", "\u3000", "é", "'", "(", ")", "SELECT ", ", ", "\"q\"", + "@", " ", "\n", "\u3000", "é", "'", "(", ")", "SELECT ", ", ", "\"q\"", "'\udcff'", + "@m('\udcff')", "@sur()", ) # fmt: skip _DIFFERENTIAL_FILE: Path = Path("models/orders_summary.sql") @@ -290,6 +295,10 @@ def _cols(relation: object) -> str: return f"SELECT * FROM {relation}" +def _surrogate() -> str: + return "x\udcff" + + def _generated() -> str: return '(SELECT id FROM __ref("orders"))' @@ -310,6 +319,7 @@ def _loaded_macro(*, name: str, function: Callable[..., object]) -> LoadedMacro: ("tag", _tag), ("cols", _cols), ("gen", _generated), + ("sur", _surrogate), ) } _DIFFERENTIAL_CONTEXT: MacroContext = MacroContext( @@ -318,7 +328,7 @@ def _loaded_macro(*, name: str, function: Callable[..., object]) -> LoadedMacro: class ExpansionFailureCapture: - """Swallow and keep the compile input error a block raises.""" + """Swallow and keep the error a block raises.""" def __init__(self) -> None: self.failure: BaseException | None = None @@ -328,7 +338,7 @@ def __enter__(self) -> ExpansionFailureCapture: def __exit__(self, error_type: object, error: BaseException | None, traceback: object) -> bool: self.failure = error - return isinstance(error, CompileInputError) + return isinstance(error, Exception) def random_macro_sql(*, rng: random.Random) -> str: @@ -363,3 +373,187 @@ def expansion_outcome(*, sql: str, bridge: MacroBridge | None) -> tuple[object, True: rendered[0], False: (type(capture.failure).__name__, str(capture.failure)), }[capture.failure is None] + + +_FAILING_MACRO_LOG: str = ( + "def _log(value: object) -> None:\n" + f' path = os.environ.get("{MACRO_CALL_LOG_ENV_VAR}")\n' + " if path:\n" + ' with open(path, "a", encoding="utf-8") as log:\n' + ' log.write(f"{value}\\n")\n\n\n' +) +_FAILING_MACROS: str = ( + '"""Macros that render, raise after a side effect or return values that are not SQL."""\n\n' + "import os\n\n\n" + _FAILING_MACRO_LOG + "def cents(column: str) -> str:\n" + ' """Convert an amount to cents."""\n' + " _log(column)\n" + ' return f"{column} * 100"\n\n\n' + "def wrap(expression: str) -> str:\n" + ' """Parenthesise an expression."""\n' + ' _log("wrap " + expression)\n' + ' return f"({expression})"\n\n\n' + "def explode(column: str) -> str:\n" + ' """Fail after logging the call."""\n' + " _log(column)\n" + ' raise ValueError(f"cannot render {column}")\n\n\n' + "def number(column: str) -> int:\n" + ' """Return a value that is not SQL."""\n' + " _log(column)\n" + " return 1\n\n\n" + "def _hidden(column: str) -> str:\n" + " return column\n" +) +_LOCAL_MACROS: str = ( + '"""Macros only the marts models can see."""\n\n' + "import os\n\n\n" + _FAILING_MACRO_LOG + "def mart_only(column: str) -> str:\n" + ' """Qualify a column for marts."""\n' + " _log(column)\n" + ' return f"marts_{column}"\n' +) +_CYCLIC_MACROS: str = ( + '"""Macros that call each other forever."""\n\n\n' + "def ping(column: str) -> str:\n" + ' """Call pong."""\n' + " return pong(column)\n\n\n" + "def pong(column: str) -> str:\n" + ' """Call ping."""\n' + " return ping(column)\n" +) +_FAILING_CALLS: tuple[str, ...] = ( + *("@cents('{arg}')",) * 24, *("@wrap(@cents('{arg}'))",) * 6, "@mart_only('{arg}')", + "@mart_only('{arg}')", "@explode('{arg}')", + "@wrap(@explode('{arg}'))", "@number('{arg}')", "@cents('{arg}', 'extra')", + "@cents('{arg}',,)", "@cents({arg})", "@nope('{arg}')", "@_hidden('{arg}')", + "@cents('{arg}'", "@cents x('{arg}')", +) # fmt: skip +_MODEL_FOLDERS: tuple[str, ...] = ("staging", "marts") +_ERROR_SPECIFICS: re.Pattern[str] = re.compile(r"'[^']*'|c\d+_\d+") +_CYCLE_CHANCE: tuple[bool, ...] = (False,) * 11 + (True,) + + +def generated_failing_macro_project(*, rng: random.Random) -> dict[str, str]: + """Return a project whose models call macros that may raise, mistype or be unreachable.""" + + models: dict[str, str] = dict( + _generated_model(rng=rng, model=model) for model in range(rng.randint(2, 6)) + ) + cyclic: dict[str, str] = {True: {"macros/cyclic.py": _CYCLIC_MACROS}, False: {}}[ + rng.choice(_CYCLE_CHANCE) + ] + return { + "sqlbuild_project.toml": MACRO_BRIDGE_PROJECT_FILES["sqlbuild_project.toml"], + "macros/common.py": _FAILING_MACROS, + "models/marts/_sqlbuild/macros/local.py": _LOCAL_MACROS, + **cyclic, + **models, + } + + +def _generated_model(*, rng: random.Random, model: int) -> tuple[str, str]: + calls: list[str] = [ + f"{rng.choice(_FAILING_CALLS).format(arg=f'c{model}_{call}')} AS c{call}" + for call in range(rng.randint(1, 3)) + ] + return ( + f"models/{rng.choice(_MODEL_FOLDERS)}/m{model}.sql", + f'MODEL (description "Generated {model}");\n\nSELECT {", ".join(calls)}\n', + ) + + +@dataclass(frozen=True) +class FailingMacroParity: + """How each engine's render of generated failing-macro projects compared with Python's.""" + + mismatches: list[list[tuple[object, object, object]]] + failures: int + successes: int + failures_after_macro_runs: int + distinct_errors: list[str] + + +def failing_macro_parity( + *, + projects: list[dict[str, str]], + root: Path, + engines: tuple[str, ...], + monkeypatch: pytest.MonkeyPatch, +) -> FailingMacroParity: + """Render every project under each engine; compare errors and macro runs with the first.""" + + project_dirs: list[Path] = [ + write_project(root=root / f"project_{index}", files=files) + for index, files in enumerate(projects) + ] + outcomes: dict[str, list[object]] = { + engine: _engine_outcomes( + project_dirs=project_dirs, + engine=engine, + log_path=root / f"{engine}.log", + monkeypatch=monkeypatch, + ) + for engine in engines + } + python_outcomes: list[object] = outcomes[engines[0]] + failed: list[bool] = [_is_failure(outcome) for outcome in python_outcomes] + failures: list[tuple[tuple[object, ...], tuple[str, ...]]] = cast( + list[tuple[tuple[object, ...], tuple[str, ...]]], list(compress(python_outcomes, failed)) + ) + return FailingMacroParity( + mismatches=[ + mismatches( + inputs=list[object](project_dirs), + expected=python_outcomes, + actual=outcomes[engine], + ) + for engine in engines[1:] + ], + failures=len(failures), + successes=len(python_outcomes) - len(failures), + failures_after_macro_runs=sum(bool(runs) for _, runs in failures), + distinct_errors=sorted( + {_ERROR_SPECIFICS.sub("''", str(error[1])) for error, _ in failures} + ), + ) + + +def _engine_outcomes( + *, project_dirs: list[Path], engine: str, log_path: Path, monkeypatch: pytest.MonkeyPatch +) -> list[object]: + return [ + engine_render_outcome( + project_dir=project_dir, engine=engine, log_path=log_path, monkeypatch=monkeypatch + ) + for project_dir in project_dirs + ] + + +def _is_failure(outcome: object) -> bool: + return isinstance(cast(tuple[object, ...], outcome)[0], tuple) + + +def engine_render_outcome( + *, project_dir: Path, engine: str, log_path: Path, monkeypatch: pytest.MonkeyPatch +) -> tuple[object, tuple[str, ...]]: + """Render under `engine`; return the error's full shape or the capture, and macro runs.""" + + _ = log_path.write_text("", encoding="utf-8") + monkeypatch.setenv(MACRO_CALL_LOG_ENV_VAR, str(log_path)) + capture: ExpansionFailureCapture = ExpansionFailureCapture() + rendered: list[object] = [None] + with capture: + rendered[0] = comparable_capture( + render_compile_inputs(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + failure: BaseException | None = capture.failure + outcome: object = { + True: rendered[0], + False: ( + type(failure).__name__, + str(failure), + getattr(failure, "code", None), + getattr(failure, "help", None), + type(getattr(failure, "__cause__", None)).__name__, + str(getattr(failure, "__cause__", None)), + ), + }[failure is None] + return outcome, tuple(log_path.read_text(encoding="utf-8").splitlines()) diff --git a/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py b/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py index 2650ab34a3..cb5135b328 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py +++ b/tests/integration/src/sqlbuild/compiler/compile/test_native_macro_bridge.py @@ -11,6 +11,7 @@ from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from tests.integration.src.sqlbuild.compiler.compile._test_types import ( + FailingMacroProjectParityTestCase, MacroBridgeFailureTestCase, MacroBridgeMemoTestCase, MacroBridgeParityTestCase, @@ -19,8 +20,11 @@ from tests.integration.src.sqlbuild.compiler.compile.helpers import ( MACRO_BRIDGE_PROJECT_FILES, MACRO_CALL_LOG_ENV_VAR, + FailingMacroParity, comparable_capture, expansion_outcome, + failing_macro_parity, + generated_failing_macro_project, random_macro_sql, render_compile_inputs, render_error, @@ -224,5 +228,45 @@ def test_given_random_macro_sql_when_expanding_through_bridge_then_matches_pytho assert bridge.stats()[0] > 0 +@pytest.mark.parametrize( + "test_case", + [ + FailingMacroProjectParityTestCase( + description="seeded projects with raising, mistyped, unparsable and unreachable macros", + seed=1043, + projects=150, + engines=("python", "native", "native-preview"), + expected_minimum_failures=100, + expected_minimum_successes=10, + expected_minimum_errors_after_macro_runs=80, + expected_minimum_distinct_errors=10, + expected_mismatches=[], + ), + ], + ids=lambda case: case.description, +) +def test_given_failing_macro_projects_when_rendering_then_every_engine_raises_python_error_once( + test_case: FailingMacroProjectParityTestCase, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + rng: random.Random = random.Random(test_case.seed) + + parity: FailingMacroParity = failing_macro_parity( + projects=[generated_failing_macro_project(rng=rng) for _ in range(test_case.projects)], + root=tmp_path, + engines=test_case.engines, + monkeypatch=monkeypatch, + ) + + assert parity.mismatches == [test_case.expected_mismatches] * (len(test_case.engines) - 1) + assert ( + parity.failures >= test_case.expected_minimum_failures, + parity.successes >= test_case.expected_minimum_successes, + parity.failures_after_macro_runs >= test_case.expected_minimum_errors_after_macro_runs, + len(parity.distinct_errors) >= test_case.expected_minimum_distinct_errors, + ) == (True, True, True, True), parity + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py index 699b51074f..9dae9cd388 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/_test_types.py @@ -10,8 +10,7 @@ class MacroBridgeFailureTestCase: description: str stage_with_bridge: Callable[[], str] - stage_without_bridge: Callable[[], str] - expected_error: type[Exception] + expected_error_message: str expected_bridged_runs: list[bool] @@ -22,12 +21,3 @@ class MacroBridgeSuccessTestCase: description: str expected_result: str expected_bridged_runs: list[bool] - - -@dataclass(frozen=True) -class MacroBridgeErrorContextTestCase: - """A stage whose Python error must not chain the bridge failure as its context.""" - - description: str - stage: Callable[[], str] - expected_context: BaseException | None diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py index b9d597ef70..401f3038df 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py @@ -6,6 +6,7 @@ from typing import cast from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge @@ -29,13 +30,24 @@ def failed_after_scan() -> str: raise CompileInputError("render failed") -def declaration_failed_after_scan() -> str: - """Fail on an `@enum` reference after the active bridge scanned a macro call.""" +def mismatched_after_scan() -> str: + """Report a native scan mismatch after the active bridge scanned a macro call.""" _scan_with_active_bridge() - raise CompileInputError( - "Unknown enum 'order_status' in 'models/orders.sql'", bridge_independent=True - ) + raise NativeStageMismatchError("native scan ended early") + + +def recording_failure(*, stage: Callable[[], str], errors: list[Exception]) -> Callable[[], str]: + """Return `stage`, recording each error it raises before it propagates.""" + + def recorded() -> str: + try: + return stage() + except Exception as error: + errors.append(error) + raise + + return recorded def _scan_with_active_bridge() -> None: diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py index 81aa6d6c78..cb996b6853 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py @@ -4,19 +4,17 @@ import pytest -from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge from sqlbuild.compiler.macro_bridge.main.run_with_macro_bridge import run_with_macro_bridge from tests.unit.src.sqlbuild.compiler.macro_bridge.main._test_types import ( - MacroBridgeErrorContextTestCase, MacroBridgeFailureTestCase, MacroBridgeSuccessTestCase, ) from tests.unit.src.sqlbuild.compiler.macro_bridge.main.helpers import ( - declaration_failed_after_scan, failed, failed_after_scan, + mismatched_after_scan, + recording_failure, recording_stage, rendered, ) @@ -38,9 +36,7 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br ) -> None: runs: list[bool] = [] - result: str = run_with_macro_bridge( - stage=recording_stage(runs=runs, stages={True: rendered, False: failed}) - ) + result: str = run_with_macro_bridge(stage=recording_stage(runs=runs, stages={True: rendered})) assert result == test_case.expected_result assert runs == test_case.expected_bridged_runs @@ -51,70 +47,45 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br "test_case", [ MacroBridgeFailureTestCase( - description="failure after a scan re-runs Python and raises its error", + description="failure after a scan raises the stage error without a re-run", stage_with_bridge=failed_after_scan, - stage_without_bridge=failed, - expected_error=CompileInputError, - expected_bridged_runs=[True, False], - ), - MacroBridgeFailureTestCase( - description="native-only failure after a scan is reported as a divergence", - stage_with_bridge=failed_after_scan, - stage_without_bridge=rendered, - expected_error=NativeStageMismatchError, - expected_bridged_runs=[True, False], + expected_error_message="render failed", + expected_bridged_runs=[True], ), MacroBridgeFailureTestCase( - description="failure before the bridge scans anything is raised without a re-run", + description="failure before the bridge scans anything raises without a re-run", stage_with_bridge=failed, - stage_without_bridge=rendered, - expected_error=CompileInputError, + expected_error_message="render failed", expected_bridged_runs=[True], ), MacroBridgeFailureTestCase( - description="declaration reference failure after a scan is raised without a re-run", - stage_with_bridge=declaration_failed_after_scan, - stage_without_bridge=rendered, - expected_error=CompileInputError, + description="a bridge mismatch after a scan stays an explicit mismatch", + stage_with_bridge=mismatched_after_scan, + expected_error_message="native scan ended early", expected_bridged_runs=[True], ), ], ids=lambda case: case.description, ) -def test_given_failing_stage_when_running_with_macro_bridge_then_only_bridge_failures_rerun( +def test_given_failing_stage_when_running_with_macro_bridge_then_raises_its_error_once( test_case: MacroBridgeFailureTestCase, ) -> None: runs: list[bool] = [] + raised_errors: list[Exception] = [] stage: Callable[[], str] = recording_stage( runs=runs, - stages={True: test_case.stage_with_bridge, False: test_case.stage_without_bridge}, + stages={True: recording_failure(stage=test_case.stage_with_bridge, errors=raised_errors)}, ) - with pytest.raises(test_case.expected_error): + with pytest.raises(Exception) as raised: _ = run_with_macro_bridge(stage=stage) - assert runs == test_case.expected_bridged_runs - assert active_macro_bridge() is None - - -@pytest.mark.parametrize( - "test_case", - [ - MacroBridgeErrorContextTestCase( - description="python error after a bridge failure", - stage=recording_stage(runs=[], stages={True: failed_after_scan, False: failed}), - expected_context=None, - ), - ], - ids=lambda case: case.description, -) -def test_given_python_error_when_rerun_after_bridge_failure_then_error_has_no_bridge_context( - test_case: MacroBridgeErrorContextTestCase, -) -> None: - with pytest.raises(CompileInputError) as raised: - _ = run_with_macro_bridge(stage=test_case.stage) - - assert raised.value.__context__ is test_case.expected_context + assert (str(raised.value), [raised.value], runs, active_macro_bridge()) == ( + test_case.expected_error_message, + raised_errors, + test_case.expected_bridged_runs, + None, + ) if __name__ == "__main__": diff --git a/website/src/content/docs/docs/concepts/macros.mdx b/website/src/content/docs/docs/concepts/macros.mdx index 24cc009ba3..09e3391d77 100644 --- a/website/src/content/docs/docs/concepts/macros.mdx +++ b/website/src/content/docs/docs/concepts/macros.mdx @@ -215,6 +215,8 @@ SQLBuild relies on this contract to reuse macro results: - [Compile reuse](/docs/cli/compile) replays an unchanged compile without running macros again. - A compile may run each distinct macro call once and reuse its result for identical calls elsewhere in the project. +- A compile that fails stops at its first error. It does not run the project's macros a second + time to report that error. - Macro call results are kept between compiles under `target/cache/` by default, so editing a model does not run its unchanged macro calls again. They are recomputed when macro or other Python files, configuration, declarations, seeds, sources, variables, the target, From 1c2b3fabe227d7192bf00f10d3dbdf9f2c9aaf15 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:47:39 +0100 Subject: [PATCH 2/4] fix(native): treat macro calls without a buildable store key as uncacheable --- .../macro_bridge/_helpers/store_keys.py | 94 ++++++++++++------- .../macro_bridge/classes/macro_bridge.py | 18 ++-- .../cli/commands/main/compile/_test_types.py | 10 ++ .../compile/test_compile_macro_call_store.py | 54 +++++++++++ .../compiler/macro_bridge/_helpers/helpers.py | 9 ++ .../macro_bridge/_helpers/test_store_keys.py | 19 +++- 6 files changed, 158 insertions(+), 46 deletions(-) diff --git a/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py b/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py index f507172914..35a6b520ad 100644 --- a/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py +++ b/src/sqlbuild/compiler/macro_bridge/_helpers/store_keys.py @@ -21,18 +21,29 @@ from sqlbuild.compiler.scopes.models import DeclarationIdentity from sqlbuild.python_nodes.models import SqlResourceRef +_UNSTABLE_KEY_ERRORS: tuple[type[Exception], ...] = ( + UncacheableValueError, + UnicodeEncodeError, + RecursionError, +) -def macro_store_token(*, loaded: LoadedMacro, identity: DeclarationIdentity | None) -> str: - """Name a macro by its name, file, declaration identity and exact source.""" - return _key_text( - [ - loaded.name, - loaded.relative_path.as_posix(), - _canonical(identity), - _native.content_digest([loaded.raw_source]), - ] - ) +def macro_store_token(*, loaded: LoadedMacro, identity: DeclarationIdentity | None) -> str | None: + """Name a macro by its name, file, identity and exact source, or None when it has no key.""" + + try: + return _encodable( + _key_text( + [ + loaded.name, + loaded.relative_path.as_posix(), + _canonical(identity), + _native.content_digest([loaded.raw_source]), + ] + ) + ) + except _UNSTABLE_KEY_ERRORS: + return None def context_store_token( @@ -41,22 +52,25 @@ def context_store_token( """Digest the whole context a macro may read, or None when a value has no stable text.""" try: - text: str = _key_text( + return _native.content_digest( [ - _canonical_context(macro_context), - None - if declarations is None - else [ - _canonical(declarations.constants), - _canonical(declarations.enums), - sorted(declarations.inaccessible_constants), - sorted(declarations.inaccessible_enums), - ], + _key_text( + [ + _canonical_context(macro_context), + None + if declarations is None + else [ + _canonical(declarations.constants), + _canonical(declarations.enums), + sorted(declarations.inaccessible_constants), + sorted(declarations.inaccessible_enums), + ], + ] + ) ] ) - except UncacheableValueError: + except _UNSTABLE_KEY_ERRORS: return None - return _native.content_digest([text]) def call_class_store_text( @@ -64,18 +78,23 @@ def call_class_store_text( macro_tokens: tuple[str, ...], context_token: str | None, prior_relations: tuple[SqlResourceRef, ...] | None, -) -> str: - """Text of one persistent call class: its macros, context and relations passed so far.""" - - return _key_text( - [ - list(macro_tokens), - context_token, - None - if prior_relations is None - else [[relation.kind.value, relation.name] for relation in prior_relations], - ] - ) +) -> str | None: + """Text of one persistent call class, or None when a relation name has no UTF-8 text.""" + + try: + return _encodable( + _key_text( + [ + list(macro_tokens), + context_token, + None + if prior_relations is None + else [[relation.kind.value, relation.name] for relation in prior_relations], + ] + ) + ) + except UnicodeEncodeError: + return None def _canonical_context(macro_context: MacroContext) -> object: @@ -141,3 +160,10 @@ def _type_name(kind: type) -> str: def _key_text(value: object) -> str: return json.dumps(value, ensure_ascii=False, separators=(",", ":")) + + +def _encodable(text: str) -> str: + """Return `text`, raising `UnicodeEncodeError` when the native store cannot hold it.""" + + _ = text.encode("utf-8") + return text diff --git a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py index d680cf40d8..731fe2ca5b 100644 --- a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py @@ -209,15 +209,17 @@ def lookup( class_id: int | None = self._class_ids.get(class_key) if class_id is None: class_id = self._class_ids[class_key] = len(self._class_ids) - if self._store_path is not None and call_class.persistent: - self._memo.set_persistent_class( - class_id, - call_class_store_text( - macro_tokens=call_class.macro_store_tokens, - context_token=call_class.context_store_token, - prior_relations=prior_relations, - ), + class_text: str | None = ( + call_class_store_text( + macro_tokens=call_class.macro_store_tokens, + context_token=call_class.context_store_token, + prior_relations=prior_relations, ) + if self._store_path is not None and call_class.persistent + else None + ) + if class_text is not None: + self._memo.set_persistent_class(class_id, class_text) found: tuple[str, list[tuple[str, str]], list[tuple[int, str, str]]] | None = ( self._memo.lookup(class_id, call_text) ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 2eb571f5e1..c0587a725b 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -953,6 +953,16 @@ class NativeMacroErrorTestCase: expected_macro_runs: list[str] +@dataclass(frozen=True) +class UnkeyableMacroCallTestCase: + """Compile arguments whose values no store key can hold; every engine must match Python.""" + + description: str + project_files: dict[str, str] + compile_args: tuple[str, ...] + expected_returncodes: tuple[int, ...] + + @dataclass(frozen=True) class MacroErrorLifecycleTestCase: """A failing macro project whose invocation lifecycle every engine reports the same way.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py index 2d610f33f0..544ed4fc50 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_macro_call_store.py @@ -2,11 +2,13 @@ from __future__ import annotations +import shutil import sys from pathlib import Path import pytest +from scripts.compiler_differential.constants import FAILURE_BASE_FILES from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( BrokenMacroCallStoreKeyTestCase, EngineMacroCallGateTestCase, @@ -16,6 +18,7 @@ SecondCompileStoreTestCase, StaleMacroModuleStoreTestCase, StaleStoreArrangement, + UnkeyableMacroCallTestCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( MACRO_CALL_STORE_ENGINE, @@ -45,6 +48,7 @@ pretend_fresh_process, recompile_in_process_after_edit, replace_project_text, + report_without_engine, rezip_flavor_between_processes, run_reuse_compile, store_files, @@ -181,6 +185,18 @@ ) +_TAGGED_STAGING: dict[str, str] = { + "models/staging/_sqlbuild/_macros/tags.py": ( + 'def tagged(ctx, expr: str) -> str:\n """Tag."""\n return expr\n' + ), + "models/staging/stg_orders.sql": ( + 'MODEL (\n description "Staged orders",\n);\n\n' + "SELECT order_id, customer_id, @tagged('amount') AS amount, status\n" + 'FROM __source("raw_orders")\n' + ), +} + + @pytest.mark.parametrize( "test_case", [ @@ -493,3 +509,41 @@ def test_given_no_proc_filesystem_when_compiling_repeatedly_then_the_store_still assert runs.returncodes == (0, 0) assert runs.logged_calls == test_case.expected_logged_calls assert runs.store_files == test_case.expected_store_files + + +@pytest.mark.parametrize( + "test_case", + [ + UnkeyableMacroCallTestCase( + description="context macro with a lone surrogate var", + project_files=_TAGGED_STAGING, + compile_args=("--vars", '{"regions": ["\\udcff"]}'), + expected_returncodes=(0, 0, 0), + ), + ], + ids=lambda case: case.description, +) +def test_given_var_no_store_key_can_hold_when_compiling_then_every_engine_matches_python( + test_case: UnkeyableMacroCallTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "orders" + runs: list[CompileReuseRun] = [] + for engine in ("python", "native", "native-preview"): + shutil.rmtree(project_dir, ignore_errors=True) + for relative_path, contents in {**FAILURE_BASE_FILES, **test_case.project_files}.items(): + write_project_file(project_dir, relative_path, contents) + runs.append( + run_reuse_compile( + project_dir=project_dir, + args=test_case.compile_args, + global_args=("--compiler-engine", engine), + ) + ) + + assert ( + tuple(run.returncode for run in runs), + {report_without_engine(run) for run in runs} == {report_without_engine(runs[0])}, + [run.compiled for run in runs] == [runs[0].compiled] * len(runs), + ) == (test_case.expected_returncodes, True, True), tuple( + run.report + run.stderr for run in runs + ) diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py index 3058392ef8..acb50cbbe0 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/helpers.py @@ -196,3 +196,12 @@ def environment_of(root: Path) -> str: project_dir=root, fingerprint=project_fingerprint(project_dir=root, model_paths=EXCLUDED_MODEL_PATHS), ) + + +def deeply_nested_list(*, depth: int) -> list[object]: + """Return a list nested `depth` levels deep.""" + + nested: list[object] = [] + for _ in range(depth): + nested = [nested] + return nested diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py index e32305902d..41d079e5b6 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/_helpers/test_store_keys.py @@ -21,6 +21,7 @@ from tests.unit.src.sqlbuild.compiler.macro_bridge._helpers.helpers import ( BASE_CONTEXT, constant_declarations, + deeply_nested_list, loaded_macro, with_vars, ) @@ -113,6 +114,16 @@ def test_given_two_contexts_when_naming_their_store_class_then_only_equal_values context=with_vars(region={"key": [object()]}), expected_stored=False, ), + ContextValueTestCase( + description="lone surrogate var", + context=with_vars(region="\udcff"), + expected_stored=False, + ), + ContextValueTestCase( + description="var nested beyond the recursion limit", + context=with_vars(region=deeply_nested_list(depth=100_000)), + expected_stored=False, + ), ], ids=lambda case: case.description, ) @@ -151,14 +162,14 @@ def test_given_context_value_when_naming_store_class_then_only_stable_values_are def test_given_loaded_macro_change_when_naming_it_then_only_the_same_file_and_source_match( test_case: MacroStoreTokenTestCase, ) -> None: - original: str = macro_store_token( + original: str | None = macro_store_token( loaded=loaded_macro( relative_path=Path("macros/common.py"), raw_source="def cents(column): ...\n" ), identity=None, ) - changed: str = macro_store_token( + changed: str | None = macro_store_token( loaded=loaded_macro(relative_path=test_case.relative_path, raw_source=test_case.raw_source), identity=None, ) @@ -196,11 +207,11 @@ def test_given_loaded_macro_change_when_naming_it_then_only_the_same_file_and_so def test_given_call_class_inputs_when_naming_it_then_only_identical_inputs_match( test_case: CallClassStoreTextTestCase, ) -> None: - plain: str = call_class_store_text( + plain: str | None = call_class_store_text( macro_tokens=("cents",), context_token=None, prior_relations=None ) - text: str = call_class_store_text( + text: str | None = call_class_store_text( macro_tokens=("cents",), context_token=test_case.context_token, prior_relations=test_case.prior_relations, From 9df116c22062d39df89f315089f52d689087fb4b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 01:04:29 +0100 Subject: [PATCH 3/4] test(native): expect one macro run per engine on failing compiles --- .../compile/_helpers/attachment/model_config.py | 2 +- .../compiler/model_config/main/_native_config_error.py | 2 +- .../commands/main/build/test_model_config_engines.py | 6 +++--- .../attachment_errors/test_native_attachment_errors.py | 6 +++--- .../sql_test_errors/test_native_sql_test_errors.py | 2 +- .../commands/main/compile/test_native_config_errors.py | 10 +++++----- .../main/compile/test_native_reference_errors.py | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py index 5f9c113de5..eb2dba4c8b 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py @@ -1001,7 +1001,7 @@ def native_model_validation( def native_validation_error( *, error: _native.NativeConfigError, values: dict[str, object] ) -> Exception: - """Return a native validator error; validators read expanded SQL, so the bridge decides.""" + """Return a native validator error, unmarked because validators read macro-expanded SQL.""" return native_config_error(error=error, bridge_independent=False, values=values) diff --git a/src/sqlbuild/compiler/model_config/main/_native_config_error.py b/src/sqlbuild/compiler/model_config/main/_native_config_error.py index 9b5ba695c2..b8814631ae 100644 --- a/src/sqlbuild/compiler/model_config/main/_native_config_error.py +++ b/src/sqlbuild/compiler/model_config/main/_native_config_error.py @@ -23,7 +23,7 @@ def native_config_error( bridge_independent: bool, values: Mapping[str, object] | None = None, ) -> Exception: - """Return Python's exception for `error`; mark it bridge-independent only if SQL is unread.""" + """Return Python's exception for `error`, marked `bridge_independent` when SQL is unread.""" if error.class_name == CONFIG_VALUE_TYPE_ERROR and error.key is not None and values is not None: return ConfigValueTypeError( diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py index f124f6dd00..d3e3ae5bfe 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_model_config_engines.py @@ -1,8 +1,8 @@ """E2E: layered, templated and validated model config builds the same on every compiler engine. -The native and preview engines build model config (path defaults, templates, storage policies) and checks the -model validators natively; a validator rejection re-runs Python for the exact error. Every engine -must build the same relations and report the same validator errors. +The native and preview engines build model config (path defaults, templates, storage policies) +and check the model validators natively with Python's exact errors; inputs only Python can judge +run in Python. Every engine must build the same relations and report the same validator errors. """ from __future__ import annotations diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py index 9ffdf0f4cf..9839838436 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py @@ -1,4 +1,4 @@ -"""Every engine reports attachment errors identically; bridge-independent ones never re-run.""" +"""Every engine reports attachment errors identically and runs each macro call once.""" from __future__ import annotations @@ -56,9 +56,9 @@ expected_error_types=_ERROR_TYPES, ), NativeAttachmentErrorTestCase( - description="a cursor intrinsic in a generic audit re-runs as it reads expanded SQL", + description="a cursor intrinsic in a generic audit reads expanded SQL", case_name="generic-audit-cursor-intrinsic", - expected_macro_calls=(1, 2, 2), + expected_macro_calls=(1, 1, 1), expected_error_types=_ERROR_TYPES, ), NativeAttachmentErrorTestCase( diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py index 4f7c22fc3b..3e227f83bd 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py @@ -1,4 +1,4 @@ -"""Every engine reports SQL test, scenario and relationship errors identically, without re-runs.""" +"""Every engine reports SQL test, scenario and relationship errors identically.""" from __future__ import annotations diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py index 3a2a19e347..e3b74d071c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_config_errors.py @@ -73,7 +73,7 @@ + _STAGING_BODY }, expected_report_text="unknown incremental_strategy 'upsert'", - expected_macro_calls=[1, 2], + expected_macro_calls=[1, 1], expected_python_fallbacks=[], ), NativeConfigErrorTestCase( @@ -85,7 +85,7 @@ + _STAGING_BODY }, expected_report_text="cursor_start must be before exclusive cursor_end", - expected_macro_calls=[1, 2], + expected_macro_calls=[1, 1], expected_python_fallbacks=[], ), NativeConfigErrorTestCase( @@ -127,7 +127,7 @@ ], ids=lambda case: case.description, ) -def test_given_config_error_when_compiling_with_preview_then_error_matches_python_once( +def test_given_config_error_when_compiling_with_preview_then_error_matches_python( test_case: NativeConfigErrorTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -195,12 +195,12 @@ def test_given_config_error_when_compiling_with_preview_then_error_matches_pytho }, engines=("python", "native-preview", "native"), expected_report_text="references unknown model 'ghost'", - expected_macro_calls=[1, 2, 2], + expected_macro_calls=[1, 1, 1], ) ], ids=lambda case: case.description, ) -def test_given_validator_error_from_macro_sql_when_compiling_then_preview_bridge_reruns_it( +def test_given_validator_error_from_macro_sql_when_compiling_then_every_engine_runs_macros_once( test_case: MacroExpandedValidatorErrorTestCase, tmp_path: Path ) -> None: project_dir: Path = tmp_path / "orders" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py index c58998d27b..97e099d6ec 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py @@ -104,11 +104,11 @@ expected_message=( f"{FAILURE_MART_PATH}: SQL reference contains an unclosed parenthesis" ), - expected_macro_calls=(1, 2, 2), + expected_macro_calls=(1, 1, 1), expected_error_types=("CompileInputError",) * 3, ), ReferenceScanErrorTestCase( - description="authored_fault_after_a_macro_is_located_without_a_python_rerun", + description="authored_fault_after_a_macro_is_located_natively", project_files={ "macros/emit.py": _EMIT_MACRO, FAILURE_MART_PATH: _MART_HEADER + "SELECT customer_id, @emit('1') AS total_amount\n" From 94bb5dee9021d49a130b0c89ef04f5353c717eae Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 01:29:14 +0100 Subject: [PATCH 4/4] test(native): use the renamed native entries field in new attachment cases --- .../attachments/test_native_attachment_project.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py index 41bc763176..1dd66a3c41 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_attachment_project.py @@ -102,7 +102,7 @@ + '__expected__orders AS (SELECT id, 1.5 AS amount, label FROM __source("order_events")),\n' + "__assert__no_rows AS (SELECT 1 FROM __source(order_events))\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql CTE " "'__expected__orders' must not reference project source", ), @@ -114,7 +114,7 @@ + "__seed__channel_codes AS (SELECT 1 AS id, 'web' AS label),\n" + "__expected__orders AS (SELECT 1 AS id FROM __source(order_events))\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql " "references unknown source 'missing_events'", ), @@ -126,7 +126,7 @@ + "__seed__channel_codes AS (SELECT 1 AS id FROM __source(order_events)),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario file tests/scenarios/orders_scenario.sql " "references unknown source 'missing_events'", ), @@ -137,7 +137,7 @@ + "WITH\n__seed__ AS (SELECT 1 AS id, 'web' AS label),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="error: SQL scenario 'tests/scenarios/orders_scenario.sql' " "must use __seed__ to identify a target", ), @@ -149,7 +149,7 @@ + "__macro__tidy AS (SELECT 'x'),\n" + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="does not support macro mock CTE '__macro__tidy'", ), AttachmentProjectTestCase( @@ -161,7 +161,7 @@ + "__expected__orders AS (SELECT 1 AS id, 1.5 AS amount, 'web' AS label " + "FROM __assert__no_rows)\n" }, - expected_preview_entries=frozenset({"SqlTestTargetCatalog"}), + expected_native_entries=frozenset({"SqlTestTargetCatalog"}), expected_outcome_fragment="check CTE '__expected__orders' must not depend on " "'__assert__no_rows'", ),