Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .claude/rules/prune-engine.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,15 @@ The compiler emits valid Snowflake SQL purely from `SNOWFLAKE_DIALECT` — see
- **`HASH()` reproducibility caveat.** BigQuery's `FARM_FINGERPRINT` is cross-time stable; Snowflake's `HASH()` is deterministic only *within a Snowflake release*. Sufficient for within-run prune determinism (Architectural Commitment #5); documented in `docs/prune-ops.md`.
- **Validation tiers (DEC-005).** Byte-exact Snowflake snapshot fixtures (`tests/fixtures/prune/compiled_sql/snowflake/`) are the authoritative shape gate. A gated `@pytest.mark.snowflake` suite (`tests/prune/test_compiler_fakesnow.py`, run `uv run pytest -m snowflake --no-cov`) executes the four built-ins through `fakesnow` (rule-semantic assertions, never `HASH()` value-equality) AND parses every fixture through `sqlglot`'s Snowflake dialect. Real-Snowflake `HASH(*)` semantics + case-folding + sampling are deferred to #124's live harness. The sqlglot parse-guard is the one that caught the `sample` reserved-word bug — keep a parser/executor in the loop for any new dialect.

### Databricks compiler dialect (issue #223)

The compiler emits valid Databricks/Spark-SQL purely from `DATABRICKS_DIALECT` — the **third** dialect on the dialect-driven seam (after BigQuery + Snowflake). Four #223-specific points worth keeping:

- **No `DATABRICKS_DIALECT` change was needed.** The dialect values shipped correct in the #221 skeleton; #223 only **verifies** them (unit tests + 32 byte-exact fixtures + a parse-guard) — `models.py` got a docstring edit, no field-value change. A discovery prototype compiled all 8 primitives × {full, sample} (incl. the anomaly two-query split across mad/zscore/percentile + dow) and parsed every statement under `sqlglot`'s `databricks` dialect: 26/26 OK. **Lesson: when a skeleton ships a dialect, the cert ticket is fixtures + tests, not compiler edits — verify, don't re-derive.**
- **The unquoted `sample` CTE alias is valid Spark — no reserved-word collision** (contrast Snowflake's `"sample"` bug, where `SAMPLE` is reserved). `SAMPLE` is not a Spark reserved word; `DATABRICKS_DIALECT` keeps the default `sample_cte_alias="sample"` and the parse-guard confirms it parses. Don't reflexively quote a new dialect's CTE alias — check the vendor's reserved-word list (the parse-guard catches it either way).
- **The `sqlglot` parse-guard is UNGATED (DEC-002 of #223) — deviates from the Snowflake precedent.** `tests/prune/test_compiler_databricks.py` runs `sqlglot.parse_one(fixture, dialect="databricks")` over all 32 fixtures in the **default** suite (no marker). Rationale: `sqlglot` is a base runtime dep (always importable in CI) AND Databricks has **no offline execution fake** (no fakesnow/DuckDB equivalent for Spark), so this parse-guard is the *sole* automated validity gate until the #226 live Free-Edition cert — gating it behind a marker CI never runs would defeat the #121 "keep a parser in the loop" lesson. The Snowflake parse-guard is gated only because it is *co-located* with fakesnow execution (a gated dep); there is no such coupling here. The guard pins a `>= 32` fixture-count floor (no vacuous empty-glob pass) + a planted-violation self-check (sqlglot raises on malformed SQL). **Decision rule for the next dialect: gate the parse-guard only if it's co-located with a gated execution dep; a pure-sqlglot parse-guard with no execution fake should run ungated.**
- **Validation tiers + import-guard.** 32 byte-exact fixtures (`tests/fixtures/prune/compiled_sql/databricks/` 16 + `…/anomaly/databricks/` 16, mirroring the Snowflake set 1:1) are the shape gate; the ungated parse-guard is the validity gate; real-Spark execution semantics are deferred to #226's live harness. `test_compiler_import_guard.py` `_FORBIDDEN_PREFIXES` grew `"databricks"` (planted-violation count 9 → 14) — defensive only; the compiler reads the `Dialect` object and never imports the SDK. Belt-and-braces snapshot leakage assertions check `FARM_FINGERPRINT` (BigQuery) / `HASH(*)` + `::` (Snowflake) are absent — backtick is deliberately NOT a discriminator (BigQuery and Databricks both backtick), so byte-exact **snapshot equality** is the real full-scan-leakage gate, with the leakage asserts as supplementary defence.

### Centralised bypass routing helper (issue #171)

Issue #171 introduced `_test_requires_source_table(test: CandidateTest, sample_strategy: str | None) -> bool` in `signalforge.prune.engine` as the single source of truth for "does this candidate variant require routing to the source table rather than a sampled/materialised temp?". Replaces inline isinstance checks at the two engine sites (`all_bypass_to_source` short-circuit + per-test `per_test_table_ref` override) — both now call the helper, eliminating the two-conditional drift class #170 QG Pass 3 caught.
Expand Down
2 changes: 1 addition & 1 deletion .claude/rules/warehouse-adapters.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ The ABC is warehouse-agnostic. v0.2 Snowflake/Postgres slot under `adapters/` wi

**Databricks skeleton (issue #221, epic #219).** `adapters/databricks.py` is the **fourth** adapter through the skeleton precedent (after BigQuery, the Postgres stub, and Snowflake), mirroring the #119 Snowflake skeleton verbatim: `DatabricksAdapter` captures the conn surface (`host` / `http_path` / `token` / `catalog` / `schema`) plus forward-compat OAuth-M2M params (`auth_type` / `client_id` / `client_secret` — epic open-decision: **PAT only for v0.x**, so `make_real_client` consumes only `token`); `dialect()` returns `DATABRICKS_DIALECT` (`quote_char='`'` backtick, **`identifier_case='lower'`** — Unity Catalog lower-folds, the *opposite* of Snowflake `'upper'`, like Postgres; `supports_qualify=True`; 64-bit **sign-bit-masked** `(xxhash64(to_json(struct(*))) & 9223372036854775807)` sampling hash — `xxhash64` not Murmur3-**32** (Spark's bare `hash(*)`) for collision stability, and the `& Long.MAX` mask not `ABS` because Spark's `ABS(Long.MIN_VALUE)` stays negative in non-ANSI mode and would skew `MOD(<expr>, bucket) < 1` (CodeRabbit catch on PR #254); `quote_qualified_per_component=True` for `` `catalog`.`schema`.`table` ``). `sample_rows` / `column_stats` / `run_test_sql` raise `NotImplementedError("…#219…")`; `materialise_sample` / `estimate_query_bytes` / `get_row_count` / `run_stats_query` **inherit the ABC typed degrade** (the four `*NotSupportedError` defaults). `from_profile` dispatches `profile.type == "databricks"` (lazy import; at the #221 skeleton stage it passed only `catalog=profile.project, schema=profile.dataset` — the real `host`/`http_path`/`token`/`catalog` profile fields + per-type validator were deferred to **#222**, exactly the Snowflake #119→#120 deferral; **#222 has since landed** — see § "Unified multi-warehouse `DbtProfileTarget`" → "Databricks arm"). Same three #119 notes apply: (1) **`__repr__` shows only `host` + `http_path` + `catalog`, never `token` / `schema` / `client_secret`** (repr-redaction, pinned by a secret-substring-absent test); (2) **warehouse-side confinement test** (`tests/warehouse/test_databricks_client_confinement.py`) — every `databricks-sql-connector` type/pyright-ignore lives only in `adapters/_databricks_client.py`; (3) `_DatabricksClientProtocol` (connection: `cursor()` / `close()`) split from `_DatabricksCursorProtocol` (`execute(...)` / `fetchall()` / `description` / `close()`). `databricks-sql-connector` ships under the `[databricks]` optional extra (+ dev group — **mirrored unlike `[airflow]`**, since the connector is lightweight + not constraints-pinned); the SDK import stays lazy inside `make_real_client`. The minimal `map_databricks_exception` stub maps auth-flavoured messages → `WarehouseAuthError`, else passthrough (message-marker based, no SDK import); the full table/column/syntax taxonomy + offline fake + gated live e2e land in **#226**.

**`DATABRICKS_DIALECT` values are provisional at the skeleton stage** — the prune compiler is never invoked for a Databricks profile yet (every op raises / degrades), so the dialect's SQL-fragment fields (the `xxhash64` hash, Spark `DATE_TRUNC('unit', date)` / `DAYOFWEEK(date)` / `PERCENTILE_CONT … WITHIN GROUP` date-arithmetic, `TIMESTAMP '{value}'` literals) are grounded-but-unexecuted. They are **certified by the #223 `sqlglot` `databricks`-dialect parse-guard + the #226 gated live run** — snapshot equality alone certifies shape, not validity (the #121/#171 lesson). `identifier_case='lower'` is ⚠️ load-bearing for #223's anchor-contract column matching; verify against a real `CREATE TABLE` round-trip before the compiler locks on it.
**`DATABRICKS_DIALECT` values are offline-certified as of #223; live execution deferred to #226.** At the #221 skeleton stage the dialect's SQL-fragment fields (the `xxhash64` hash, Spark `DATE_TRUNC('unit', date)` / `DAYOFWEEK(date)` / `PERCENTILE_CONT … WITHIN GROUP` date-arithmetic, `TIMESTAMP '{value}'` literals) were grounded-but-unexecuted. **#223 has since landed** the prune-compiler cert: the compiler emits valid Databricks SQL purely from `DATABRICKS_DIALECT` (no field-value change was needed — the #221 values were correct), pinned by 32 byte-exact fixtures (`tests/fixtures/prune/compiled_sql/databricks/` + `…/anomaly/databricks/`) and an **ungated** `sqlglot` `databricks`-dialect parse-guard (`tests/prune/test_compiler_databricks.py`, DEC-002 — runs in the default suite because `sqlglot` is a base dep and Databricks has no offline execution fake; see `prune-engine.md` § "Databricks compiler dialect"). Snapshot equality certifies *shape*, the parse-guard certifies *syntactic validity*; real-Spark execution semantics + a real `CREATE TABLE` round-trip (the `identifier_case='lower'` lower-fold assumption, load-bearing for anomaly-grain column matching) remain deferred to the **#226** gated live Free-Edition run — snapshot/parse equality alone does not certify the engine *accepts* the SQL (the #121/#171 lesson).

**⚠️ `TableRef.project` length blocks Unity Catalog catalog names (gotcha for #224).** `TableRef.project` validates as a GCP project id (`validate_project_id`, 6–30 chars), so a Unity Catalog catalog like `main` (4 chars) **fails `TableRef` construction**. The #221 skeleton tests sidestep with `project=None` (two-part `dataset.name`). #224's sampling path threads catalog names into `TableRef.project` (three-part `catalog.schema.table` via `quote_qualified_per_component=True`) and will hit this — either relax the project-id validation for the Databricks dialect or carry the catalog on a separate field. Surfaced while writing the skeleton stub tests; do NOT assume `TableRef` accepts arbitrary catalog identifiers.

Expand Down
2 changes: 1 addition & 1 deletion docs/prune-ops.md
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ concerns are explicitly deferred:
- **Per-decision `bytes_billed` recording (DEC-027).** The adapter does not surface job stats in v0.1; the diagnostic probe (US-003) reads them via `INFORMATION_SCHEMA.JOBS_BY_USER` out-of-band rather than through the adapter API. v0.2 extends the adapter's seam to return job stats so the `PruneDecision` can carry the figure natively.
- **Per-test `timeout_ms` threading.** `PruneConfig.test_timeout_seconds` is documented but not yet threaded through `WarehouseAdapter.run_test_sql` per call. The plumbing exists in `make_query_job_config` (DEC-013, AR-B2 of issue #6); surfacing it through the public adapter signature is a v0.2 task.
- **Test batching — Q4=B / Q4=C optimisations.** The Phase-1 plan catalogues two cost optimisations (per-column `COUNTIF` batching; temp-table-materialised sample). v0.1 does not adopt either. US-003 produces the data needed to evaluate the temp-table option in v0.2.
- **Multi-warehouse adapters.** Postgres, Databricks, Redshift adapters slot in behind `WarehouseAdapter` without prune changes once their adapters land. The prune compiler is fully dialect-driven (DEC-025), reading all warehouse-specific SQL from the `Dialect` value object, never branching on dialect `name`. **Snowflake compiler support landed in issue #121** (see § "Snowflake compiler dialect" above); its live warehouse harness is #124. A new vendor populates a `Dialect` and the compiler emits correct SQL with no compiler change.
- **Multi-warehouse adapters.** Postgres, Databricks, Redshift adapters slot in behind `WarehouseAdapter` without prune changes once their adapters land. The prune compiler is fully dialect-driven (DEC-025), reading all warehouse-specific SQL from the `Dialect` value object, never branching on dialect `name`. **Snowflake compiler support landed in issue #121** (see § "Snowflake compiler dialect" above); its live warehouse harness is #124. **Databricks compiler support landed in issue #223:** `DATABRICKS_DIALECT` emits Spark/Databricks SQL (backtick quoting, `xxhash64` sign-bit-masked sampling hash, Spark date-arithmetic fragments), pinned by byte-exact snapshot fixtures under `tests/fixtures/prune/compiled_sql/databricks/` and certified for syntactic validity by an **ungated** `sqlglot` `databricks`-dialect parse-guard over every fixture (`tests/prune/test_compiler_databricks.py`, runs in the default suite — no marker — because `sqlglot` is a base dep and Databricks has no offline execution fake); real-Spark execution semantics are deferred to the live harness in #226. A new vendor populates a `Dialect` and the compiler emits correct SQL with no compiler change.
- **Confidence intervals on `always-passes`.** Surfacing "less than or equal to 3/N upper-bound failure rate at 95 percent confidence" (rule of three) on the decision record so reviewers can calibrate the always-pass verdict. Also covers great-expectations-style `mostly:` thresholds.
- **Historical always-pass evidence.** Running candidate tests against multiple `run_results.json` snapshots to assert "never failed in last N runs." The Phase-1 plan considers this for the `failed-on-known-clean-data` evidence channel and defers to v0.2.
- **dbt-utils test types.** `dbt_utils.unique_combination_of_columns`, `dbt_utils.accepted_range`, `dbt_utils.expression_is_true`, etc. The drafter's `CandidateTest` union has six variants — the four generic schema tests plus the `custom_sql` business-rule escape hatch (issue #116) plus `row_count_between` (#169); the prune compiler compiles all six. **One `dbt_expectations` macro graduated in #169:** `dbt_expectations.expect_table_row_count_to_be_between` is recognised by `prune-existing` and promoted to the structured `row_count_between` variant (see `docs/ingest-ops.md` § "Recognition of `expect_table_row_count_to_be_between`"). Other namespaced dbt-utils / dbt-expectations macros remain v0.2+ territory (a `custom_sql` test can express many of them by hand in the meantime).
Expand Down
Loading
Loading