From 3d19b8f5b3cc39e68683f3a2aee9847c977a8412 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 29 Oct 2026 17:13:48 +0000 Subject: [PATCH 01/11] feat(repo): replay grades record each credited hit; report --hits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dokimi:review_replay grade kept only the ids of the threads it credited (details.threads_hit), not which finding hit each one. To check a hit, the evaluation of IntelPython/dpnp#3099 had to rerun the location matching by hand next to gold.json and answer.txt, and that is how it found credits by chance: a finding on the right lines about something else. Separately, an answer whose findings JSON does not parse is graded parse_error with every recall 0, but no summary showed it, so an arm that makes answers harder to parse read as a recall loss. - grade_replay adds details.matches: per credited pair, in threads_hit order, the thread (id, file, line, comment cut to 200 characters), the finding (its index, repository-relative file, line as matched, severity, message cut to 200) and their distance. The matching and the metrics are unchanged, so VERSION stays 1; the graders digest a batch is pinned to changes with the source as before. - repo report --hits lists those pairs by task, arm, model and rep, two lines each (--json: records with task, arm, model, rep and cell), counts the cells graded before matches existed, and says so when the batch has no review_replay cell. The default report is unchanged. - repo report counts, per row, the cells whose grade.json has parse_error, as a note under the row ("agent answer(s) whose findings JSON did not parse") and as parse_errors in --json. - Contract §4.5 and §9, the grader docstring and a changelog fragment. Signed-off-by: Anton Volkov --- ...1029_120000_antonwolfy_replay-hit-pairs.md | 4 + dokimi/docs/repo-evaluation-contract.md | 19 ++- dokimi/dokimi/commands/repo_run.py | 11 +- dokimi/dokimi/repos/graders/review.py | 35 ++++-- dokimi/dokimi/repos/report.py | 100 ++++++++++++++-- dokimi/tests/test_repo_builtin_graders.py | 23 ++++ dokimi/tests/test_repo_report_hits.py | 111 ++++++++++++++++++ 7 files changed, 284 insertions(+), 19 deletions(-) create mode 100644 dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md create mode 100644 dokimi/tests/test_repo_report_hits.py diff --git a/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md new file mode 100644 index 000000000..5c29acb67 --- /dev/null +++ b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md @@ -0,0 +1,4 @@ +### Added + +- **Replay hits you can check** — a `dokimi:review_replay` grade now records `details.matches`: per credited thread, the thread (id, anchor, gold comment) and the finding that hit it (its index, location, severity, message) with their distance in lines, so a hit can be read from `grade.json` alone. The metrics are unchanged. `dokimi repo report --hits` lists those pairs by task, arm, model and rep (`--json` for records), and counts the cells graded before `matches` existed. +- **Parse errors in `repo report`** — a row notes how many of its cells had an agent answer whose findings JSON did not parse (`grade.json` `parse_error`), and `--json` gives `parse_errors` per row, so a lower recall from answers that did not parse can be told from one from worse reviews. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 38d9c96d8..da4b1d917 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -616,7 +616,7 @@ failed the task, and on a task with `score` that is a failed run with score | `grader` | data next to `task.json` (key that renames it) | `pass` | `metrics` | `details` | |---|---|---|---|---| | `dokimi:review_seeds` | `manifest.json` (`manifest`); oracle answer `oracle.json` (`oracle_answer`), a list of findings | `null`; for a manifest with no seeds (a clean-diff control), `true` when no finding is `blocker` or `major` | `n_findings`, `n_severe`, `n_decoys`, `unmatched`, `recall` (`null` with no seeds) | `seeds_hit`, `decoys_flagged`, `recall_by_class` (`"hit/total"`) | -| `dokimi:review_replay` | `gold.json` (`gold`); `task.json` `window`, default 5 | `null` (score-only) | `n_findings`, `n_severe`, `n_threads`, `recall` (= `recall_all`), `recall_addressed`, `precision_proxy` | `threads_hit`, `recall_by_kind`, `decoys_flagged` (always `[]`) | +| `dokimi:review_replay` | `gold.json` (`gold`); `task.json` `window`, default 5 | `null` (score-only) | `n_findings`, `n_severe`, `n_threads`, `recall` (= `recall_all`), `recall_addressed`, `precision_proxy` | `threads_hit`, `matches`, `recall_by_kind`, `decoys_flagged` (always `[]`) | `manifest.json` is `{"seeds": [{id, class, file, lines, kw}], "decoys": [{id, file, lines, kw}]}`; `file` `"*"` matches any file, and `kw` is a @@ -637,6 +637,12 @@ review thread of a merged PR, anchored on a line of the reviewed revision. - A finding on a thread's file within `window` lines of its anchor can hit it. Matching is one-to-one and nearest first: one finding credits at most one thread, and one thread at most one finding. +- `matches` holds one object per credited pair, in `threads_hit` order: the + thread's `thread` (id), `thread_file`, `thread_line` and `comment`, the + finding's `finding` (its index among the answer's objects), `file` + (repository-relative), `line` (as read for matching), `severity` and + `message`, and their `distance` in lines; `comment`, `severity` and + `message` are cut to 200 characters. `repo report --hits` lists them (§9). - `recall_addressed` counts only threads with `addressed: true` (the code changed in response), `recall_by_kind` groups by `kind`, and `precision_proxy` is the share of findings within the window of any @@ -1311,7 +1317,16 @@ python cli.py repo compare repo-runs/replay --baseline guidance:none --metric re When cells carry `result.json` `process` (§4.1), a line under the row gives the mean of each count over the cells that have it (`guidance_read` counted as paths; `counter:` for `process_metrics`); a `null` or absent - `process` is left out, not counted as zero. + `process` is left out, not counted as zero. A row whose cells include + agent answers with no findings JSON that parsed (`grade.json` + `parse_error`, §4.5) notes how many; `--json` gives `parse_errors` on every + row. `repo report --hits` prints, instead of the table, each credited pair of + the `dokimi:review_replay` cells (`details.matches`, §4.5) by task, arm, + model and rep: the thread and its comment, then the finding that hit it. + `--hits --json` gives `{batch, replay_cells, without_matches, hits}`, one + record in `hits` per pair (`task`, `arm`, `model`, `rep`, `cell` and the + match's fields). A cell graded before `matches` was recorded is counted + (`N cell(s) graded without hit details`), not listed. - `repo compare` reduces cells to one value per (task, arm, model) first: repetitions share a task's difficulty and are not paired across arms, so the task is the unit. Per treatment arm, model and measure (gate share, score, diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 035933761..34d2ab57f 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -378,13 +378,17 @@ def _harness_bin(name: str) -> Path: def cmd_repo_report(args: argparse.Namespace) -> None: - """Per (task, arm, model): gated success, grader failures, missing cells, cost, turns.""" - from dokimi.repos.report import format_report, summarize + """Per (task, arm, model): gated success, grader failures, missing cells, cost, turns; or the replay hits.""" + from dokimi.repos.report import format_hits, format_report, load_hits, summarize batch_dir = Path(args.batch_dir) if not batch_dir.is_dir(): print(f"✗ not a directory: {batch_dir}", file=sys.stderr) sys.exit(1) + if args.hits: + found = load_hits(batch_dir) + print(json.dumps(found, indent=2) if args.json else format_hits(found)) + return summary = summarize(batch_dir) print(json.dumps(summary, indent=2) if args.json else format_report(summary)) @@ -535,6 +539,9 @@ def register(repo_sub, positive_int, source_help: str, cache_help: str) -> None: report_p = repo_sub.add_parser("report", help="Per (task, arm, model) table over a batch directory") report_p.add_argument("batch_dir", help="/ of `repo run`") report_p.add_argument("--json", action="store_true", help="Print the summary as JSON") + report_p.add_argument("--hits", action="store_true", + help="Instead of the table: each (thread, finding) pair that credited a " + "dokimi:review_replay thread, by task, arm and rep (grade.json details.matches)") report_p.set_defaults(func=cmd_repo_report) compare_p = repo_sub.add_parser( diff --git a/dokimi/dokimi/repos/graders/review.py b/dokimi/dokimi/repos/graders/review.py index 721f2cfc8..18706012d 100644 --- a/dokimi/dokimi/repos/graders/review.py +++ b/dokimi/dokimi/repos/graders/review.py @@ -41,7 +41,12 @@ to one and nearest first, so one finding credits at most one thread and one thread at most one finding. Location is the only criterion (no judge, deterministic), so ``pass`` is null: humans miss problems and raise -non-defects, and neither recall nor precision is a verdict. +non-defects, and neither recall nor precision is a verdict. ``matches`` lists +each credited pair in ``threads_hit`` order, so a grade can be checked without +the task: the thread (``thread``, ``thread_file``, ``thread_line``, +``comment``), the finding (``finding``, its index among the answer's objects; +``file`` repository-relative and ``line`` as matched; ``severity``, +``message``) and their ``distance``; strings are cut to 200 characters. ``precision_proxy`` is the share of findings within the window of any thread, a lower bound on precision. ``recall`` equals ``recall_all`` and ``decoys_flagged`` is ``[]``, so a seeds-style report applies unchanged. @@ -216,18 +221,33 @@ def _ratio(a, b): def _replay_pairs(gold, findings, window): - """``(pairs, near)``: every (distance, finding index, thread id) within ``window``, and how many - findings are near any thread.""" + """``(pairs, near)``: every (distance, finding index, thread id, thread) within ``window``, and how + many findings are near any thread.""" pairs, near = [], 0 for i, f in enumerate(findings): path, _, line = finding_fields(f) - close = [(abs(line - g["line"]), i, g["id"]) for g in gold + close = [(abs(line - g["line"]), i, g["id"], g) for g in gold if same_file(g, path) and abs(line - g["line"]) <= window] pairs += close near += bool(close) return pairs, near +CUT = 200 # characters of a comment or message a match keeps + + +def _cut(value): + return "" if value is None else str(value)[:CUT] + + +def _match(g, i, f, distance): + """One credited (thread, finding) pair of ``details.matches``: enough to check it without the task.""" + path, _, line = finding_fields(f) + return {"thread": g["id"], "thread_file": g["file"], "thread_line": g["line"], "comment": _cut(g.get("comment")), + "finding": i, "file": path, "line": line, "severity": _cut(f.get("severity")), + "message": _cut(f.get("message")), "distance": distance} + + def grade_replay(task, task_dir, answer): """``review_replay``: score-only, ``recall`` over the human threads.""" gold = _read_json(task_dir, task.get("gold", "gold.json")) @@ -237,16 +257,17 @@ def grade_replay(task, task_dir, answer): return {"pass": False, "parse_error": True, "recall": zero, "recall_all": zero, "recall_addressed": _ratio(0, sum(bool(g.get("addressed")) for g in gold))} pairs, near = _replay_pairs(gold, findings, task.get("window", 5)) - hit, used = set(), set() + hit, used, credited = set(), set(), {} # nearest first: one finding per thread, one thread per finding; ids in _id_key order, as oneDAL's min() - for _, i, gid in sorted(pairs, key=lambda p: (p[0], p[1], _id_key(p[2]))): + for dist, i, gid, g in sorted(pairs, key=lambda p: (p[0], p[1], _id_key(p[2]))): if i not in used and gid not in hit: used.add(i) hit.add(gid) + credited[gid] = _match(g, i, findings[i], dist) addressed = [g for g in gold if g.get("addressed")] recall_all = _ratio(len(hit), len(gold)) return {"pass": None, "n_findings": len(findings), "n_severe": n_severe(findings), - "threads_hit": _ids(hit), "n_threads": len(gold), + "threads_hit": _ids(hit), "matches": [credited[gid] for gid in _ids(hit)], "n_threads": len(gold), "recall_all": recall_all, "recall": recall_all, "recall_addressed": _ratio(sum(g["id"] in hit for g in addressed), len(addressed)), "recall_by_kind": _per_group(gold, "kind", hit), diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index 30a0d6e69..a467ce838 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -31,6 +31,15 @@ A row's ``process`` holds the mean ``result.json`` ``process`` counts of its cells (:mod:`.process_counts`), printed on a line of its own under the row. + +A row's ``parse_errors`` counts its cells whose ``grade.json`` has +``metrics.parse_error`` true: the grader ran, and the agent's answer held no +findings JSON that parsed, so the run counts as failed (contract §4.5). The +table notes it only when it is not 0. + +``--hits`` (:func:`load_hits`) lists instead each credited (thread, finding) +pair of the ``dokimi:review_replay`` cells, from ``grade.json`` +``details.matches``. """ from __future__ import annotations @@ -126,6 +135,7 @@ def summarize(batch_dir: Path) -> dict[str, Any]: if expecting: row["skill_expected"] = len(expecting) row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in expecting) + row["parse_errors"] = sum(_metrics(batch_dir / r["cell"]).get("parse_error") is True for r in recs) rows.append(row) return {"batch": str(batch_dir), "cells": len(cells), "missing": missing, "rows": rows, "unsupported": load_unsupported(batch_dir), @@ -153,14 +163,7 @@ def format_report(summary: dict[str, Any]) -> str: turns = "-" if r["turns"] is None else f"{r['turns']:.1f}" lines.append(f"{r['task']:<{width}} {r['arm']:<{arm_w}} {r['model']:<{model_w}} {r['n']:>3} " f"{result:<22} {r['grader_failures']:>11} {r['missing']:>7} {cost:>8} {turns:>6}") - noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") - notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] - notes += [f"{r[k]}/{r[d]} {k.replace('_', ' ')}" - for k, d in (("skill_invoked", "skill_measured"), ("skill_selected_correct", "skill_expected")) - if k in r] - if "uptake" in r: - notes.append(f"arm's additions used in {r['uptake']['used']}/{r['uptake']['cells']} cell(s)" - + unmeasured_note(r["uptake"])) + notes = _row_notes(r) if notes: lines.append(" " * (width + 2) + " " + ", ".join(notes)) lines += process_line(r.get("process"), width) @@ -176,6 +179,87 @@ def format_report(summary: dict[str, Any]) -> str: return "\n".join(lines) +def _row_notes(r: dict[str, Any]) -> list[str]: + noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") + notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] + if r.get("parse_errors"): # the grader ran; the agent's findings JSON is what failed + notes.append(f"{r['parse_errors']} agent answer(s) whose findings JSON did not parse (counted as failed)") + notes += [f"{r[k]}/{r[d]} {k.replace('_', ' ')}" + for k, d in (("skill_invoked", "skill_measured"), ("skill_selected_correct", "skill_expected")) + if k in r] + if "uptake" in r: + notes.append(f"arm's additions used in {r['uptake']['used']}/{r['uptake']['cells']} cell(s)" + + unmeasured_note(r["uptake"])) + return notes + + +def _grade_doc(cell_dir: Path) -> dict[str, Any]: + """The cell's ``grade.json``; empty when it is missing or not a JSON object (a grader failure says why).""" + try: + doc = json.loads((cell_dir / "grade.json").read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + return doc if isinstance(doc, dict) else {} + + +def _metrics(cell_dir: Path) -> dict[str, Any]: + found = _grade_doc(cell_dir).get("metrics") + return found if isinstance(found, dict) else {} + + +REPLAY = "dokimi:review_replay" + + +def load_hits(batch_dir: Path) -> dict[str, Any]: + """``repo report --hits``: each credited pair of the batch's ``dokimi:review_replay`` cells, from their + ``grade.json`` ``details.matches``, by task, arm, model and rep. ``replay_cells`` counts the cells whose + ``grade.json`` names that grader; ``without_matches`` those graded before ``matches`` was recorded.""" + cells, _ = load_cells(batch_dir) + hits: list[dict[str, Any]] = [] + replay = without = 0 + for rec in sorted(cells, key=lambda r: (r["task"], r["arm"], r["model"], r["rep"])): + grade = _grade_doc(batch_dir / rec["cell"]) + if grade.get("grader") != REPLAY: + continue + replay += 1 + details = grade.get("details") + details = details if isinstance(details, dict) else {} + matches = details.get("matches") + if not isinstance(matches, list): # a parse error or a grader error credits no thread: nothing is missing + without += "threads_hit" in details + continue + hits += [{"task": rec["task"], "arm": rec["arm"], "model": rec["model"], "rep": rec["rep"], + "cell": rec["cell"], **m} for m in matches if isinstance(m, dict)] + return {"batch": str(batch_dir), "replay_cells": replay, "without_matches": without, "hits": hits} + + +def format_hits(found: dict[str, Any], width: int = 100) -> str: + """Two lines per credited pair: the thread and its comment, then the finding that hit it.""" + if not found["replay_cells"]: + return f"{found['batch']}: no {REPLAY} cell" + lines = [f"{found['batch']}: {len(found['hits'])} credited hit(s) in {found['replay_cells']} {REPLAY} cell(s)"] + task = cell = None + for h in found["hits"]: + if h["task"] != task: + task = h["task"] + lines.append(str(task)) + if h["cell"] != cell: + cell = h["cell"] + lines.append(f" {h['arm']} {h['model']} r{h['rep']}") + lines.append(f" {h.get('thread')} {h.get('thread_file')}:{h.get('thread_line')} " + + _one_line(h.get("comment"), width)) + lines.append(f" finding {h.get('finding')} {h.get('file')}:{h.get('line')} {h.get('severity')}, " + f"{h.get('distance')} line(s) away: " + _one_line(h.get("message"), width)) + if found["without_matches"]: + lines.append(f"{found['without_matches']} cell(s) graded without hit details") + return "\n".join(lines) + + +def _one_line(text: Any, width: int) -> str: + flat = " ".join(str(text or "").split()) + return flat if len(flat) <= width else flat[:width - 3] + "..." + + def _unsupported_lines(cells: list[dict[str, Any]]) -> list[str]: by_task: dict[tuple[str, str], int] = defaultdict(int) for u in cells: diff --git a/dokimi/tests/test_repo_builtin_graders.py b/dokimi/tests/test_repo_builtin_graders.py index aaa0f3b41..fe3bcaece 100644 --- a/dokimi/tests/test_repo_builtin_graders.py +++ b/dokimi/tests/test_repo_builtin_graders.py @@ -204,6 +204,29 @@ def test_replay_is_one_to_one_nearest_first_within_the_window(tmp_path): "recall_addressed": 0.0} +def test_replay_matches_record_each_credited_pair_nearest_first(tmp_path): + d = _task_dir(tmp_path, "replay_mul") + # object 1 (line 4) is near both threads: t_doc at 0 takes it, so t_op goes to object 2 at 1, not object 0 at 4 + findings = ["not an object", _f(9, message="far"), _f(4, severity="major", message="near both"), + _f("6", file="/eval/run/workspace/src/calc.py", message="m" * 300)] + g = review.grade_replay({}, d, fence(findings)) + assert g["threads_hit"] == ["t_doc", "t_op"] and g["recall"] == 1.0 and g["precision_proxy"] == 1.0 + assert g["matches"] == [ + {"thread": "t_doc", "thread_file": "src/calc.py", "thread_line": 4, "comment": "docstring?", + "finding": 1, "file": "src/calc.py", "line": 4, "severity": "major", "message": "near both", "distance": 0}, + {"thread": "t_op", "thread_file": "src/calc.py", "thread_line": 5, + "comment": "mul adds instead of multiplying\nsecond line", "finding": 2, "file": "src/calc.py", "line": 6, + "severity": "minor", "message": "m" * 200, "distance": 1}] + # the entry script keeps them under details, so grade.json alone is enough to check a hit + doc = json.loads((_script(tmp_path, "grade", "review_replay", "replay_mul", fence(findings)) / "grade.json") + .read_text()) + assert doc["details"]["matches"] == g["matches"] and "matches" not in doc["metrics"] and check_grade(doc) == [] + assert review.grade_replay({}, d, fence([_f(30)]))["matches"] == [] + # t_op is credited first (distance 0), but matches follow threads_hit + later = review.grade_replay({}, d, fence([_f(5), _f(3)]))["matches"] + assert [(m["thread"], m["finding"], m["distance"]) for m in later] == [("t_doc", 1, 1), ("t_op", 0, 0)] + + @pytest.mark.outcome("cjm-builtin-review-graders") def test_matching_rules_as_the_contract_states_them(tmp_path): """§4.5: same file only, kw case-insensitive, replay within ``window`` (default 5) inclusive.""" diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py new file mode 100644 index 000000000..b4c465ed0 --- /dev/null +++ b/dokimi/tests/test_repo_report_hits.py @@ -0,0 +1,111 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""`repo report --hits` (each credited replay pair) and the parse-error count of `repo report`, over hand-written +batches graded by the real ``dokimi:review_replay`` grader.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from dokimi.cli import build_parser +from dokimi.repos.graders import main, review +from dokimi.repos.report import format_hits, format_report, load_hits, summarize +from tests.test_repo_builtin_graders import GOLD, _f, fence +from tests.test_repo_compare import _cell + +REPLAY = "dokimi:review_replay" + + +def _graded(tmp: Path, answer: str, *, old: bool = False, grader: str = REPLAY) -> dict: + """``grade.json`` as the entry script writes it for ``answer`` against ``GOLD``; ``old``: as graded before + ``details.matches`` existed.""" + (tmp / "gold").mkdir(exist_ok=True) + (tmp / "gold" / "gold.json").write_text(json.dumps(GOLD)) + metrics, details = main.split_result(review.grade_replay({}, tmp / "gold", answer)) + if old: + details.pop("matches") + return {"schema": "repo_grade.v1", "grader": grader, "pass": None, "metrics": metrics, + "details": {**details, "grader_version": review.VERSION}, "errors": []} + + +def _put(batch: Path, task: str, arm: str, rep: int, grade: dict) -> None: + _cell(batch, task, arm, rep, score=grade["metrics"].get("recall") or 0.0) + (batch / f"{task}__{arm.replace(':', '-')}__m-1__r{rep}" / "grade.json").write_text(json.dumps(grade)) + + +def _batch(tmp: Path) -> Path: + batch = tmp / "b1" + todo = _f(6, message="The TODO is repeated five\ntimes and has no issue URL") + _put(batch, "pr3098-t2", "guidance:none", 1, _graded(tmp, fence([todo]))) + _put(batch, "pr3098-t2", "guidance:none", 2, _graded(tmp, fence([_f(4, message="x" * 150), _f(5, message="mul adds")]))) + _put(batch, "pr3098-t2", "guidance:raw", 1, _graded(tmp, fence([_f(5)]), old=True)) + # the parse error the dpnp run hit: a "line" JSON cannot read, so the whole fence is unreadable + unparsed = '```json\n[{"file": "dpnp/dpnp_iface_indexing.py", "line": 1:1, "message": "m"}]\n```' + _put(batch, "pr3014-t1", "guidance:none", 2, _graded(tmp, unparsed)) + _put(batch, "seeds", "guidance:none", 1, _graded(tmp, fence([_f(5)]), grader="dokimi:review_seeds")) + return batch + + +def test_hits_list_each_credited_pair_by_task_arm_and_rep(tmp_path): + batch = _batch(tmp_path) + found = load_hits(batch) + assert (found["replay_cells"], found["without_matches"]) == (4, 1) # the parse error has no hit to miss + assert [(h["task"], h["arm"], h["rep"], h["thread"], h["finding"], h["distance"]) for h in found["hits"]] == [ + ("pr3098-t2", "guidance:none", 1, "t_op", 0, 1), + ("pr3098-t2", "guidance:none", 2, "t_doc", 0, 0), ("pr3098-t2", "guidance:none", 2, "t_op", 1, 0)] + assert found["hits"][0] == { + "task": "pr3098-t2", "arm": "guidance:none", "model": "m-1", "rep": 1, "cell": "pr3098-t2__guidance-none__m-1__r1", + "thread": "t_op", "thread_file": "src/calc.py", "thread_line": 5, + "comment": "mul adds instead of multiplying\nsecond line", "finding": 0, "file": "src/calc.py", "line": 6, + "severity": "minor", "message": "The TODO is repeated five\ntimes and has no issue URL", "distance": 1} + assert format_hits(found).splitlines() == [ + f"{batch}: 3 credited hit(s) in 4 {REPLAY} cell(s)", + "pr3098-t2", + " guidance:none m-1 r1", + " t_op src/calc.py:5 mul adds instead of multiplying second line", + " finding 0 src/calc.py:6 minor, 1 line(s) away: The TODO is repeated five times and has no issue URL", + " guidance:none m-1 r2", + " t_doc src/calc.py:4 docstring?", + " finding 0 src/calc.py:4 minor, 0 line(s) away: " + "x" * 97 + "...", + " t_op src/calc.py:5 mul adds instead of multiplying second line", + " finding 1 src/calc.py:5 minor, 0 line(s) away: mul adds", + "1 cell(s) graded without hit details"] + + +def _cli(capsys, *argv: str) -> str: + args = build_parser().parse_args(["repo", "report", *argv]) + args.func(args) + return capsys.readouterr().out + + +def test_report_hits_flag_prints_the_pairs_and_the_default_report_stays_the_table(tmp_path, capsys): + batch = _batch(tmp_path) + text = _cli(capsys, str(batch), "--hits") + assert text.startswith(f"{batch}: 3 credited hit(s)") and "finding 0 src/calc.py:6" in text + records = json.loads(_cli(capsys, str(batch), "--hits", "--json")) + assert [(h["cell"], h["thread"]) for h in records["hits"]][:2] == [ + ("pr3098-t2__guidance-none__m-1__r1", "t_op"), ("pr3098-t2__guidance-none__m-1__r2", "t_doc")] + assert records["without_matches"] == 1 + table = _cli(capsys, str(batch)) + assert table.startswith(f"{batch}: 5 finished cells") and "credited" not in table and "finding 0" not in table + + +def test_hits_on_a_batch_with_no_replay_cell_say_so(tmp_path): + _cell(tmp_path, "a", "guidance:none", 1) + found = load_hits(tmp_path) + assert found["hits"] == [] and found["replay_cells"] == 0 + assert format_hits(found) == f"{tmp_path}: no {REPLAY} cell" + + +def test_report_counts_answers_that_did_not_parse_per_row(tmp_path): + batch = _batch(tmp_path) + rows = {(r["task"], r["arm"]): r for r in summarize(batch)["rows"]} + unparsed = rows[("pr3014-t1", "guidance:none")] + assert unparsed["parse_errors"] == 1 and unparsed["grader_failures"] == 0 # the agent failed, not the grader + assert [r["parse_errors"] for k, r in rows.items() if k[0] != "pr3014-t1"] == [0, 0, 0] + lines = format_report(summarize(batch)).splitlines() + at = next(i for i, line in enumerate(lines) if line.startswith("pr3014-t1")) + assert lines[at + 1].strip() == "1 agent answer(s) whose findings JSON did not parse (counted as failed)" + assert sum("did not parse" in line for line in lines) == 1 From 29fed9c1f246fb7d6110e636390a16e7cacb50bb Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 29 Oct 2026 18:03:51 +0000 Subject: [PATCH 02/11] docs(repo): say why the replay grader version stays 1 with details.matches A field added to details that leaves every metric and pass as they were does not bump VERSION; a reader tells such a grade by the field being present. Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/graders/review.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/repos/graders/review.py b/dokimi/dokimi/repos/graders/review.py index 18706012d..7eeeac56f 100644 --- a/dokimi/dokimi/repos/graders/review.py +++ b/dokimi/dokimi/repos/graders/review.py @@ -63,7 +63,9 @@ import json import re -VERSION = "1" # bump when a result for the same inputs changes; grade.json records it +# Bump when a result for the same inputs changes (a metric or ``pass``); grade.json records it. A field added +# to ``details`` that leaves the result as it was (``matches``) does not: a reader tells it by its presence. +VERSION = "1" SEVERE = ("blocker", "major") From 8e43a2b7edb467520386301450d682ecddb9c909 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 29 Oct 2026 18:03:51 +0000 Subject: [PATCH 03/11] test(repo): report --hits and the parse-error count on unreadable grades Covers a grade.json that is not JSON or not an object, details that is not an object, non-object and textless entries in matches, metrics that is not an object and a non-boolean parse_error. Signed-off-by: Anton Volkov --- dokimi/tests/test_repo_report_hits.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index b4c465ed0..0e3b97ed9 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -109,3 +109,20 @@ def test_report_counts_answers_that_did_not_parse_per_row(tmp_path): at = next(i for i, line in enumerate(lines) if line.startswith("pr3014-t1")) assert lines[at + 1].strip() == "1 agent answer(s) whose findings JSON did not parse (counted as failed)" assert sum("did not parse" in line for line in lines) == 1 + + +def test_hits_and_report_skip_unreadable_grades_and_list_a_match_without_text(tmp_path): + batch = tmp_path / "b1" + bad = {1: "{not json", 2: "[]", + 3: json.dumps({"grader": REPLAY, "metrics": {"parse_error": "yes"}, "details": []}), + 4: json.dumps({"grader": REPLAY, "metrics": [], "details": {"threads_hit": ["t"], + "matches": [None, "t", {"thread": "t", "finding": 0, "distance": 0}]}})} + for rep, text in bad.items(): + _cell(batch, "a", "guidance:none", rep) + (batch / f"a__guidance-none__m-1__r{rep}" / "grade.json").write_text(text) + found = load_hits(batch) + assert (found["replay_cells"], found["without_matches"]) == (2, 0) # 1 and 2 name no grader + assert [(h["rep"], h["thread"]) for h in found["hits"]] == [(4, "t")] + assert format_hits(found).splitlines()[-2:] == [ + " t None:None ", " finding 0 None:None None, 0 line(s) away: "] + assert [r["parse_errors"] for r in summarize(batch)["rows"]] == [0] # a non-boolean parse_error is not one From 0a7539aff36c136296d961d607e18ed851b4804d Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 29 Oct 2026 19:28:40 +0100 Subject: [PATCH 04/11] fix(repo): report --hits refuses a cell with no rep, as the table does After #238, repo report refuses a result.json of the wrong shape with a CellFileError naming it (exit 1). load_cells checks task, arm and model; --hits also sorts by rep, which the table never reads, so a cell without it stopped --hits with a KeyError traceback. load_hits now refuses it the same way: "/result.json has no usable `rep`". Signed-off-by: Anton Volkov --- dokimi/dokimi/repos/report.py | 3 +++ dokimi/tests/test_repo_report_hits.py | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index 68deadce2..f04895dd5 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -340,6 +340,9 @@ def load_hits(batch_dir: Path) -> dict[str, Any]: ``grade.json`` ``details.matches``, by task, arm, model and rep. ``replay_cells`` counts the cells whose ``grade.json`` names that grader; ``without_matches`` those graded before ``matches`` was recorded.""" cells, _ = load_cells(batch_dir) + bad = next((r for r in cells if not isinstance(r.get("rep"), int)), None) # load_cells checks the rest of the key + if bad is not None: + raise CellFileError(f"{batch_dir / bad['cell'] / 'result.json'} has no usable `rep`") hits: list[dict[str, Any]] = [] replay = without = 0 for rec in sorted(cells, key=lambda r: (r["task"], r["arm"], r["model"], r["rep"])): diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index 0e3b97ed9..8d07f62c6 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -9,6 +9,8 @@ import json from pathlib import Path +import pytest + from dokimi.cli import build_parser from dokimi.repos.graders import main, review from dokimi.repos.report import format_hits, format_report, load_hits, summarize @@ -126,3 +128,15 @@ def test_hits_and_report_skip_unreadable_grades_and_list_a_match_without_text(tm assert format_hits(found).splitlines()[-2:] == [ " t None:None ", " finding 0 None:None None, 0 line(s) away: "] assert [r["parse_errors"] for r in summarize(batch)["rows"]] == [0] # a non-boolean parse_error is not one + + +def test_hits_refuse_a_cell_with_no_rep_as_the_table_refuses_a_malformed_cell(tmp_path, capsys): + batch = _batch(tmp_path) + path = batch / "pr3098-t2__guidance-none__m-1__r1" / "result.json" + rec = json.loads(path.read_text()) + del rec["rep"] # the table never reads it; --hits sorts by it + path.write_text(json.dumps(rec)) + with pytest.raises(SystemExit) as stopped: + _cli(capsys, str(batch), "--hits") + assert stopped.value.code == 1 + assert capsys.readouterr().err.strip() == f"✗ {path} has no usable `rep`" From 89f282af6c12be44702aca479c90e5765c759dff Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 29 Oct 2026 19:23:14 +0000 Subject: [PATCH 05/11] test(repo): pin "" for missing match text and rep order of --hits Review comment 4416180649: test_replay_matches_record_each_credited_pair_ nearest_first now grades a thread with no `comment` against a finding with no `severity` or `message` and compares the match with a literal whose three fields are "". Before, every input supplied those strings, so `_cut` could turn None into "None" and the suite still passed. Review comment 4416180698: test_hits_list_each_credited_pair_by_task_ arm_and_rep adds a replay cell with rep 10 next to rep 2. By directory name r10 sorts before r2, so the expected listing, rep 1, 2, then 10, now fails if load_hits drops its (task, arm, model, rep) sort or sorts the rep as a string. Signed-off-by: Anton Volkov --- dokimi/tests/test_repo_builtin_graders.py | 5 +++++ dokimi/tests/test_repo_report_hits.py | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/dokimi/tests/test_repo_builtin_graders.py b/dokimi/tests/test_repo_builtin_graders.py index fe3bcaece..e877d88c8 100644 --- a/dokimi/tests/test_repo_builtin_graders.py +++ b/dokimi/tests/test_repo_builtin_graders.py @@ -222,6 +222,11 @@ def test_replay_matches_record_each_credited_pair_nearest_first(tmp_path): .read_text()) assert doc["details"]["matches"] == g["matches"] and "matches" not in doc["metrics"] and check_grade(doc) == [] assert review.grade_replay({}, d, fence([_f(30)]))["matches"] == [] + # a thread with no comment and a finding with no severity or message keep "", not "None" + write(tmp_path, "bare/gold.json", json.dumps([{"id": "t", "file": "src/calc.py", "line": 5, "kind": "docs"}])) + assert review.grade_replay({}, tmp_path / "bare", fence([{"file": "src/calc.py", "line": 5}]))["matches"] == [ + {"thread": "t", "thread_file": "src/calc.py", "thread_line": 5, "comment": "", + "finding": 0, "file": "src/calc.py", "line": 5, "severity": "", "message": "", "distance": 0}] # t_op is credited first (distance 0), but matches follow threads_hit later = review.grade_replay({}, d, fence([_f(5), _f(3)]))["matches"] assert [(m["thread"], m["finding"], m["distance"]) for m in later] == [("t_doc", 1, 1), ("t_op", 0, 0)] diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index 8d07f62c6..20b024d92 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -52,18 +52,21 @@ def _batch(tmp: Path) -> Path: def test_hits_list_each_credited_pair_by_task_arm_and_rep(tmp_path): batch = _batch(tmp_path) + # by directory name r10 sorts before r2; the hits follow rep order + _put(batch, "pr3098-t2", "guidance:none", 10, _graded(tmp_path, fence([_f(5)]))) found = load_hits(batch) - assert (found["replay_cells"], found["without_matches"]) == (4, 1) # the parse error has no hit to miss + assert (found["replay_cells"], found["without_matches"]) == (5, 1) # the parse error has no hit to miss assert [(h["task"], h["arm"], h["rep"], h["thread"], h["finding"], h["distance"]) for h in found["hits"]] == [ ("pr3098-t2", "guidance:none", 1, "t_op", 0, 1), - ("pr3098-t2", "guidance:none", 2, "t_doc", 0, 0), ("pr3098-t2", "guidance:none", 2, "t_op", 1, 0)] + ("pr3098-t2", "guidance:none", 2, "t_doc", 0, 0), ("pr3098-t2", "guidance:none", 2, "t_op", 1, 0), + ("pr3098-t2", "guidance:none", 10, "t_op", 0, 0)] assert found["hits"][0] == { "task": "pr3098-t2", "arm": "guidance:none", "model": "m-1", "rep": 1, "cell": "pr3098-t2__guidance-none__m-1__r1", "thread": "t_op", "thread_file": "src/calc.py", "thread_line": 5, "comment": "mul adds instead of multiplying\nsecond line", "finding": 0, "file": "src/calc.py", "line": 6, "severity": "minor", "message": "The TODO is repeated five\ntimes and has no issue URL", "distance": 1} assert format_hits(found).splitlines() == [ - f"{batch}: 3 credited hit(s) in 4 {REPLAY} cell(s)", + f"{batch}: 4 credited hit(s) in 5 {REPLAY} cell(s)", "pr3098-t2", " guidance:none m-1 r1", " t_op src/calc.py:5 mul adds instead of multiplying second line", @@ -73,6 +76,9 @@ def test_hits_list_each_credited_pair_by_task_arm_and_rep(tmp_path): " finding 0 src/calc.py:4 minor, 0 line(s) away: " + "x" * 97 + "...", " t_op src/calc.py:5 mul adds instead of multiplying second line", " finding 1 src/calc.py:5 minor, 0 line(s) away: mul adds", + " guidance:none m-1 r10", + " t_op src/calc.py:5 mul adds instead of multiplying second line", + " finding 0 src/calc.py:5 minor, 0 line(s) away: ", "1 cell(s) graded without hit details"] From 9092ed4a1edab41355a796829f618675082acf02 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 29 Oct 2026 19:30:25 -0700 Subject: [PATCH 06/11] fix(repo): report reads grade.json as the host does; --hits skips grader failures `repo report` reads every cell's grade.json for parse_errors, and --hits for the matches. _grade_doc read it with a plain read_text, so a FIFO blocked the whole table, a link out of the batch was followed, and a deeply nested file raised RecursionError. It now goes through grading.read_grade_json, the reader read_grade uses (lstat, O_NOFOLLOW, and RecursionError as no grade). read_grade gets the RecursionError case too: it used to crash the grading of such a cell. --hits also credits no match of a cell whose result says grader_failure, which the table scores as 0. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/repos/grading.py | 15 +++++--- dokimi/dokimi/repos/report.py | 12 +++---- dokimi/tests/test_repo_report_hits.py | 49 +++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/dokimi/dokimi/repos/grading.py b/dokimi/dokimi/repos/grading.py index c4d0174e6..0c0cbf942 100644 --- a/dokimi/dokimi/repos/grading.py +++ b/dokimi/dokimi/repos/grading.py @@ -226,18 +226,25 @@ def replaced_files(run_dir: Path) -> list[str]: return out -def read_grade(run_dir: Path) -> tuple[dict[str, Any] | None, list[str]]: - path = run_dir / "grade.json" +def read_grade_json(path: Path) -> tuple[Any, list[str]]: + """The JSON value of a cell's ``grade.json``, read as the host must: a link, FIFO or other non-regular file + is not read, and a file nested too deep to parse is no grade. ``(None, problems)`` when it is not one.""" try: if not stat.S_ISREG(os.lstat(path).st_mode): return None, ["grade.json is not a regular file"] # O_NOFOLLOW: a link swapped in after the lstat is not followed either with os.fdopen(os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)), "rb") as f: - doc = json.loads(f.read().decode("utf-8")) + return json.loads(f.read().decode("utf-8")), [] except FileNotFoundError: return None, ["no grade.json written"] - except (OSError, ValueError) as exc: + except (OSError, ValueError, RecursionError) as exc: return None, [f"grade.json is not JSON: {exc}"] + + +def read_grade(run_dir: Path) -> tuple[dict[str, Any] | None, list[str]]: + doc, problems = read_grade_json(run_dir / "grade.json") + if problems: + return None, problems problems = check_grade(doc) return (None, [f"grade.json is not {GRADE_SCHEMA}: {p}" for p in problems]) if problems else (doc, []) diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index f04895dd5..b7c895718 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -54,7 +54,7 @@ import math -from .grading import SCORE_BOUND +from .grading import SCORE_BOUND, read_grade_json from .process_counts import process_line, process_means from .runner import UNSUPPORTED_FILE, slug @@ -319,11 +319,9 @@ def _row_notes(r: dict[str, Any]) -> list[str]: def _grade_doc(cell_dir: Path) -> dict[str, Any]: - """The cell's ``grade.json``; empty when it is missing or not a JSON object (a grader failure says why).""" - try: - doc = json.loads((cell_dir / "grade.json").read_text(encoding="utf-8")) - except (OSError, ValueError): - return {} + """The cell's ``grade.json``, read as the grader's host reads it; empty when it is missing, not a regular + file, too deep to parse or not a JSON object (a grader failure says why).""" + doc, _ = read_grade_json(cell_dir / "grade.json") return doc if isinstance(doc, dict) else {} @@ -350,6 +348,8 @@ def load_hits(batch_dir: Path) -> dict[str, Any]: if grade.get("grader") != REPLAY: continue replay += 1 + if rec["grade"]["status"] == "grader_failure": # the table scores this cell 0: no hit is credited to it + continue details = grade.get("details") details = details if isinstance(details, dict) else {} matches = details.get("matches") diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index 20b024d92..64341938d 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -7,6 +7,8 @@ from __future__ import annotations import json +import os +import threading from pathlib import Path import pytest @@ -146,3 +148,50 @@ def test_hits_refuse_a_cell_with_no_rep_as_the_table_refuses_a_malformed_cell(tm _cli(capsys, str(batch), "--hits") assert stopped.value.code == 1 assert capsys.readouterr().err.strip() == f"✗ {path} has no usable `rep`" + + +def test_a_grade_nested_too_deep_or_linked_out_of_the_batch_is_no_grade(tmp_path): + batch = tmp_path / "b1" + outside = tmp_path / "outside.json" # a link out of the batch is not followed + outside.write_text(json.dumps({"grader": REPLAY, "metrics": {"parse_error": True}, + "details": {"matches": [{"thread": "outside", "finding": 0}]}})) + _put(batch, "a", "guidance:none", 1, _graded(tmp_path, fence([_f(5)]))) + _cell(batch, "a", "guidance:none", 2, score=0.0) + _cell(batch, "a", "guidance:none", 3, score=0.0) + (batch / "a__guidance-none__m-1__r2" / "grade.json").write_text("[" * 200000) # RecursionError to parse + link = batch / "a__guidance-none__m-1__r3" / "grade.json" + link.unlink() + link.symlink_to(outside) + assert summarize(batch)["rows"][0]["parse_errors"] == 0 + found = load_hits(batch) + assert (found["replay_cells"], [h["rep"] for h in found["hits"]]) == (1, [1]) + + +def test_a_grade_that_is_a_fifo_is_no_grade_and_does_not_block_the_report(tmp_path): + batch = tmp_path / "b1" + _put(batch, "a", "guidance:none", 1, _graded(tmp_path, fence([_f(5)]))) + _cell(batch, "a", "guidance:none", 2, score=0.0) + fifo = batch / "a__guidance-none__m-1__r2" / "grade.json" + fifo.unlink() + os.mkfifo(fifo) + out: dict = {} + reader = threading.Thread(target=lambda: out.update(summary=summarize(batch), hits=load_hits(batch)), + daemon=True) # a read of a FIFO waits for a writer: the thread is kept apart + reader.start() + reader.join(10) + if reader.is_alive(): # open the write end so the blocked read ends + os.close(os.open(fifo, os.O_WRONLY)) + reader.join(10) + assert not reader.is_alive() and "summary" in out, "a grade.json that is a FIFO blocked the report" + assert out["summary"]["rows"][0]["parse_errors"] == 0 + assert (out["hits"]["replay_cells"], [h["rep"] for h in out["hits"]["hits"]]) == (1, [1]) + + +def test_hits_credit_no_match_of_a_cell_the_table_scores_as_a_grader_failure(tmp_path): + batch = tmp_path / "b1" # written, then the run failed: the table counts the cell as a grader failure + _cell(batch, "a", "guidance:none", 1, failure=True) + (batch / "a__guidance-none__m-1__r1" / "grade.json").write_text(json.dumps(_graded(tmp_path, fence([_f(5)])))) + _put(batch, "a", "guidance:none", 2, _graded(tmp_path, fence([_f(5)]))) + assert [r["grader_failures"] for r in summarize(batch)["rows"]] == [1] + found = load_hits(batch) + assert (found["replay_cells"], [h["rep"] for h in found["hits"]]) == (2, [2]) From 414855f9002a7b659f88ca256e5fd931f10be4ef Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 30 Oct 2026 10:00:13 +0000 Subject: [PATCH 07/11] fix(repo): parse_errors skips grader failures; score rows say scored 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review comment 4418950465 (the parse_errors half): a cell whose result.json says grader_failure was counted both in the row's grader_failures and, when its grade.json had metrics.parse_error, in parse_errors. summarize now leaves it out of parse_errors, as 9092ed4 leaves it out of --hits. Review comment 4418950480: the parse-error note said "(counted as failed)" on every row, but a score-only row (every dokimi:review_replay row) has no failure count; the cell is in the mean as a score of 0. The note now says "(scored 0)" on a score row and keeps "(counted as failed)" on a gate row. The module docstring, contract §9 and the changelog fragment say both. Signed-off-by: Anton Volkov --- ...1029_120000_antonwolfy_replay-hit-pairs.md | 2 +- dokimi/docs/repo-evaluation-contract.md | 6 +++-- dokimi/dokimi/repos/report.py | 12 ++++++---- dokimi/tests/test_repo_report_hits.py | 22 ++++++++++++++++++- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md index 5c29acb67..12f2bc15a 100644 --- a/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md +++ b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md @@ -1,4 +1,4 @@ ### Added - **Replay hits you can check** — a `dokimi:review_replay` grade now records `details.matches`: per credited thread, the thread (id, anchor, gold comment) and the finding that hit it (its index, location, severity, message) with their distance in lines, so a hit can be read from `grade.json` alone. The metrics are unchanged. `dokimi repo report --hits` lists those pairs by task, arm, model and rep (`--json` for records), and counts the cells graded before `matches` existed. -- **Parse errors in `repo report`** — a row notes how many of its cells had an agent answer whose findings JSON did not parse (`grade.json` `parse_error`), and `--json` gives `parse_errors` per row, so a lower recall from answers that did not parse can be told from one from worse reviews. +- **Parse errors in `repo report`** — a row notes how many of its cells had an agent answer whose findings JSON did not parse (`grade.json` `parse_error`): scored 0 on a score-only row, counted as failed on a gate row, and not counted for a cell already counted as a grader failure. `--json` gives `parse_errors` per row, so a lower recall from answers that did not parse can be told from one from worse reviews. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index da4b1d917..329d20496 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -1319,8 +1319,10 @@ python cli.py repo compare repo-runs/replay --baseline guidance:none --metric re as paths; `counter:` for `process_metrics`); a `null` or absent `process` is left out, not counted as zero. A row whose cells include agent answers with no findings JSON that parsed (`grade.json` - `parse_error`, §4.5) notes how many; `--json` gives `parse_errors` on every - row. `repo report --hits` prints, instead of the table, each credited pair of + `parse_error`, §4.5) notes how many, `(scored 0)` on a score-only row and + `(counted as failed)` on a gate row; a cell whose `result.json` says + `grader_failure` is counted under `grader_fail` instead. `--json` gives + `parse_errors` on every row. `repo report --hits` prints, instead of the table, each credited pair of the `dokimi:review_replay` cells (`details.matches`, §4.5) by task, arm, model and rep: the thread and its comment, then the finding that hit it. `--hits --json` gives `{batch, replay_cells, without_matches, hits}`, one diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index b7c895718..4eff02921 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -34,8 +34,10 @@ A row's ``parse_errors`` counts its cells whose ``grade.json`` has ``metrics.parse_error`` true: the grader ran, and the agent's answer held no -findings JSON that parsed, so the run counts as failed (contract §4.5). The -table notes it only when it is not 0. +findings JSON that parsed, so the run counts as failed (contract §4.5). A +cell whose ``result.json`` says ``grader_failure`` is counted there, not +here. The table notes it only when it is not 0: ``scored 0`` on a score row, +``counted as failed`` on a gate row. ``--hits`` (:func:`load_hits`) lists instead each credited (thread, finding) pair of the ``dokimi:review_replay`` cells, from ``grade.json`` @@ -260,7 +262,8 @@ def summarize(batch_dir: Path) -> dict[str, Any]: if expecting: row["skill_expected"] = len(expecting) row["skill_selected_correct"] = sum(bool(u["skill_selected_correct"]) for u in expecting) - row["parse_errors"] = sum(_metrics(batch_dir / r["cell"]).get("parse_error") is True for r in recs) + row["parse_errors"] = sum(r["grade"]["status"] != "grader_failure" # counted in grader_failures + and _metrics(batch_dir / r["cell"]).get("parse_error") is True for r in recs) rows.append(row) return {"batch": str(batch_dir), "cells": len(cells), "missing": missing, "rows": rows, "unsupported": load_unsupported(batch_dir), @@ -308,7 +311,8 @@ def _row_notes(r: dict[str, Any]) -> list[str]: noted = ("agent_timeouts", "agent_errors", "over_budget", "cost_unknown") notes = [f"{r[k]} {k.replace('_', ' ')}" for k in noted if r[k]] if r.get("parse_errors"): # the grader ran; the agent's findings JSON is what failed - notes.append(f"{r['parse_errors']} agent answer(s) whose findings JSON did not parse (counted as failed)") + counted = "scored 0" if r["kind"] == "score" else "counted as failed" + notes.append(f"{r['parse_errors']} agent answer(s) whose findings JSON did not parse ({counted})") notes += [f"{r[k]}/{r[d]} {k.replace('_', ' ')}" for k, d in (("skill_invoked", "skill_measured"), ("skill_selected_correct", "skill_expected")) if k in r] diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index 64341938d..b4521b6d1 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -117,10 +117,19 @@ def test_report_counts_answers_that_did_not_parse_per_row(tmp_path): assert [r["parse_errors"] for k, r in rows.items() if k[0] != "pr3014-t1"] == [0, 0, 0] lines = format_report(summarize(batch)).splitlines() at = next(i for i, line in enumerate(lines) if line.startswith("pr3014-t1")) - assert lines[at + 1].strip() == "1 agent answer(s) whose findings JSON did not parse (counted as failed)" + assert lines[at + 1].strip() == "1 agent answer(s) whose findings JSON did not parse (scored 0)" # in the mean assert sum("did not parse" in line for line in lines) == 1 +def test_the_parse_error_note_of_a_gate_row_says_counted_as_failed(tmp_path): + _cell(tmp_path, "gate", "guidance:none", 1, success=False, metrics={"parse_error": True}) + _cell(tmp_path, "gate", "guidance:none", 2) + lines = format_report(summarize(tmp_path)).splitlines() + at = next(i for i, line in enumerate(lines) if line.startswith("gate")) + assert "1/2 pass" in lines[at] + assert lines[at + 1].strip() == "1 agent answer(s) whose findings JSON did not parse (counted as failed)" + + def test_hits_and_report_skip_unreadable_grades_and_list_a_match_without_text(tmp_path): batch = tmp_path / "b1" bad = {1: "{not json", 2: "[]", @@ -195,3 +204,14 @@ def test_hits_credit_no_match_of_a_cell_the_table_scores_as_a_grader_failure(tmp assert [r["grader_failures"] for r in summarize(batch)["rows"]] == [1] found = load_hits(batch) assert (found["replay_cells"], [h["rep"] for h in found["hits"]]) == (2, [2]) + + +def test_parse_errors_leave_out_a_cell_the_table_counts_as_a_grader_failure(tmp_path): + batch = tmp_path / "b1" + unparsed = _graded(tmp_path, "no findings JSON here") + assert unparsed["metrics"]["parse_error"] is True + _cell(batch, "a", "guidance:none", 1, failure=True, score=0.0) # the grade was written, then the run failed + (batch / "a__guidance-none__m-1__r1" / "grade.json").write_text(json.dumps(unparsed)) + _put(batch, "a", "guidance:none", 2, unparsed) + row = summarize(batch)["rows"][0] + assert (row["grader_failures"], row["parse_errors"]) == (1, 1) From 6421d626a26e062c7c23f744078028c82564055c Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 30 Oct 2026 10:00:28 +0000 Subject: [PATCH 08/11] docs(cli): repo report --hits help says by task, arm, model and rep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review comment 4418950473: the --hits help said the pairs are listed "by task, arm and rep", but load_hits sorts and format_hits groups them by task, arm, model and rep, as contract §9 and the changelog fragment already say. The help now says so too. Signed-off-by: Anton Volkov --- dokimi/dokimi/commands/repo_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index cc6fafd2f..e4992e0d9 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -551,7 +551,7 @@ def register(repo_sub, positive_int, source_help: str, cache_help: str) -> None: report_p.add_argument("--json", action="store_true", help="Print the summary as JSON") report_p.add_argument("--hits", action="store_true", help="Instead of the table: each (thread, finding) pair that credited a " - "dokimi:review_replay thread, by task, arm and rep (grade.json details.matches)") + "dokimi:review_replay thread, by task, arm, model and rep (grade.json details.matches)") report_p.set_defaults(func=cmd_repo_report) compare_p = repo_sub.add_parser( From e53ab205b915089d10a1f8037897f94d582a2a4b Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 30 Oct 2026 10:01:57 +0000 Subject: [PATCH 09/11] test(repo): FIFO grade test skips without mkfifo and fails on a block Review comment 4420760178: the test called os.mkfifo unguarded, an AttributeError on Windows. It is now skipped where os.mkfifo is missing, as tests/test_task_format_detect.py does. Review comment 4420760207: a report that opened the FIFO could still pass, because the cleanup opened the write end, the blocked read got EOF and the empty file read as no grade. The test now records whether the first join timed out, then opens the write end (non-blocking, a bounded number of times) until every blocked read has ended, and only then asserts that it did not time out. Signed-off-by: Anton Volkov --- dokimi/tests/test_repo_report_hits.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index b4521b6d1..c85c41f4e 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -6,6 +6,7 @@ from __future__ import annotations +import contextlib import json import os import threading @@ -176,6 +177,7 @@ def test_a_grade_nested_too_deep_or_linked_out_of_the_batch_is_no_grade(tmp_path assert (found["replay_cells"], [h["rep"] for h in found["hits"]]) == (1, [1]) +@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="needs os.mkfifo") def test_a_grade_that_is_a_fifo_is_no_grade_and_does_not_block_the_report(tmp_path): batch = tmp_path / "b1" _put(batch, "a", "guidance:none", 1, _graded(tmp_path, fence([_f(5)]))) @@ -188,10 +190,14 @@ def test_a_grade_that_is_a_fifo_is_no_grade_and_does_not_block_the_report(tmp_pa daemon=True) # a read of a FIFO waits for a writer: the thread is kept apart reader.start() reader.join(10) - if reader.is_alive(): # open the write end so the blocked read ends - os.close(os.open(fifo, os.O_WRONLY)) - reader.join(10) - assert not reader.is_alive() and "summary" in out, "a grade.json that is a FIFO blocked the report" + blocked = reader.is_alive() + for _ in range(30): # open the write end so each blocked read ends (the table's, then the one of --hits) + if not reader.is_alive(): + break + with contextlib.suppress(OSError): # ENXIO: no read of the FIFO is waiting right now + os.close(os.open(fifo, os.O_WRONLY | os.O_NONBLOCK)) + reader.join(1) + assert not blocked, "a grade.json that is a FIFO blocked the report" assert out["summary"]["rows"][0]["parse_errors"] == 0 assert (out["hits"]["replay_cells"], [h["rep"] for h in out["hits"]["hits"]]) == (1, [1]) From 29aff4fef06b51bee6779e166d195355463c51bc Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 30 Oct 2026 10:43:27 +0000 Subject: [PATCH 10/11] test(cli): allowlist repo report --hits and refresh the CLI snapshot The merge with main brings the flag registry of #254: its ratchet test fails a spelling that is neither canonical in dokimi/cli_flags.py nor on tests/data/cli_flags_allowlist.txt, and CI fails a stale docs/reference/cli-interface.json. `--hits` switches repo report from the table to the list of credited replay hits; no registry concept names a view switch, so it gets an allowlist row with that reason, and the snapshot is regenerated with what `make cli-snapshot` runs (the only change is the --hits option). #298's scripts/check_comment_blocks.py --base origin/main --head HEAD finds no comment block over two lines among the lines this PR adds or edits, so no comment changes. Signed-off-by: Anton Volkov --- dokimi/docs/reference/cli-interface.json | 16 ++++++++++++++++ dokimi/tests/data/cli_flags_allowlist.txt | 1 + 2 files changed, 17 insertions(+) diff --git a/dokimi/docs/reference/cli-interface.json b/dokimi/docs/reference/cli-interface.json index 3778ecf3d..ad1f9d568 100644 --- a/dokimi/docs/reference/cli-interface.json +++ b/dokimi/docs/reference/cli-interface.json @@ -10792,6 +10792,22 @@ "required": false, "type": null }, + { + "action": "_StoreTrueAction", + "choices": null, + "const": true, + "default": false, + "group_key": null, + "group_required": null, + "hidden": false, + "is_flag": true, + "key": "--hits", + "kind": "option", + "nargs": 0, + "position": null, + "required": false, + "type": null + }, { "action": "_StoreTrueAction", "choices": null, diff --git a/dokimi/tests/data/cli_flags_allowlist.txt b/dokimi/tests/data/cli_flags_allowlist.txt index 6e9ed384d..be3e93b1e 100644 --- a/dokimi/tests/data/cli_flags_allowlist.txt +++ b/dokimi/tests/data/cli_flags_allowlist.txt @@ -76,6 +76,7 @@ --grounded-from-topics --groups --harnesses +--hits # repo report replay hits (#297) adds it: a view switch has no registry concept yet --image --include-disabled # novelty routing (#172) adds it: no registry concept yet; a F1b migration names it --include-resources From 5210ebe35785ddfe495224fb419922eb97ea2322 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 30 Oct 2026 10:44:37 +0000 Subject: [PATCH 11/11] fix(repo): repo report --hits keeps the cell's key over a match's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review comment 4422546939: load_hits built each hit as the cell's task, arm, model, rep and cell followed by **match, so a details.matches entry with a key of one of those names moved the hit to another task or repetition in both --hits and --hits --json. The match is now spread first and the result.json values last. format_hits only reads these records, so it needs no change. A new test grades a cell whose match carries all five colliding keys and checks that the JSON records and the text listing show the cell's own task, arm, model, rep and cell. The contract (§9) and the changelog fragment say the five come from result.json. Signed-off-by: Anton Volkov --- .../20261029_120000_antonwolfy_replay-hit-pairs.md | 2 +- dokimi/docs/repo-evaluation-contract.md | 3 ++- dokimi/dokimi/repos/report.py | 4 ++-- dokimi/tests/test_repo_report_hits.py | 11 +++++++++++ 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md index 12f2bc15a..685e7e1e7 100644 --- a/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md +++ b/dokimi/changelog.d/20261029_120000_antonwolfy_replay-hit-pairs.md @@ -1,4 +1,4 @@ ### Added -- **Replay hits you can check** — a `dokimi:review_replay` grade now records `details.matches`: per credited thread, the thread (id, anchor, gold comment) and the finding that hit it (its index, location, severity, message) with their distance in lines, so a hit can be read from `grade.json` alone. The metrics are unchanged. `dokimi repo report --hits` lists those pairs by task, arm, model and rep (`--json` for records), and counts the cells graded before `matches` existed. +- **Replay hits you can check** — a `dokimi:review_replay` grade now records `details.matches`: per credited thread, the thread (id, anchor, gold comment) and the finding that hit it (its index, location, severity, message) with their distance in lines, so a hit can be read from `grade.json` alone. The metrics are unchanged. `dokimi repo report --hits` lists those pairs by task, arm, model and rep, as the cell's `result.json` names them even when a match carries a key of the same name (`--json` for records), and counts the cells graded before `matches` existed. - **Parse errors in `repo report`** — a row notes how many of its cells had an agent answer whose findings JSON did not parse (`grade.json` `parse_error`): scored 0 on a score-only row, counted as failed on a gate row, and not counted for a cell already counted as a grader failure. `--json` gives `parse_errors` per row, so a lower recall from answers that did not parse can be told from one from worse reviews. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 20a37514b..38bcfaaea 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -1346,7 +1346,8 @@ python cli.py repo compare repo-runs/replay --baseline guidance:none --metric re model and rep: the thread and its comment, then the finding that hit it. `--hits --json` gives `{batch, replay_cells, without_matches, hits}`, one record in `hits` per pair (`task`, `arm`, `model`, `rep`, `cell` and the - match's fields). A cell graded before `matches` was recorded is counted + match's fields; the first five come from the cell's `result.json`, and a + match key of the same name does not replace them). A cell graded before `matches` was recorded is counted (`N cell(s) graded without hit details`), not listed. - `repo compare` reduces cells to one value per (task, arm, model) first: repetitions share a task's difficulty and are not paired across arms, so the diff --git a/dokimi/dokimi/repos/report.py b/dokimi/dokimi/repos/report.py index 4eff02921..76a4385ec 100644 --- a/dokimi/dokimi/repos/report.py +++ b/dokimi/dokimi/repos/report.py @@ -360,8 +360,8 @@ def load_hits(batch_dir: Path) -> dict[str, Any]: if not isinstance(matches, list): # a parse error or a grader error credits no thread: nothing is missing without += "threads_hit" in details continue - hits += [{"task": rec["task"], "arm": rec["arm"], "model": rec["model"], "rep": rec["rep"], - "cell": rec["cell"], **m} for m in matches if isinstance(m, dict)] + hits += [{**m, "task": rec["task"], "arm": rec["arm"], "model": rec["model"], "rep": rec["rep"], + "cell": rec["cell"]} for m in matches if isinstance(m, dict)] # result.json's key wins over a match's return {"batch": str(batch_dir), "replay_cells": replay, "without_matches": without, "hits": hits} diff --git a/dokimi/tests/test_repo_report_hits.py b/dokimi/tests/test_repo_report_hits.py index c85c41f4e..50acc93ef 100644 --- a/dokimi/tests/test_repo_report_hits.py +++ b/dokimi/tests/test_repo_report_hits.py @@ -103,6 +103,17 @@ def test_report_hits_flag_prints_the_pairs_and_the_default_report_stays_the_tabl assert table.startswith(f"{batch}: 5 finished cells") and "credited" not in table and "finding 0" not in table +def test_a_match_key_named_like_the_cell_key_does_not_move_the_hit(tmp_path, capsys): + batch = tmp_path / "b1" + grade = _graded(tmp_path, fence([_f(5)])) + grade["details"]["matches"][0].update(task="other-task", arm="guidance:raw", model="m-9", rep=99, cell="elsewhere") + _put(batch, "a", "guidance:none", 1, grade) + records = json.loads(_cli(capsys, str(batch), "--hits", "--json")) + assert [(h["task"], h["arm"], h["model"], h["rep"], h["cell"], h["thread"]) for h in records["hits"]] == [ + ("a", "guidance:none", "m-1", 1, "a__guidance-none__m-1__r1", "t_op")] + assert _cli(capsys, str(batch), "--hits").splitlines()[1:3] == ["a", " guidance:none m-1 r1"] + + def test_hits_on_a_batch_with_no_replay_cell_say_so(tmp_path): _cell(tmp_path, "a", "guidance:none", 1) found = load_hits(tmp_path)