feat(DEV-1809): import column/table/dataset comments during schema ingestion

.claude/skills/slayer-overview.md

@@ -12,7 +12,7 @@ SLayer is a lightweight, agent-first semantic layer. Instead of writing raw SQL,
 - **SQLGenerator** — takes an EnrichedQuery (not SlayerQuery) and converts it to SQL via sqlglot (dialect-aware: postgres, mysql, bigquery, etc.)
 - **SlayerSQLClient** — executes SQL via SQLAlchemy with retry logic and statement timeouts
 - **Storage** — YAML or SQLite backends for model and datasource configs
-- **Ingestion** — auto-generates models from DB schema with rollup-style FK joins (denormalized LEFT JOINs). It can be triggered manually (`slayer ingest`, `ingest_datasource_models`, `POST /ingest`) or **on every server boot** via `slayer serve --ingest-on-startup` / `slayer mcp --ingest-on-startup` (also `SLAYER_INGEST_ON_STARTUP=1`, or `create_app/create_mcp_server(ingest_on_startup=True)` programmatically). It is idempotent and continues on per-datasource failures.
+- **Ingestion** — auto-generates models from DB schema with rollup-style FK joins (denormalized LEFT JOINs), importing DB column/table comments into `description`s (fill-if-empty, never overwriting; BigQuery also fills the datasource description from the dataset description; SQLite has no comments). It can be triggered manually (`slayer ingest`, `ingest_datasource_models`, `POST /ingest`) or **on every server boot** via `slayer serve --ingest-on-startup` / `slayer mcp --ingest-on-startup` (also `SLAYER_INGEST_ON_STARTUP=1`, or `create_app/create_mcp_server(ingest_on_startup=True)` programmatically). It is idempotent and continues on per-datasource failures.
 - **Interfaces** — MCP server (stdio via `slayer mcp`, SSE via `slayer serve` at `/mcp/sse`), REST API (FastAPI on port 5143), Python SDK, and two read-only wire-protocol facades for BI tools: Arrow Flight SQL (`slayer flight-serve`, port 5144) and Postgres (`slayer pg-serve`, port 5145; the connection `database` selects the SLayer datasource)
 
 ## Key Models

.github/workflows/ci.yml

@@ -141,6 +141,13 @@ jobs:
         timeout-minutes: 5
         run: poetry run python examples/bigquery/verify.py
 
+      - name: Run BigQuery integration tests
+        if: steps.gate.outputs.skip != 'true'
+        timeout-minutes: 10
+        env:
+          GCP_PROJECT_ID: ${{ secrets.GCP_PROJECT_ID }}
+        run: poetry run pytest tests/integration/test_integration_bigquery.py -m integration -v
+
       - name: Dump server logs on failure
         if: failure() && steps.gate.outputs.skip != 'true'
         run: cat /tmp/slayer-bq.log

DECISIONS.md

