Four sm verbs grade a statement a person supplied, part by part by sandeep-agami · Pull Request #288 · AgamiAI/agami-core

sandeep-agami · 2026-09-11T22:09:01Z

Spec: ACE-114 (feature reconcile-evidence, brief F10)

Do not merge without manual review.

Summary

agami-reconcile is learning to take a trusted query as evidence rather than as the answer. The skill will decide meaning and talk to the person; everything deterministic about a statement belongs in the CLI. This PR is that half, and nothing yet calls it: four sm verbs, one shared probe, and their tests. The skill change comes in later PRs.

What changed

sm claims <root> --sql-file A --against-sql-file B reports where two statements differ in the seven claims the golden runner already compares (tables, filter predicates, date window, group keys, join keys, ordering, limit). golden_claims.compare_statements had been reachable from the runner and the save door and from no command.
sm compare-results <root> --golden-csv --generated-csv [--match] [--golden-sql-file] [--bounds] says whether two result CSVs say the same thing, through the golden comparator, so a table-shaped answer is judged the way an answer key is. Numeric text is put back as Decimal before the comparator sees a cell, and an empty cell reads as NULL, because the CSV wire lost both.
sm join-probes <root> --sql-file S names, for every join the statement wrote, whether the semantic model declares a relationship between those two tables and whether the written key matches a declared one, and emits the overlap and cardinality probe SQL that would show whether the keys really resolve. It describes; it never grades. The overlap probe is the one introspection already trusts, extracted as introspect.overlap_sql so there is one spelling of it.
sm filter-values plan <root> --sql-file S names, for every string value typed into a filter (=, <>, IN, NOT IN; LIKE is reported as a pattern and not probed), the column it binds to, whether the semantic model's list of values holds it, the near miss under a case-and-whitespace fold, and the probes that would settle it. sm filter-values judge <root> --plan P --results DIR reads the probe CSVs back and grades each value confirmed, model_gap, query_defect or unresolved, with the tier that decided it and the rows the value selects.

None of the four runs SQL. The skill runs every emitted probe through the same execution tier a question takes, with the same guards, and hands the CSVs back.

Three refusals, on purpose

A near miss is a case and whitespace fold only. 'Paid' matching the listed paid is exact under that fold; an edit-distance suggestion is a guess, and this package does not guess about values.
choice_field: {} reads as not yet decoded and grades unresolved until a probe answers. Reading it as "no legal values" would fail every literal on every such column.
A column marked sensitive is never probed for a value. Only the count of distinct values, which names nothing, is emitted for it.

Review, second push

The Agami review panel (correctness, silent failures, test coverage, /code-review, security-review, the house rubric) ran on the first push. Every finding that reproduced is fixed in the second commit:

Joins are classified with the receipt's own flags, in the receipt's order, so a CTE that shadows a declared table is undeclarable, a USING, NATURAL, comma join or unreadable declared on: is undetermined, and only undeclared joins get probes. A join between declared tables on a key the semantic model does not declare is wrong_key. Every empty probe list says why. A self-join on one column no longer crashes.
The overlap probe runs only from a column that is not a declared key; cardinality is probed once per column at the top level and not at all for a declared key.
filter-values plan no longer attributes a CTE's or derived table's column to the real table, reads numeric literals, reports BETWEEN and IS NULL as skipped, hoists the distinct probe to one per column, drops the cardinality probe nothing read, marks exists_folded conditional, and never sends a value carrying a backslash or a control character to the warehouse (the security finding: on backslash-escaping engines a doubled quote can end the literal early).
filter-values judge treats a zero-byte CSV as a probe that failed, leaves an unlisted value open until the existence probe answers, reads the distinct limit from the plan, and carries the operator with rows_with_value.
Dialect.limited spells the row limit once, with TOP after DISTINCT; overlap_sql uses it, so the 50-row sample holds on TOP and FETCH engines too.
The CSV reader lives beside the comparator as result_from_csv, keeps padded ids as text, and refuses an empty file; compare-results defaults to the comparator's own exact; bad bounds and unreadable CSVs come back as JSON errors; filter-values is two argparse subcommands; claims says which side could not be read.

