feat: review inner joins that drop most grouped rows at answer_commit

[anandgupta42]

Issue for this PR

No linked issue (feat). Draft: pending a live evaluation before it is marked ready.

Type of change

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

What does this PR do?

Why. An analyst agent often materializes two tables into the session scratch database, inner-joins them on an ID whose format or coverage differs between the sides, and commits an aggregate over the few rows that survived (for example 23 of 513 patients), sometimes even writing "23 patients" in basis. Nothing tells it that most of the grouped population vanished in the join. A check on every sql_execute was tried before and rejected: it ran probes on the shared scratch connection for every query and keyed on the wrong query. This PR checks once, at answer_commit, on the query that produced the committed values.

What changed (packages/opencode/src/altimate/tools/answer-commit-join-coverage.ts, wired into answer-commit.ts):

• Locating the answer query. Among the current turn's completed sql_execute parts, the one whose output shows every committed value (numbers above 10 or with decimals, exact or rounded/truncated at the committed places; non-numeric strings of 3+ chars). It abstains when there is no such evidence (e.g. a lone integer 5), nothing matches, a matching output was truncated, or two matching queries differ in SQL text. It does not fall back to the latest query.
• Join extraction from DuckDB's own json_serialize_sql AST: one SELECT, no CTE/subquery/table function, 2 to 3 scratch base tables joined by INNER joins (ON, USING or comma join with a WHERE equality), up to 2 = key conjuncts per pair. Keys may be wrapped in upper/lower/trim/ltrim/rtrim/btrim/substr/substring/left/right, CAST, COALESCE. Every WHERE conjunct must belong to one side and becomes that side's filter. Anything else (outer joins, IS NOT DISTINCT FROM, lpad/concat/regexp_replace, cross-side filters, 16+ digit literals) means it skips.
• Measurement without the join. Per pair, one statement: each side's own-filtered count(*) and count(*) ... WHERE EXISTS (match in the other side). These are semi-joins built from single-table projections that the deserializer produces from the agent's own AST nodes, so a fan-out join (20k x 20k on one key, answer with LIMIT 1) costs milliseconds, not 400 M joined rows.
• Gates before any data is read (catalog only): base tables in scratch.main, no view of the same name, at most 500,000 rows (estimate); each key column fixed-width, or VARCHAR whose pragma_storage_info Max String Length is known for segments covering every row, is at most 256 and has no pending in-place updates (those leave segment stats stale); rows x key width at most 16 M chars per side; every called function built in (a user macro named trim skips); every column a side filter reads passes the same width gate; at most 8 WHERE conjuncts per side and at most 64 AST expression nodes across all key and filter expressions (so work cannot grow with predicate count, IN lists or COALESCE arguments); effective scratch query timeout at least 5 s. Constant-only WHERE conjuncts (1 = 0) skip.
• Firing rule. A side whose columns the GROUP BY references (no GROUP BY means both sides) keeps < 50% of its >= 20 rows, and basis/rows.note do not already state " of " (or /, out of). A GROUP BY name that is both a select alias and an input column abstains, and so do GROUP BY ALL, SELECT DISTINCT and window-function queries, whose grouped side is not read from an explicit GROUP BY. Losses on the measure side alone do not fire.
• Review. A one-time answer_commit review: rejection (status: "error", title Answer not committed) with the measured counts and a conditional instruction: look for a covering key/source, or state "M of N" in basis and recommit unchanged. The unchanged recommit is accepted. Metadata carries evidence.joins, final_query_part and join_check_ms.
• One review per turn, also under concurrency. answer_commit reserves the session's review slot for the current user message id synchronously, before any await. A concurrent second commit of the same turn sees the reservation and commits without review. The slot is released when nothing fired. A history without a user message id (the MCP adapter on main has none) is never reviewed.
• Disabled by ALTIMATE_SQL_AUTO_CHECK=off (0, false). unresolved commits are not checked. Any error is swallowed and the commit proceeds.

Known limitations (low, documented rather than fixed): the review evidence is kept only on the rejection's metadata (the accepted recommit does not copy it); the once-per-turn reservation assumes one committer per session, which holds on the native path.

Not included: the MCP adapter changes the check needs over MCP (recording input on tool parts, stable user-boundary ids). The adapter is not on main; that change lives on the MCP branch.

Cost side effects

