From 80e20defc5ab2656a7cf69b8f6a956969a0ab180 Mon Sep 17 00:00:00 2001 From: Sandeep Date: Sun, 13 Sep 2026 19:51:15 -0700 Subject: [PATCH] reconcile: the card reads agreement, says could not compare, names the failed side; 2e unordered One differing cell printed "0% of the values match" for a column that was there; the values row now reads the comparator's share ("9 of 10 rows match", "differs in total on 1 of 10 rows"), and "same rows, different columns" needs the paired columns to agree on every row. Every error row said "agami's query failed" although the cause was often the person's statement, two empty results or a bare question; the card reads the cause from the row's files, offers agami again only when agami failed, and sends a question-only row to the grading page. The result pill reads "could not compare", or "same query, answer not compared" / "different query, answer not compared" when the claims could be read, the `outputs` claim ("selects") included, so query equivalence stands on its own when the data cannot be compared. The near-miss sentence shows only on a defect. Phase 2e passes `--unordered`: a different sort is a different query, never a different answer. Pins moved on purpose: "identical, row for row" reads "identical"; the error sentence carries its cause; "could not run" reads "could not compare"; the 2e pin names --unordered. Spec: ACE-134 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 13 ++ plugins/agami/scripts/reconcile.py | 120 +++++++++++++++--- plugins/agami/shared/part-ledger.md | 2 +- .../shared/reconcile-report-template.html | 3 +- plugins/agami/skills/agami-reconcile/SKILL.md | 6 +- tests/test_reconcile_learning_loop_skill.py | 3 +- tests/test_reconcile_report_items.py | 94 +++++++++++++- tests/test_reconcile_result_and_fix.py | 12 +- 8 files changed, 228 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 694e1177..a7191c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,6 +172,19 @@ below corresponds to one such version. ### Fixed +- **The reconcile card reads how many rows agree, names the side that failed, and still says what + the two queries are when the data could not be compared.** One differing cell used to print "0% of + the values match" for a column that was there; the values row now reads the comparator's share + ("9 of 10 rows match, differs in total on 1 of 10 rows"), and "same rows, different columns" needs + the paired columns to agree on every row. Every error row said "agami's query failed" although the + cause was often the person's statement, two empty results or a bare question; the card now reads + the cause from the row's files ("This row could not be compared: your query did not run: …"), + offers agami again only when agami failed, and sends a question-only row to the grading page. The + result pill reads "could not compare", or "same query, answer not compared" and "different query, + answer not compared" when the claims could be read, the new `outputs` claim ("selects") included. + Phase 2e passes `--unordered`, so a different sort is a different query and never a different + answer. (ACE-134) + - **A join on the key the model declares one-to-one is no longer reported as a fan trap because a second edge exists between the same two tables.** The fan and chasm pre-flight matched a declared edge to a join by table pair and never read the columns the join wrote, so a subclass view joined diff --git a/plugins/agami/scripts/reconcile.py b/plugins/agami/scripts/reconcile.py index aa6f62e0..4decd1fd 100644 --- a/plugins/agami/scripts/reconcile.py +++ b/plugins/agami/scripts/reconcile.py @@ -1484,7 +1484,7 @@ def next_chunk(run_dir: Path, size: int = CHUNK_SIZE) -> dict: _STATE = {CONFIRMED: "held", QUERY_DEFECT: "defect", MODEL_GAP: "gap", UNRESOLVED: "open", NOTED: "noted"} _STATE_WORDS = {"held": "passed", "defect": "a mistake in your query", "gap": "a gap in the semantic model", "open": "could not check", "noted": "noticed", "differs": "the two queries differ"} -_CLAIM_KEYS = {"tables": "tables read", "filter_predicates": "filters", "date_window": "date window", +_CLAIM_KEYS = {"tables": "tables read", "outputs": "selects", "filter_predicates": "filters", "date_window": "date window", "group_keys": "grouped by", "join_keys": "join keys", "ordering": "ordered by", "limit": "limit"} # The words a ledger part's grade takes on the page, by part family and grade. Every cell on the # page comes from this table, the run's files, or the receipt; none is written by hand. @@ -1742,13 +1742,33 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N if renamed: rows[-1]["renamed"] = [[a, b] for a, b in pairs if a != b] acc = result_set.get("accuracy") + share = result_set.get("paired_row_share") + agreement = list(result_set.get("column_agreement") or []) + n_rows = result_set.get("golden_row_count") if acc is not None: same = float(acc) >= 1.0 + paired_word = f"the {len(pairs)} paired column{'s' if len(pairs) != 1 else ''}" if same: - add("values", "held", "identical, row for row", None) + add("values", "held", "identical", None) + elif pairs and share is not None: + # The comparator says how many rows agree over the paired columns, and which pair + # disagrees on how many rows; the card reads those numbers rather than a 0. + if float(share) >= 1.0: + add("values", "held", f"identical on {paired_word}", None, + note="a column of yours has no partner; the paired columns match") + else: + if isinstance(n_rows, int) and n_rows > 0: + agree = round(float(share) * n_rows) + text = f"{agree} of {n_rows} rows match" + weak = [f"{a} on {n_rows - round(g * n_rows)} of {n_rows} rows" + for (a, _b), g in zip(pairs, agreement) if isinstance(g, (int, float)) and g < 1.0] + else: + text, weak = f"{float(share):.0%} of the rows match", [] + add("values", "defect", text, None, note=("differs in " + ", ".join(weak)) if weak else None) elif pairs and result_set.get("unmatched_golden_columns"): - add("values", "held", f"identical on the {len(pairs)} paired column{'s' if len(pairs) != 1 else ''}", None, - note="the score is 0 only because a column of yours has no partner; the paired columns match") + # An older score file without the share: every pair it reports agreed by construction. + add("values", "held", f"identical on {paired_word}", None, + note="a column of yours has no partner; the paired columns match") else: add("values", "defect", f"{float(acc):.0%} of the values match", None, note=result_set.get("reason")) else: @@ -1824,7 +1844,7 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N yours = f"{_fmt(ev.get('dropped'))} of {_fmt(ev.get('total'))} {ev.get('left')} rows" elif fam == "question_fit" and ev.get("fit"): yours = {"plausible": "yes", "doubtful": "doubtful", "no_question": "no question given"}.get(ev["fit"], ev["fit"]) - elif fam == "literal" and ev.get("near_miss"): + elif fam == "literal" and ev.get("near_miss") and state == "defect": yours = f"matches no rows; the data spells it {ev['near_miss']}" for mention in ev.get("prose") or []: if isinstance(mention, dict) and mention.get("text"): @@ -1833,7 +1853,7 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N return rows, words -_DEFINITIONAL = {"tables read", "filters", "date window", "join keys", "grouped by"} +_DEFINITIONAL = {"tables read", "selects", "filters", "date window", "join keys", "grouped by"} _RESULT_LABEL = { ("matches", "same"): "match", ("matches", "different"): "same answer, different query", ("matches", "not_comparable"): "match", @@ -1847,6 +1867,41 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N _FIX_OWNER = {"query": "you", "semantic_model": "model", "examples": "agami", "question": "question", "ask_again": "agami", "none": "nothing"} +_ERROR_LEAD = { + "agami_failed": "agami's query failed", + "yours_failed": "your query did not run", + "nothing_to_compare": "both queries returned no rows, so there is nothing to compare", + "no_ground_truth": "there is nothing to compare against; agami's answer is graded on the grading page", + "unknown": "the run's files do not say why", +} + + +def _first_line(text: Any) -> str: + """The first line of an error, the only line a card shows.""" + return str(text).strip().splitlines()[0].strip() if text else "" + + +def _error_cause(rec: dict) -> str | None: + """Why an `error` row could not be compared, read from the row's files and never assumed: the + person's statement did not run (or was refused), agami wrote no statement or its run failed, + both results were empty, the row has nothing to compare against, or the files do not say.""" + if (rec.get("status") or "error") != "error": + return None + parts = ((rec.get("ledger") or {}).get("rows") or []) if isinstance(rec.get("ledger"), dict) else [] + by_part = {p.get("part"): p for p in parts} + for part in ("runs", "scope"): + if part in by_part and by_part[part].get("verdict") != CONFIRMED: + return "yours_failed" + if not rec.get("sql") or (rec.get("recorded") is None and rec.get("actual") is None and rec.get("error")): + return "agami_failed" + score = (rec.get("comparison") or {}).get("result_set") if isinstance(rec.get("comparison"), dict) else None + if score and score.get("status") == "unscored": + return "nothing_to_compare" + if not rec.get("statement") and rec.get("expected") is None: + return "no_ground_truth" + return "unknown" + + def _result(rec: dict, diff: list[dict]) -> dict: """The result in two facts read by code: whether the data matches, and whether the two queries are the same. `label` is the plain-word pill; `unchecked` counts the checks on your query that could @@ -1879,7 +1934,7 @@ def _result(rec: dict, diff: list[dict]) -> dict: if by_name.get(name) == "unknown" and part_verdicts.get(part) == CONFIRMED: by_name[name] = "agrees" statuses = list(by_name.values()) - definitional_unknown = any(by_name.get(n) == "unknown" for n in ("tables", "filter_predicates", "date_window", "join_keys", "group_keys")) + definitional_unknown = any(by_name.get(n) == "unknown" for n in ("tables", "outputs", "filter_predicates", "date_window", "join_keys", "group_keys")) if (not claims or all(st == "unknown" for st in statuses)) and not columns_differ: query = "not_comparable" elif any(st == "differs" for st in statuses) or columns_differ: @@ -1894,7 +1949,11 @@ def _result(rec: dict, diff: list[dict]) -> dict: # A check counts once: a claim the ledger carries as a part is counted as that part. graded_claims = {"date_window", "filter_predicates"} if part_verdicts else set() unchecked = sum(1 for p in parts if p.get("verdict") == UNRESOLVED) + sum(1 for n, st in by_name.items() if st == "unknown" and n not in graded_claims) - label = "could not run" if data == "could_not_compare" else _RESULT_LABEL[(data, query)] + if data == "could_not_compare": + # The data could not be compared; the two statements still say what they are. + label = {"same": "same query, answer not compared", "different": "different query, answer not compared"}.get(query, "could not compare") + else: + label = _RESULT_LABEL[(data, query)] differing = sorted(r["key"] for r in diff if (r["state"] in ("defect", "differs") or (r["key"] == "columns" and r["state"] == "noted")) and r["key"] in _DEFINITIONAL | {"ordered by", "limit", "columns"}) @@ -1913,6 +1972,11 @@ def _values_agree_on_shared_columns(rec: dict) -> bool: if acc is not None and float(acc) >= 1.0: return True # agami returned everything you did, and more pairs = score.get("column_pairs") or [] + share = score.get("paired_row_share") + if pairs and share is not None: + # The comparator says whether the paired columns agree on every row; before it did, every + # pair it reported agreed by construction. + return float(share) >= 1.0 and bool(score.get("unmatched_golden_columns") or score.get("unmatched_generated_columns")) if pairs: return bool(score.get("unmatched_golden_columns") or score.get("unmatched_generated_columns")) # an older score file without pairs: fall back to the matched share of the columns @@ -1932,6 +1996,11 @@ def _fix(rec: dict, diff: list[dict], result: dict) -> str: fit = next((p for p in parts if p.get("part") == "question_fit"), None) cols = next((r for r in diff if r["key"] == "columns"), None) if (rec.get("status") or "error") == "error": + cause = _error_cause(rec) + if cause == "yours_failed": + return "semantic_model" if any(p.get("verdict") == MODEL_GAP for p in parts) else "query" + if cause in ("nothing_to_compare", "no_ground_truth"): + return "none" return "ask_again" if rec.get("status") == "expected_doubtful" or any(p.get("verdict") == QUERY_DEFECT for p in parts): return "query" @@ -1971,7 +2040,12 @@ def _change_for_fix(fix: str, rec: dict, diff: list[dict]) -> tuple[list[str], l extra = list((cols or {}).get("yours_hi") or []) mistakes = _measured_mistakes(rec) prefill = {"change": "", "fix": "", "reword": rec.get("question") or "", "example": ""} - if fix == "query": + cause = _error_cause(rec) + if fix == "query" and cause == "yours_failed": + error = _first_line(rec.get("error")) + change = [f"Your query did not run{': ' + error if error else ''}. Fix it, then run this row again."] + todo = ["Your query: fix it so it runs, then re-run."] + elif fix == "query": if mistakes: change = [f"Fix your query: {', '.join(mistakes)}. Then run this row again."] prefill["fix"] = "; ".join(mistakes) @@ -1994,10 +2068,19 @@ def _change_for_fix(fix: str, rec: dict, diff: list[dict]) -> tuple[list[str], l change = [("Reword the question, or change your query, so they ask the same thing. " + reason).strip()] todo = ["The question: reword it and re-run."] elif fix == "ask_again": - if rec.get("status") == "error": + if cause == "agami_failed": change, todo = list(_OWNER_CHANGE["agami"][0]), list(_OWNER_CHANGE["agami"][1]) + elif cause == "unknown": + change = ["Run this row again; it could not be compared and the run's files do not say why."] + todo = ["Run the row again."] else: change, todo = list(_FIX_CHANGE["ask_again"][0]), list(_FIX_CHANGE["ask_again"][1]) + elif fix == "none" and cause == "nothing_to_compare": + change = ["Both queries returned no rows, so there is nothing to compare. Widen the date window or the filters in your query, then run this row again."] + todo = ["Your query: widen the window or the filters, then re-run."] + elif fix == "none" and cause == "no_ground_truth": + change = ["Grade agami's answer on the grading page; there is nothing to compare it against."] + todo = ["Grade the answer on the grading page."] else: change, todo = list(_OWNER_CHANGE["keep"][0]), list(_OWNER_CHANGE["keep"][1]) if fit and fit.get("verdict") != CONFIRMED and fit.get("note") and fix != "question": @@ -2022,14 +2105,17 @@ def _owner(rec: dict, diff: list[dict]) -> str: fit_ok = (fit is not None and fit.get("verdict") == CONFIRMED) if needs_fit else (fit is None or fit.get("verdict") == CONFIRMED) return "keep" if one_cell and fit_ok else "nothing" if status == "error": - return "agami" + cause = _error_cause(rec) + if cause == "yours_failed": + return "model" if any(p.get("verdict") == MODEL_GAP for p in parts) else "you" + return "nothing" if cause in ("nothing_to_compare", "no_ground_truth") else "agami" if status == "expected_doubtful" or any(p.get("verdict") == QUERY_DEFECT for p in parts): return "you" if any(p.get("verdict") == MODEL_GAP for p in parts): return "model" cols = next((r for r in diff if r["key"] == "columns"), None) extra_yours = bool(cols and cols.get("yours_hi")) - if (extra_yours or differing & {"ordered by", "limit"}) and not (differing & {"tables read", "filters", "join keys", "grouped by", "values", "rows"}): + if (extra_yours or differing & {"ordered by", "limit"}) and not (differing & {"tables read", "selects", "filters", "join keys", "grouped by", "values", "rows"}): # The two answers hold the same rows and differ in what the person's query returns or how # it orders them: that is the query to change, not a definition and not the question. return "you" @@ -2097,7 +2183,10 @@ def _sentence(rec: dict, diff: list[dict]) -> str: if status == "expected_doubtful": return f"Your query has a mistake ({', '.join(mistakes or red) or 'see the red rows'}), so the number you expected is doubtful." if status == "error": - return f"agami's query failed{': ' + rec['error'] if rec.get('error') else ''}." + cause = _error_cause(rec) or "unknown" + error = _first_line(rec.get("error")) + with_error = cause in ("agami_failed", "yours_failed", "unknown") and error + return f"This row could not be compared: {_ERROR_LEAD[cause]}{': ' + error if with_error else ''}." where = red + gaps return f"The two answers do not match. What differs: {', '.join(where)}." if where else "The two answers do not match, and no check explains why." @@ -2150,8 +2239,9 @@ def report_items(run_dir: Path) -> list[dict]: keep_ok = legacy_owner == "keep" owner = "keep" if (keep_ok and fix in ("none", "examples")) else _FIX_OWNER[fix] change, todo, prefill = _change_for_fix(fix, rec, diff) - if fix == "none" and not keep_ok: - # nothing to fix, and not kept either: say why in the words the status gives + if fix == "none" and not keep_ok and (rec.get("status") or "error") != "error": + # nothing to fix, and not kept either: say why in the words the status gives. An error + # row with nothing to fix already says its cause (nothing to compare, or no ground truth). change, todo = _change("nothing", rec, diff) if result["data"] == "matches" and result["query"] == "different": clause = ("The two queries differ in: " + ", ".join(result["differs_in"]) + ("; the match may not hold on other data." if set(result["differs_in"]) & _DEFINITIONAL else "; a cosmetic difference.")) diff --git a/plugins/agami/shared/part-ledger.md b/plugins/agami/shared/part-ledger.md index f96e2b5e..1ed406b3 100644 --- a/plugins/agami/shared/part-ledger.md +++ b/plugins/agami/shared/part-ledger.md @@ -35,7 +35,7 @@ downstream: a join that could not be graded leaves the fan-out check on its aggr | `values_declared:.` | `filter-values.judge.json` `columns` | one per filtered column. `populated` → confirmed; `absent` or `empty` with the distinct probe `listed` (under 26 values) → model_gap of kind `description`, the same finding family as a stale list; `overflow` → noted, no list is expected of a wide column; `empty` → noted; `failed` or `not_run` → unresolved; a sensitive column → noted | | `dropped_rows:-` | `.dropped_rows.csv` | noted, never a grade: ` of rows have no partner`, counted over the whole table before the statement's own filters, and naming the other side as not counted (an inner join drops from both); a probe planned but not run → noted, nothing claimed; no probe planned → no part | | `question_fit` | `question_fit.json`, the skill's Phase 1.5g reading of whether the statement answers its question | `plausible` → confirmed, by reading, and the note says so; `doubtful` → unresolved with the reason, so the row grades `match_unverified` at best and never reaches the keep-offer; `no_question` → no part, and only for a statement that came with no question: against a row that carries one it is a contradiction, and the findings verb and the report page refuse to keep such a row; absent after a run that succeeded → unresolved, the fit was not checked. The one part graded by judgment: it can withhold a row and never proves anything about the semantic model | -| `predicates`, `date_window` | `claims.json`, only with `--with-claims` | `agrees` → confirmed; `differs` → noted (kind `different_query`, both sides named; a fact about the pair that never grades yours, shown on the report page as "same answer, different query"); `unknown` → unresolved, except a `date_window` that is `null` on both sides when `unreadable` says both statements parsed and `temporal_predicates` is zero on both sides, which is confirmed (neither writes a date filter, so there is nothing to disagree about); a count above zero is a window written in a shape the reader does not fold, and stays open. A difference is reported, never judged here | +| `predicates`, `date_window` | `claims.json`, only with `--with-claims` | `agrees` → confirmed; `differs` → noted (kind `different_query`, both sides named; a fact about the pair that never grades yours, shown on the report page as "same answer, different query"); `unknown` → unresolved, except a `date_window` that is `null` on both sides when `unreadable` says both statements parsed and `temporal_predicates` is zero on both sides, which is confirmed (neither writes a date filter, so there is nothing to disagree about); a count above zero is a window written in a shape the reader does not fold, and stays open. A difference is reported, never judged here. The other six claims (tables, what is selected, group keys, join keys, ordering, limit) are not parts: the report page reads them from `claims.json` as rows of the diff grid, and a difference in any of them makes the two statements a different query. The `ordering` claim is the one place row order is judged; `sm compare-results --unordered` never compares it | **The verdict is the weakest part:** `query_defect` outranks `unresolved`, which outranks `model_gap`, which outranks `confirmed`. The counts travel with it so a reader sees what else was there. diff --git a/plugins/agami/shared/reconcile-report-template.html b/plugins/agami/shared/reconcile-report-template.html index db878068..ddd3118c 100644 --- a/plugins/agami/shared/reconcile-report-template.html +++ b/plugins/agami/shared/reconcile-report-template.html @@ -169,7 +169,8 @@

Send to Claude

// the one filter row, and the older status chips step aside. const labels = {}; const RESULT_CLASS = { 'match': 'held', 'same answer, different query': 'open', 'same rows, different columns': 'open', - 'different answer': 'defect', 'same query, different answer': 'defect', 'could not run': 'noted' }; + 'different answer': 'defect', 'same query, different answer': 'defect', 'could not compare': 'noted', + 'same query, answer not compared': 'open', 'different query, answer not compared': 'defect' }; DATA.items.forEach(i => { if (i.result && i.result.label) labels[i.result.label] = (labels[i.result.label] || 0) + (passes(i, 'result') ? 1 : 0); }); const results = document.getElementById('result-chips'); results.classList.toggle('filtering', view.result.size > 0); diff --git a/plugins/agami/skills/agami-reconcile/SKILL.md b/plugins/agami/skills/agami-reconcile/SKILL.md index 1ad544f2..54292d75 100644 --- a/plugins/agami/skills/agami-reconcile/SKILL.md +++ b/plugins/agami/skills/agami-reconcile/SKILL.md @@ -295,10 +295,10 @@ For a row whose `expected` is one number, Phase 2c's diff is the comparison. For ```bash bash "$AGAMI_PLUGIN_ROOT/scripts/sm" compare-results "$ROOT" \ --golden-csv rows//statement.csv --generated-csv rows//actual.csv \ - --match values --golden-sql-file rows//statement.sql > rows//comparison.json + --match values --unordered > rows//comparison.json ``` -`accuracy` of `1.0` is a match. The person's statement is handed over as `--golden-sql-file` for one reason only: whether it ordered its rows. +`accuracy` of `1.0` is a match. Row order is never part of this comparison (`--unordered`): the `ordering` claim below says whether the two statements sort the same way, and a different sort is a different query, not a different answer. The score also says how far the two tables agree when they do not match: `paired_row_share` and `column_agreement` beside `column_pairs`, which the report page reads as "9 of 10 rows match". When the row carries both statements, name where they differ before anyone reads two receipts side by side: @@ -309,7 +309,7 @@ python3 "$AGAMI_PLUGIN_ROOT/scripts/reconcile.py" ledger --row-dir rows/ --wi **This is the row's one ledger run.** Every file Phase 1.5 wrote is still there, so the grades are the same ones 1.5 would have produced, plus the two claim parts. When agami's own run failed and there is no statement to compare against, run it here without `--with-claims`. Never run it twice. -The seven claims (tables, filter predicates, date window, group keys, join keys, ordering, limit) say which part differs; they never say who is right. Then set the row's `status` with `reconcile.py status`, from the diff's `match` and the ledger's verdict. For a table there is no `diff`: pass `--match true` when `compare-results` reports `accuracy` of `1.0`, `false` otherwise, and `none` when it could not score. +The eight claims (tables, what is selected, filter predicates, date window, group keys, join keys, ordering, limit) say which part differs; they never say who is right. Two statements are the same query only when every claim that could be read agrees, what they select included; when the data could not be compared, the page still says "same query, answer not compared" or "different query, answer not compared". Then set the row's `status` with `reconcile.py status`, from the diff's `match` and the ledger's verdict. For a table there is no `diff`: pass `--match true` when `compare-results` reports `accuracy` of `1.0`, `false` otherwise, and `none` when it could not score. ### 2f — Write the findings diff --git a/tests/test_reconcile_learning_loop_skill.py b/tests/test_reconcile_learning_loop_skill.py index 4049296a..4bc0c544 100644 --- a/tests/test_reconcile_learning_loop_skill.py +++ b/tests/test_reconcile_learning_loop_skill.py @@ -135,7 +135,8 @@ def test_compare_and_findings_sit_between_the_record_and_present(): assert (SKILL.index("### 2d — Build the row record") < SKILL.index("### 2e — Compare") < SKILL.index("### 2f — Write the findings") < SKILL.index("## Phase 3: Present")) compare = _between(SKILL, "### 2e — Compare", "### 2f") - assert "compare-results" in compare and "--golden-sql-file" in compare + assert "compare-results" in compare and "--unordered" in compare and "--golden-sql-file" not in compare + assert "Row order is never part of this comparison" in compare and "eight claims" in compare assert 'sm" claims' in compare and "--with-claims" in compare assert "they never say who is right" in compare findings = _between(SKILL, "### 2f — Write the findings", "## Phase 3: Present") diff --git a/tests/test_reconcile_report_items.py b/tests/test_reconcile_report_items.py index c9f43cdd..f645bce5 100644 --- a/tests/test_reconcile_report_items.py +++ b/tests/test_reconcile_report_items.py @@ -79,14 +79,14 @@ def test_a_table_that_differs_in_columns_names_the_extra_columns_and_blames_the_ rows = {r["key"]: r for r in item["diff"]} assert rows["rows"]["state"] == "held" and rows["rows"]["yours"] == "21 rows" and rows["rows"]["agami"] == "21 rows" assert rows["columns"]["state"] == "defect" and rows["columns"]["yours_hi"] == ["planned_ship_date", "delivered_at", "channel"] and rows["columns"].get("agami_hi") in (None, []) - assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical, row for row" + assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical" assert rows["date window"]["state"] == "open" and rows["date window"]["agami"] == "placed_at ≥ 2025-06-01" and rows["date window"]["yours"] == "could not read" assert rows["date window"]["note"] == "a shape the claims reader does not fold" assert rows["caveats read"]["state"] == "noted" and item["words"] == ['orders: "shipped_at is the time anchor for order reporting."'] # The same rows come back; only the columns the person's query returns differ: the query is what to change. assert item["owner"] == "you" and item["single_cell"] is False and item["expected"] == "21 rows" assert item["change"] == ["Your query returns columns the question did not ask for: planned_ship_date, delivered_at, channel. Remove them, or name them in the question."] - assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical, row for row" + assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical" assert item["sentence"].startswith("The two answers do not match. What differs: columns") @@ -290,7 +290,7 @@ def test_the_first_reviews_findings(tmp_path): claim_rows = [r for r in items[3]["diff"] if r["key"] in ("claims", "tables read", "filters", "limit")] assert [r["key"] for r in claim_rows] == ["claims"] and claim_rows[0]["state"] == "open" assert "limit" not in items[3]["sentence"] and "claims" in items[3]["sentence"] - assert items[4]["owner"] == "agami" and items[4]["sentence"] == "agami's query failed: boom." and items[4]["status"] == "error" + assert items[4]["owner"] == "agami" and items[4]["sentence"] == "This row could not be compared: agami's query failed: boom." and items[4]["status"] == "error" assert len(items) == 5 and items[5]["status"] == "match" and items[5]["owner"] == "keep" # 2 again, the other way: a difference in values beside extra columns of yours is not just the query both = dict(TABLE_DIFF, row=6, comparison={"result_set": {"accuracy": 0.4, "reason": "values differ", "unmatched_golden_columns": [], "golden_row_count": 21, "generated_row_count": 21}}) @@ -335,3 +335,91 @@ def test_keep_allowed_on_the_items_equals_the_parsers_keep_gate(tmp_path): keepable = pr.keepable_rows(run) assert {i["row"]: i["keep_allowed"] for i in items} == {r: (r in keepable) for r in (1, 2, 3)} == {1: True, 2: False, 3: True} + + +# --- ACE-134: the values row reads the comparator's agreement; an error row names the side that failed --- + +def _table(row, **score): + base = {"accuracy": 0.9, "reason": "9 of the answer key's 10 rows matched, and the generated statement returned 10 rows", + "unmatched_golden_columns": [], "unmatched_generated_columns": [], "golden_row_count": 10, "generated_row_count": 10, + "column_pairs": [["customer", "customer"], ["total", "amount"]], "column_agreement": [1.0, 0.9], "paired_row_share": 0.9, + "order_sensitive": False} + base.update(score) + return dict(TABLE_DIFF, row=row, recorded={"columns": ["customer", "amount"], "rows": []}, + statement_recorded={"columns": ["customer", "total"], "row_count": 10}, comparison={"result_set": base}) + + +def test_one_differing_cell_reads_as_rows_that_match_and_the_column_that_differs(tmp_path): + items = {i["row"]: i for i in reconcile.report_items(_run(tmp_path, [_table(1)]))} + rows = {r["key"]: r for r in items[1]["diff"]} + assert rows["values"]["state"] == "defect" and rows["values"]["yours"] == "9 of 10 rows match" + assert rows["values"]["note"] == "differs in total on 1 of 10 rows" + assert rows["columns"]["state"] == "held" and rows["columns"]["renamed"] == [["total", "amount"]] + assert items[1]["result"]["data"] == "differs" and items[1]["result"]["label"] == "different answer" + + +def test_paired_columns_agreeing_on_every_row_beside_an_unpaired_one_read_partly(tmp_path): + partly = _table(1, accuracy=0.0, reason="no generated column carries the values of: channel", + unmatched_golden_columns=["channel"], column_agreement=[1.0, 1.0], paired_row_share=1.0) + partly["statement_recorded"] = {"columns": ["customer", "total", "channel"], "row_count": 10} + # ...and not partly when the paired columns disagree too: the values differ, whatever the columns. + both = _table(2, accuracy=0.0, unmatched_golden_columns=["channel"], column_agreement=[1.0, 0.9], paired_row_share=0.9) + both["statement_recorded"] = {"columns": ["customer", "total", "channel"], "row_count": 10} + items = {i["row"]: i for i in reconcile.report_items(_run(tmp_path, [partly, both]))} + rows = {r["key"]: r for r in items[1]["diff"]} + assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical on the 2 paired columns" + assert items[1]["result"]["data"] == "partly" and items[1]["result"]["label"] == "same rows, different columns" + assert items[2]["result"]["data"] == "differs" + + +def test_an_older_score_without_the_share_keeps_the_paired_columns_grace(tmp_path): + old = _table(1, accuracy=0.0, unmatched_golden_columns=["channel"]) + del old["comparison"]["result_set"]["column_agreement"]; del old["comparison"]["result_set"]["paired_row_share"] + old["statement_recorded"] = {"columns": ["customer", "total", "channel"], "row_count": 10} + rows = {r["key"]: r for r in reconcile.report_items(_run(tmp_path, [old]))[0]["diff"]} + assert rows["values"]["state"] == "held" and rows["values"]["yours"] == "identical on the 2 paired columns" + + +def test_each_error_cause_names_the_side_that_failed(tmp_path): + yours = dict(ERROR, row=1, statement="SELECT ...", error="relation orders_v does not exist", + ledger={"rows": [_part("runs", "query_defect", note="the database named a missing table")], "verdict": "query_defect", "counts": {}}) + scope = dict(ERROR, row=2, statement="SELECT ...", error="refused: table_scope", + ledger={"rows": [_part("runs", "unresolved"), _part("scope", "model_gap", note="a table the semantic model does not declare")], "verdict": "model_gap", "counts": {}}) + agami = dict(ERROR, row=3, statement="SELECT ...", error="the generator's answer did not carry a statement this run could read") + empty = dict(ERROR, row=4, statement="SELECT ...", sql="SELECT ...", recorded={"columns": ["n"], "rows": []}, + statement_recorded={"columns": ["n"], "row_count": 0}, error=None, + comparison={"result_set": {"status": "unscored", "accuracy": None, "reason": "both result sets are empty, so the comparison would check no value", + "golden_row_count": 0, "generated_row_count": 0}}) + question_only = dict(ERROR, row=5, sql="SELECT ...", recorded={"columns": ["n"], "rows": [[3]]}, error=None) + unknown = dict(ERROR, row=6, statement="SELECT ...", sql="SELECT ...", recorded={"columns": ["n"], "rows": [[3]]}, error="boom\nmore") + items = {i["row"]: i for i in reconcile.report_items(_run(tmp_path, [yours, scope, agami, empty, question_only, unknown]))} + got = {r: (i["fix"], i["owner"], i["sentence"]) for r, i in items.items()} + assert got == { + 1: ("query", "you", "This row could not be compared: your query did not run: relation orders_v does not exist."), + 2: ("semantic_model", "model", "This row could not be compared: your query did not run: refused: table_scope."), + 3: ("ask_again", "agami", "This row could not be compared: agami's query failed: the generator's answer did not carry a statement this run could read."), + 4: ("none", "nothing", "This row could not be compared: both queries returned no rows, so there is nothing to compare."), + 5: ("none", "nothing", "This row could not be compared: there is nothing to compare against; agami's answer is graded on the grading page."), + 6: ("ask_again", "agami", "This row could not be compared: the run's files do not say why: boom."), + } + assert items[1]["change"][0] == "Your query did not run: relation orders_v does not exist. Fix it, then run this row again." + assert items[3]["change"][0].startswith("Ask the question again in other words; agami's query failed") + assert items[4]["change"][0].startswith("Both queries returned no rows") + assert items[5]["change"][0].startswith("Grade agami's answer on the grading page") + assert items[6]["change"][0].startswith("Run this row again") + assert all(i["result"]["label"] == "could not compare" for i in items.values()) + + +def test_the_two_statements_still_say_what_they_are_when_the_data_could_not_be_compared(tmp_path): + agree = [{"name": n, "status": "agrees", "generated": v, "golden": v} for n, v in + (("tables", ["orders"]), ("outputs", ["sum(orders.amount)"]), ("filter_predicates", ["eq(orders.status, 'paid')"]), + ("date_window", None), ("group_keys", []), ("join_keys", []), ("ordering", []), ("limit", None))] + same = dict(ERROR, row=1, statement="SELECT ...", error="the generator did not answer within the time this run allows", claims={"claims": agree}) + differ = dict(same, row=2, claims={"claims": [dict(c, status="differs", generated=["avg(orders.amount)"]) if c["name"] == "outputs" else c for c in agree]}) + items = {i["row"]: i for i in reconcile.report_items(_run(tmp_path, [same, differ]))} + assert items[1]["result"]["label"] == "same query, answer not compared" and items[1]["result"]["query"] == "same" + assert items[2]["result"]["label"] == "different query, answer not compared" and items[2]["result"]["query"] == "different" + selects = next(r for r in items[2]["diff"] if r["key"] == "selects") + assert selects["state"] == "differs" and selects["yours"] == ["sum(orders.amount)"] and selects["agami"] == ["avg(orders.amount)"] + # A structural match never makes the row keepable: the gate is the data's. + assert items[1]["keep_allowed"] is False and items[1]["status"] == "error" diff --git a/tests/test_reconcile_result_and_fix.py b/tests/test_reconcile_result_and_fix.py index 32f4d703..181e8fef 100644 --- a/tests/test_reconcile_result_and_fix.py +++ b/tests/test_reconcile_result_and_fix.py @@ -53,7 +53,7 @@ def test_every_result_label_and_its_fix(tmp_path): 5: ("same query, different answer", "differs", "same", "ask_again"), 6: ("different answer", "differs", "different", "examples"), 7: ("different answer", "differs", "not_comparable", "question"), - 8: ("could not run", "could_not_compare", "not_comparable", "ask_again"), + 8: ("could not compare", "could_not_compare", "not_comparable", "ask_again"), 9: ("different answer", "differs", "not_comparable", "query"), 10: ("different answer", "differs", "different", "semantic_model"), } @@ -140,3 +140,13 @@ def test_identical_column_names_with_different_row_counts_are_one_column_set(tmp assert rows["columns"]["state"] == "held" and not rows["columns"].get("yours_hi") and not rows["columns"].get("agami_hi") assert item["result"]["data"] == "differs" and "columns" not in item["result"]["differs_in"] + + +def test_a_different_projection_alone_is_a_different_query(tmp_path): + """The eighth claim: two statements that select different expressions are not the same query, + however the other seven agree.""" + only_outputs = dict(SCALAR_MATCH, row=2, claims=_claims(("tables", "agrees", ["orders"], ["orders"]), + ("outputs", "differs", ["sum(orders.amount)"], ["avg(orders.amount)"]))) + items = _items(tmp_path, [only_outputs]) + assert items[2]["result"]["query"] == "different" and items[2]["result"]["label"] == "same answer, different query" + assert items[2]["result"]["differs_in"] == ["selects"] and items[2]["fix"] == "examples"