@@ -78,3 +78,4 @@ implementation detail. Include issue refs when known.
 - 2026-08-12 — Dotted dimension join-path binding (DEV-1780): a dotted dimension/time-dimension path resolves only when every hop is a direct join. Previously a hop that was not a direct join fell through leniently — the enriched dim kept its `A__B` alias in SELECT/GROUP BY but `_resolve_joins` emitted no join, shipping invalid SQL (unbound table alias). Filters and cross-model measures already rejected such paths; only dimensions/time-dimensions had the hole (the shared `_resolve_dotted_dim_with_stage_fallback` lenient branch). Fix is an engine routing pre-pass (`SlayerQueryEngine._route_dotted_dimension_refs`, run in `_enrich` before `enrich_query`, gated on `enforce_join_binding and source_model_origin is None`): it normalizes root-prefixes via `strip_source_model_prefix`, then for each dotted ref tries the explicit direct-join walk and, on `_NoJoinError`, routes via a datasource-scoped `JoinGraph`. A SHORT FORM (one model segment, e.g. `Consumer.name`) with exactly ONE route to the target auto-resolves — the ref is rewritten to the full routed path, so the result key is the full path (`root.Subscription.Customer.Consumer.name`), consistent with "joined dims keep the full path". Ambiguous (≥2 routes), unreachable (0), and explicit multi-hop chains with a broken hop are REJECTED with `UnresolvableDimensionJoinError(SlayerError, ValueError)` (mirrors the DEV-1645 `UnresolvableOrderColumnError` reject-don't-emit-invalid-SQL doctrine); the message suggests the short form when the target is uniquely reachable, else the shortest deterministic full path (`JoinGraph.shortest_path`), else nothing. `JoinGraph.count_simple_paths(root, target, cap=2)` classifies routes — it counts ALL simple paths (a 2-hop + 3-hop route is genuinely ambiguous; auto-picking the shorter would silently change join semantics), reverse-reachability-pruned and cycle-guarded. The rewrite map is also applied to matching `OrderItem.column` refs and `main_time_dimension` so dependent references stay consistent. Deliberate limits (conservative, prefer reject over a wrong route): routing runs only within a single datasource (`model.data_source` truthy; the graph is datasource-scoped) and is deferred when named-query stages are in scope (their virtual models aren't in the stored graph) — those refs fall through to the guard. A post-`_resolve_joins` safety-net guard in `enrich_query` (same gate) raises `UnresolvableDimensionJoinError` for any dim/time-dim whose alias is absent from `resolved_joins`, guaranteeing the invariant even for direct `enrich_query` callers; the re-rooted cross-model CTE enrichment passes `enforce_join_binding=False` (it legitimately carries source-local shared dims like `orders.status` that never bind to a base-table join). Out of scope: the multi-stage lenient cross-stage fall-through (`test_unresolvable_dotted_ref_falls_through`, where distinguishing a genuine error from a re-rooting artifact is unsolved) and leaf-column-missing-on-a-valid-path (the alias IS bound there — a different failure class).
 - 2026-08-16 — FK-derived joins name the MODEL, not the live object (DEV-1688 / DEV-1741 / #279). Model names strip `__` (reserved for join paths), so an FK to `reports__patient__drug` used to persist a join targeting a model that cannot exist; `_generate_joins` now takes the live→model map, and a target whose object was skipped on a sanitization collision drops its join rather than dangling. Stores written before the fix self-heal on the normal re-ingest path rather than via a schema migration — no version bump, and the repair demands the sanitized target AND identical `join_pairs` to match a freshly-generated join, so it can only rename the join the bug produced (name-only matching would collapse `a__b` and `a___b` onto one target and trip the duplicate-target guard, turning a merely-dangling store into a failed re-ingest). A store that never re-ingests keeps a join that was already broken.
 - 2026-08-18 — Two BigQuery-only failures fixed. (1) The DEV-1444 outer wrap re-parses already-emitted SQL, and BigQuery parses a quoted dotted alias (`` `orders.created_at` ``) into one part per segment, so the old qualifier strip (`col.set("table", None)`) both emitted an empty backtick pair — `400 Syntax error: Invalid empty identifier` on any computed measure plus ORDER BY — and, once that was fixed by replacing the node, silently dropped the model prefix, turning a syntax error into an unresolved name. `SqlDialect._outer_order_column` therefore re-resolves the column by picking the LONGEST part-suffix the inner SELECT actually projects (checked as a quoted identifier against `inner_sql`, which at that point still carries canonical dotted aliases — `rewrite_emitted_sql` mangles them afterwards). That subsumes the `_base.`-qualified form `_assemble_combined_sql` emits and keeps Postgres/DuckDB/MySQL output byte-identical, including the untouched bare-column and no-match fallbacks; hidden ORDER-BY hoists resolve too, which a `public`-list match would have missed. (2) `_get_columns_fallback` queried the bare `information_schema.columns`, which BigQuery resolves as `<project>.information_schema.columns` — a project-level view a dataset-scoped service account (the normal least-privilege setup) cannot read, so every per-table introspection 403'd. It now qualifies by dataset (`` `<dataset>`.INFORMATION_SCHEMA.COLUMNS ``), taken from `schema` or from the dotted table name, and drops the now-redundant `table_schema` predicate. Related hardening: `_live_schema_for_datasource` raises `IntrospectionUnavailable` when EVERY table in a datasource failed rather than returning an empty map, because empty is indistinguishable from "every table was dropped" — drift then reported a `WholeModelDelete` for every model in the tenant off one credential error, and `--force-clean` would act on it. `_collect_sql_table_diffs` catches it and returns no verdict; type refinement catches it and keeps the persisted types.
+- 2026-08-20 — DB comments imported at ingestion (DEV-1809 / #317): column comments → `Column.description`, table comments → `SlayerModel.description`, BigQuery dataset description → `DatasourceConfig.description` — strictly fill-if-empty (DEV-1356 additive-only; no provenance tracking, so a DB comment edit never propagates over an existing description — clear it and re-ingest). Inspector path covers Postgres/MySQL/SQL Server/Snowflake/ClickHouse/BigQuery; per-dialect fallback comment SQL for MySQL/Snowflake/ClickHouse/DuckDB/Postgres (DuckDB reachable only via the fallback; SQL Server/BigQuery fallbacks stay types-only — disproportionate SQL for a path their Inspector already covers). Schema-level comments have no generic API, hence BigQuery-only datasource descriptions; comment fetching is always best-effort, never a failed ingest; dbt/OSI curated descriptions win over DB comments at creation time; drift detection ignores comment text.

docs/concepts/ingestion.md

@@ -42,6 +42,7 @@ Tables with no FK references use their plain table name with no joins.
 SLayer introspects each table's column types and generates a model:
 
 - **One `Column`** per non-joined column on the source table — name, `type` inferred from the database (`string` / `number` / `boolean` / `time` / `date`), `primary_key=True` for PKs. Whether each column is used as a group-by dimension or as an aggregation source is decided per query.
+- **Column and table comments** stored in the database become `Column.description` and `SlayerModel.description` (see [Comments and descriptions](#comments-and-descriptions) below).
 - **`unique=True`** for columns that alone form a `UNIQUE` constraint or unique index. PK columns are not stamped redundantly — `primary_key` already implies uniqueness. Composite uniqueness is evaluated per key-set during join-cardinality inference rather than being flattened onto individual columns.
 - **A column literally named `count`** is renamed to `count_col` to avoid clashing with the always-available `*:count`.
 - **No auto-generated `measures`** — `SlayerModel.measures` is the named-formula library and stays empty after ingestion. You can add named formulas later via the API/MCP if you want bare-name shortcuts (`{"formula": "aov"}`).
@@ -54,6 +55,36 @@ All models use `sql_table` (the source table) plus `joins` (direct FK joins only
 
 Each FK join also gets a structural [`cardinality`](models.md#join-cardinality) guess from the key constraints alone (no data is read): `many_to_one` by default, upgrading to `one_to_one` when the source key is itself unique. A side counts as unique only when some PK/unique key-set is a subset of the join key — if `(a)` is unique then `(a, b)` is too, but a constraint on `(a, b)` does not make `(a)` unique. When the target key cannot be *verified* unique from its constraints, cardinality is left unset rather than guessed. To infer it from the data instead, run `slayer validate-models --cardinality`.
 
+### Comments and descriptions
+
+Textual metadata stored in the database is imported during ingestion, so
+agents see the same documentation the DBA wrote:
+
+- **Column comments** → `Column.description`
+- **Table comments** → `SlayerModel.description`
+- **BigQuery dataset description** → the datasource's `description`
+  (BigQuery only — no cross-database API exists for schema-level comments)
+
+The import is strictly **fill-if-empty**: a comment lands only where the
+existing `description` is empty. Hand-written descriptions are never
+overwritten, on first ingest or any re-ingest — consistent with the
+additive-only re-ingestion contract. To re-import a comment, clear the
+description and re-run `slayer ingest`. Imported descriptions feed the
+search corpus like any other description, so the next embedding refresh
+picks them up.
+
+Per-database support:
+
+| Database | Column comments | Table comments |
+|---|---|---|
+| BigQuery | ✓ (field descriptions) | ✓ (table description) |
+| Postgres, MySQL, SQL Server, Snowflake, ClickHouse | ✓ | ✓ |
+| DuckDB | ✓ (`COMMENT ON`) | ✓ |
+| SQLite | — (no such feature) | — |
+
+Comment fetching is best-effort: a database or driver that cannot surface
+comments simply yields models without descriptions, never a failed ingest.
+
 ### SQLite affinity probing
 
 SQLite's declared column types are affinity hints, not strict constraints: a column declared `INTEGER` can store `INTEGER`, `REAL`, `TEXT`, or `BLOB` values per row. To prevent silent truncation downstream (a column declared `INTEGER` but actually storing `0.99` would cast to `0` and break `AVG`/`SUM` results), SLayer runs an additional value-level probe on SQLite ingestion for every column the inspector reports as `INTEGER`-affinity.
@@ -276,7 +307,7 @@ Ingest-on-startup: N/M datasources ingested (K failed: name1, name2)
 `slayer ingest` (and the equivalent MCP / REST entry points) is idempotent by default — re-runs are safe. For each in-scope live table:
 
 - **No persisted model with that name** → ingest from scratch via the path above.
-- **Existing `sql_table`-mode model** → append new columns and joins from the live schema. Existing user metadata is **never** overwritten — `description`, `label`, `format`, `meta`, and `allowed_aggregations` are preserved verbatim. The only in-place updates are strictly additive gap-fills: a join's `cardinality` is set only when it is currently unset (a value you chose is never replaced), and a column's `unique` is only ever turned on, never off. Filling either one is enough to trigger a save, so a re-ingest that adds no columns or joins still persists newly-discovered constraint metadata.
+- **Existing `sql_table`-mode model** → append new columns and joins from the live schema. Existing user metadata is **never** overwritten — `description`, `label`, `format`, `meta`, and `allowed_aggregations` are preserved verbatim. The only in-place updates are strictly additive gap-fills: a join's `cardinality` is set only when it is currently unset (a value you chose is never replaced), a column's `unique` is only ever turned on, never off, and an **empty** `description` (column and model level) is filled from the live DB comment. Filling any of these is enough to trigger a save, so a re-ingest that adds no columns or joins still persists newly-discovered metadata. Filled descriptions are reported per model (`ModelAddition.described_columns`, `model_described`; the CLI prints `+descriptions: …`).
 - **Existing `sql`-mode or query-backed model with the matching name** → skipped silently; those are user-authored.
 
 With the default YAML storage, two live tables whose quoted names differ only by letter case (`"Orders"` vs `orders`) cannot both be persisted — model names collide as filenames on macOS / Windows, so the save is rejected (`IdCollisionError`). The first table wins; the second surfaces as a per-model entry in `IdempotentIngestResult.errors` (or a per-model message on the CLI / MCP paths) without aborting the rest of the ingest. SQLite storage persists both.

docs/database-support.md

@@ -19,14 +19,17 @@ against the live service in CI when they are.
 | **ClickHouse** | `tests/integration/test_integration_clickhouse.py` (`testcontainers[clickhouse]`) | `examples/clickhouse/` |
 | **SQL Server** | `tests/integration/test_integration_sqlserver.py` (`testcontainers`, `msodbcsql18` + `unixodbc-dev` on the runner) | `examples/sqlserver/` |
 | **Snowflake** | `tests/integration/test_integration_snowflake.py` (skips without `~/.snowflake/connections.toml`; profile name overridable via `$SLAYER_SNOWFLAKE_CONNECTION`) | `examples/snowflake/` (no Docker) |
-| **BigQuery** | `examples/bigquery/verify.py` driven by CI against `bigquery-public-data.thelook_ecommerce` (gated on `GCP_PROJECT_ID` / `GCP_SA_KEY_B64` repo secrets) | `examples/bigquery/` (no Docker — managed service) |
-
-BigQuery does not yet have a pytest-style integration suite; its CI coverage
-runs the example's `verify.py` directly via `.github/workflows/ci.yml`. That
-exercises auto-ingestion, basic projection, joins, time-grain dimensions, and
-the cardinality / sum-of-grouped-equals-total invariants — enough to catch
-emitted-SQL regressions, but the verify-script tier is shallower than the
-testcontainers suites.
+| **BigQuery** | `tests/integration/test_integration_bigquery.py` (live ingestion against a temp dataset in the billing project; skips without `GCP_PROJECT_ID` + ADC) plus `examples/bigquery/verify.py` driven by CI against `bigquery-public-data.thelook_ecommerce` (gated on `GCP_PROJECT_ID` / `GCP_SA_KEY_B64` repo secrets) | `examples/bigquery/` (no Docker — managed service) |
+
+BigQuery CI coverage has two layers, both in the `bigquery-example` job of
+`.github/workflows/ci.yml`: the example's `verify.py` exercises query
+execution against the public dataset (basic projection, joins, time-grain
+dimensions, and the cardinality / sum-of-grouped-equals-total invariants),
+and `test_integration_bigquery.py` exercises live schema ingestion — creating
+a temporary dataset with table/column/dataset descriptions in the billing
+project, asserting comment import and idempotent re-ingest semantics, then
+deleting it. The service account therefore needs dataset create/delete rights
+in the billing project (e.g. `roles/bigquery.user`), not just `jobUser`.
 
 ## Tier 2 — code-covered
 
@@ -87,6 +90,10 @@ mapping lives in `SqlDialect.build_approx_count_distinct`.
 
 ### SQLite caveats
 
+SQLite has no table or column comments, so ingestion imports no descriptions
+there (every other Tier 1 database's comments are imported — see
+[Ingestion — Comments and descriptions](concepts/ingestion.md#comments-and-descriptions)).
+
 SQLite has a much smaller built-in math/stat catalog than the other supported
 engines. SLayer registers Python aggregate and scalar UDFs on every new SQLite
 connection via SQLAlchemy's `connect` event (see
@@ -225,6 +232,11 @@ plus `$GCP_PROJECT_ID` for billing). The `bigquery://` driver requires the
 - **No FK introspection.** BigQuery exposes no foreign-key metadata via
   `INFORMATION_SCHEMA`, so auto-ingestion cannot discover joins. Hand-declare
   `ModelJoin`s on the model.
+- **Descriptions ARE imported.** Auto-ingestion maps BigQuery column
+  descriptions → `Column.description`, table descriptions →
+  `SlayerModel.description`, and the dataset description → the datasource's
+  `description` (fill-if-empty; see
+  [Ingestion — Comments and descriptions](concepts/ingestion.md#comments-and-descriptions)).
 - **Dotted alias mangling.** BigQuery rejects column names containing `.`
   (output schema names must match `[A-Za-z_][A-Za-z0-9_]*`), so SLayer
   rewrites `<model>.<column>` aliases (`orders._count`,

slayer/cli.py

@@ -2237,6 +2237,15 @@ def _run_datasources_create(args, storage):
         sys.exit(1)
 
     _persist_ingested_models(report.models, storage, assume_yes=args.yes)
+
+    if report.schema_description and not ds.description:
+        ds = ds.model_copy(update={"description": report.schema_description})
+        try:
+            run_sync(storage.save_datasource(ds))
+            print("Datasource description imported.")
+        except Exception as e:
+            print(f"Could not save datasource description: {e}")
+
     # After persistence, so the sections comment on what was just written. Exit
     # stays 0 — creating the datasource succeeded, whatever was skipped/hidden.
     _print_ingest_drift_and_errors(report, data_source=ds.name)