This adds automatic queries on user data, only when all of these hold: the check is on, the commit is fields/rows, the answer query is located unambiguously, and it ran on the session scratch DuckDB (local, ephemeral; never a remote warehouse, never a local DuckDB file, never SQLite). Per commit, at most once per user turn: ~6 catalog statements (json_serialize_sql, duckdb_views, duckdb_tables, duckdb_columns, duckdb_functions, pragma_storage_info), then for each of at most 2 pairs (key and filter expressions capped at 64 AST nodes, at most 8 filter conjuncts per side) one json_deserialize_sql per side and one count statement that is O(|L| + |R|) with a hash table bounded by the catalog gate (<= 500,000 rows per table, <= 16 M key chars per side). The join is never executed and no model is called. All statements run inside one withScratchConnection lease with readOnly, are never aborted, and the check skips when the inherited query timeout is below 5 s. Measured locally: 500k x 500k wrapped VARCHAR keys < 1 s in the test (probe: 79 ms); the fan-out case is milliseconds. Added commit latency when it runs is those statements plus any operation already holding the scratch lease.

How did you verify your code works?

Unit/integration, bun, real DuckDB through the session scratch connector and withScratchConnection (test/altimate/tools/answer-commit-join-coverage.test.ts, 22 tests, pass at da9c781511):

• fires: sparse patients/labs (23 of 513, exact rejection text), customers joined to paid orders (20 of 100, the documented false-positive class), unqualified keys, upper(trim()) key with right-side filters, comma join, USING, scratch.-qualified names; the unchanged recommit is accepted
• silent/skips: full coverage (commit accepted), basis already states "23 of 513", measure side only, check off, 8 KB keys (length gate), table with a pending update, 600k-row table, inherited timeout 1 s, LEFT JOIN, IS NOT DISTINCT FROM, subquery, CTE, cross-side filter, regexp_replace/lpad/LIKE keys, 3 key conjuncts, wide integer literal, WHERE 1 = 0, user macro shadowing trim, 1,024 LIKE conjuncts, 9 conjuncts on one side (8 still measured), a 100-item IN list, a 100-argument COALESCE key, GROUP BY ALL, SELECT DISTINCT, window-only select
• fan-out 20k x 20k with LIMIT 1: measured 20000/20000 (asserted < 5 s; measured in milliseconds); 500k x 500k: 250000/500000 (asserted < 5 s)
• location: answer query picked over a later sanity query; rounding vs scaling; abstains on zero evidence, two differing matching queries, truncation, values split across queries; remote warehouse; history without a turn id; the review slot honors a review marker or two prior commit rejections in the turn history even for a fresh session id
• concurrency (native path): two concurrent identical commits in one turn give exactly one review; a new turn is reviewed again
• test/altimate/tools/answer-commit.test.ts, test/agent/answer-commit.test.ts, test/cli/run/budget-finalization.test.ts: pass at da9c781511 (one subprocess test hit its 5 s timeout once under load and passed on rerun). bun run typecheck: pass. Upstream marker check (--markers --base main --strict): pass. No tool schema changed.

Not run: any live agent session or evaluation (the next step, and the reason this is a draft); the full CI suite (left to CI).

Reviews: local Codex (gpt-6.1-sol, read-only) of c8923faf83 found 3 P1s (constant-only WHERE dropped, side-filter columns unbounded, literal case folded in SQL comparison), fixed in 743a356f96. A second Codex pass of 743a356f96 found 1 P1 (filter work unbounded by predicate count: 1,024 LIKE conjuncts on 300k rows measured in 4.1 s against a 12.6 ms answer) and an independent review found a medium (GROUP BY ALL/DISTINCT/window queries treated every table as grouped); both fixed in da9c781511 with regressions. da9c781511 has not had another Codex pass. Open P2s: a COALESCE constant can widen a key past its column width (capped at 64 chars); the bullet can ask to acknowledge an already acknowledged side when the other side fired; reviewedTurn keeps one entry per reviewed session.

Screenshots / recordings

Not a UI change.

Checklist

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

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[anandgupta42]

Closing after a live evaluation. On a sparse-join question the check fired on 15 of 16 trials, with no false positives across 20 trials on join-heavy questions where it should stay silent, and about 100 ms added per commit. It did not change the outcome: every agent acknowledged the measured coverage ("23 of 513") and recommitted the same answer, because the loss came from the choice of source rather than the join itself. Since it adds automatic queries on user data without a measured benefit, I'm not shipping it. The design (per-side counts plus EXISTS semi-joins, never re-running the join, catalog byte and node caps) and the evaluation are kept in the internal research notes in case we revisit it as a product feature.