fix(advise): inverted IS NOT NULL on current sqlglot, and the CI gaps that hid it

[hanslemm]

Follow-ups from the Batch 2 reviews, plus one shipped correctness bug the follow-up work
uncovered.

The bug: IS NOT NULL was read as IS NULL on any current sqlglot

sqlglot moved IS NOT NULL from a wrapping Not(Is(...)) node to a negate=True flag on the
Is node itself, in 30.13 — inside the range this package declares (sqlglot>=30.12,<31).
_role() read only the parent, so on any newer sqlglot every IS NOT NULL was classified as a
NULL_CHECK: the exact inverse.

That is not a cosmetic misclassification. ADV004 turns these roles straight into a partial
index's WHERE clause, so it proposed

CREATE INDEX ON ... (col) WHERE guard IS NULL;

for a workload filtering guard IS NOT NULL — an index over precisely the complement of the
rows the queries touch, at MEDIUM confidence.

uv.lock pins 30.12, so development, CI and every review on the Batch 2 branch ran the one
version where the code was correct. A fresh pip install sqlquality resolved 30.14 and got the
inverted predicate. _is_negated now accepts both encodings — the flag is additive, so neither
the version floor nor the ceiling has to move and users are not pushed to a particular sqlglot
to get correct DDL.

The regression test builds both trees directly rather than parsing, because parsing only
exercises whichever encoding the installed version produces — which is why the existing
parse-based test could not fail on 30.12. Verified: reverting to parent-only polarity turns the
new test RED under 30.12 and 30.14, while the old test reddens only under 30.14.

Two CI blind spots, both closed

The bug above was invisible to CI for a structural reason, and so was a second invariant:

• highest-deps installs with --resolution highest instead of reading uv.lock, so every
  job no longer tests the same single point in the declared dependency ranges. An upstream
  release that breaks a declared range now fails here rather than in someone's generated DDL.
• no-extras runs the default suite after a plain uv sync — no extras, no container — and
  fails on any skip. Every existing job passes --all-extras, so nothing verified the
  documented promise that uv run pytest works out of the box. A test that acquires such a
  dependency becomes a skip, which reads as success in a green run.

I verified the no-extras guard both ways: it passes today (576 → 583 passed, no psycopg
installed, zero skips), and it fires on a deliberately introduced importorskip.

Review follow-ups

The MEDIUM rung now says why. ADV001 and ADV007 both reached MEDIUM in silence when the NDV
catalog had nothing for the column — which reads as a considered middling judgement rather than
what it is: the selectivity check did not run. Both now disclose it, in the same words, and only
at that rung (with an unknown row count the proposal is already LOW for a different reason, and a
second sentence would misattribute it).

Four behaviours nothing was checking, each now pinned and each verified RED under a mutation
of the line it claims to pin:

• ADV001's and ADV007's new disclosures
• ADV007's (-cost_ms, column) tiebreak — two equally hot join keys reshuffled between runs
  without it
• _coverage_warning's <= boundary — flipping it to <, so the warning fires at exactly 20%,
  passed the entire suite. The test asserts at, above and below the threshold, so it fails
  whichever way the comparison is flipped.

Three comments that were no longer true. _identifier_pattern named ADV006's wide-table
detection as its main caller, which stopped being true when ADV006 moved to parsing.
_dedupe_by_ddl claimed tied proposals "carry no information that distinguishes them" when
rationale and title can differ — what they lack is anything a tie-break could act on.
ADV008's unreadable-index-list sentence omitted the "— check before applying" its two siblings
carry.

Verification

• 583 passed, 14 deselected — zero skips
• 14 passed integration against live postgres:16
• Suite green under both sqlglot 30.12 (locked) and 30.14 (highest allowed)
• ruff check, ruff format --check, mypy src/sqlquality clean

Still open

• ADV004 never discloses that it skipped coverage checks, and it is the only index-creating
  rule that never calls _covered. Pre-existing; the fix is a design question, not wiring.
• The integration compose binds host port 55432, which collides with a container from
  another project on at least one dev machine. When that happens docker compose up does not
  bind and the suite talks to whatever else is listening. Worth either picking a less common
  port or failing loudly on a version/database mismatch.

🤖 Generated with Claude Code