feat(advise): harden the Postgres adapter, and put a real Postgres in the loop

.github/workflows/release.yml

@@ -9,6 +9,15 @@ name: Release
 on:
   push:
     tags:
+      # Release tags are bare semver: 0.3.0, not v0.3.0.
+      #
+      # `v*` is kept only as a safety net. Tag filters are globs, not regexes, so a bare
+      # pattern cannot be made to reject a `v` prefix — and a pushed tag that matches
+      # nothing fails *silently*: no workflow run appears, so it looks like a release that
+      # is merely slow rather than one that never started. Matching both means an
+      # out-of-habit `v0.3.0` still publishes. Drop the `v*` line if you would rather that
+      # mistake fail loudly.
+      - "[0-9]*.[0-9]*.[0-9]*"
       - "v*"
 
 jobs:

CONTRIBUTING.md

@@ -48,3 +48,18 @@ uv run mypy src/sqlquality
 - Any user-visible change (new flag, changed exit code, changed output, new config key)
   needs a `CHANGELOG.md` entry under the current unreleased version, in the appropriate
   `Added` / `Changed` / `Fixed` / `BREAKING` section.
+
+## Integration tests (optional)
+
+`advise`'s introspection SQL is only checked for drift by the default suite. To run it
+against a real Postgres:
+
+```bash
+docker compose -f tests/integration/docker-compose.yml up -d
+uv run pytest -m integration
+docker compose -f tests/integration/docker-compose.yml down
+```
+
+These are deselected by default, so `uv run pytest` stays green without Docker. Point them
+at your own server with `SQLQUALITY_TEST_DSN`. They need the `postgres` extra
+(`uv sync --extra postgres`).

README.md

@@ -315,7 +315,7 @@ missing driver degrades with an install hint instead of a traceback.
 | `--schema` | `public` | Schema to introspect. **One at a time** — passing two exits 2, see Limitations. |
 | `--since` | — | Window, e.g. `7d`. **Not honored on Postgres** — see Prerequisites below. |
 | `--limit` | `500` | Max query-history rows to read. |
-| `--min-cost-share` | `0.01` | Suppress proposals below this share of workload cost. |
+| `--min-cost-share` | `0.01` | Suppress proposals below this share of workload cost. Applies to the **cost-weighted** rules (ADV001, ADV004, ADV005, ADV006); the index-hygiene rules **ADV002 and ADV003 carry no cost evidence and are always reported**, whatever the threshold. |
 | `--keep-literals` | off | Do **not** redact literal values from query text. |
 | `--timeout` | `30` | Statement timeout in seconds (rejected outside 1–3600). |
 | `--dry-run` | off | Print every statement the adapter would issue, then exit 0 **without connecting**. |
@@ -355,6 +355,13 @@ only way to retain them, and the report states which mode produced it. `advise`
 writes to your database — proposed DDL only ever goes to a file (`--ddl`) for you to
 review and apply by hand.
 
+One rendering quirk worth knowing before you read a report: `pg_stat_statements` replaces
+an interval literal with its own parameter marker (`interval $2`), and sqlglot renders that
+back as `INTERVAL '2'`. So `created_at > CURRENT_TIMESTAMP - INTERVAL '2'` in a report
+stamped `"redacted": true` means **the interval was parameterised**, not that someone wrote
+a two-something interval — the `2` is Postgres's parameter index. Nothing leaked, but the
+statement is not valid SQL to copy out and run.
+
 **Prerequisites and limits:**
 
 - **`pg_stat_statements`** must be installed (`shared_preload_libraries` +
@@ -382,11 +389,9 @@ review and apply by hand.
 
 - **HIGH** — cost share above `--min-cost-share`, **and** supporting catalog stats present
   (e.g. NDV), **and** confirmation that the proposed index does not already exist.
-- **MEDIUM** — cost evidence is solid but a catalog input is missing or stale. ADV002 and
-  ADV003 are capped at MEDIUM unconditionally: `idx_scan` only accumulates since the last
-  statistics reset, so zero scans can never prove an index is unused across a full business
-  cycle, and ADV003 compares column lists without being able to see a partial index's
-  predicate.
+- **MEDIUM** — cost evidence is solid but a catalog input is missing or stale. ADV002 is
+  capped at MEDIUM unconditionally: `idx_scan` only accumulates since the last statistics
+  reset, so zero scans can never prove an index is unused across a full business cycle.
 - **LOW** — thin evidence, and specifically **any check that could not be run**: the row
   count is unknown so the small-table floor could not be applied, or the existing-index
   list was denied so "no index already covers this" could not be confirmed. Absent
@@ -779,16 +784,17 @@ LLM suggestions unavailable: The 'anthropic' package is required for AnthropicPr
   be most of your hot reads. They are counted, and the skip line calls them `filtered`
   rather than pretending they were introspection or DDL, but they are not analyzed.
   Unwrapping to the inner `SELECT` is a follow-up.
-- **Expression indexes are invisible to the catalog query.** Postgres's `pg_index.indkey`
-  holds `0` for an expression column, which matches no `pg_attribute` row, so ADV001 may
-  propose a plain-column index whose `lower(col)` expression-index equivalent already
-  exists.
-- **ADV003 cannot see partial-index predicates.** It compares column lists only, so it
-  could recommend dropping a partial index in favor of a wider full index that does not
-  actually cover the same rows. Its confidence is capped at MEDIUM for that reason, and
-  the caveat is repeated in the proposal's own rationale — a README does not travel
-  inside the `.sql` file you run. `advise` cannot tell you which of the two indexes is
-  partial or expression-based; you have to check.
+- **Expression indexes are read but not matched.** `advise` now sees that an index on
+  `lower(status)` exists and names it in the proposal's evidence, but it cannot tell whether
+  that index already serves a lookup on `status` — so it proposes and says so, rather than
+  suppressing or ignoring. Confirm before applying.
+- **ADV003 only compares plain indexes.** A pair where either index carries a `WHERE`
+  predicate or an indexed expression is skipped entirely rather than proposed at lower
+  confidence: a partial index exists to serve a subset, so recommending its removal is
+  likely wrong rather than merely uncertain. Plain pairs are reported at HIGH.
+- **A partial index does not suppress a proposal.** `idx ON orders(status) WHERE
+  shipped_at IS NULL` does not serve `WHERE status = $1`, so it is not treated as covering
+  a candidate index — it is named in the evidence instead.
 - **One schema per run.** Every catalog fact is keyed on the bare relation name — table
   sizes, NDV statistics, index lists and the `qualify()` schema all merge across schemas —
   so `orders` in two schemas would alias into one another and the last catalog row read