DEV-1562: live-Metabase end-to-end pg-facade integration test suite

.github/workflows/ci.yml

@@ -1,5 +1,8 @@
 name: CI
 
+permissions:
+  contents: read
+
 on:
   push:
     branches: [main]
@@ -51,8 +54,13 @@ jobs:
         # + testcontainers + (for SQL Server) the ODBC Driver 18 — provisioned
         # only in those workflows. Exclude them from the always-on integration
         # job; they run when their respective dialect files change.
+        #
+        # DEV-1562: the live-Metabase e2e suite (`metabase_e2e` marker) has
+        # its own workflow at .github/workflows/pg-facade-e2e.yml — same
+        # reasoning, exclude here so this job isn't on the hook for booting
+        # a Metabase container.
         run: |
-          poetry run pytest tests/ -v -m integration --timeout=180 \
+          poetry run pytest tests/ -v -m "integration and not metabase_e2e" --timeout=180 \
             --ignore=tests/integration/test_integration_mysql.py \
             --ignore=tests/integration/test_integration_clickhouse.py \
             --ignore=tests/integration/test_integration_sqlserver.py

.github/workflows/pg-facade-e2e.yml

@@ -0,0 +1,89 @@
+name: PG Facade Live-Metabase E2E
+
+permissions:
+  contents: read
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - 'slayer/pg_facade/**'
+      - 'slayer/facade/**'
+      - 'slayer/demo/**'
+      - 'tests/integration/test_metabase_e2e.py'
+      - 'tests/integration/conftest_metabase.py'
+      - 'tests/integration/_pg_serve_helpers.py'
+      - 'tests/integration/conftest.py'
+      - 'pyproject.toml'
+      - 'poetry.lock'
+      - '.github/workflows/pg-facade-e2e.yml'
+  pull_request:
+    branches: [main]
+    paths:
+      - 'slayer/pg_facade/**'
+      - 'slayer/facade/**'
+      - 'slayer/demo/**'
+      - 'tests/integration/test_metabase_e2e.py'
+      - 'tests/integration/conftest_metabase.py'
+      - 'tests/integration/_pg_serve_helpers.py'
+      - 'tests/integration/conftest.py'
+      - 'pyproject.toml'
+      - 'poetry.lock'
+      - '.github/workflows/pg-facade-e2e.yml'
+
+jobs:
+  metabase-e2e:
+    runs-on: ubuntu-latest
+    timeout-minutes: 25
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python 3.11
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+
+      - name: Install Poetry
+        run: pip install poetry
+
+      - name: Install dependencies
+        run: poetry install -E all --with dev
+
+      - name: Install jafgen (for demo data generation)
+        run: poetry run pip install git+https://github.com/rossbowen/jaffle-shop-generator.git@09557a1118b000071f8171aa97d54d5029bf0f0b
+
+      - name: Cache Metabase image
+        id: mb_cache
+        uses: actions/cache@v4
+        with:
+          path: /tmp/metabase-image.tar
+          key: metabase-image-v0.62.1.5-${{ runner.os }}-${{ runner.arch }}
+
+      - name: Load Metabase image from cache
+        if: steps.mb_cache.outputs.cache-hit == 'true'
+        run: docker load -i /tmp/metabase-image.tar
+
+      - name: Pull and save Metabase image
+        if: steps.mb_cache.outputs.cache-hit != 'true'
+        run: |
+          docker pull metabase/metabase:v0.62.1.5
+          docker save metabase/metabase:v0.62.1.5 -o /tmp/metabase-image.tar
+
+      - name: Run live-Metabase e2e suite
+        timeout-minutes: 15
+        run: poetry run pytest -m metabase_e2e tests/integration/test_metabase_e2e.py -v --timeout=300
+
+      - name: Dump Metabase container logs on failure
+        if: failure()
+        run: |
+          if [ -f /tmp/slayer-metabase-e2e-container.log ]; then
+            echo "=== /tmp/slayer-metabase-e2e-container.log ==="
+            cat /tmp/slayer-metabase-e2e-container.log
+          else
+            echo "No fixture-side log dump found — falling back to docker logs."
+            for cid in $(docker ps -aq --filter ancestor=metabase/metabase:v0.62.1.5); do
+              echo "=== container $cid ==="
+              docker logs --tail=400 "$cid" || true
+            done
+          fi

