fix(analytics): move the DuckDB live suite out of tests/integration, and add the BI docs
CI (SIAX Cloud) / security (push) Successful in 28s
CI (SIAX Cloud) / integration (push) Successful in 42s
Security Scan (SIAX Cloud) / security (push) Successful in 21s
CI (SIAX Cloud) / quality (push) Successful in 1m2s
CI (SIAX Cloud) / security (pull_request) Successful in 23s
CI (SIAX Cloud) / integration (pull_request) Successful in 40s
CI (SIAX Cloud) / quality (pull_request) Successful in 58s
Security Scan (SIAX Cloud) / security (pull_request) Successful in 20s

CI's integration job failed, and it failed CORRECTLY. That job provides a live
OpenSearch and a live ClickHouse and then asserts that nothing in
tests/integration/ skipped, on the stated principle that "a skipped suite must
never masquerade as a green integration gate". The DuckDB suite needs a Python
interpreter that job does not provide, so it skipped, and the guard fired — the
first suite in that directory ever to skip there.

Moving the file to tests/duckdb/ respects the guard rather than weakening it.
Loosening it to tolerate skips would have traded a real gate for one green job,
which is the trade this estate keeps regretting. The suite still runs, still
fails rather than skipping when DUCKDB_PYTHON is set and DuckDB is broken, and
is still pinned by the committed evidence.

Also fixed: "does NOT ship the Iceberg table" asserted against the FILESYSTEM
when the claim is about what is COMMITTED. The live suite rebuilds that table
locally, so the assertion failed whenever both suites ran in one pass — it was
testing the wrong thing rather than catching a real problem. It now asserts
against `git ls-files`, and also asserts the parquet/csv fixtures ARE tracked,
so a version that stopped shipping any fixture would fail too.

docs/analytics/BI_QUERY_METRIC_QUALITY.md is organised around what is PROVEN
and what is NOT, because the second list is the one that gets dropped. It
records the five findings and the six open items, including that BI-P029's
MetricFlow was never executed and that BI-P021's budget is a bound rather than
a hard cap (measured 130x overshoot at a 100ms poll).

It also reports, without changing it, a latent flaw in .gitea/workflows/ci.yml
(a protected file this branch does not touch): the integration guard's second
pattern `Tests  [0-9]+ passed` never matches, because vitest writes that
summary with ANSI colour codes between the words. The guard is therefore
STRICTER than it reads — "any skip fails" rather than "skipped and nothing
passed" — so nothing is currently unguarded, but the escape hatch does not
work and someone will eventually be puzzled by it.