Not changed, on purpose: the three column resolvers in the tree still differ in case policy (pre-existing; a runtime change, not this PR's), and quote_lit still doubles only the quote (pre-existing; introspection's probe mode uses it with warehouse-sampled values, filed separately).

Verification

tests/test_semantic_model_cli_reconcile_verbs.py: 49 tests over a synthetic three-table shop, including the CTE-shadow, USING, unreadable-on:, self-join, no-single-engine, SQL Server row-limit, empty-probe-file, padded-id, bad-bounds and NOT IN cases the review asked for.
tests/test_reconcile.py, tests/test_ah111_reconcile_promotion_skill.py, tests/test_semantic_model_cli.py, tests/test_golden_claims.py, tests/test_ah099_comparator.py, tests/test_semantic_model_introspect.py pass untouched (512 total with the new file).
uv run dev.py check: ruff and gitleaks clean, no vendored-lib drift, 5597 passed, 12 skipped, 1 pre-existing failure: tests/test_golden_run.py::test_a_client_that_cannot_be_found_still_fails_as_a_generation fails identically on untouched origin/main on this machine because a client binary exists at /opt/homebrew/bin/claude. Not touched by this change.
Run tests with uv run dev.py test or uvx --with pytest-cov --with-editable "packages/agami-core[model,server]" pytest …. A bare uv run pytest falls through to the system pytest and imports the package from whichever checkout that environment was installed from.

Review round 2

A second panel pass found six more places a missing or unreadable input became a confident grade. All fixed in the second commit, each with a regression test:

A distinct probe that returned no values at all (header-only file) graded query_defect; it now falls through to the existence tier and stays unresolved when the column holds nothing.
A filter shape the literal walker does not read (OR, a function over the column, a cast, a column-to-column comparison) yielded neither a literal nor a skipped row, so the plan read as all-clear; each is now one skipped row naming its class. A negative number is read as the value typed.
Literals are capped at 200 with dropped counted, mirroring the join cap.
A cross join and a join between two declared keys now say why no overlap probe was written.
A zero-byte folded-probe file is named in the note like the other two.
sm claims emits temporal_predicates per side so a ledger can tell "neither statement filtered on a date" from "a window was written in a shape the resolver does not fold".
Every verb that reads a SQL file answers a missing one with {"error": "unreadable_sql_file"} and exit 2.

Verbs test file: 55 passed.

Review round 3

From a test run on a real profile (findings tracked in the design record). Third commit, 62 verb tests passing:

