Skip to content

Commit 330c21b

Browse files
committed
fix(native): compare catalog shapes on cold compiles and record deferrals after the session
Keep the binding catalog's schemas and analysis shapes in captures and ignore them only on warm stages, where Python's analysis cache skips catalog preparation. Record per-model deferrals once the session completes, and fall back to Python when provide or finish panics.
1 parent 6d8861e commit 330c21b

11 files changed

Lines changed: 182 additions & 13 deletions

File tree

‎crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use sqlbuild_analysis::assembly::analysis_session::models::{
1515
use sqlbuild_analysis::assembly::analysis_session::types::{Pairs, Shapes};
1616
use sqlbuild_analysis::semantic_validation::types::DiagnosticRow;
1717

18+
use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic;
19+
1820
use crate::bindings::_helpers::boundary::panics::compiler_error;
1921
use crate::bindings::models::ProjectCatalog;
2022
use crate::bindings::types::CompilerDetach;
@@ -124,16 +126,18 @@ impl NativeModelAnalysisSession {
124126
return false;
125127
};
126128
let answers: Vec<DeferredAnalysis> = results.into_iter().map(deferred_analysis).collect();
127-
let provided: Result<(), String> = provide_deferred_analyses(&mut session, answers);
129+
let provided: Result<(), String> =
130+
catch_compiler_panic(|| provide_deferred_analyses(&mut session, answers));
128131
self.inner = Some(session);
129132
self.kept(provided).is_some()
130133
}
131134

132135
/// Every model's outcome once the session is done, or None to analyse in Python.
133136
fn finish(&mut self) -> Option<FinishRow> {
134137
let session: AnalysisSession = self.inner.take()?;
135-
let outcome: Result<SessionOutcome, String> = finish_analysis_session(session);
136-
self.kept(outcome).map(finish_row)
138+
let outcome: Result<FinishRow, String> =
139+
catch_compiler_panic(|| finish_analysis_session(session).map(finish_row));
140+
self.kept(outcome)
137141
}
138142

139143
/// Why the session handed the analysis back to Python, when it did.

‎scripts/compiler_differential/_helpers/comparing/capture.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
from scripts.compiler_differential.models import Divergence
1818

1919

20-
def first_capture_difference(*, left: Path, right: Path) -> Divergence | None:
21-
"""Return the first logical JSON pointer, or line for invalid JSON, where captures differ."""
20+
def first_capture_difference(
21+
*, left: Path, right: Path, ignored: frozenset[str] = frozenset()
22+
) -> Divergence | None:
23+
"""Return the first JSON pointer outside `ignored`, or line for invalid JSON, that differs."""
2224

2325
try:
2426
left_capture: CaptureFile = CaptureFile(left)
@@ -27,6 +29,7 @@ def first_capture_difference(*, left: Path, right: Path) -> Divergence | None:
2729
left=left_capture.root,
2830
right=right_capture.root,
2931
resolve=(left_capture.resolve, right_capture.resolve),
32+
ignored=ignored,
3033
)
3134
except (json.JSONDecodeError, UnicodeDecodeError):
3235
return _first_line_difference(left=left, right=right)

‎scripts/compiler_differential/_helpers/comparing/compare.py‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,14 @@
1313

1414

1515
def first_json_difference(
16-
*, left: object, right: object, pointer: str = "", resolve: NodeResolvers | None = None
16+
*,
17+
left: object,
18+
right: object,
19+
pointer: str = "",
20+
resolve: NodeResolvers | None = None,
21+
ignored: frozenset[str] = frozenset(),
1722
) -> Divergence | None:
18-
"""Return the first JSON pointer whose value, type, or key order differs, via `resolve`."""
23+
"""Return the first JSON pointer outside `ignored` whose value, type or key order differs."""
1924

