feat: note silent row loss on joins in `sql_execute` results

Closed
AN
2d ago
+594-0

Issue for this PR

Closes #

N/A: feat change; no linked issue required.

Type of change

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

What does this PR do?

Why. An analyst agent can join a population table to a fact table, silently keep a small fraction of the population, and report the result as fact. In real agent sessions a join kept 23 of 513 entities and the agent stated the 23 without questioning it. The existing sql_check keys/fanout checks can measure this, but agents almost never call sql_check, so the signal has to appear on the sql_execute result they already read.

What changed (before → after). After a successful sql_execute read:

  • Before: no signal when a join drops most of its left side.
  • After: when an equality join between two base tables keeps under half of its left rows, the result carries one note, for example: join cohort → facts on MemberId = MemberId: WARNING coverage loss: 6 of 8 left rows (75.0%) have no match; the inner join drops them. Before reporting results over the joined rows, confirm this loss is expected ... or look for a different key, column or source .... Healthy joins add nothing. Metadata gets join_coverage_warning: true when it fires.

How it works (src/altimate/tools/sql-join-coverage.ts):

  • Parses the SQL with DuckDB's own parser (json_serialize_sql); considers at most two INNER/LEFT joins whose inputs are both plain base tables joined only by column equalities.
  • WHERE conjuncts that reference only one side are applied to that side, so the count uses the same population as the query. Only simple predicates are rebuilt (comparisons, BETWEEN, IN lists, IS [NOT] NULL, casts of columns and constants); a conjunct with a function call (LIKE included), CASE or subquery, or one spanning both sides, another table or a bare column, skips the check. A LEFT JOIN with a right-side WHERE conjunct is skipped, since that filter drops the NULL-extended rows after the join. Subquery sides, CTE names, column alias lists and samples are skipped, not rebuilt.
  • Runs only on the session scratch database and local DuckDB files. Each side must resolve to exactly one local DuckDB base table of at most 500,000 rows (catalog estimate), with no view of the same name anywhere. Remote warehouses, SQLite and cloud/remote DuckDB paths (md:, motherduck:, ducklake:, URLs) are never queried.
  • Never interrupts the engine (no short timeout, session deadline or abort on these queries; a local engine timeout restarts its helper and drops session state, and scratch data cannot move to an isolated connector). The work is bounded by construction instead: the size ceiling plus simple-predicate filters keep the count to milliseconds (a semi-join at the ceiling measured 9 ms), and the note is dropped if the whole check exceeds 250 ms.
  • Requires an explicit warehouse (without one the query may have run through dbt). Every error or ambiguity returns nothing, and the query result is never altered or blocked.
  • ALTIMATE_SQL_AUTO_CHECK=off (or 0/false) disables it; default on.

Deployment readiness. Self-contained; no config, data or migration.

User impact. Analyst and other sql_execute users on scratch/local DuckDB see an extra advisory line only on joins that drop over half of their left rows.

How did you verify your code works?

  • bun run typecheck: clean.
  • New test/altimate/tools/sql-join-coverage.test.ts (12 tests, real DuckDB scratch, a local DuckDB file and SQLite): low coverage note with counts; exact 50% threshold silent; NULL keys never match; LEFT JOIN wording; multi-key and reversed operands; WHERE filters on the left, right and both sides (comparisons, BETWEEN, IN, IS NOT NULL), and unsplittable or function-bearing WHERE (LIKE, lower(), OR, subquery) skipped; LEFT JOIN with a right-side WHERE skipped; non-equality, OR, USING, bare columns, subquery sides, CTE names, column alias lists and parse errors silent; views, a view shadowing a table name, and tables over the ceiling skipped (the ceiling test fails if the ceiling is removed) with scratch state intact; local DuckDB file checked; SQLite, md: DuckDB and Postgres skipped; missing warehouse and missing tables return nothing; and through SqlExecuteTool the note reaches output and metadata only on low coverage, with identical rows when the flag is off.
  • Existing consumers of sql_execute: 568 tests pass across sql-execute, sql-execute-query-from, scratch-sessions, precedence-guard-order, sql-tool-cancellation, warehouse-failure-visibility, sql-materialize(-concurrency), sql-validation-adversarial, sql-comment-safety, sql-result-lint, sql-check, tool-schema-anthropic, feature-discovery-e2e, tracing-bigint and the new suite.
  • script/upstream/analyze.ts --markers --base main --strict: ok.
  • Reviews: two local read-only Codex reviews (heads 0681ee5486, 9d224a97fe) found engine-restart, deadline, remote-DuckDB, view-shadowing and identifier issues, all addressed (the design was narrowed to local DuckDB base tables, never interrupted). Codex then ran out of credits; the framework's independent Claude review of cf12a3facf found the WHERE-filter issue (fixed in 26d4d6d922) and three low items (fixed). Bot findings since (LEFT JOIN right-side filters; bounding work that outlives the budget) are fixed in 070b58de04.

Cost side effects

One extra counting query per eligible join (at most two per sql_execute call), only on the local scratch database or local DuckDB files and only when both tables are at most 500,000 rows; it never runs against billed warehouses. No model calls. Disable with ALTIMATE_SQL_AUTO_CHECK=off.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR
  • I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • SQL query results can now include an advisory when a supported join matches fewer than half of the filtered rows. The warning may also appear in response metadata.
    • Join coverage checks can be disabled with an environment setting. Checks that are unsupported, ambiguous, or exceed the time limit are skipped; query results remain unchanged.
  • Tests
    • Added coverage for warning conditions, supported and skipped query shapes, configuration, and SQL tool output.