A declared join now carries what the semantic model knows: declared_cardinality (every relationship between the two tables, direction, one side, and whether the written columns are that edge's) and unique_by_model for both written columns. It used to be the emptiest entry in the file, for the one case the model had already answered.
Every inner join on one column pair plans a dropped_rows_probe: the left table's rows with no partner on the right, as a correlated NOT EXISTS in WHERE (the one form Snowflake, Oracle and BigQuery all run). Withheld with a reason for LEFT and FULL joins, self-joins, cross joins, the size guard and no single engine. A fact for the ledger to state, never to grade.
The judge says what the model declared: declared on every verdict, a note on any warehouse-decided grade over an undeclared column, and a columns map with one entry per filtered column (listed, overflow, empty, failed, not_run), so an undeclared value list is said once per column.
An undetermined aggregate carries reason: which of the four blindnesses it hit. The decision is unchanged.
A matched output column carries the metric's source_tables, so a match by shape on another table can be told apart.

Two wire contracts widen deliberately; their exact key-set pins were updated in test_ace060_trap_free_aggregates.py and test_ace088_receipt_sections.py.

Review round 4

A fourth panel pass on the third commit. Fixed in the fourth commit, each with a regression test (verbs test file: 70 passing):

LEFT SEMI and LEFT ANTI joins were read as LEFT joins and reported as dropping nothing (Databricks, Trino). The probe now reads the join's kind: a semi join is probed, an anti join is withheld with its own reason.
The dropped-rows probe names the side it did not count (unexamined), since an inner join drops from both sides and the probe counts one.
A table of unknown size withholds the whole-table probe (the size guard was fail-open when no row estimate existed), and whole-table probes are capped at twenty per statement.
Uniqueness by the semantic model is one rule for both emitters; the judge's unreadable branch has the same keys as the readable one; an impossible guard is gone; new signatures are annotated; two comments stop saying "the model".

Rebase note (2026-09-13). Replayed onto main at 99e44a8 with every branch below it; content unchanged, force-pushed with lease. The base had moved by four commits (#292, #297, #306, #308); the changelog was union-merged.

🤖 Generated with Claude Code

Copilot

🔵 Needs a closer look

It adds new CLI verbs and SQL-emitting probe logic (and the PR explicitly requests manual review), and there are correctness issues to address before merge.

Pull request overview

Adds a new “reconcile evidence” CLI surface for semantic-model analysis so agami-reconcile can treat a trusted SQL statement as evidence (not an answer) and deterministically compare/plan/judge key statement parts without executing SQL directly.

Changes:

Adds four new sm verbs (claims, compare-results, join-probes, filter-values plan/judge) to diff statements, compare CSV results via the golden comparator, and plan/judge join/value probes.
Introduces semantic_model.probes to emit join/value probe SQL and to grade filter literals from returned probe CSVs.
Extracts introspect.overlap_sql for a shared overlap-probe SQL spelling and adds a dedicated test suite for the new verbs.

File summaries

File	Description
tests/test_semantic_model_cli_reconcile_verbs.py	New end-to-end CLI tests covering the four reconcile verbs and their JSON contracts.
packages/agami-core/src/semantic_model/probes.py	New probe planning/judging logic for joins and filter literals (no DB execution).
packages/agami-core/src/semantic_model/introspect.py	Extracts overlap probe SQL into a reusable helper.
packages/agami-core/src/semantic_model/cli.py	Wires up the new `sm` verbs and adds CSV-to-ExecResult parsing for comparator use.
CHANGELOG.md	Documents the new verbs and the shared overlap probe helper.

Review details

Files reviewed: 5/5 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

sandeep-agami · 2026-09-11T22:38:00Z

+    return (
        f"SELECT COUNT(*) AS matched FROM (SELECT DISTINCT {col_f} AS v FROM {fq_from} "
        f"WHERE {col_f} IS NOT NULL {('LIMIT 50' if dialect.limit_style=='limit' else '')}) src "
        f"WHERE EXISTS (SELECT 1 FROM {fq_to} t WHERE t.{col_t} = src.v)"
    )


Agreed, and fixed in the next push: the sample is now bounded through a new Dialect.limited(select, n), which spells TOP / FETCH FIRST / LIMIT in one place (and puts TOP after DISTINCT, which T-SQL requires). overlap_sql uses it, so the 50-row cap now holds on every engine; the LIMIT-dialect statement is byte-identical to before.

sandeep-agami · 2026-09-11T22:38:02Z

+    negated = False
+    if isinstance(conj, exp.Not) and isinstance(conj.this, exp.In):
+        conj, negated = conj.this, True
+    if isinstance(conj, exp.In) and isinstance(conj.this, exp.Column):
+        for lit in conj.expressions:
+            if isinstance(lit, exp.Literal) and lit.is_string:
+                yield conj.this, lit.this, "not in" if negated else "in", False
+        return


Checked against sqlglot directly: a NOT IN (...) parses to Not(this=In(...)) on postgres, mysql, snowflake, tsql and the generic grammar, and exp.In has no negation argument (In.arg_types is this/expressions/query/unnest/field/is_global). The comment in test_aggregation_enforcement.py names exp.In as the node inside the Not, not as a negated In. The Not(In) unwrap here is therefore the right shape, and test_judge_carries_the_operator_so_a_count_is_not_misread now pins that a NOT IN literal is reported with op not in.

agami-reconcile is learning to take a trusted query as evidence rather than as the answer. The skill decides meaning and talks to the person; everything deterministic about the statement belongs in the CLI, and this is that half. - `sm claims` reports where two statements differ, in the seven claims the golden runner already compares. `golden_claims.compare_statements` had been reachable from the runner and the save door and from no command. - `sm compare-results` says whether two result CSVs say the same thing, through the golden comparator, so a table-shaped answer is judged the way an answer key is. Numeric text is put back as Decimal before the comparator sees a cell, because the CSV wire lost the types. - `sm join-probes` names, for every join a statement wrote, whether the semantic model declares a relationship between those two tables and whether the written key matches the declared one, and emits the overlap and cardinality probes that would show whether the keys resolve. The overlap probe is the one introspection already trusts, now shared as `introspect.overlap_sql`. - `sm filter-values plan` names, for every value typed into a filter, the column it binds to, whether the semantic model's list of values holds it, and the probes that would settle it. `sm filter-values judge` reads the probe results back and grades each value confirmed, model_gap, query_defect or unresolved. None of the four runs SQL. The skill runs every emitted probe through the same execution tier a question takes, with the same guards, and hands the CSVs back. Three refusals are deliberate. A near miss is a case and whitespace fold only, never an edit distance. An empty list of values reads as not yet decoded, never as no legal values. A column marked sensitive is never probed for a value, so a probe cannot confirm private values one guess at a time. Spec: ACE-114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Review found that probes.py rendered open questions as settled claims and read failed probes as answers. Every finding below reproduced against the fixture. - join-probes classifies each join with the receipt's own flags, in the receipt's order: right_declarable, pinned, left_declarable, the reduced pairs, and a declaration whose `on:` could not be read. A CTE shadowing a declared table is `undeclarable`; USING, NATURAL, a comma join and an unreadable declaration are `undetermined`; a join between declared tables on a key the semantic model does not declare is `wrong_key`. Probes are emitted only for `undeclared`, and every empty probe list says why. - The overlap probe runs only from a column that is not a declared key, since sampling a parent key against its children proves nothing about the join. Cardinality probes are per column, once, at the top level, and null where the semantic model already says the column is unique. - filter-values plan subtracts CTE names and computed relations before placing a column, reads numeric literals too, reports BETWEEN and IS NULL as skipped, hoists the distinct probe to one per column, drops the cardinality probe nothing read, marks exists_folded as conditional on exists returning zero, and never sends a value carrying a backslash or a control character to the warehouse: engines quote it differently, and the doubled quote that is safe on one is a breakout on another. - filter-values judge treats a zero-byte probe CSV as a probe that failed, leaves an unlisted value open until the existence probe answers, reads the distinct limit off the plan, carries the operator, and names the count for what it is (rows_with_value). - Dialect.limited spells TOP / FETCH FIRST / LIMIT in one place and puts TOP after DISTINCT; overlap_sql uses it, so the 50-row sample holds on every engine. - The CSV reader moves beside the comparator as result_from_csv, keeps a padded id as text, and refuses an empty file. compare-results defaults to the comparator's own match level, and reports a bad band or an unreadable CSV as JSON rather than a traceback. filter-values is two argparse subcommands with required flags. claims says which side could not be read. - No bare "the model" anywhere in the diff. Spec: ACE-114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Second review round on the four verbs. Every change is one more place a missing or unreadable input used to become a confident answer. - filter-values judge: a distinct probe that returned no values at all (a header-only file) is no evidence about the value typed; the grade falls through to the existence tier, and stays unresolved when that tier says the column holds nothing either. A zero-byte folded probe is named in the note like the other two. - filter-values plan: a filter shape the walker does not read (two conditions joined by OR, a function over the column, a cast, a comparison against another column) is one `skipped` row naming its class, never an empty list; a negative number is the value typed; literals are capped at 200 with `dropped` counted, the way joins already were. - join-probes: a cross join and a join between two declared keys each say why no overlap probe was written. - claims: `temporal_predicates` counts, per side, the conjuncts that speak of time. Two zeros are what lets a ledger read a `date_window` of `unknown` as "neither statement filtered on a date". - Every verb that reads a SQL file answers a missing one with `{"error": "unreadable_sql_file"}` and exit 2 instead of a traceback and an empty stdout. Spec: ACE-114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ss names itself Third round, from a test run on a real profile. Four facts now travel that used to stay in the analysis that had them in hand. - join-probes: a declared or wrong-key join carries `declared_cardinality` (every relationship the model declares between the two tables, its direction, its one side, and whether the written columns are that edge's columns) and `unique_by_model` for both written columns from the declared key or the table's grain. No probe is planned for it; the declaration has answered. Every inner join on one column pair also carries a `dropped_rows_probe`: the left table's rows with no partner on the right, whole table, as a correlated NOT EXISTS in WHERE, the one spelling every engine runs. Withheld, with the reason, for LEFT and FULL joins, self-joins, cross joins, unplaced columns, the size guard and a datasource with no single engine. - filter-values judge: every verdict carries `declared` (populated, empty, absent) and a grade the warehouse decided over an undeclared column says so. `columns` is one entry per filtered column with what the model declares and what the distinct probe showed (listed, overflow, empty, failed, not_run), so a column nobody declared a list for is said once, not once per value. - pre-flight: an `undetermined` aggregate carries `reason`, the one blindness it hit (no column named; a column attributed to no single table; a name bound to a computed relation; a table the model does not declare). The decision is unchanged; the boolean it reads is now derived from the reason. - receipt: a matched output column carries the metric's `source_tables`, so a match by shape on a table the statement never reads can be told from a match on one it does. Two wire contracts widen on purpose and their key-set pins move with them: tests/test_ace060_trap_free_aggregates.py (aggregate items gain `reason`) and tests/test_ace088_receipt_sections.py (output items gain `source_tables`). Spec: ACE-114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… count, and never scans a table of unknown size Fourth review round on the verbs. - A LEFT SEMI or LEFT ANTI join parsed as a LEFT join and was reported as dropping nothing. The probe now reads the join's kind: a semi join drops exactly what the probe counts and is probed; an anti join keeps only the rows with no partner, so its count is the join's result and it is withheld with its own reason. - The probe counts one side, the left. It now names the other as `unexamined`, so a reader never takes one count for the whole story. - The size guard returned false when a table had no row estimate, so a profile introspected without estimates got a whole-table NOT EXISTS per join. A table of unknown size now withholds the probe, and whole-table probes are capped at twenty per statement. - Uniqueness by the semantic model is one rule (a declared key or the whole grain) read by both emitters; the judge's unreadable branch carries the same keys as its readable one; a guard for an impossible condition is gone; new signatures are annotated. Spec: ACE-114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

sandeep-agami requested a review from ashwin-agami as a code owner September 11, 2026 22:09

Copilot AI lite review requested due to automatic review settings September 11, 2026 22:09

Copilot started reviewing on behalf of sandeep-agami September 11, 2026 22:09 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

sandeep-agami and others added 5 commits September 13, 2026 16:33

sandeep-agami force-pushed the ACE-114-reconcile-sm-verbs branch from 8fe1ea5 to 6102ae5 Compare September 13, 2026 23:36

Navigation Menu

Uh oh!

Four sm verbs grade a statement a person supplied, part by part - #288

Four sm verbs grade a statement a person supplied, part by part#288
sandeep-agami wants to merge 5 commits into
mainfrom
ACE-114-reconcile-sm-verbs

sandeep-agami commented Sep 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

sandeep-agami Sep 11, 2026

Uh oh!

sandeep-agami Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sandeep-agami commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Three refusals, on purpose

Review, second push

Verification

Review round 2

Review round 3

Review round 4

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

sandeep-agami Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

sandeep-agami Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sandeep-agami commented Sep 11, 2026 •

edited

Loading