CLAUDE.md

@@ -240,12 +240,16 @@ poetry run pytest tests/integration/ -m integration
 poetry run pytest tests/integration/test_integration.py -m integration        # SQLite
 poetry run pytest tests/integration/test_integration_postgres.py -m integration  # Postgres
 poetry run pytest tests/integration/test_integration_duckdb.py -m integration    # DuckDB
+
+# Run live-Metabase end-to-end suite (DEV-1562) — needs Docker
+poetry run pytest -m metabase_e2e tests/integration/test_metabase_e2e.py -v
 ```
 
 - Unit tests: `tests/test_models.py`, `test_sql_generator.py`, `test_storage.py`, `test_sqlite_storage.py`, `test_mcp_server.py`
 - Integration tests (SQLite): `tests/integration/test_integration.py`
 - Integration tests (Postgres): `tests/integration/test_integration_postgres.py` — uses pytest-postgresql (auto-spawns temp Postgres)
 - Integration tests (DuckDB): `tests/integration/test_integration_duckdb.py` — uses duckdb directly (no Docker)
+- **Live-Metabase e2e (DEV-1562)**: `tests/integration/test_metabase_e2e.py` (marker: `metabase_e2e`). Boots one `metabase/metabase:v0.62.1.5` container alongside two token-protected pg-serve instances (per-session random tokens, both bound on `0.0.0.0` so the container reaches them via `host.docker.internal`; the second instance backs the L.2 / L.3 bad-password tests). Drives ~62 cases against the real Metabase REST API + pgjdbc protocol. Skips cleanly when Docker is unavailable. Local one-shot ≈ 4 minutes. CI runs only on PRs that touch `slayer/pg_facade/`, `slayer/facade/`, `slayer/demo/`, the e2e test files, or `pyproject.toml` / `poetry.lock` (see `.github/workflows/pg-facade-e2e.yml`).
 - Shared fixtures in `tests/conftest.py`
 
 ## Linting

docs/interfaces/pg-facade.md

@@ -165,3 +165,33 @@ The facade is pure-stdlib; the extra exists only to keep the install path consis
 ```bash
 pip install "motley-slayer[pg_facade]"
 ```
+
+## Testing your changes
+
+For wire-level / translator changes, the unit suite under `tests/test_pg_facade*.py` covers
+each component in isolation. Behaviour at the *interaction boundary* with a real BI client
+is covered by the live-Metabase end-to-end suite (DEV-1562):
+
+```bash
+poetry run pytest -m metabase_e2e tests/integration/test_metabase_e2e.py -v
+```
+
+The suite needs Docker; it boots `metabase/metabase:v0.62.1.5` alongside two
+token-protected pg-serve processes (per-session random tokens, both bound on `0.0.0.0`
+so the container reaches them via `host.docker.internal`; the second backs the L.2 / L.3
+bad-password tests) and drives ~62 cases through the real `pgjdbc` protocol — bootstrap +
+sync, MBQL aggregations and time-grain breakouts, native-SQL probes, wire-format
+round-trips, transactions, concurrency, and error envelopes. Skips cleanly when Docker is
+unavailable.
+
+Known limitations (each tracked by a strict-`xfail` against a Linear ticket — the day the
+referenced gap is fixed, the test XPASSes and CI flips red, prompting a lift):
+LEFT JOIN-with-subquery projection (DEV-1565), CAST(col AS type) projection (DEV-1566),
+catalog fingerprint measure leak (DEV-1567), MBQL aggregation-ordinal refs in HAVING /
+ORDER BY (DEV-1568), per-connection `SET` state (DEV-1569), Bind-path typed-sentinel for
+INT-vs-empty-string (DEV-1570), and **Metabase week breakouts** (DEV-1572 — Metabase emits
+a Sunday-week wrapper that doesn't match SLayer's Monday-based `WEEK` granularity).
+
+CI fires automatically on PRs touching `slayer/pg_facade/`, `slayer/facade/`,
+`slayer/demo/`, the
+e2e test files, or `pyproject.toml` / `poetry.lock`.

pyproject.toml

@@ -128,6 +128,7 @@ testpaths = ["tests"]
 addopts = "--ignore=tests/perf"
 markers = [
     "integration: marks tests as integration tests (require a real database)",
+    "metabase_e2e: live-Metabase end-to-end suite (requires Docker)",
 ]
 asyncio_mode = "auto"
 

slayer/facade/translator.py

@@ -293,9 +293,19 @@ def _apply_strip_prefix(
 def _detect_time_grain_date_trunc(
     node: exp.Expression,
 ) -> Optional[Tuple[TimeGranularity, exp.Column]]:
+    """Plain ``DATE_TRUNC(<unit>, <col>)`` detector.
+
+    Does NOT unwrap any day offsets on the column side — a bare
+    ``DATE_TRUNC('week', col + INTERVAL '1 day')`` is a user-written
+    shifted bucket, not the Metabase Sunday-week wrapper, and must
+    fall through to the regular "unsupported projection" error.
+    The full Sunday-week pattern is handled by
+    ``_detect_sunday_week_wrapper`` which requires BOTH the outer ``-1
+    day`` shift and the inner ``+1 day`` shift to be present together.
+    """
     unit = node.args.get("unit")
     col = node.this
-    if unit is None or not isinstance(col, exp.Column):
+    if unit is None:
         return None
     # The unit is a string literal under the dialect-less parse and a bare
     # identifier (``exp.Var``) under the Postgres dialect's TIMESTAMP_TRUNC.
@@ -306,9 +316,104 @@ def _detect_time_grain_date_trunc(
     grain = _TIME_GRAIN_NAMES.get(unit_str)
     if grain is None:
         return None
+    if isinstance(col, exp.Cast):
+        col = col.this
+    if not isinstance(col, exp.Column):
+        return None
     return grain, col
 
 
+def _detect_sunday_week_wrapper(
+    node: exp.Expression,  # NOSONAR(S1172) — placeholder until DEV-1572 lands
+) -> Optional[Tuple[TimeGranularity, exp.Column]]:
+    """Stub — Sunday-week wrapper recognition is deferred to DEV-1572.
+
+    DEV-1562 PR #182 rounds 4-5 added detection of Metabase's full
+    Sunday-week shape — ``(CAST(DATE_TRUNC('week', col + INTERVAL '1 day')
+    AS DATE) + INTERVAL '-1 day')`` — and mapped it to ``WEEK(col)``.
+    Codex round-10 flagged that as a correctness bug: SLayer's existing
+    ``WEEK`` granularity is Monday-based (``date.weekday()``) and silently
+    swapping Metabase's Sunday buckets for Monday ones shifts row labels
+    by a day and reshuffles the row→bucket assignment.
+
+    Until SLayer grows a real ``WEEK_SUNDAY`` granularity per dialect
+    (tracked in DEV-1572), this stub returns ``None`` so the wrapper
+    falls through to the existing "Unsupported projection expression"
+    error — failing loudly with a clear message is better than silently
+    bucketing the wrong way. The Metabase week-breakout test in the e2e
+    suite is xfail(strict=True) referencing DEV-1572 and will XPASS the
+    day the real granularity lands.
+    """
+    return None
+
+
+def _day_interval_sign(node: exp.Expression) -> Optional[int]:
+    """Return ``+1`` for ``INTERVAL '1 day'``, ``-1`` for ``INTERVAL '-1 day'``,
+    or ``None`` if ``node`` isn't a one-day interval at all.
+
+    Recognised forms:
+    * Dialect-less parse: ``INTERVAL '1 day'`` / ``INTERVAL '-1 day'`` — the
+      literal carries the unit string.
+    * Postgres dialect: ``INTERVAL '1' DAY`` — literal is the magnitude,
+      unit is a separate ``DAY`` node.
+    """
+    if not isinstance(node, exp.Interval):
+        return None
+    val = node.this
+    if not isinstance(val, exp.Literal):
+        return None
+    s = str(val.this).strip().lower().replace("'", "")
+    unit = node.args.get("unit")
+    unit_str = ""
+    if unit is not None:
+        unit_str = str(unit.this if hasattr(unit, "this") else unit).lower()
+    if s in {"1", "-1"}:
+        if not unit_str.startswith("day"):
+            return None
+        return 1 if s == "1" else -1
+    if s == "1 day":
+        return 1
+    if s == "-1 day":
+        return -1
+    return None
+
+
+def _unwrap_signed_day_offset(
+    node: exp.Expression, *, expected_sign: int,
+) -> exp.Expression:
+    """If ``node`` shifts by exactly ``expected_sign`` days (``+1`` or ``-1``)
+    via a single ADD/SUB of ``INTERVAL '1 day'`` / ``INTERVAL '-1 day'``,
+    return the inner expression. Otherwise return ``node`` unchanged.
+
+    Direction matters: ``expected_sign=-1`` matches Metabase's outer
+    Sunday-week wrapper (``<expr> + INTERVAL '-1 day'`` or
+    ``<expr> - INTERVAL '1 day'``) but NOT the inverse, so a legitimate
+    user-written ``DATE_TRUNC('week', x + INTERVAL '1 day')`` outside the
+    Sunday-week wrapper stays preserved (would not match
+    ``expected_sign=-1``). ``expected_sign=+1`` matches the inner
+    column-side shift (``<col> + INTERVAL '1 day'`` or
+    ``<col> - INTERVAL '-1 day'``).
+    """
+    if isinstance(node, exp.Paren):
+        inner = _unwrap_signed_day_offset(node.this, expected_sign=expected_sign)
+        if inner is not node.this:
+            return inner
+    if isinstance(node, exp.Add):
+        # Adding +1 day → net +1; adding -1 day → net -1.
+        right_sign = _day_interval_sign(node.expression)
+        if right_sign is not None and right_sign == expected_sign:
+            return node.this
+        left_sign = _day_interval_sign(node.this)
+        if left_sign is not None and left_sign == expected_sign:
+            return node.expression
+    if isinstance(node, exp.Sub):
+        # Subtracting +1 day → net -1; subtracting -1 day → net +1.
+        right_sign = _day_interval_sign(node.expression)
+        if right_sign is not None and -right_sign == expected_sign:
+            return node.this
+    return node
+
+
 def _detect_time_grain_single_arg(
     node: exp.Expression,
 ) -> Optional[Tuple[TimeGranularity, exp.Column]]:
@@ -351,6 +456,19 @@ def _detect_time_grain(node: exp.Expression) -> Optional[Tuple[TimeGranularity,
         unwrapped = _detect_time_grain(node.this)
         if unwrapped is not None:
             return unwrapped
+    # DEV-1562 / DEV-1558 round-20 follow-up: Metabase emits Sunday-based
+    # week truncation as the full wrapper
+    # ``CAST((CAST(DATE_TRUNC('week', col + INTERVAL '1 day') AS DATE) +
+    # INTERVAL '-1 day') AS DATE)``. Detect the complete pattern as a single
+    # match — partial wrappers (just the outer -1d, or just the inner +1d)
+    # are NOT Sunday-week and must keep raising as unsupported projections.
+    sunday_week = _detect_sunday_week_wrapper(node)
+    if sunday_week is not None:
+        return sunday_week
+    if isinstance(node, exp.Paren):
+        recur = _detect_time_grain(node.this)
+        if recur is not None:
+            return recur
     if isinstance(node, (exp.DateTrunc, exp.TimestampTrunc)):
         match = _detect_time_grain_date_trunc(node)
         if match is not None: