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")