diff --git a/CHANGELOG.md b/CHANGELOG.md index c4083fd8..7fef7408 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,10 @@ below corresponds to one such version. queries the same) as the pill, and the fix (your query, the semantic model, the examples, the question, agami again, nothing) as the action. A query written differently is a noted fact, no longer a blocker on a matching answer. (ACE-127) +- The reconcile card after a second read: the question is the title, columns are compared by the data + they carry (the compare-results score names `column_pairs` and `unmatched_generated_columns`), a bare + column reads as its table's column in the claims reader, the change text and the decision boxes derive + from the one fix, an `example` decision joins the block, the checks panel folds. (ACE-128) - From the first test of the grid: a plain column in a list query is no longer graded as a missing metric (the receipt's output items say whether they aggregate); a date window written against the clock (`date_trunc('year', current_date) + interval`) resolves and compares against another such diff --git a/packages/agami-core/src/semantic_model/comparator.py b/packages/agami-core/src/semantic_model/comparator.py index 3d4e6dbb..149d4e4c 100644 --- a/packages/agami-core/src/semantic_model/comparator.py +++ b/packages/agami-core/src/semantic_model/comparator.py @@ -410,6 +410,11 @@ class ItemScore: accuracy: Optional[float] reason: str unmatched_golden_columns: tuple[str, ...] = () + # Which golden column paired with which generated column, by VALUES, and which generated columns + # paired with none. A renamed column is the same column here; a reader that compared names would + # call it missing. Additive; empty where no column-level comparison ran. + column_pairs: tuple[tuple[str, str], ...] = () + unmatched_generated_columns: tuple[str, ...] = () golden_row_count: Optional[int] = None generated_row_count: Optional[int] = None order_sensitive: Optional[bool] = None @@ -629,6 +634,32 @@ def _judge( return _Verdict("error", None, f"{match!r} is not a match level this comparison knows") +def _column_pairs( + 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.""" + if match not in ("exact", "values") or not golden.rows or not generated.rows: + return (), () + 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 (), () + try: + pairing, _unmatched = match_columns( + golden.columns, golden.rows, generated.columns, generated.rows, + ordered=ordered, quantize=match == "values", + ) + 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 + def compare_result_sets( golden: ExecResult, generated: ExecResult, @@ -657,11 +688,14 @@ 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) return ItemScore( status=verdict.status, accuracy=verdict.accuracy, reason=verdict.reason, unmatched_golden_columns=verdict.unmatched, + column_pairs=pairs, + unmatched_generated_columns=extra, golden_row_count=len(golden.rows), generated_row_count=len(generated.rows), order_sensitive=ordered, diff --git a/packages/agami-core/src/semantic_model/golden_claims.py b/packages/agami-core/src/semantic_model/golden_claims.py index 2dd8cd83..e71200ea 100644 --- a/packages/agami-core/src/semantic_model/golden_claims.py +++ b/packages/agami-core/src/semantic_model/golden_claims.py @@ -235,6 +235,12 @@ def _rendered(node: "exp.Expression", aliases: dict[str, str], depth: int) -> st if isinstance(node, exp.Column): qualifier = node.table if not qualifier: + # An unqualified column in a SELECT that reads exactly one table belongs to that table, + # so `opened` and `r.opened` are one key; with two tables in scope it stays bare, since + # guessing an owner would make two different columns one key. + tables = {rt._tkey(rt._bare(t)) for t in aliases.values()} + if len(tables) == 1: + return f"{next(iter(tables))}.{node.name.lower()}" return node.name.lower() # The schema and catalog parts are dropped with the alias: `sales.orders.region` and # `orders.region` name one column, and `_bare` has already stripped the schema off the @@ -464,7 +470,10 @@ def _unit_name(node: "exp.Expression | None") -> Optional[str]: return _RELATIVE_UNITS.get(str(text).strip().strip("'\"").lower()) -def _relative_bound(node: "exp.Expression | None") -> Optional[str]: +_MAX_RELATIVE_DEPTH = 8 + + +def _relative_bound(node: "exp.Expression | None", depth: int = _MAX_RELATIVE_DEPTH) -> Optional[str]: """The bound a node spells RELATIVE to the run date, as words: `today`, `start of this year`, `start of this year + 7 month`, `today - 30 day`. None for any other shape. @@ -474,36 +483,38 @@ def _relative_bound(node: "exp.Expression | None") -> Optional[str]: date it computed would be a bound neither statement wrote. It is compared only against another relative bound (`_window_status`), never against a literal date. """ - if node is None: + if node is None or depth <= 0: + # A relative bound deeper than a handful of steps is not a window anyone wrote; past the + # budget it reads None, so `read_claims` keeps its promise never to raise on a pathological tree. return None if isinstance(node, exp.Paren): - return _relative_bound(node.this) + return _relative_bound(node.this, depth - 1) if isinstance(node, exp.Cast): - return _relative_bound(node.this) + return _relative_bound(node.this, depth - 1) if isinstance(node, exp.CurrentDate): return "today" if isinstance(node, exp.CurrentTimestamp) or (isinstance(node, exp.Anonymous) and str(node.this).lower() in ("now", "getdate", "sysdate", "current_timestamp")): return "now" if isinstance(node, (exp.DateTrunc, exp.TimestampTrunc)): - inner = _relative_bound(node.this if isinstance(node, exp.TimestampTrunc) else node.args.get("this")) + inner = _relative_bound(node.this if isinstance(node, exp.TimestampTrunc) else node.args.get("this"), depth - 1) unit = _unit_name(node.args.get("unit")) if isinstance(node, exp.DateTrunc): # sqlglot's DateTrunc holds the unit in `unit` and the value in `this`; some dialects # parse the argument order the other way round, so both are tried. - inner = _relative_bound(node.this) or _relative_bound(node.args.get("unit")) + inner = _relative_bound(node.this, depth - 1) or _relative_bound(node.args.get("unit"), depth - 1) unit = _unit_name(node.args.get("unit")) or _unit_name(node.this) if inner in ("today", "now") and unit: return f"start of this {unit}" return None if isinstance(node, (exp.Add, exp.Sub)): - base = _relative_bound(node.this) + base = _relative_bound(node.this, depth - 1) step = _interval_words(node.expression) if base and step: return _step(base, isinstance(node, exp.Add), *step) return None date_add_types = tuple(t for t in (exp.DateAdd, getattr(exp, "TsOrDsAdd", None)) if t is not None) if isinstance(node, date_add_types + (exp.DateSub,)): - base = _relative_bound(node.this) + base = _relative_bound(node.this, depth - 1) n = node.expression unit = _unit_name(node.args.get("unit")) count: Optional[int] = None diff --git a/plugins/agami/scripts/parse_reconcile_intake.py b/plugins/agami/scripts/parse_reconcile_intake.py index 788f3981..38168971 100644 --- a/plugins/agami/scripts/parse_reconcile_intake.py +++ b/plugins/agami/scripts/parse_reconcile_intake.py @@ -30,7 +30,7 @@ _KEYS = {"profile", "reconcile-run", "intake"} -def _key_of(line: str): +def _key_of(line: str) -> str | None: low = line.strip().lower() for k in _KEYS: if low.startswith(k + ":") or low == k + ":": @@ -144,8 +144,15 @@ def main(argv=None) -> int: print(json.dumps({"ok": False, "data": None, "anomalies": [{"kind": "bad_argument", "detail": str(exc)}], "needs_judgment": {"kind": "bad_argument", "ask": "pass the rows file `reconcile.py intake` wrote and the pasted block"}}, indent=2)) return 2 + if not isinstance(intake, dict) or not isinstance(intake.get("rows"), list): + print(json.dumps({"ok": False, "data": None, "anomalies": [{"kind": "bad_argument", "detail": "the rows file is not the output of `reconcile.py intake`"}], + "needs_judgment": {"kind": "bad_argument", "ask": "pass the rows file `reconcile.py intake` wrote (an object with a `rows` list)"}}, indent=2)) + return 2 known = {r.get("row", n) for n, r in enumerate(intake.get("rows", []), 1)} - data, anomalies, needs = parse(text, known, run=args.run) + # The block must name the run it is applied to. When --run is not given, the run is the folder + # --out lands in, so a block from another run can never be applied by leaving the flag off. + run = args.run or (Path(args.out).expanduser().resolve().parent.name if args.out else None) + data, anomalies, needs = parse(text, known, run=run) counts = None if needs is None: applied, counts = apply(intake, data["decisions"]) diff --git a/plugins/agami/scripts/parse_reconcile_report.py b/plugins/agami/scripts/parse_reconcile_report.py index 80423bef..7d563f93 100644 --- a/plugins/agami/scripts/parse_reconcile_report.py +++ b/plugins/agami/scripts/parse_reconcile_report.py @@ -38,12 +38,32 @@ from pathlib import Path _KEYS = {"profile", "reconcile-run", "decisions"} -_DECISIONS = frozenset({"keep", "change", "fix", "reword", "nothing"}) -_WITH_WORDS = frozenset({"change", "fix", "reword"}) +_DECISIONS = frozenset({"keep", "change", "fix", "reword", "example", "nothing"}) +_WITH_WORDS = frozenset({"change", "fix", "reword", "example"}) _FIELDS = ("row", "decision", "words") _DROPPED_KINDS = frozenset({"unknown_decision", "decision_missing_row", "row_decided_twice", "decision_not_an_object", "keep_not_offered", "words_ignored_on_keep", - "words_ignored_on_nothing", "words_not_text"}) + "words_ignored_on_nothing", "words_not_text", "example_not_offered"}) + + +def example_blocked_rows(run_dir: Path) -> set[int]: + """Rows whose ledger holds a part the data proved wrong: a statement with a mistake in it is never + offered as a prompt example, whatever the page suggested. Read from each row's ledger.json.""" + blocked: set[int] = set() + rows_dir = run_dir / "rows" + if not rows_dir.is_dir(): + return blocked + for row_dir in rows_dir.iterdir(): + ledger = row_dir / "ledger.json" + if not row_dir.name.isdigit() or not ledger.exists(): + continue + try: + parts = json.loads(ledger.read_text(encoding="utf-8")).get("rows", []) + except (OSError, ValueError): + continue + if any(isinstance(p, dict) and p.get("verdict") == "query_defect" for p in parts): + blocked.add(int(row_dir.name)) + return blocked def keepable_rows(run_dir: Path) -> set[int]: @@ -81,7 +101,7 @@ def keepable_rows(run_dir: Path) -> set[int]: return keep -def _key_of(line: str): +def _key_of(line: str) -> str | None: low = line.strip().lower() for k in _KEYS: if low.startswith(k + ":") or low == k + ":": @@ -112,7 +132,8 @@ def _sections(text: str) -> tuple[dict, list[str]]: return out, repeated -def parse(text: str, keepable: set[int] | None = None, run: str | None = None) -> tuple[dict, list, dict | None]: +def parse(text: str, keepable: set[int] | None = None, run: str | None = None, + example_blocked: set[int] | None = None) -> tuple[dict, list, dict | None]: sec, repeated = _sections(text) anomalies: list = [{"kind": "key_repeated", "detail": key} for key in repeated] needs: dict | None = None @@ -156,6 +177,9 @@ def parse(text: str, keepable: set[int] | None = None, run: str | None = None) - if row in seen: anomalies.append({"kind": "row_decided_twice", "row": row}) continue + if decision == "example" and row in (example_blocked or set()): + anomalies.append({"kind": "example_not_offered", "row": row}) + continue # dropped like a keep the run did not offer; _DROPPED_KINDS carries the kind if decision == "keep" and row not in (keepable or set()): # The offer's predicate belongs to the ledger: a keep the run's own files do not # allow is not the person's to grant from a page. @@ -200,7 +224,7 @@ def main(argv=None) -> int: print(json.dumps({"ok": False, "data": None, "anomalies": [{"kind": "bad_argument", "detail": str(exc)}], "needs_judgment": {"kind": "bad_argument", "ask": "the block file could not be read"}}, indent=2)) return 2 - data, anomalies, needs = parse(text, keepable_rows(run_dir), run=run_dir.name) + data, anomalies, needs = parse(text, keepable_rows(run_dir), run=run_dir.name, example_blocked=example_blocked_rows(run_dir)) print(json.dumps({"ok": needs is None, "data": data, "anomalies": anomalies, "needs_judgment": needs}, indent=2)) return 0 diff --git a/plugins/agami/scripts/reconcile.py b/plugins/agami/scripts/reconcile.py index 27f76cf6..aa6f62e0 100644 --- a/plugins/agami/scripts/reconcile.py +++ b/plugins/agami/scripts/reconcile.py @@ -438,11 +438,16 @@ def _rows_from_json(items: Any, *, file: str, source: str | None) -> tuple[list[ return rows, skipped +_MAX_INTAKE_BYTES = 20 * 1024 * 1024 + + def _rows_from_file(path: Path, source: str | None) -> tuple[list[dict], list[dict]]: """One file's rows and the lines it could not use. The extension decides how lines are cut: `.json` is a list, `.sql` is statements split on `;`, `.txt` and `.md` are one question per line, and everything else is CSV.""" file = path.name + if path.stat().st_size > _MAX_INTAKE_BYTES: + raise ValueError(f"{file} is {path.stat().st_size // (1024 * 1024)} MB; the intake reads files up to {_MAX_INTAKE_BYTES // (1024 * 1024)} MB. Export fewer rows, or split the file.") text = path.read_text(encoding="utf-8") suffix = path.suffix.lower() if suffix == ".json": @@ -1394,7 +1399,7 @@ def row_status(match: bool | None, ledger_verdict: str | None) -> str: # -------------------------------------------------------------------------------------------- -# next-chunk: the run works five rows at a time, and rows.jsonl is its checkpoint +# next-chunk: the run works five rows at a time, and rows.jsonl is its checkpoint. # -------------------------------------------------------------------------------------------- CHUNK_SIZE = 5 @@ -1473,12 +1478,12 @@ def next_chunk(run_dir: Path, size: int = CHUNK_SIZE) -> dict: # -------------------------------------------------------------------------------------------- -# report-items: the report page's items, templated from what the run wrote, never written by hand +# report-items: the report page's items, templated from what the run wrote, never written by hand. # -------------------------------------------------------------------------------------------- _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"} + "open": "could not check", "noted": "noticed", "differs": "the two queries differ"} _CLAIM_KEYS = {"tables": "tables read", "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 @@ -1711,22 +1716,41 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N add("rows", "held" if same_rows else "defect", yours_text, agami_text) yc = list(((rec.get("statement_recorded") or {}).get("columns")) or []) ac = list(((rec.get("recorded") or {}).get("columns")) or []) + pairs = [tuple(p) for p in (result_set.get("column_pairs") or []) if isinstance(p, (list, tuple)) and len(p) == 2] if yc or ac: - only_yours, only_agami = _only(yc, ac), _only(ac, yc) + values_compared = same_rows and (bool(pairs) or bool(result_set.get("unmatched_golden_columns"))) + if values_compared: + # Columns are compared by the values they carry, never by name: the comparator says + # which of yours paired with which of agami's, and a renamed column is the same column. + only_yours = [c for c in (result_set.get("unmatched_golden_columns") or []) if c in yc] + only_agami = list(result_set.get("unmatched_generated_columns") or []) + renamed = [f"{a} → {b}" for a, b in pairs if a != b] + else: + # No values comparison ran (the row counts differ, or an older score file): names are + # all there is. Identical names are one column set; the rows check carries the counts. + only_yours, only_agami, renamed = _only(yc, ac), _only(ac, yc), [] if not only_yours and not only_agami: col_state = "held" elif only_yours: col_state = "defect" else: col_state = "noted" # agami returned more than asked; nothing of yours is missing - add("columns", col_state, yc, ac, yours_hi=only_yours, agami_hi=only_agami, - note=(f"the comparison scores on columns; {len(result_set.get('unmatched_golden_columns') or [])} unmatched scored 0" - if result_set.get("unmatched_golden_columns") else (f"agami returned columns your query did not: {', '.join(only_agami)}" if col_state == "noted" else None))) + # No sentence: the tokens carry the difference the way a diff does. A column only yours + # has reads as removed, one only agami's as added, and a pair with two names is marked in + # place so the reader sees they hold the same values. + add("columns", col_state, yc, ac, yours_hi=only_yours, agami_hi=only_agami) + if renamed: + rows[-1]["renamed"] = [[a, b] for a, b in pairs if a != b] acc = result_set.get("accuracy") if acc is not None: same = float(acc) >= 1.0 - add("values", "held" if same else "defect", "identical, row for row" if same else f"{float(acc):.0%} of the values match", - None, note=None if same else result_set.get("reason")) + if same: + add("values", "held", "identical, row for row", 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") + else: + add("values", "defect", f"{float(acc):.0%} of the values match", None, note=result_set.get("reason")) else: match = rec.get("match") if rec.get("match") is not None else (scalar or {}).get("match") if rec.get("status") == "error": @@ -1757,15 +1781,20 @@ def add(key, state, yours=None, agami=None, note=None, yours_hi=None, agami_hi=N status = claim.get("status") if yours is None and agami is None and status in ("agrees", "same"): continue - state = "held" if status in ("agrees", "same") else "defect" if status == "differs" else "open" + state = "held" if status in ("agrees", "same") else "differs" if status == "differs" else "open" note = None - if name == "date_window" and state == "open": - part = parts.get("date_window") or {} - note = part.get("note") or "the window could not be read from one of the two queries" + graded = parts.get({"date_window": "date_window", "filter_predicates": "predicates"}.get(name, "")) + if state == "open" and graded and graded.get("verdict") == CONFIRMED: + # The ledger read this claim with more context (no date filter anywhere, say) and confirmed it. + state, note = "held", None + if name == "date_window": + yours, agami = yours or "no date filter", agami or "no date filter" + elif name == "date_window" and state == "open": + note = (graded or {}).get("note") or "the window could not be read from one of the two queries" yours = yours or "could not read" agami = agami or "could not read" add(_CLAIM_KEYS.get(name, name), state, yours, agami, note=note, - yours_hi=_only(yours, agami) if state == "defect" else None, agami_hi=_only(agami, yours) if state == "defect" else None) + yours_hi=_only(yours, agami) if state == "differs" else None, agami_hi=_only(agami, yours) if state == "differs" else None) # 3 · every part of the person's statement the ledger graded, with agami's side where a receipt says. filters, metrics = _receipt_filters(agami_receipt), _receipt_metrics(agami_receipt) @@ -1830,8 +1859,8 @@ def _result(rec: dict, diff: list[dict]) -> dict: cols = rows.get("columns"); values = rows.get("values") same_rows = rows["rows"]["state"] == "held" values_ok = values is None or values["state"] == "held" - if same_rows and values_ok and (cols is None or cols["state"] == "held"): - data = "matches" + if same_rows and values_ok and (cols is None or cols["state"] in ("held", "noted")): + data = "matches" # a column only agami returned is noticed, not a difference in the answer elif same_rows and cols is not None and cols["state"] != "held" and _values_agree_on_shared_columns(rec): data = "partly" else: @@ -1842,36 +1871,55 @@ def _result(rec: dict, diff: list[dict]) -> dict: claims = ((rec.get("claims") or {}).get("claims") or []) if isinstance(rec.get("claims"), dict) else [] statuses = [c.get("status") for c in claims] columns_differ = "columns" in rows and rows["columns"]["state"] != "held" + by_name = {c.get("name"): c.get("status") for c in claims} + parts = ((rec.get("ledger") or {}).get("rows") or []) if isinstance(rec.get("ledger"), dict) else [] + part_verdicts = {p.get("part"): p.get("verdict") for p in parts} + # A claim the ledger graded confirmed (no date filter anywhere, say) is not unknown for this purpose. + for name, part in (("date_window", "date_window"), ("filter_predicates", "predicates")): + 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")) 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: # The seven claims do not cover the projection; two queries that return different columns # are different queries even when every claim agrees. query = "different" + elif definitional_unknown: + # Nothing differs, but a claim that decides sameness could not be read: sameness is not established. + query = "not_comparable" else: query = "same" - parts = ((rec.get("ledger") or {}).get("rows") or []) if isinstance(rec.get("ledger"), dict) else [] - unchecked = sum(1 for p in parts if p.get("verdict") == UNRESOLVED) + sum(1 for st in statuses if st == "unknown") + # 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)] differing = sorted(r["key"] for r in diff - if (r["state"] == "defect" or (r["key"] == "columns" and r["state"] == "noted")) + if (r["state"] in ("defect", "differs") or (r["key"] == "columns" and r["state"] == "noted")) and r["key"] in _DEFINITIONAL | {"ordered by", "limit", "columns"}) return {"data": data, "query": query, "label": label, "unchecked": unchecked, "differs_in": differing} def _values_agree_on_shared_columns(rec: dict) -> bool: - """A table compare whose only difference is the column set: every golden column the generated side - carries matched by value, so the score is exactly the matched share.""" + """A table compare whose only difference is the column set. The comparator pairs columns by their + values, so every pair it reports agrees by construction; the answer is partly the same when at + least one pair exists beside a column of yours with no partner or a column of agami's with none. + The score itself is 0.0 whenever any column of yours is unpaired, so it cannot be the test.""" score = (rec.get("comparison") or {}).get("result_set") if isinstance(rec.get("comparison"), dict) else None if not score: return False + acc = score.get("accuracy") + 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 [] + 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 cols = list(((rec.get("statement_recorded") or {}).get("columns")) or []) unmatched = list(score.get("unmatched_golden_columns") or []) - acc = score.get("accuracy") if acc is None or not cols: return False - if float(acc) >= 1.0: - return True # agami returned everything you did, and more matched = len(cols) - len(unmatched) return matched > 0 and abs(float(acc) - matched / len(cols)) < 1e-6 @@ -1913,13 +1961,57 @@ def _fix(rec: dict, diff: list[dict], result: dict) -> str: } +def _change_for_fix(fix: str, rec: dict, diff: list[dict]) -> tuple[list[str], list[str], dict]: + """The card's change text, its to-do and the words each decision box starts with, all from the one + fix. The three used to come from three places and could disagree on one card.""" + parts = ((rec.get("ledger") or {}).get("rows") or []) if isinstance(rec.get("ledger"), dict) else [] + fit = next((p for p in parts if p.get("part") == "question_fit"), None) + gaps = [r["key"] for r in diff if r["state"] == "gap"] + cols = next((r for r in diff if r["key"] == "columns"), None) + extra = list((cols or {}).get("yours_hi") or []) + mistakes = _measured_mistakes(rec) + prefill = {"change": "", "fix": "", "reword": rec.get("question") or "", "example": ""} + if fix == "query": + if mistakes: + change = [f"Fix your query: {', '.join(mistakes)}. Then run this row again."] + prefill["fix"] = "; ".join(mistakes) + elif extra: + change = [f"Your query returns columns the question did not ask for: {', '.join(extra)}. Remove them, or name them in the question."] + prefill["fix"] = "remove " + ", ".join(extra) + prefill["reword"] = (rec.get("question") or "").rstrip(".?") + f", with {', '.join(extra)}?" + else: + change = ["Fix your query where the marks are red, then run this row again."] + todo = ["Your query: fix the red rows, then re-run."] + elif fix == "semantic_model": + change = [f"The semantic model is missing: {', '.join(gaps)}. Add them through /agami-save-correction." if gaps + else "Decide which definition your team means. A change to the semantic model goes through /agami-save-correction."] + todo = [f"The semantic model: {', '.join(gaps)}." if gaps else "The semantic model: decide the definition."] + prefill["change"] = ("add " + ", ".join(gaps)) if gaps else "" + elif fix == "examples": + change, todo = list(_FIX_CHANGE["examples"][0]), list(_FIX_CHANGE["examples"][1]) + elif fix == "question": + reason = (fit or {}).get("note") or "" + 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": + change, todo = list(_OWNER_CHANGE["agami"][0]), list(_OWNER_CHANGE["agami"][1]) + else: + change, todo = list(_FIX_CHANGE["ask_again"][0]), list(_FIX_CHANGE["ask_again"][1]) + 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": + change.append(f"Also: {fit['note']}") + return change, todo, prefill + + def _owner(rec: dict, diff: list[dict]) -> str: """Who acts, from the evidence, in this order: a mistake in the query is the person's; a gap the ledger measured is the semantic model's; a question read differently is the question's.""" status = rec.get("status") parts = ((rec.get("ledger") or {}).get("rows") or []) if isinstance(rec.get("ledger"), dict) else [] fit = next((p for p in parts if p.get("part") == "question_fit"), None) - differing = {r["key"] for r in diff if r["state"] == "defect"} + differing = {r["key"] for r in diff if r["state"] in ("defect", "differs")} if status == "match": # The keep-offer (Phase 3e, and the page's keep gate) is made only for a one-cell answer whose # statement, if any, answers its question; a matched table or a doubtful fit is not offered. @@ -1994,7 +2086,7 @@ def _measured_mistakes(rec: dict) -> list[str]: def _sentence(rec: dict, diff: list[dict]) -> str: status = rec.get("status") - red = [r["key"] for r in diff if r["state"] == "defect" and r["key"] not in ("answer", "rows", "values")] + red = [r["key"] for r in diff if r["state"] in ("defect", "differs") and r["key"] not in ("answer", "rows", "values")] mistakes = _measured_mistakes(rec) open_ = [r["key"] for r in diff if r["state"] == "open"] gaps = [r["key"] for r in diff if r["state"] == "gap"] @@ -2018,8 +2110,12 @@ def resume(reconcile_dir: Path) -> dict | None: for run_dir in candidates: try: state = next_chunk(run_dir) - except (json.JSONDecodeError, ValueError): - continue + except json.JSONDecodeError: + continue # an intake.json that is not JSON is not a run to resume + except ValueError as exc: + # A corrupt checkpoint is refused, never skipped: skipping would resume an older run and + # leave this one's row to be run twice later. + raise ValueError(f"{run_dir}: {exc}") from exc if not state["complete"]: return {"run_dir": str(run_dir), "finished": state["finished"], "remaining": state["remaining"], "chunk_rows": state["chunk_rows"], "progress": state["progress"]} @@ -2034,6 +2130,9 @@ def report_items(run_dir: Path) -> list[dict]: items = [] for rec in records: rec = dict(rec, status=rec.get("status") or "error") + if isinstance(rec.get("error"), str): + # The page shows the classifier's one line, never a driver's message with a host or a table in it. + rec["error"] = rec["error"].strip().splitlines()[0][:200] if rec["error"].strip() else None n = rec.get("row") row_dir = run_dir / "rows" / str(n) agami_receipt = None @@ -2049,18 +2148,18 @@ def report_items(run_dir: Path) -> list[dict]: legacy_owner = _owner(rec, diff) # keep is the fix "nothing" on a row the keep gate accepts; the page's keep offer is that keep_ok = legacy_owner == "keep" - owner = "keep" if (fix == "none" and keep_ok) else _FIX_OWNER[fix] - change, todo = _change(owner if owner != "agami" else legacy_owner, rec, diff) - if fix in _FIX_CHANGE: - change, todo = list(_FIX_CHANGE[fix][0]), list(_FIX_CHANGE[fix][1]) - if fix == "ask_again" and rec.get("status") == "error": - change, todo = list(_OWNER_CHANGE["agami"][0]), list(_OWNER_CHANGE["agami"][1]) - if fix == "none" and not keep_ok and rec.get("status") == "match": + 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 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.")) else: clause = None + cols_row = next((r for r in diff if r["key"] == "columns"), None) + if cols_row and cols_row.get("agami_hi") and not cols_row.get("yours_hi"): + clause = ((clause + " ") if clause else "") + f"agami also returned: {', '.join(cols_row['agami_hi'])}." prov = rec.get("provenance") or {} shape_words = {"a": "a question", "b": "a question with your SQL", "c": "a number from your dashboard", "d": "a number with the SQL behind it"} source = ", ".join(p for p in (prov.get("source"), f"{prov['file']}:{prov['line']}" if prov.get("file") and prov.get("line") else prov.get("file"), @@ -2078,8 +2177,8 @@ def report_items(run_dir: Path) -> list[dict]: "source": source or None, "status": rec.get("status") or "error", "expected": expected, "answer": answer, "delta_pct": (round(delta * 100, 1) if isinstance(delta, (int, float)) and not isinstance(delta, bool) else None), - "single_cell": bool(single), "owner": owner, "keep_allowed": owner == "keep", "diff": diff, - "result": result, "fix": fix, "fix_words": _FIX_WORDS[fix], + "single_cell": bool(single), "owner": owner, "keep_allowed": keep_ok, "diff": diff, + "result": result, "fix": fix, "fix_words": _FIX_WORDS[fix], "prefill": prefill, "sentence": _sentence(rec, diff) + (" " + clause if clause else ""), "words": words, "disagreement": None, "change": list(change), "todo": list(todo), "sql_yours": rec.get("statement") or None, "sql_agami": rec.get("sql") or None, @@ -2153,7 +2252,11 @@ def main(argv: list[str] | None = None) -> int: if not root.is_dir(): print(f"reconcile resume: directory not found: {root}", file=sys.stderr) return 2 - found = resume(root) + try: + found = resume(root) + except ValueError as exc: + print(f"reconcile resume: {exc}", file=sys.stderr) + return 2 print(json.dumps(found, indent=2)) # Exit 4, "nothing to do": every run under the directory is complete, or there is none. return 0 if found else 4 @@ -2245,10 +2348,11 @@ def main(argv: list[str] | None = None) -> int: if missing: print(f"reconcile intake: file not found: {', '.join(missing)}", file=sys.stderr) return 2 + csv.field_size_limit(sys.maxsize) # a long SQL cell is a row, not an error try: result = intake(paths, source=args.source) - except (json.JSONDecodeError, UnicodeDecodeError) as exc: - print(f"reconcile intake: could not read the input: {exc}", file=sys.stderr) + except (json.JSONDecodeError, UnicodeDecodeError, ValueError, csv.Error, RecursionError) as exc: + print(f"reconcile intake: could not read the input: {str(exc).splitlines()[0][:300]}", file=sys.stderr) return 2 if not result["rows"]: # Exit 4, "nothing to do", kept apart from 2 so the skill can say which happened: diff --git a/plugins/agami/scripts/render_reconcile_intake.py b/plugins/agami/scripts/render_reconcile_intake.py index b42a7e47..5bb1ec51 100644 --- a/plugins/agami/scripts/render_reconcile_intake.py +++ b/plugins/agami/scripts/render_reconcile_intake.py @@ -84,8 +84,6 @@ def _validate_item(item: dict, idx: int) -> None: raise ValueError(f"item {idx}: '{key}' must be text") if item.get("line") is not None and (not isinstance(item["line"], int) or isinstance(item["line"], bool)): raise ValueError(f"item {idx}: 'line' must be a whole number") - if item.get("line") is not None and (not isinstance(item["line"], int) or isinstance(item["line"], bool)): - raise ValueError(f"item {idx}: 'line' must be a whole number") if item.get("statement_preview") and len(item["statement_preview"]) > 80: raise ValueError(f"item {idx}: 'statement_preview' is at most 80 characters") diff --git a/plugins/agami/scripts/render_reconcile_report.py b/plugins/agami/scripts/render_reconcile_report.py index 2910e4df..2fe04490 100644 --- a/plugins/agami/scripts/render_reconcile_report.py +++ b/plugins/agami/scripts/render_reconcile_report.py @@ -41,14 +41,14 @@ # What one card may carry, beat by beat. Every text field is DISPLAY text the skill already wrote in # plain language; the lists are one sentence per line. A `rows` or `recorded` key is refused. _FIELDS = ("row", "label", "question", "source", "status", "expected", "answer", "delta_pct", "single_cell", - "owner", "read", "how", "words", "disagreement", "change", "checks", "todo", "report_path", "diff", "sentence", "sql_yours", "sql_agami", "keep_allowed", "result", "fix", "fix_words") + "owner", "read", "how", "words", "disagreement", "change", "checks", "todo", "report_path", "diff", "sentence", "sql_yours", "sql_agami", "keep_allowed", "result", "fix", "fix_words", "prefill") _LISTS = ("read", "how", "words", "change", "todo") -_DIFF_KEYS = ("key", "state", "yours", "agami", "note", "yours_hi", "agami_hi") +_DIFF_KEYS = ("key", "state", "yours", "agami", "note", "yours_hi", "agami_hi", "renamed") _STATUSES = {"match", "match_unverified", "mismatch", "expected_doubtful", "error"} # Who acts in beat 4, which colors the fourth column: the person's query, the semantic model, the # question, agami's answer (a worked example), keep, or nothing. _OWNERS = {"you", "model", "question", "agami", "keep", "nothing"} -_CHECK_STATES = {"held", "defect", "open", "gap", "noted"} +_CHECK_STATES = {"held", "defect", "open", "gap", "noted", "differs"} _LAYOUTS = ("auto", "cards", "audit") _DATA_RESULTS = {"matches", "partly", "differs", "could_not_compare"} _QUERY_RESULTS = {"same", "different", "not_comparable"} @@ -97,6 +97,9 @@ def _validate_item(item: dict, idx: int) -> None: raise ValueError(f"item {idx}: diff '{hi}' must be a list of the tokens to highlight") if row.get("note") is not None and not isinstance(row["note"], str): raise ValueError(f"item {idx}: diff 'note' must be text") + renamed = row.get("renamed") + if renamed is not None and not (isinstance(renamed, list) and all(isinstance(p, list) and len(p) == 2 and all(isinstance(x, str) for x in p) for p in renamed)): + raise ValueError(f"item {idx}: diff 'renamed' must be a list of [yours, agami] name pairs") if "rows" in row or "recorded" in row: raise ValueError(f"item {idx}: result rows are never rendered, not even inside a diff row") if item.get("keep_allowed") is not None and not isinstance(item["keep_allowed"], bool): @@ -110,6 +113,9 @@ def _validate_item(item: dict, idx: int) -> None: raise ValueError(f"item {idx}: 'fix' must be one of {sorted(_FIXES)}") if item.get("fix_words") is not None and not isinstance(item["fix_words"], str): raise ValueError(f"item {idx}: 'fix_words' must be text") + prefill = item.get("prefill") + if prefill is not None and (not isinstance(prefill, dict) or not all(isinstance(k, str) and isinstance(v, str) for k, v in prefill.items())): + raise ValueError(f"item {idx}: 'prefill' must map a decision to the words its box starts with") if item.get("sentence") is not None and not isinstance(item["sentence"], str): raise ValueError(f"item {idx}: 'sentence' must be text") for key in ("sql_yours", "sql_agami"): diff --git a/plugins/agami/shared/reconcile-grades-template.html b/plugins/agami/shared/reconcile-grades-template.html index a755028a..b8c581bc 100644 --- a/plugins/agami/shared/reconcile-grades-template.html +++ b/plugins/agami/shared/reconcile-grades-template.html @@ -28,7 +28,7 @@
-