fix(sql-check): bound key probes and report directional coverage

Merged
AN
16h ago
+244-29

Issue for this PR

Closes #1491

Stacked on feat/analyst-harness-dab-2026-10 (#1441)

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The sql_check keys check gave misleading verdicts and was too slow on large warehouses. A key set that was a proper subset of the other side was reported as "unmatched keys found", integer keys produced spurious prefix/suffix findings, and large text key sets exceeded a 5s query deadline.

Root cause: the verdict required the exact match count to equal both distinct counts (so a complete left subset failed); the bidirectional substring predicate ran over all distinct key pairs with integers cast to text (nested-loop join, false positives such as 12 vs 123); and normalization functions were embedded in correlated predicates.

Before -> after:

  • Subset: "unmatched keys found" -> "left coverage complete", with independent left/right coverage shares and counts. Empty sides report coverage as unavailable.
  • Integer keys (including DuckDB unsigned/huge integers): spurious prefix/suffix and normalized matches -> text diagnostics skipped, reported as n/a.
  • Large text key sets (7k left / 130k right, SQLite and DuckDB): >5s timeout -> under 1s. Normalized forms are materialized per distinct key, right-side forms grouped before equality joins, unmatched samples use anti-joins, and the prefix/suffix probe is bounded to 100 unmatched left and 100 right keys (disclosed in the output).
  • Counts still cover all source keys; at most five unmatched values per side are returned.

Deferred / follow-ups (Codex review, both P2, not fixed here):

  • DuckDB collated string keys (e.g. NOCASE/NOACCENT) can make join counts exceed one match per left key; per-left-key counting would harden this.
  • DuckDB BIGNUM is not in the integer-type allowlist.
  • Live PostgreSQL not exercised (sandbox could not start a server); AS MATERIALIZED needs PostgreSQL 12+.
  • Prefix/suffix detection is intentionally sampled.

How did you verify your code works?

  • bun test test/altimate/tools/sql-check.test.ts: 76 pass, 0 fail (16 new tests; the large-set cases failed before the fix).
  • bun test test/altimate/tools (24 files): 632 pass, 3 fail, 1 error. The failures are in datamate-list-integrations and sample-setup (5s timeouts), unrelated to this change; they also fail in a clean detached worktree at the base (2 fail, 2 errors, flaky).
  • bun test test/altimate/tryExecuteViaDbt.test.ts: 4 pass.
  • bun run typecheck in packages/opencode: 0 diagnostics.
  • script/upstream/analyze.ts --markers --base feat/analyst-harness-dab-2026-10 --strict: pass. The changed files are under src/altimate/** and absent from the v1.4.0 baseline tag, so no markers are required.
  • git diff --check: clean.
  • Independent Codex review: only two P2 findings (listed above).

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

šŸ¤– Generated with Claude Code

[!NOTE]
Medium Risk
Changes diagnostic SQL and verdict semantics for a core analyst tool; behavior shifts (subset = success) could surprise existing callers, though scope is limited to the keys check path.

Overview
Fixes misleading keys verdicts and timeouts in sql_check by rewriting the keys SQL plan and tightening when text diagnostics run.

Verdict and reporting now treat complete left-side exact matches as left coverage complete (not failure when the right side is larger). Metadata adds left_coverage / right_coverage shares, a clearer verdict line, and empty-input coverage as n/a.

Integer keys (including DuckDB unsigned/huge integer types) skip trim/normalize/leading-zero and prefix/suffix probes so values like 12 vs 123 no longer produce false format findings; those metrics surface as n/a with an explicit skip message.

Performance and correctness materialize per-distinct-key normalization, aggregate right-side forms before joins (so collisions do not inflate match counts), use joins/anti-joins instead of correlated EXISTS scans, and cap the prefix/suffix probe to up to 100 unmatched left keys Ɨ 100 right keys (disclosed in output). Large-set tests expect completion within a 5s deadline on SQLite and DuckDB.

Tests add subset coverage, integer skip behavior, normalization collision counting, bounded prefix diagnostics, empty-side coverage, mixed-type SQLite behavior, DuckDB unsigned/hugeint coverage, and 7k/130k key performance cases.

Reviewed by Cursor Bugbot for commit 4711728471e967a03e3181e0ad7bc9a993330b75. Bugbot is set up for automated code reviews on this repo. Configure here.

Known follow-ups

  • Prefix/suffix probe compares against a sampled 100-key right side (disclosed in the output); the keys query uses AS MATERIALIZED, which needs PostgreSQL 12 or later; collation-aware exact coverage and BIGNUM key detection are not handled.