957 tests pass with ClickHouse, Postgres and DuckDB all live.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
2026-09-07 03:08:11 +02:00
parent 2bfbd3f159
commit 3ee023017b
9 changed files with 183 additions and 16 deletions
+155
View File
@@ -0,0 +1,155 @@
# BI-P014, P016–P022, P025–P026, P028–P031 — query, metric, quality
Extends the analytics core merged in `0pt#34` (BI-P002..P012). Nothing there is
rebuilt.
This document is organised around what is **proven** and what is **not**,
because the second list is the one that gets dropped.
---
## Where things live
| prompt | module | live proof |
| -------------------------------------- | ----------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
| BI-P014 DuckDB provider | `src/analytics/providers/duckdb-provider.ts` | `tests/duckdb/duckdb-provider.live.test.ts` |
| BI-P016 query request contract | `src/analytics/contract/query-request.ts`, `compile-clickhouse.ts`, `compile-semantic.ts` | unit only (pure compilation) |
| BI-P017 analyst SQL contract | `src/analytics/sql/` | `tests/integration/analyst-sql-clickhouse.integration.test.ts` |
| BI-P018 execution model | `src/analytics/execution/query-execution-record.ts` | `sql/009` + Postgres realinfra |
| BI-P019 cancellation | `src/analytics/execution/cancellation.ts` | `tests/integration/query-cancellation-budget.integration.test.ts` |
| BI-P020 query cache | `src/analytics/cache/query-cache.ts` | unit only (no engine involved) |
| BI-P021 cost / budget | `src/analytics/cost/query-budget.ts` | same live suite as BI-P019 |
| BI-P022 dbt contract | `src/analytics/transform/dbt-contract.ts`, `analytics/dbt/` | contract only — execution is `n0d#38` |
| BI-P025 quality provider | `src/analytics/quality/` | `tests/integration/data-quality-clickhouse.integration.test.ts` |
| BI-P026 freshness | `src/analytics/quality/freshness.ts` | unit + `sql/009` defaults |
| BI-P028/P031 metric model + governance | `src/analytics/metrics/metric-model.ts` | Postgres trigger, realinfra |
| BI-P029 semantic provider | `src/analytics/metrics/semantic-provider.ts` | **partially — see below** |
| BI-P030 semantic registry | `src/analytics/metrics/semantic-registry.ts` | unit + Postgres constraints |
---
## Five findings that changed the implementation
Each was found by writing a test that could fail, and each is pinned so it
cannot quietly stop being true.
### 1. ClickHouse `readonly = 1` on a user profile forbids **every** request setting
Measured on 24.8.14.39. A user whose profile carries `readonly = 1` answers
`Code: 164. Cannot modify 'max_rows_to_read' setting in readonly mode` to any
setting sent with the statement. So BI-P012's ceilings **cannot travel with an
analyst statement** — they are baked into the user's `SETTINGS PROFILE` at
provisioning. Proven both ways: an over-budget scan fails with code 158 from
the profile with no setting sent, and an attempt to raise it from the request
fails with 164.
### 2. `CREATE ROW POLICY IF NOT EXISTS` is a silent no-op on re-provisioning
Dropping a user empties the policy's apply-to list. The next provisioning run
sees the policy exists and does nothing, so it survives with
`apply_to_list = []` — applying to **nobody** — while `system.row_policies`
still shows the correct `select_filter`. The restricted user read 100 of 100
rows, **both tenants**, from the base table.
Fixed with `OR REPLACE`, plus `rowPolicyVerificationSql()` which turns "the
policy is applied" from an assumption into a query, and a test asserting
`applied_to_user = 1` per object. `CREATE SETTINGS PROFILE IF NOT EXISTS` has
the identical shape and is fixed the same way.
### 3. DuckDB's `allowed_directories` **alone** is a no-op
Measured on 1.5.5. With `allowed_directories` set to one mount directory and
nothing else, `read_csv('/etc/passwd')` **returned the file** and an `https://`
URL was fetched over the network. Only `enable_external_access = false`
alongside it makes the allow-list bind.
The obvious spelling of this control appears in `duckdb_settings()`, looks like
a sandbox, and permits everything. The measurement is pinned by a test that
never skips, so simplifying the bootstrap breaks the build rather than the
guarantee.
### 4. `pg_constraint.conname` is not schema-scoped
`SELECT 1 FROM pg_constraint WHERE conname = '...'` matches a same-named
constraint in **any** schema, so `sql/009` applied into a second schema
silently skipped every guarded `CHECK`. Every guard now scopes via
`conrelid = '<table>'::regclass`.
### 5. `describe.skipIf(flagSetInBeforeAll)` skips **always**
Vitest evaluates `skipIf` at collection time. Ten tests reported green while
skipping against a live, reachable ClickHouse. Replaced with the repo's
env-var gate, which **fails** when the variable is set and the server is down.
---
## What is NOT proven
- **BI-P029 / MetricFlow was never executed.** No Python runtime is
provisioned for this repo's CI, and BI-P029's "pin a currently verified
version" needs a licence and supply-chain review that has not happened.
`MetricFlowProvider.providerReadiness()` returns `verified: false` and names
all four open items. What _is_ proven is that the engine-neutral spec
compiles to a MetricFlow-shaped query and that canonical SIAX metric ids
survive a provider swap — tested against a second, deliberately
different-named provider so the equality is not trivially true.
- **The Superset integration path and the AI Analyst comparison** (BI-P029's
other two DoD surfaces) are not implemented.
- **dbt is not run here.** `DBT_RUNTIME_PIN` has both review flags `false`, and
`assertDbtRuntimeCertified` refuses a production run until they are true.
- **DuckDB Arrow IPC** is unproven: the `arrow` extension is a network install
the offline environment could not download. `readiness()` says so.
- **BI-P021's budget is a bound, not a hard cap.** In-flight enforcement is
granular to the poll interval. Measured: a 1,000,000-row window cancelled the
query at **130,883,409 rows**, 92 ms in — a 130× overshoot. Still a bound
(unwatched, the same query read 6.5 billion rows in one second), but anyone
quoting it as a hard cap is wrong.
- **The per-process window store does not bind across replicas.**
`describeEnforcement()` reports `"with N replicas a tenant can spend up to N
times this budget"`. A shared store (Valkey, or the CL0UD quota service)
behind `SharedWindowStore` is required for it to bind.
---
## Running the live suites
```bash
docker run -d --name ch -p 28123:8123 -e CLICKHOUSE_SKIP_USER_SETUP=1 \
clickhouse/clickhouse-server:24.8
docker run -d --name pg -p 25433:5432 -e POSTGRES_PASSWORD=test \
-e POSTGRES_DB=opt postgres:16-alpine
CLICKHOUSE_URL=http://127.0.0.1:28123 pnpm vitest run tests/integration
ANALYTICS_TEST_DATABASE_URL=postgres://postgres:test@127.0.0.1:25433/opt \
pnpm vitest run tests/analytics/analytics-governance-postgres.realinfra.test.ts
python -m venv .venv && .venv/bin/pip install duckdb pyarrow 'pyiceberg[sql-sqlite]'
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/duckdb
```
Every one of these **fails** rather than skipping when its variable is set and
the backend is unreachable.
---
## One note for whoever maintains `.gitea/workflows/ci.yml`
Not changed here — it is a protected file, and this branch touched none.
The integration job's guard is:
```sh
if grep -qE "skipped" /tmp/itest.log && ! grep -qE "Tests [0-9]+ passed" /tmp/itest.log
```
Vitest writes that summary with ANSI colour codes, so the literal text is
`Tests [22m [1m[32m81 passed[39m…`. The second pattern therefore **never
matches**, and the guard is effectively "any skip fails the job" rather than
the "skipped _and_ nothing passed" it reads as. That is stricter than intended,
not looser, so nothing is currently unguarded — but the escape hatch does not
work, and someone will eventually be puzzled by it. Piping through
`sed 's/\x1b\[[0-9;]*m//g'` before the grep would restore the intended
behaviour.
This is how the DuckDB suite's placement was caught, so the guard earned its
keep either way.
+1 -1
View File
@@ -1,6 +1,6 @@
{
"duckdb_version": "1.5.5",
"recorded_at": "2026-09-07T00:53:52.439Z",
"recorded_at": "2026-09-07T01:07:03.476Z",
"parquet": {
"SE": 394.5,
"NO": 105.5
@@ -2,7 +2,7 @@
"prompt": "BI-P017",
"claim": "Every bypass attempt, executed directly against a live ClickHouse as the tenant's restricted user WITH THE STATEMENT GUARD REMOVED, fails to return a foreign-tenant row, an ungranted-table row, or server metadata.",
"clickhouse_version": "24.8.14.39",
"recorded_at": "2026-09-07T00:19:07.198Z",
"recorded_at": "2026-09-07T01:07:07.196Z",
"base_table_rows_both_tenants": 100,
"tenant_view_rows_alpha": 50,
"attempts_total": 64,
@@ -572,7 +572,7 @@
"leaked_ungranted_table_row": false,
"leaked_server_metadata": false,
"guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: \"SYSTEM\" is not permitted in a read-only analyst session",
"engine_response_head": "ALPHA_ROW_0 ALPHA_ROW_1 ALPHA_ROW_2 ALPHA_ROW_3 ALPHA_ROW_4 ALPHA_ROW_5 ALPHA_ROW_6 ALPHA_ROW_7 ALPHA_ROW_8 ALPHA_ROW_9 ALPHA_ROW_10 ALPHA_ROW_11 ALPHA_ROW_12 A"
"engine_response_head": "events sales events sales ALPHA_ROW_0 ALPHA_ROW_1 ALPHA_ROW_2 ALPHA_ROW_3 ALPHA_ROW_4 ALPHA_ROW_5 ALPHA_ROW_6 ALPHA_ROW_7 ALPHA_ROW_8 ALPHA_ROW_9 ALPHA_ROW_10 A"
},
{
"id": "unterminated-comment",
@@ -656,7 +656,7 @@
"leaked_ungranted_table_row": false,
"leaked_server_metadata": false,
"guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: \"SYSTEM\" is not permitted in a read-only analyst session",
"engine_response_head": "opt_analytics events 12706469-5605-436f-94ff-fc3c112aec6f MergeTree 0 ['/var/lib/clickhouse/store/127/12706469-5605-436f-94ff-fc3c112aec6f/'] /var/lib/clickhous"
"engine_response_head": "opt_analytics events 30087fa6-7696-4216-abc7-507d8ab01e10 MergeTree 0 ['/var/lib/clickhouse/store/300/30087fa6-7696-4216-abc7-507d8ab01e10/'] /var/lib/clickhous"
},
{
"id": "describe-subquery",
@@ -1,20 +1,20 @@
{
"clickhouse_version": "24.8.14.39",
"recorded_at": "2026-09-07T00:25:59.706Z",
"recorded_at": "2026-09-07T01:07:02.373Z",
"counterfactual_still_running_after_2s": true,
"cancellation": {
"rows_read_before_cancel": 9124163046,
"elapsed_ms_to_verified_stop": 35,
"rows_read_before_cancel": 4955255022,
"elapsed_ms_to_verified_stop": 110,
"verified_stopped": true,
"clickhouse_query_log_type": "ExceptionWhileProcessing",
"clickhouse_exception_code": 394
},
"budget_in_flight": {
"budget_max_rows_scanned": 1000000,
"observed_rows_read_at_cancel": 127743777,
"observed_elapsed_ms_at_cancel": 12,
"observed_rows_read_at_cancel": 215980518,
"observed_elapsed_ms_at_cancel": 60,
"polls_before_breach": 1,
"overshoot_factor": 128,
"overshoot_factor": 216,
"overshoot_note": "in-flight enforcement is granular to the poll interval; the query overshot the window by this factor before the poll that caught it. A bound, not a hard cap.",
"cancellation_verified": true,
"clickhouse_exception_code": 394,
+1 -1
View File
@@ -1,6 +1,6 @@
{
"clickhouse_version": "24.8.14.39",
"recorded_at": "2026-09-07T00:40:02.745Z",
"recorded_at": "2026-09-07T01:07:17.084Z",
"not_null": {
"evaluated": 118,
"violations": 5
+1 -1
View File
@@ -39,7 +39,7 @@
* refuses a session missing any of them.
*
* Evidence: evidence/bi-p014/duckdb-sandbox.json, written by
* tests/integration/duckdb-provider.integration.test.ts, which runs the bypass
* tests/duckdb/duckdb-provider.live.test.ts, which runs the bypass
* corpus with LAYER 1 REMOVED and requires layer 2 to refuse it anyway.
*/
import path from 'node:path';
+1 -1
View File
@@ -4,7 +4,7 @@
* BI-P014 — the parts provable without a DuckDB, plus the pin on the parts
* that are not.
*
* The live suite (tests/integration/duckdb-provider.integration.test.ts) only
* The live suite (tests/duckdb/duckdb-provider.live.test.ts) only
* runs when DUCKDB_PYTHON is set. On a runner without it, it contributes
* nothing — so the recorded evidence is committed and THIS file, which never
* skips, checks it. In particular it pins the finding that makes layer 2 real:
@@ -1,6 +1,6 @@
/**
* tests/integration/duckdb-provider.integration.test.ts
* ======================================================
* tests/duckdb/duckdb-provider.live.test.ts
* ==========================================
* BI-P014 — the DuckDB provider against a real DuckDB.
*
* Two things are proven here that a unit test cannot:
@@ -22,6 +22,18 @@
*
* Gated on DUCKDB_PYTHON, an interpreter with the `duckdb` package. With it
* set and DuckDB broken, this suite FAILS; it does not degrade to a skip.
*
* ── WHY THIS IS NOT UNDER tests/integration/ ──────────────────────────────
* It was, and CI's integration job failed — correctly. That job provides a
* live OpenSearch and a live ClickHouse and then asserts that NOTHING in
* tests/integration/ skipped, on the stated principle that "a skipped suite
* must never masquerade as a green integration gate". A DuckDB suite needing
* a Python interpreter that job does not provide is not one of the suites that
* job is a gate for, so it skipped, and the guard fired.
*
* Moving the file respects that guard rather than weakening it. Loosening the
* guard to tolerate skips would have traded a real gate for one green job —
* which is the trade this estate keeps regretting.
*/
import { execFileSync } from 'node:child_process';
import fs from 'node:fs';
+1 -1
View File
@@ -14,7 +14,7 @@ To rebuild the Iceberg fixture locally:
```
python -m venv .venv && .venv/bin/pip install duckdb pyarrow 'pyiceberg[sql-sqlite]'
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/integration/duckdb-provider.integration.test.ts
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/duckdb/duckdb-provider.live.test.ts
```
`iceberg_scan` is pointed at an EXACT metadata JSON file rather than a table