From 5c2bc5e5ce8992fdc7a7b4e779ae03df79ec6e86 Mon Sep 17 00:00:00 2001 From: Sandeep Date: Sun, 13 Sep 2026 19:36:31 -0700 Subject: [PATCH] comparator: pair columns by agreement, take an unordered door; claims: read the outputs One differing cell unpaired a whole column, so the score read "no generated column carries the values of: total" for a column agreeing on nine rows of ten, and every reader keyed on the pairs saw none. Columns still pair on whole-vector equality first; what is left pairs by name when both sides spell one (qualifier and case dropped), or by the highest share of agreeing rows when more than half agree. The score then counts rows and carries `column_agreement` beside `column_pairs` and `paired_row_share` over the paired columns. The pass mark does not move. `compare_result_sets(ordered=False)` and `sm compare-results --unordered` compare the rows as a set whatever ORDER BY the statement wrote, for reconcile, whose ordering is a claim of its own. The golden run never passes it. The claims reader gains an eighth claim, `outputs`: what each statement selects, alias peeled, so two statements are the same query only when they select the same expressions. It reports and never gates. Every reader that counted seven counts eight. Pins moved on purpose: reversed rows under one name pair by name and disagree row for row; a boolean against an int of the same name, and a null against an empty string, pair at agreement 0; the exact level's failing pair reads 0.5; the claim count and names. Spec: ACE-131 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 24 +++ packages/agami-core/src/semantic_model/cli.py | 13 +- .../src/semantic_model/comparator.py | 175 +++++++++++++++--- .../src/semantic_model/golden_claims.py | 48 +++-- .../agami/scripts/render_golden_datasets.py | 2 +- .../shared/golden-datasets-template.html | 4 +- tests/test_ah099_comparator.py | 104 +++++++++-- tests/test_comparator_column_pairs.py | 3 + tests/test_golden_claims.py | 45 ++++- tests/test_golden_run.py | 5 +- tests/test_render_golden_datasets.py | 2 +- tests/test_render_golden_run.py | 2 +- ...test_semantic_model_cli_reconcile_verbs.py | 19 +- 13 files changed, 366 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7225ef6b..bdad721a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,18 @@ below corresponds to one such version. ### Changed +- **The result comparator pairs a column that mostly agrees instead of calling it missing.** One + differing cell used to unpair a column: the score fell to 0 with "no generated column carries the + values of: total" for a column agreeing on nine rows of ten, and every reader keyed on the pairs + saw an empty list. Columns still pair on whole-vector equality first; what is left pairs by name + when the two sides spell one (the qualifier and case dropped), or by the highest share of agreeing + rows when more than half agree. The score then counts rows ("9 of the answer key's 10 rows + matched") and carries `column_agreement` beside `column_pairs` and `paired_row_share` over the + paired columns. An item still passes at exactly 1.0; a golden column with no partner at all still + scores 0 with its name. Three pins moved with it: a same-named column of another type, a null + against an empty string, and one differing row now read as a pair that disagrees, not a column + that is absent. (ACE-131) + - **A golden run pays for the model's description once, not once per question.** Every question starts its own client, and every one re-sent the whole model — about 35k tokens on a 22-area profile — in a prompt that could not be reused, because it began with the client's own system @@ -37,6 +49,18 @@ below corresponds to one such version. ### Added +- **An eighth claim, `outputs`, says what a statement selects.** `sm claims` and the golden run + compared tables, filters, date window, group keys, join keys, ordering and limit, and never the + projection, so two statements selecting different expressions could still read as the same + query. The new claim reads each output expression with its alias peeled (`total` and + `o.amount_total` over one `SUM(orders.amount)` agree; `SUM` against `AVG` differs; `SELECT *` is the + one key `*`). It reports and never gates. Every reader that counted seven now counts eight. + (ACE-131) +- **`sm compare-results --unordered`** compares the rows as a set whatever ORDER BY either statement + wrote, for a caller whose ordering is a claim of its own (reconcile's 2e, in ACE-134). The + comparator's `compare_result_sets` takes the same as `ordered=False`; the golden run is unchanged. + (ACE-131) + - **Four `sm` verbs that grade a statement a person supplied, part by part.** `agami-reconcile` is learning to take a trusted query as evidence rather than as the answer, and these are the deterministic checks it will lean on. `sm claims` reports where two statements differ, in the seven diff --git a/packages/agami-core/src/semantic_model/cli.py b/packages/agami-core/src/semantic_model/cli.py index 1e45f25c..54f1f0b2 100644 --- a/packages/agami-core/src/semantic_model/cli.py +++ b/packages/agami-core/src/semantic_model/cli.py @@ -313,10 +313,10 @@ def _read_sql_file(path: str) -> Optional[str]: def cmd_claims(args) -> int: - """Where two statements differ, in the seven claims the golden runner already compares. + """Where two statements differ, in the eight claims the golden runner already compares. `golden_claims.compare_statements` has been reachable from the runner and the save door and from no command; this is that command. A side that could not be read says so, rather than - leaving seven `unknown` claims to explain themselves.""" + leaving eight `unknown` claims to explain themselves.""" from .golden_claims import compare_statements, count_temporal_predicates, read_claims org = L.load_datasource(args.root) grammar = _grammar(org) @@ -371,7 +371,8 @@ def cmd_compare_results(args) -> int: if golden_sql is None: return 2 score = compare_result_sets(golden, generated, match=args.match, golden_sql=golden_sql, - bounds=bounds, dialect=_grammar(org)) + bounds=bounds, dialect=_grammar(org), + ordered=False if args.unordered else None) _print_json(dataclasses.asdict(score)) return 0 @@ -1348,7 +1349,7 @@ def build_parser() -> argparse.ArgumentParser: help="optional freshness timestamp for the receipt's tables section") sp.set_defaults(func=cmd_receipt) - sp = sub.add_parser("claims", help="where two statements differ: the seven claims the golden runner compares, as a diff") + sp = sub.add_parser("claims", help="where two statements differ: the eight claims the golden runner compares, as a diff") sp.add_argument("root") sp.add_argument("--sql-file", required=True, dest="sql_file") sp.add_argument("--against-sql-file", required=True, dest="against_sql_file") @@ -1361,7 +1362,9 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("--match", default="exact", choices=["exact", "values", "shape", "bounded", "nonempty"], help="the comparator's own default is exact; reconcile passes values for a number that may carry a float tail") sp.add_argument("--golden-sql-file", default=None, dest="golden_sql_file", - help="the answer key's statement, read only for whether it ordered its rows") + help="the answer key's statement, read only for whether it ordered its rows; not read when --unordered is given") + sp.add_argument("--unordered", action="store_true", + help="compare the rows as a set whatever ORDER BY either statement wrote; reconcile passes it because the ordering claim carries the order") sp.add_argument("--bounds", default=None, help="JSON with min_rows/max_rows/min_value/max_value, for --match bounded") sp.set_defaults(func=cmd_compare_results) diff --git a/packages/agami-core/src/semantic_model/comparator.py b/packages/agami-core/src/semantic_model/comparator.py index 149d4e4c..0486b764 100644 --- a/packages/agami-core/src/semantic_model/comparator.py +++ b/packages/agami-core/src/semantic_model/comparator.py @@ -312,7 +312,46 @@ def _column_vectors( return vectors -def match_columns( +class ColumnPairing(NamedTuple): + """How the golden columns paired with the generated ones, and how far each pair agrees. + + `agreement` is per golden index: the share of rows on which the pair's cells are equal, 1.0 for + a pair the exact stage made. It is what lets a reader say "9 of 10 rows agree on this column" + where the old all-or-nothing pairing could only say the column was missing. + """ + + pairing: dict[int, int] + unmatched: tuple[str, ...] + agreement: dict[int, float] + + +def _folded_name(name: str) -> str: + """A column name as two statements would spell the same column: lowercase, the qualifier off, so + `o.Total` and `total` are one name.""" + return name.rsplit(".", 1)[-1].strip().lower() + + +def _agreement( + left: tuple[tuple[str, Any], ...], right: tuple[tuple[str, Any], ...], ordered: bool +) -> float: + """The share of rows on which two column vectors agree: position by position when the order is + part of the answer, as a multiset when it is not. The vectors are the same length here.""" + if not left: + return 0.0 + if ordered: + overlap = sum(1 for a, b in zip(left, right) if a == b) + else: + overlap = sum((Counter(left) & Counter(right)).values()) + return overlap / len(left) + + +# A best-effort pair needs more than half the rows to agree. Below that the two columns share a few +# values by coincidence (an id and a count on a short result), and pairing them would put the wrong +# column's difference on the card. Strict, so a two-row result agreeing on one row does not pair. +_MAJORITY = 0.5 + + +def pair_columns( golden_columns: Sequence[str], golden_rows: Sequence[Sequence[Any]], generated_columns: Sequence[str], @@ -320,13 +359,22 @@ def match_columns( *, ordered: bool, quantize: bool = False, -) -> tuple[dict[int, int], tuple[str, ...]]: - """Pair golden columns with the generated columns carrying the same values. - - Returns the golden-index → generated-index pairing and the golden column names that found no - partner. Neither a column's NAME nor its position is ever consulted: a generated statement - that aliases the total and selects it second still answered the question, and a statement that - reused the golden name for a different value did not. +) -> ColumnPairing: + """Pair golden columns with generated columns, by values first and then by best effort. + + Stage one pairs on whole value-vector equality and consults neither a column's name nor its + position: a generated statement that aliases the total and selects it second still answered the + question. It is greedy and deliberately NOT a maximum-matching algorithm: equality is transitive, + so the candidate sets are equivalence classes, partners inside one class are interchangeable, and + taking the first unclaimed one can never strand a later column that had an option of its own. + + Stage two is for what stage one left: one differing cell would otherwise unpair a column that is + plainly there, and the score would read "no generated column carries the values of total" for a + column agreeing on nine rows of ten. Over the golden columns still unmatched, in order, and the + generated columns still unclaimed: a candidate with the same folded name pairs at any agreement + (the name says it is the same column; the agreement says how much of it differs); otherwise the + candidate with the highest share of agreeing rows pairs when that share is above one half, ties + going to generated order. A golden column with no such partner is reported unmatched, as before. """ golden_vectors = _column_vectors( golden_columns, golden_rows, ordered=ordered, quantize=quantize @@ -334,22 +382,58 @@ def match_columns( generated_vectors = _column_vectors( generated_columns, generated_rows, ordered=ordered, quantize=quantize ) - # Greedy, and deliberately NOT a maximum-matching algorithm. A golden column pairs with a - # generated one only when their value vectors are equal, and equality is transitive: the - # candidate sets are equivalence classes, so two golden columns either compete for exactly the - # same partners or for none of the same. Partners inside one class are interchangeable, so - # taking the first unclaimed one can never strand a later column that had an option of its own - # — there is no augmenting path to find, and adding one back would be dead weight. unclaimed: dict[tuple[tuple[str, Any], ...], list[int]] = {} for index, vector in enumerate(generated_vectors): unclaimed.setdefault(vector, []).append(index) pairing: dict[int, int] = {} + agreement: dict[int, float] = {} for index, vector in enumerate(golden_vectors): partners = unclaimed.get(vector) if partners: pairing[index] = partners.pop(0) + agreement[index] = 1.0 + + claimed = set(pairing.values()) + for index, vector in enumerate(golden_vectors): + if index in pairing: + continue + candidates = [i for i in range(len(generated_vectors)) if i not in claimed] + if not candidates: + continue + wanted = _folded_name(golden_columns[index]) + by_name = [i for i in candidates if _folded_name(generated_columns[i]) == wanted] + if by_name: + chosen = by_name[0] + else: + shares = [(_agreement(vector, generated_vectors[i], ordered), -i) for i in candidates] + best_share, negative_index = max(shares) + if best_share <= _MAJORITY: + continue + chosen = -negative_index + pairing[index] = chosen + claimed.add(chosen) + agreement[index] = _agreement(vector, generated_vectors[chosen], ordered) + unmatched = tuple(name for index, name in enumerate(golden_columns) if index not in pairing) - return pairing, unmatched + return ColumnPairing(pairing, unmatched, agreement) + + +def match_columns( + golden_columns: Sequence[str], + golden_rows: Sequence[Sequence[Any]], + generated_columns: Sequence[str], + generated_rows: Sequence[Sequence[Any]], + *, + ordered: bool, + quantize: bool = False, +) -> tuple[dict[int, int], tuple[str, ...]]: + """`pair_columns` as the golden-index → generated-index pairing and the golden column names that + found no partner, the shape every caller and pin has read since Slice 1.""" + paired = pair_columns( + golden_columns, golden_rows, generated_columns, generated_rows, + ordered=ordered, quantize=quantize, + ) + return paired.pairing, paired.unmatched def _project( @@ -419,6 +503,13 @@ class ItemScore: generated_row_count: Optional[int] = None order_sensitive: Optional[bool] = None notes: tuple[str, ...] = () + # Beside `column_pairs`, pair for pair: the share of rows on which that pair agrees (1.0 for a + # pair made on whole-vector equality). And the share of rows on which EVERY paired column agrees, + # computed even when a golden column has no partner, so a reader can tell "identical on the + # paired columns, one column missing" from "the paired columns disagree too". None when nothing + # paired or the row counts differ. Additive, like the pairs. + column_agreement: tuple[float, ...] = () + paired_row_share: Optional[float] = None class _Verdict(NamedTuple): @@ -634,31 +725,45 @@ def _judge( return _Verdict("error", None, f"{match!r} is not a match level this comparison knows") -def _column_pairs( +def _pairing_facts( golden: ExecResult, generated: ExecResult, match: MatchLevel, ordered: bool -) -> tuple[tuple[tuple[str, str], ...], tuple[str, ...]]: - """(golden column, generated column) pairs matched by values, and the generated columns left - over, for the two levels that pair columns at all. Never raises; a shape the pairing cannot read - reports nothing rather than failing a score that already ran.""" +) -> tuple[tuple[tuple[str, str], ...], tuple[str, ...], tuple[float, ...], Optional[float]]: + """(golden column, generated column) pairs, the generated columns left over, each pair's + agreement and the share of rows every pair agrees on, for the two levels that pair columns at + all. Never raises; a shape the pairing cannot read reports nothing rather than failing a score + that already ran.""" + nothing: tuple[tuple[tuple[str, str], ...], tuple[str, ...], tuple[float, ...], Optional[float]] + nothing = ((), (), (), None) if match not in ("exact", "values") or not golden.rows or not generated.rows: - return (), () + return nothing if len(golden.rows) != len(generated.rows): # Pairing is by value vectors, and two vectors of different length are never equal, so # every column would read unpaired: not a fact about the columns, only about the counts, # which the score already reports. Nothing is claimed here. - return (), () + return nothing + quantize = match == "values" try: - pairing, _unmatched = match_columns( + paired = pair_columns( golden.columns, golden.rows, generated.columns, generated.rows, - ordered=ordered, quantize=match == "values", + ordered=ordered, quantize=quantize, ) + share: Optional[float] = None + if paired.pairing: + overlap, golden_count, _generated_count = compare_rows( + golden.rows, generated.rows, paired.pairing, ordered=ordered, quantize=quantize + ) + share = _accuracy(overlap, golden_count) except Exception: # The score itself has already reported a ragged or malformed result as an error with a # value-free reason; the pairing is a courtesy on top and must never turn that into a raise. - return (), () - pairs = tuple((golden.columns[g], generated.columns[i]) for g, i in sorted(pairing.items())) - extra = tuple(name for i, name in enumerate(generated.columns) if i not in set(pairing.values())) - return pairs, extra + return nothing + ordered_pairs = sorted(paired.pairing.items()) + pairs = tuple((golden.columns[g], generated.columns[i]) for g, i in ordered_pairs) + agreement = tuple(paired.agreement[g] for g, _ in ordered_pairs) + taken = set(paired.pairing.values()) + extra = tuple(name for i, name in enumerate(generated.columns) if i not in taken) + return pairs, extra, agreement, share + def compare_result_sets( golden: ExecResult, @@ -668,6 +773,7 @@ def compare_result_sets( golden_sql: Optional[str] = None, bounds: Optional[GoldenBounds] = None, dialect: Optional[str] = None, + ordered: Optional[bool] = None, ) -> ItemScore: """Score one generated result against its answer key. Never raises. @@ -675,8 +781,15 @@ def compare_result_sets( generated statement is deliberately not a parameter: the ordering that has to hold is the one the ANSWER KEY asked for, so a generated statement that drops the ORDER BY is still judged against it rather than excused by it. + + `ordered`, when given, decides instead of the statement. Reconcile passes False: it compares two + trusted statements whose ordering is a claim of its own, so the rows are a set here and the + ORDER BY is judged where it is named. The golden run never passes it. """ - ordered, note = has_top_level_order_by(golden_sql, dialect=dialect) + if ordered is None: + ordered, note = has_top_level_order_by(golden_sql, dialect=dialect) + else: + note = None if ordered else "row order was not compared" try: verdict = _judge(golden, generated, match, bounds, ordered) except RaggedRow as exc: @@ -688,7 +801,7 @@ def compare_result_sets( verdict = _Verdict( "error", None, f"the comparison failed with an unexpected {type(exc).__name__}" ) - pairs, extra = _column_pairs(golden, generated, match, ordered) + pairs, extra, agreement, share = _pairing_facts(golden, generated, match, ordered) return ItemScore( status=verdict.status, accuracy=verdict.accuracy, @@ -700,6 +813,8 @@ def compare_result_sets( generated_row_count=len(generated.rows), order_sensitive=ordered, notes=(note,) if note else (), + column_agreement=agreement, + paired_row_share=share, ) diff --git a/packages/agami-core/src/semantic_model/golden_claims.py b/packages/agami-core/src/semantic_model/golden_claims.py index e71200ea..1c03b078 100644 --- a/packages/agami-core/src/semantic_model/golden_claims.py +++ b/packages/agami-core/src/semantic_model/golden_claims.py @@ -3,10 +3,10 @@ A golden item that fails on its numbers says the two statements returned different rows. It cannot say *why*, and "why" is the whole value of the failure: a window off by a quarter, a required filter left out and a genuinely different question all look identical from a row count. This module is the -sentence after that one. It reads each statement into seven claims about what the statement asks +sentence after that one. It reads each statement into eight claims about what the statement asks for, compares them claim by claim, and hands the caller a structured diff. -**It is a describer with two gates, and the split is the design.** Five of the seven claims are +**It is a describer with two gates, and the split is the design.** Six of the eight claims are REPORTED — a difference in them is a fact for a person to read, not a verdict — and exactly two are allowed to decide anything: @@ -53,11 +53,13 @@ DIFFERS = "differs" UNKNOWN = "unknown" -# Exactly seven, and the tuple is the contract: an eighth claim is a change to what a golden item -# is allowed to assert about a statement, not an implementation detail of this module. The order is -# the order a diff renders in. +# Exactly eight, and the tuple is the contract: a new claim is a change to what a golden item is +# allowed to assert about a statement, not an implementation detail of this module (the eighth, +# `outputs`, was added by ACE-131 so that "the same query" can mean "selects the same things" too). +# The order is the order a diff renders in. CLAIM_NAMES = ( "tables", + "outputs", "filter_predicates", "date_window", "group_keys", @@ -113,13 +115,19 @@ def as_dict(self) -> dict[str, Any]: @dataclass class ClaimSet: - """What one statement asks for, in the seven terms two statements are compared in. + """What one statement asks for, in the eight terms two statements are compared in. Every field defaults to its own empty value so that `ClaimSet(unreadable=…)` is the whole of the unreadable case; `read_claims` is the only constructor, and it always fills all of them. """ tables: frozenset[str] = frozenset() # bare, case-folded + # What the statement selects: one key per output expression with its alias peeled, so two + # statements that select the same expressions under different names agree, and a statement + # that selects a different expression differs. `SELECT *` is the one key `*`. A describer, + # never a gate: the projection is what an answer's columns come from, and a comparison of the + # columns' VALUES lives in the comparator, not here. + outputs: frozenset[str] = frozenset() filter_predicates: frozenset[str] = frozenset() # normalized keys, not the statement's text # Every column any predicate the statement writes mentions — the `must_filter` gate's input, # and a different question from the one above: *is this column constrained anywhere* rather @@ -141,6 +149,7 @@ class ClaimSet: def as_dict(self) -> dict[str, Any]: return { "tables": sorted(self.tables), + "outputs": sorted(self.outputs), "filter_predicates": sorted(self.filter_predicates), # Bounded here rather than at the source: a quoted identifier is written by whoever # wrote the statement, and this is the one claim value that is held raw so the gate can @@ -162,7 +171,7 @@ def _join_keys_as_list(keys: "frozenset[frozenset[tuple[str, str]]]") -> list[li def read_claims(sql: str, *, dialect: str) -> ClaimSet: - """Read one statement into its seven claims. Never raises: an input this module cannot read + """Read one statement into its eight claims. Never raises: an input this module cannot read comes back as a `ClaimSet` whose `unreadable` says so.""" tree, why = rt._parse_reporting(sql, dialect=dialect) if tree is None: @@ -187,6 +196,7 @@ def read_claims(sql: str, *, dialect: str) -> ClaimSet: # written, and these are the values that would otherwise arrive in the diff at whatever # length and with whatever line breaks the statement gave them. tables=frozenset(rt._echo_name(rt._tkey(ref.bare)) for ref in rt._table_references(select)), + outputs=_outputs(select, aliases), filter_predicates=frozenset(_expression_key(node, aliases) for node in conjuncts), filtered_columns=_constrained_columns(select), date_window=_resolve_date_window(conjuncts, aliases), @@ -197,6 +207,22 @@ def read_claims(sql: str, *, dialect: str) -> ClaimSet: ) +def _outputs(select: "exp.Select", aliases: dict[str, str]) -> frozenset[str]: + """What the statement selects, one key per output expression, the alias peeled off first: an + alias is the author's name for a value and not the value. `SELECT *` (and `t.*`) is the key `*`, + because a star selects whatever the table has and no list of names can be read out of it here. + `DISTINCT` is not read: this claim says what is selected, not how many times.""" + keys = [] + for node in select.expressions: + if isinstance(node, exp.Alias): + node = node.this + if isinstance(node, exp.Star) or (isinstance(node, exp.Column) and isinstance(node.this, exp.Star)): + keys.append("*") + continue + keys.append(_expression_key(node, aliases)) + return frozenset(keys) + + def _expression_key(node: "exp.Expression", aliases: dict[str, str]) -> str: """One expression reduced to the key two statements compare it by. @@ -710,7 +736,7 @@ def _join_keys( @dataclass class Claim: - """One of the seven claims, and whether the two statements agree on it. + """One of the eight claims, and whether the two statements agree on it. `generated` and `golden` are the claim's own value on each side, in the JSON-able form `ClaimSet.as_dict` renders it — identifiers, bounds and counts, never a statement. @@ -744,9 +770,9 @@ def as_dict(self) -> dict[str, Any]: @dataclass class ClaimDiff: - """What two statements say about each other: seven claims, and whatever gated.""" + """What two statements say about each other: eight claims, and whatever gated.""" - claims: list[Claim] # exactly seven, in CLAIM_NAMES order + claims: list[Claim] # exactly eight, in CLAIM_NAMES order gates: list[GateVerdict] # empty when nothing gates @property @@ -793,7 +819,7 @@ def diff_claims( if unreadable: claims.append(Claim(name=name, status=UNKNOWN, generated=None, golden=None)) continue - # Six of the seven are decided on the RENDERED value, which `as_dict` has already sorted — + # Seven of the eight are decided on the RENDERED value, which `as_dict` has already sorted — # so every claim that is a set underneath compares order-insensitively for free, and the # value a reader is shown is the same value the status was decided from. The window is the # exception, because its own rule ignores one of its fields. diff --git a/plugins/agami/scripts/render_golden_datasets.py b/plugins/agami/scripts/render_golden_datasets.py index 4458f14d..4ce3f6fe 100644 --- a/plugins/agami/scripts/render_golden_datasets.py +++ b/plugins/agami/scripts/render_golden_datasets.py @@ -195,7 +195,7 @@ def _coverage(manifest: dict, datasets: list) -> dict[str, Any]: nothing, which is the false comfort this tab exists to remove. Tables and metrics are two different strengths of evidence and are kept under separate keys for - that reason. A table is one of the seven claims a statement is read into, so "this answer key + that reason. A table is one of the eight claims a statement is read into, so "this answer key reads `orders`" is a fact about the statement. A metric is not one of those seven, so it is matched by name against the statement's text — a weaker signal, and the page says so. """ diff --git a/plugins/agami/shared/golden-datasets-template.html b/plugins/agami/shared/golden-datasets-template.html index c895a248..89e326fa 100644 --- a/plugins/agami/shared/golden-datasets-template.html +++ b/plugins/agami/shared/golden-datasets-template.html @@ -628,12 +628,12 @@

Send to Claude

c.tables_untouched, 'gap')); root.appendChild(coverageGroup( 'Tables the confirmed answer keys read', - 'Read out of each statement itself — one of the seven claims a statement is read into — ' + 'Read out of each statement itself — one of the eight claims a statement is read into — ' + 'rather than from what the author declared it uses.', c.tables_exercised)); root.appendChild(coverageGroup( 'Metrics no confirmed answer key names', - 'Weaker evidence than the tables above: a metric is not one of those seven claims, so this ' + 'Weaker evidence than the tables above: a metric is not one of those eight claims, so this ' + 'is matched by name against the answer key text. A metric spelled out by hand in a ' + 'statement reads as absent here.', c.metrics_unnamed, 'gap')); diff --git a/tests/test_ah099_comparator.py b/tests/test_ah099_comparator.py index c96d7f7a..1a667fe9 100644 --- a/tests/test_ah099_comparator.py +++ b/tests/test_ah099_comparator.py @@ -514,14 +514,16 @@ def test_a_duplicated_golden_column_leaves_one_unmatched(): assert unmatched[0] in ("a", "b") -def test_a_bool_column_does_not_match_an_int_column(): +def test_a_bool_column_pairs_with_the_int_column_of_its_name_and_agrees_on_no_row(): # Slice 1's tags exist for this: `is_active` as a real boolean against the 0/1 SQLite stores - # is a different answer, and raw equality would have called it a match. - pairing, unmatched = c.match_columns( - ["is_active"], [(True,), (False,)], ["is_active"], [(1,), (0,)], ordered=True + # is a different answer, and raw equality would have called it a match. The two columns share + # a name, so they are paired as the same column; the agreement says every row differs, and the + # score then reads "0 of 2 rows matched" rather than "no generated column carries is_active". + paired = c.pair_columns( + ["is_active"], [(True,), (False,)], ["is_active"], [(1,), (0,)], ordered=True, quantize=False ) - assert pairing == {} - assert unmatched == ("is_active",) + assert paired.pairing == {0: 0} and paired.unmatched == () + assert paired.agreement == {0: 0.0} def test_column_values_in_a_different_row_order_match_when_unordered(): @@ -532,12 +534,18 @@ def test_column_values_in_a_different_row_order_match_when_unordered(): assert unmatched == () -def test_column_values_in_a_different_row_order_do_not_match_when_ordered(): +def test_column_values_in_a_different_row_order_pair_by_name_and_disagree_row_for_row_when_ordered(): + # The column is there under its own name, so it pairs; the rows then disagree position by + # position, which is what an ordered comparison is for. The score says so in rows, not columns. pairing, unmatched = c.match_columns( ["channel"], [("web",), ("store",)], ["channel"], [("store",), ("web",)], ordered=True ) - assert pairing == {} - assert unmatched == ("channel",) + assert pairing == {0: 0} and unmatched == () + golden = c.ExecResult(columns=["channel"], rows=[("web",), ("store",)]) + generated = c.ExecResult(columns=["channel"], rows=[("store",), ("web",)]) + score = c.compare_result_sets(golden, generated, golden_sql="SELECT channel FROM t ORDER BY channel") + assert score.accuracy == 0.0 and score.reason.startswith("0 of the answer key's 2 rows matched") + assert score.column_pairs == (("channel", "channel"),) and score.column_agreement == (0.0,) def test_a_mixed_type_column_sorts_without_raising_when_unordered(): @@ -553,11 +561,13 @@ def test_a_mixed_type_column_sorts_without_raising_when_unordered(): def test_matching_forwards_quantize(): golden_rows = [(Decimal("1.00000000001"),)] generated_rows = [(Decimal("1.0"),)] - assert c.match_columns(["v"], golden_rows, ["v"], generated_rows, ordered=True)[0] == {} - pairing, _ = c.match_columns( - ["v"], golden_rows, ["v"], generated_rows, ordered=True, quantize=True - ) - assert pairing == {0: 0} + # Under another name the two pair only when quantize makes the cells equal... + assert c.match_columns(["v"], golden_rows, ["w"], generated_rows, ordered=True)[0] == {} + assert c.match_columns(["v"], golden_rows, ["w"], generated_rows, ordered=True, quantize=True)[0] == {0: 0} + # ...and under the same name they pair either way, quantize deciding whether the one row agrees. + strict = c.pair_columns(["v"], golden_rows, ["v"], generated_rows, ordered=True, quantize=False) + loose = c.pair_columns(["v"], golden_rows, ["v"], generated_rows, ordered=True, quantize=True) + assert strict.agreement == {0: 0.0} and loose.agreement == {0: 1.0} def test_matching_a_ragged_row_is_surfaced_as_this_module_s_error(): @@ -807,7 +817,9 @@ def test_a_null_does_not_match_the_empty_string_end_to_end(): generated = _res(["note"], [("",)]) score = c.compare_result_sets(golden, generated, golden_sql=_UNORDERED) assert score.accuracy == 0.0 - assert score.unmatched_golden_columns == ("note",) + # The column pairs by its name and the one row disagrees: a null is not an empty string. + assert score.column_pairs == (("note", "note"),) and score.column_agreement == (0.0,) + assert score.unmatched_golden_columns == () def test_a_boolean_agrees_with_its_text_spelling_but_never_with_an_int(): @@ -818,7 +830,9 @@ def test_a_boolean_agrees_with_its_text_spelling_but_never_with_an_int(): assert spelled.accuracy == 1.0 stored = c.compare_result_sets(golden, _res(["is_active"], [(1,), (0,)]), golden_sql=_UNORDERED) assert stored.accuracy == 0.0 - assert stored.unmatched_golden_columns == ("is_active",) + # Paired by name, agreeing on no row: the difference is in every value, not in a missing column. + assert stored.column_pairs == (("is_active", "is_active"),) and stored.column_agreement == (0.0,) + assert stored.unmatched_golden_columns == () # --- the five levels ------------------------------------------------------------------------- @@ -828,7 +842,7 @@ def test_a_boolean_agrees_with_its_text_spelling_but_never_with_an_int(): "level, golden, generated, bounds, expected", [ ("exact", _res(["orders"], [(1,), (2,)]), _res(["orders"], [(1,), (2,)]), None, 1.0), - ("exact", _res(["orders"], [(1,), (2,)]), _res(["orders"], [(1,), (9,)]), None, 0.0), + ("exact", _res(["orders"], [(1,), (2,)]), _res(["orders"], [(1,), (9,)]), None, 0.5), # same name, one of two rows agrees: the share, not a missing column # A twelfth-digit difference is inside `values`' tolerance and outside `exact`'s. ( "values", @@ -1302,3 +1316,59 @@ def test_the_module_exports_only_its_public_surface(): # The scoring call and the value it hands back. Everything else is an internal these tests # reach as a module attribute, and `MatchLevel`/`GoldenBounds` belong to `golden`. assert set(c.__all__) == {"compare_result_sets", "ItemScore"} + + +# --- pairing by agreement -------------------------------------------------------------------------- + +_TEN = [(f"c{i}", i * 10) for i in range(10)] + + +def test_a_column_agreeing_on_most_rows_pairs_and_the_score_counts_the_rows_that_match(): + golden = c.ExecResult(columns=["customer", "total"], rows=_TEN) + rows = list(_TEN) + rows[3] = ("c3", 31) + generated = c.ExecResult(columns=["customer", "amount"], rows=rows) + score = c.compare_result_sets(golden, generated, match="values", ordered=False) + assert score.column_pairs == (("customer", "customer"), ("total", "amount")) + assert score.column_agreement == (1.0, 0.9) + assert score.accuracy == 0.9 and score.paired_row_share == 0.9 + assert score.reason.startswith("9 of the answer key's 10 rows matched") + assert score.unmatched_golden_columns == () and score.unmatched_generated_columns == () + + +def test_a_differently_named_column_agreeing_on_a_minority_of_rows_stays_unmatched(): + golden = c.ExecResult(columns=["customer", "total"], rows=_TEN) + rows = [(name, value if i < 3 else value + 1) for i, (name, value) in enumerate(_TEN)] + generated = c.ExecResult(columns=["customer", "amount"], rows=rows) + score = c.compare_result_sets(golden, generated, match="values", ordered=False) + assert score.unmatched_golden_columns == ("total",) and score.unmatched_generated_columns == ("amount",) + assert score.accuracy == 0.0 and score.reason == "no generated column carries the values of: total" + # The customer column paired and agrees on every row: the share over the paired columns says so. + assert score.column_pairs == (("customer", "customer"),) and score.paired_row_share == 1.0 + + +def test_a_same_named_column_pairs_at_any_agreement(): + golden = c.ExecResult(columns=["customer", "total"], rows=_TEN) + generated = c.ExecResult(columns=["customer", "o.total"], rows=[(n, v + 1) for n, v in _TEN]) + score = c.compare_result_sets(golden, generated, match="values", ordered=False) + assert score.column_pairs == (("customer", "customer"), ("total", "o.total")) + assert score.column_agreement == (1.0, 0.0) + assert score.accuracy == 0.0 and score.reason.startswith("0 of the answer key's 10 rows matched") + + +def test_the_caller_can_ask_for_the_rows_as_a_set_whatever_the_statement_ordered(): + golden = c.ExecResult(columns=["channel"], rows=[("web",), ("store",)]) + generated = c.ExecResult(columns=["channel"], rows=[("store",), ("web",)]) + ordered_sql = "SELECT channel FROM t ORDER BY channel" + score = c.compare_result_sets(golden, generated, golden_sql=ordered_sql, ordered=False) + assert score.accuracy == 1.0 and score.order_sensitive is False + assert score.notes == ("row order was not compared",) + # The statement still decides when the caller says nothing. + assert c.compare_result_sets(golden, generated, golden_sql=ordered_sql).accuracy == 0.0 + + +def test_nothing_pairs_when_the_row_counts_differ_and_the_share_is_absent(): + golden = c.ExecResult(columns=["customer"], rows=_TEN) + generated = c.ExecResult(columns=["customer"], rows=_TEN[:9]) + score = c.compare_result_sets(golden, generated, match="values", ordered=False) + assert score.column_pairs == () and score.column_agreement == () and score.paired_row_share is None diff --git a/tests/test_comparator_column_pairs.py b/tests/test_comparator_column_pairs.py index eef6932f..12547f00 100644 --- a/tests/test_comparator_column_pairs.py +++ b/tests/test_comparator_column_pairs.py @@ -15,10 +15,12 @@ def test_renamed_and_extra_columns_are_reported_as_pairs_and_extras(): score = compare_result_sets(golden, generated, match="values") assert score.accuracy == 0.0 and score.unmatched_golden_columns == ("channel",) assert score.column_pairs == (("number", "o.number"), ("status", "o.status")) and score.unmatched_generated_columns == ("region",) + assert score.column_agreement == (1.0, 1.0) and score.paired_row_share == 1.0 generated2 = ExecResult(columns=["o.number", "o.status", "o.channel", "region"], rows=[("A1", "paid", "web", "EU"), ("A2", "open", "shop", "US")]) score2 = compare_result_sets(golden, generated2, match="values") assert score2.accuracy == 1.0 and score2.column_pairs == (("number", "o.number"), ("status", "o.status"), ("channel", "o.channel")) assert score2.unmatched_generated_columns == ("region",) and score2.unmatched_golden_columns == () + assert score2.column_agreement == (1.0, 1.0, 1.0) and score2.paired_row_share == 1.0 def test_a_scalar_pair_serialises_and_other_levels_report_nothing(): @@ -34,4 +36,5 @@ def test_different_row_counts_report_no_pairs_and_no_extras(): generated = ExecResult(columns=["department", "pending_items"], rows=[("a", 1), ("b", 2), ("c", 3)]) score = compare_result_sets(golden, generated, match="values") assert score.accuracy == 0.0 and score.column_pairs == () and score.unmatched_generated_columns == () and score.unmatched_golden_columns == () + assert score.column_agreement == () and score.paired_row_share is None diff --git a/tests/test_golden_claims.py b/tests/test_golden_claims.py index 6b23cc05..b3b06ad5 100644 --- a/tests/test_golden_claims.py +++ b/tests/test_golden_claims.py @@ -389,8 +389,8 @@ def _parsed_or_none(text: str, engine: str): # The statement a golden item would carry: a filtered, windowed, joined, grouped, ordered, capped -# aggregate over the demo shop. Every one of the seven claims is present in it, which is what lets -# the rewrite below assert that all seven AGREE rather than that none of them differs. +# aggregate over the demo shop. Every one of the eight claims is present in it, which is what lets +# the rewrite below assert that all eight AGREE rather than that none of them differs. GOLDEN_SHAPE = ( "SELECT o.region, SUM(o.amount) AS revenue " "FROM orders o JOIN customers c ON o.customer_id = c.id " @@ -401,15 +401,18 @@ def _parsed_or_none(text: str, engine: str): @pytest.mark.parametrize("engine", ENGINES) class TestComparingTwoStatements: - """Seven claims out, and exactly two of them allowed to decide anything.""" + """Eight claims out, and exactly two of them allowed to decide anything.""" - def test_the_claim_set_is_exactly_seven_claims(self, engine): - """Seven is the contract, not an implementation detail: an eighth claim changes what a - golden item is allowed to assert about a statement.""" + def test_the_claim_set_is_exactly_eight_claims(self, engine): + """Eight is the contract, not an implementation detail: a new claim changes what a golden + item is allowed to assert about a statement. The eighth, `outputs`, was added on purpose + (ACE-131) so two statements can be called the same query only when they select the same + expressions; it reports and never gates.""" diff = _diff(GOLDEN_SHAPE, GOLDEN_SHAPE, engine) assert gc.CLAIM_NAMES == ( "tables", + "outputs", "filter_predicates", "date_window", "group_keys", @@ -417,9 +420,33 @@ def test_the_claim_set_is_exactly_seven_claims(self, engine): "ordering", "limit", ) - assert len(diff.claims) == 7 + assert len(diff.claims) == 8 assert tuple(claim.name for claim in diff.claims) == gc.CLAIM_NAMES + def test_the_outputs_claim_reads_what_is_selected_with_the_aliases_peeled(self, engine): + a = "SELECT o.region, SUM(o.amount) AS revenue FROM orders o GROUP BY o.region" + b = "SELECT SUM(orders.amount) AS total_revenue, orders.region FROM orders GROUP BY orders.region" + claim = _claim(_diff(a, b, engine), "outputs") + assert claim.status == gc.AGREES + assert claim.generated == claim.golden == ["orders.region", "sum(orders.amount)"] + + def test_the_outputs_claim_differs_on_a_different_expression(self, engine): + a = "SELECT region, SUM(amount) AS revenue FROM orders GROUP BY region" + b = "SELECT region, AVG(amount) AS revenue FROM orders GROUP BY region" + assert _claim(_diff(a, b, engine), "outputs").status == gc.DIFFERS + + def test_a_bare_column_in_a_single_table_statement_is_its_tables_column_in_the_outputs_claim(self, engine): + claim = _claim(_diff("SELECT o.total FROM orders o", "SELECT total FROM orders", engine), "outputs") + assert claim.status == gc.AGREES and claim.golden == ["orders.total"] + + def test_select_star_differs_from_a_column_list_in_the_outputs_claim(self, engine): + claim = _claim(_diff("SELECT * FROM orders", "SELECT region FROM orders", engine), "outputs") + assert claim.status == gc.DIFFERS and claim.generated == ["*"] + + def test_the_outputs_claim_never_gates(self, engine): + diff = _diff("SELECT region FROM orders", "SELECT SUM(amount) FROM orders", engine) + assert _claim(diff, "outputs").status == gc.DIFFERS and not diff.gated + def test_an_aliased_reordered_rewrite_agrees_on_every_claim(self, engine): """The property the whole module rests on: two spellings of one question produce identical claims. If this ever reports a difference, every difference the module reports is @@ -433,7 +460,7 @@ def test_an_aliased_reordered_rewrite_agrees_on_every_claim(self, engine): ) diff = _diff(rewritten, GOLDEN_SHAPE, engine) - assert [claim.status for claim in diff.claims] == [gc.AGREES] * 7 + assert [claim.status for claim in diff.claims] == [gc.AGREES] * 8 assert diff.gates == [] assert diff.gated is False @@ -614,7 +641,7 @@ def test_an_unparseable_statement_reports_unknown_rather_than_raising(self, engi the same reason.""" diff = _diff("SELECT FROM WHERE ,", GOLDEN_SHAPE, engine, must_filter=["region"]) - assert [claim.status for claim in diff.claims] == [gc.UNKNOWN] * 7 + assert [claim.status for claim in diff.claims] == [gc.UNKNOWN] * 8 assert diff.gates == [] assert diff.gated is False diff --git a/tests/test_golden_run.py b/tests/test_golden_run.py index 30bf8d19..a5c99d44 100644 --- a/tests/test_golden_run.py +++ b/tests/test_golden_run.py @@ -389,14 +389,15 @@ def test_a_required_filter_left_out_turns_a_would_be_pass_into_a_fail(chokepoint assert result.failed == 1 and result.gating_failures == 1 -def test_a_scored_item_carries_the_seven_claims(chokepoint): +def test_a_scored_item_carries_the_eight_claims(chokepoint): """A failing item's whole value is the sentence after 'the rows disagree', so the diff rides on every item that had two statements to read.""" result = _run(_dataset(_item()), _StubGenerator(), _SpyExecutor()) claims = result.outcomes[0].claims assert [claim["name"] for claim in claims["claims"]] == [ - "tables", "filter_predicates", "date_window", "group_keys", "join_keys", "ordering", "limit", + "tables", "outputs", "filter_predicates", "date_window", "group_keys", "join_keys", "ordering", + "limit", ] assert claims["gated"] is False and result.outcomes[0].passed diff --git a/tests/test_render_golden_datasets.py b/tests/test_render_golden_datasets.py index 477fef20..b3c2cb91 100644 --- a/tests/test_render_golden_datasets.py +++ b/tests/test_render_golden_datasets.py @@ -569,7 +569,7 @@ def test_the_dialect_comes_from_the_model_rather_than_a_default(artifacts): def test_metrics_are_reported_apart_from_the_tables(artifacts): - """A metric is not one of the seven claims a statement is read into, so it is matched by name + """A metric is not one of the eight claims a statement is read into, so it is matched by name against the statement text — weaker evidence than a table claim, kept under its own key so the tab cannot present the two as the same thing.""" coverage = _payload(_rendered(artifacts))["coverage"] diff --git a/tests/test_render_golden_run.py b/tests/test_render_golden_run.py index 207df665..9691417a 100644 --- a/tests/test_render_golden_run.py +++ b/tests/test_render_golden_run.py @@ -57,7 +57,7 @@ def _claims(generated: list[str], golden: list[str], status: str = "differs") -> dict[str, Any]: - """A statement difference in the shape the run writes it — seven claims, tables first.""" + """A statement difference in the shape the run writes it — the claims, tables first.""" rest = [ {"name": name, "status": "agrees", "generated": None, "golden": None} for name in ( diff --git a/tests/test_semantic_model_cli_reconcile_verbs.py b/tests/test_semantic_model_cli_reconcile_verbs.py index b05d6b64..8103fd61 100644 --- a/tests/test_semantic_model_cli_reconcile_verbs.py +++ b/tests/test_semantic_model_cli_reconcile_verbs.py @@ -4,7 +4,7 @@ skill decides meaning and talks to the person; everything deterministic about the statement lives in the CLI, and these are those parts: -* `claims` - where two statements differ, in the seven claims `golden_claims` already reads +* `claims` - where two statements differ, in the eight claims `golden_claims` already reads * `compare-results` - whether two result sets say the same thing, through the golden comparator * `join-probes` - for every join the statement wrote: the receipt's own status for it, whether the written key matches the declared one, and the probe SQL that would show whether @@ -178,6 +178,23 @@ def test_compare_results_scores_one_when_the_two_tables_say_the_same_thing(tmp_p assert d["order_sensitive"] is False +def test_compare_results_unordered_compares_the_rows_as_a_set_whatever_the_statement_ordered(tmp_path): + """Reconcile never compares row order: the ordering claim says whether the two statements sort + the same way. `--unordered` says so to the comparator, whatever ORDER BY the statement wrote.""" + _model(tmp_path) + g = _write(tmp_path, "g.csv", "region,total\nEU,4.2\nUS,10\n") + a = _write(tmp_path, "a.csv", "region,total\nUS,10\nEU,4.2\n") + s = _write(tmp_path, "s.sql", "SELECT region, SUM(total) AS total FROM orders GROUP BY region ORDER BY total DESC") + rc, out = _run(["compare-results", str(tmp_path), "--golden-csv", g, "--generated-csv", a, + "--match", "values", "--golden-sql-file", s]) + assert rc == 0 and json.loads(out)["accuracy"] == 0.0 + rc, out = _run(["compare-results", str(tmp_path), "--golden-csv", g, "--generated-csv", a, + "--match", "values", "--golden-sql-file", s, "--unordered"]) + d = json.loads(out) + assert rc == 0 and d["accuracy"] == 1.0 and d["order_sensitive"] is False + assert d["notes"] == ["row order was not compared"] + + def test_compare_results_defaults_to_the_comparators_own_level_and_scores_zero_on_a_difference(tmp_path): _model(tmp_path) g = _write(tmp_path, "g.csv", "total\n10\n")