2025
if resolve is not None:
2126
left_digest: str | None = shared_digest(left)
@@ -28,12 +33,16 @@ def first_json_difference(
2833
right_object: dict[str, object] | None = as_json_object(right)
2934
if left_object is not None and right_object is not None:
3035
return _first_object_difference(
31-
left=left_object, right=right_object, pointer=pointer, resolve=resolve
36+
left=left_object, right=right_object, pointer=pointer, resolve=resolve, ignored=ignored
3237
)
3338
if isinstance(left, list) and isinstance(right, list):
3439
for index, (left_item, right_item) in enumerate(zip(left, right, strict=False)):
3540
found: Divergence | None = first_json_difference(
36-
left=left_item, right=right_item, pointer=f"{pointer}/{index}", resolve=resolve
41+
left=left_item,
42+
right=right_item,
43+
pointer=f"{pointer}/{index}",
44+
resolve=resolve,
45+
ignored=ignored,
3746
)
3847
if found is not None:
3948
return found
@@ -99,17 +108,20 @@ def _first_object_difference(
99108
right: dict[str, object],
100109
pointer: str,
101110
resolve: NodeResolvers | None,
111+
ignored: frozenset[str],
102112
) -> Divergence | None:
103113
for key in (*left, *(key for key in right if key not in left)):
104114
location: str = f"{pointer}/{json_pointer_token(key)}"
115+
if location in ignored:
116+
continue
105117
if key not in left or key not in right:
106118
return _divergence(
107119
location=location,
108120
left=_shown(value=left.get(key, MISSING_VALUE), resolve=resolve, side=0),
109121
right=_shown(value=right.get(key, MISSING_VALUE), resolve=resolve, side=1),
110122
)
111123
found: Divergence | None = first_json_difference(
112-
left=left[key], right=right[key], pointer=location, resolve=resolve
124+
left=left[key], right=right[key], pointer=location, resolve=resolve, ignored=ignored
113125
)
114126
if found is not None:
115127
return found

‎scripts/compiler_differential/_helpers/comparing/comparison.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
strip_report_fields,
2020
)
2121
from scripts.compiler_differential.constants import (
22+
ANALYSIS_CACHE_STATE_POINTERS,
23+
COLD_CAPTURE_COMMAND,
2224
MISSING_VALUE,
2325
MODEL_RESOURCE_TYPE,
2426
PLAN_LABEL,
@@ -266,7 +268,11 @@ def _capture_differences(
266268
differences.append(
267269
(
268270
artifact,
269-
first_capture_difference(left=left_files[name], right=right_files[name]),
271+
first_capture_difference(
272+
left=left_files[name],
273+
right=right_files[name],
274+
ignored=_cache_state_pointers(command),
275+
),
270276
)
271277
)
272278
return differences
@@ -278,3 +284,9 @@ def _presence_divergence(*, left_present: bool, right_present: bool) -> Divergen
278284
left="present" if left_present else MISSING_VALUE,
279285
right="present" if right_present else MISSING_VALUE,
280286
)
287+
288+
289+
def _cache_state_pointers(command: str) -> frozenset[str]:
290+
"""Catalog state Python's warm analysis cache skips; compared only on the cold compile."""
291+
292+
return frozenset() if command == COLD_CAPTURE_COMMAND else ANALYSIS_CACHE_STATE_POINTERS

‎scripts/compiler_differential/constants.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ def short_model_names(*, model: Model, ctx: RuleContext) -> list[Finding]:
8686
COLD_COMPILE: DifferentialCommand = DifferentialCommand(
8787
label="compile", arguments=("compile", "--json", "--manifest", "--dag")
8888
)
89+
COLD_CAPTURE_COMMAND: str = f"0-{COLD_COMPILE.label}"
90+
ANALYSIS_CACHE_STATE_POINTERS: frozenset[str] = frozenset(
91+
{"/binding_catalog/schemas", "/binding_catalog/analysis_shapes"}
92+
)
8993
WARM_COMPILE: DifferentialCommand = DifferentialCommand(
9094
label="compile-warm", arguments=("compile", "--json")
9195
)

‎src/sqlbuild/compiler/analysis_session/classes/native_model_analysis.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import logging
6+
from collections import Counter
67
from collections.abc import Sequence
78
from typing import Any
89

@@ -62,6 +63,7 @@ def __init__(
6263
request.column_nullability_by_table
6364
)
6465
self._kept: dict[tuple[int, str], PolyglotAnalysisResult] = {}
66+
self._deferrals: Counter[str] = Counter()
6567

6668
def analyses(
6769
self, session: _native.NativeModelAnalysisSession
@@ -81,6 +83,8 @@ def analyses(
8183
self._record_catalog_changes(
8284
schema_additions=schema_additions, analysis_names=analysis_names
8385
)
86+
for kind, count in self._deferrals.items():
87+
record_analysis_deferral(kind=kind, count=count)
8488
self._python.record_uncached()
8589
return {
8690
request.model_input.model_file.file_path.stem: self._model_analysis(
@@ -123,7 +127,7 @@ def _answer(self, deferral: DeferralRow) -> DeferredRow:
123127
shapes: dict[str, dict[str, str]] = {name: dict(shape) for name, shape in schemas}
124128
analysis: PolyglotAnalysisResult
125129
if kind == DEFERRAL_ANALYSIS:
126-
record_analysis_deferral(kind=DEFERRAL_LEGACY_ANALYSIS)
130+
self._deferrals[DEFERRAL_LEGACY_ANALYSIS] += 1
127131
analysis = self._python.analyze_deferred(
128132
model=model,
129133
precomputed=PythonModelAnalysis.legacy_precomputed(
@@ -136,7 +140,7 @@ def _answer(self, deferral: DeferralRow) -> DeferredRow:
136140
column_nullability_by_table=self._nullability,
137141
)
138142
else:
139-
record_analysis_deferral(kind=DEFERRAL_ENRICHMENT)
143+
self._deferrals[DEFERRAL_ENRICHMENT] += 1
140144
analysis = self._python.enrich(model=model, input_schemas=shapes)
141145
self._kept[(model, kind)] = analysis
142146
return deferred_row(model=model, analysis=analysis)

‎tests/e2e/scripts/compiler_differential/helpers.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,20 @@ def _perturbed(*, until, **stages):
7373
frontier.native_frontier = _perturbed
7474
"""
7575

76+
CATALOG_PERTURBATION: str = """
77+
import sqlbuild.compiler.analysis_session.classes.native_model_analysis as native_analysis
78+
79+
_original = native_analysis.NativeModelAnalysis._record_catalog_changes
80+
81+
82+
def _perturbed(self, **changes):
83+
_original(self, **changes)
84+
self._catalog.schemas["perturbed_by_test"] = {"order_id": "INTEGER"}
85+
86+
87+
native_analysis.NativeModelAnalysis._record_catalog_changes = _perturbed
88+
"""
89+
7690
DEFERRAL_PERTURBATION: str = """
7791
import json
7892
import os

‎tests/e2e/scripts/compiler_differential/test_compiler_differential.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
WheelSiteReportTestCase,
1919
)
2020
from tests.e2e.scripts.compiler_differential.helpers import (
21+
CATALOG_PERTURBATION,
2122
COMPILED_PROJECT_CAPTURE,
2223
DEFERRAL_PERTURBATION,
2324
DISCOVERY_PERTURBATION,
@@ -181,6 +182,39 @@ def test_given_perturbed_native_render_when_comparing_then_compile_inputs_captur
181182
assert not any(text in output for text in test_case.expected_absent), output
182183

183184

185+
@pytest.mark.parametrize(
186+
"test_case",
187+
[
188+
HarnessRunTestCase(
189+
description="wrong_native_catalog_shape",
190+
extra_arguments=(),
191+
expected_exit_code=1,
192+
expected_lines=("DIFF project/waffle_shop",),
193+
expected_patterns=(
194+
r"- stage capture 0-compile/\d+-compiled_project\.json at /binding_catalog/schemas",
195+
),
196+
expected_absent=("warm/", "3-plan/"),
197+
)
198+
],
199+
ids=lambda case: case.description,
200+
)
201+
def test_given_wrong_native_catalog_shape_when_comparing_then_only_the_cold_capture_differs(
202+
test_case: HarnessRunTestCase, tmp_path: Path, capsys: pytest.CaptureFixture[str]
203+
) -> None:
204+
exit_code: int = run_compiler_differential(
205+
harness_arguments(
206+
work_dir=tmp_path / "work",
207+
extra=perturbation_arguments(tmp_path / "perturbation", source=CATALOG_PERTURBATION),
208+
)
209+
)
210+
211+
output: str = capsys.readouterr().out
212+
assert exit_code == test_case.expected_exit_code, output
213+
assert all(line in output for line in test_case.expected_lines), output
214+
assert all(re.search(pattern, output) for pattern in test_case.expected_patterns), output
215+
assert not any(text in output for text in test_case.expected_absent), output
216+
217+
184218
@pytest.mark.parametrize(
185219
"test_case",
186220
[

‎tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,13 @@ class AnalysisFallbackTestCase:
2525
allow_compact_analysis: bool
2626
keeps_catalog: bool
2727
expected_kind: str
28+
29+
30+
@dataclass(frozen=True)
31+
class SessionFailureTestCase:
32+
"""A session that fails after Python answered deferrals, so Python analyses everything."""
33+
34+
description: str
35+
seed: int
36+
model_count: int
37+
expected_kinds: dict[str, int]

‎tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import pytest
1515

16+
import sqlbuild._native as native_module
1617
import sqlbuild.compiler.compile._helpers.assembly.project as project_assembly
1718
from sqlbuild.adapter.contract.models import ExpressionInferenceProfile
1819
from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter
@@ -394,3 +395,36 @@ def _analysis_views(analyses: dict[str, ModelSqlAnalysis] | None) -> object:
394395

395396
def _catalog_view(catalog: Any) -> tuple[object, ...]:
396397
return (dict(catalog.schemas), dict(catalog.analysis_shapes))
398+
399+
400+
class FailingProvideSession:
401+
"""A native session whose answers to deferrals are refused, as a failed session refuses."""
402+
403+
def __init__(self, session: Any) -> None:
404+
self._session: Any = session
405+
self.answered: int = 0
406+
self.failure: str = "injected session failure"
407+
408+
def run(self) -> object:
409+
return self._session.run()
410+
411+
def provide(self, answers: list[object]) -> bool:
412+
self.answered += len(answers)
413+
return False
414+
415+
def finish(self) -> object:
416+
return self._session.finish()
417+
418+
419+
def failing_provide_sessions(*, monkeypatch: pytest.MonkeyPatch) -> list[FailingProvideSession]:
420+
"""Make every native session refuse Python's answers; return the sessions started."""
421+
422+
started: list[FailingProvideSession] = []
423+
original: Callable[..., Any] = native_module.start_model_analysis_session
424+
425+
def start(catalog: object, request: tuple[object, ...]) -> FailingProvideSession:
426+
started.append(FailingProvideSession(original(catalog, request)))
427+
return started[-1]
428+
429+
monkeypatch.setattr(native_module, "start_model_analysis_session", start)
430+
return started

0 commit comments

Comments
 (0)