diff --git a/dokimi/changelog.d/20261018_221020_napetrov_builtin-review-graders.md b/dokimi/changelog.d/20261018_221020_napetrov_builtin-review-graders.md new file mode 100644 index 00000000..41c8c71a --- /dev/null +++ b/dokimi/changelog.d/20261018_221020_napetrov_builtin-review-graders.md @@ -0,0 +1,3 @@ +### Added + +- **Built-in review graders, `task.json` `grader: "dokimi:review_seeds"` and `"dokimi:review_replay"`** — a review task is now data only: `manifest.json` (seeds and decoys) or `gold.json` (human review threads) next to `task.json`, graded by framework code instead of the repository's `grade` and `oracle` entry points. `repo tasks check` and `repo run` use them with no other change. They run in the grading container on the image's `python3` (standard library only, 3.8 or newer), from a read-only mount at `/eval/graders` the agent's container never gets; an image without `python3` is a grader failure that says so. The matching rules are oneDAL's `review` and `review_replay` graders', with the same metrics for the same answers; `grade.json` records `details.grader_version`. An answer with no findings JSON is a failed run with `recall` 0 and `parse_error`, not a grader failure. Any other grader name goes to the repository's entry points as before, and the `dokimi:` prefix is reserved (contract §4.5). diff --git a/dokimi/changelog.d/20261021_120000_napetrov_batch-pins-graders.md b/dokimi/changelog.d/20261021_120000_napetrov_batch-pins-graders.md new file mode 100644 index 00000000..75e5a4a9 --- /dev/null +++ b/dokimi/changelog.d/20261021_120000_napetrov_batch-pins-graders.md @@ -0,0 +1,3 @@ +### Changed + +- **A batch is pinned to the built-in grader sources too.** When a task names a `dokimi:` grader, `repo run` hashes the built-in graders into `_eval/batch.json` (`graders`) next to `eval_dir` and `image`, and refuses to resume a batch after they change, as it does for an edit to `.agent-eval/grade.py`; `repo compare` warns when two batches ran from different graders. Before, a grader change moved scores (for example `replay_mul`) between cells of one batch with no warning. A batch stored before this change with such a task has no `graders` pin and is refused on resume: start a fresh `--batch` dir. A batch with no such task is not pinned to them and resumes as before. diff --git a/dokimi/docs/repo-evaluation-contract.md b/dokimi/docs/repo-evaluation-contract.md index 89c7444b..7b4c9e9c 100644 --- a/dokimi/docs/repo-evaluation-contract.md +++ b/dokimi/docs/repo-evaluation-contract.md @@ -126,7 +126,7 @@ outside every workspace and passes its absolute path to entry points as `answer.txt` are written to `{run_dir}` after the agent's container exits. - `grade` and `oracle` run in a separate container from the same image, after the agent's container has exited. That container mounts `{eval_dir}` and - `{source_dir}` read-only and `{run_dir}` read-write. It gets a `HOME` of its + `{source_dir}` read-only and `{run_dir}` read-write (and, for a built-in grader (§4.5), the framework's grader code at `/eval/graders`, read-only). It gets a `HOME` of its own (`{run_dir}/grade-home`, made empty before each run; the agent never had it), `PYTHONNOUSERSITE=1`, and git configuration overrides that disable `core.fsmonitor` and hooks (`core.hooksPath=/dev/null`); the workspace's @@ -139,8 +139,9 @@ outside every workspace and passes its absolute path to entry points as containers share. A grader that imports or runs workspace code runs the agent's code. - **Fixed paths.** Inside every container `{run_dir}` is `/eval/run`, - `{eval_dir}` is `/eval/content` and `{source_dir}` is `/eval/source`. The - agent container mounts the workspace at `/eval/run/workspace`, the same path + `{eval_dir}` is `/eval/content`, `{source_dir}` is `/eval/source`, and + the built-in graders' code is `/eval/graders` (grading container only, §4.5). + The agent container mounts the workspace at `/eval/run/workspace`, the same path graders see. An absolute path in an answer or grade therefore means the same thing after a run directory is archived or regraded on another host. On oneDAL, 14 of 308 copied runs regraded differently because build answers @@ -243,7 +244,7 @@ Rules: ## 3. `task.json` The framework interprets the **core** and **workspace** keys. Every other key -belongs to the repository's grader and is passed through unread. +belongs to the repository's grader and is passed through unread, except `grader`, which the framework reads to tell a `dokimi:` name (§4.5) from a repository's. | Key | Required | Read by | Meaning | |---|---|---|---| @@ -251,7 +252,7 @@ belongs to the repository's grader and is passed through unread. | `family` | yes | framework (reporting) | free string; common values: `build`, `fix`, `feature`, `test`, `review` | | `base` | yes | framework | commit to snapshot: a hex SHA, optionally followed by `^` (the parent, the usual form for a mined fix). Branch names, tags and `HEAD` are refused, because repetitions of one task must get one snapshot. Use the full 40-character SHA; a shorter one is accepted with a warning. It must resolve in the source. `meta.json` records it as `base_rev` and the resolved full SHA as `base_sha` | | `prompt` | yes | framework | what the agent is told | -| `grader` | yes | repository | grader name, interpreted by the `grade` entry point | +| `grader` | yes | framework for `dokimi:` (§4.5); repository otherwise | grader name; any other name is interpreted by the `grade` entry point | | `setup_patches` | no | framework | applied after the snapshot, before the arm (for example an injected bug) | | `review_patch` | no | framework | applied with `git am` as the HEAD commit under review | | `gate` | no | framework | what counts as success; default `["pass"]`. Each entry is a boolean metric name, which must be `true`, or `{"metric": , "min": }` for a numeric metric | @@ -468,27 +469,115 @@ against the repository. Every other key, per claim and at the top level, is free-form and stored with the scan. A fixed minimum lets L0 merge any repository's checks with its own without knowing their rules. +### 4.5 Built-in graders + +A task whose `grader` is `dokimi:` is graded by the framework, not by +the repository's `grade` and `oracle` entry points; any other name goes to +those entry points. Such a task is data only: its directory holds +`task.json`, the patch and the JSON files the grader reads. The `dokimi:` +prefix is the framework's: an unknown `dokimi:` name fails `tasks check` and +the run's grade, and is never handed to the repository. + +A built-in grader runs where `grade` and `oracle` run: in the grading +container, with the same mounts, network and environment (§4.2). The +framework's grader code is mounted read-only at `/eval/graders` (never into +the agent's container) and runs as +`python3 -I -B /eval/graders/main.py grade|oracle /eval/run`, +where `` is the task's directory under `{eval_dir}`. It uses the +standard library only and the image's `python3` (3.8 or newer). `-I` keeps +`PYTHONPATH` from `image.env` and the workspace off `sys.path`; `-B` prevents +writing `.pyc` files to the read-only mount. An image with no `python3` on +`PATH` is a grader failure that says so: the grader exits 127 and its +`.err` file shows `exec python3 failed`. Any +other exit 127 is reported as a plain `grade exited 127`. + +- `grade` reads `task.json` and `answer.txt` from `{run_dir}` and the data + files from the task's directory, and writes `grade.json` (§4.2): numbers, + booleans and `null` under `metrics`; strings, lists and objects under + `details`, with `details.grader_version`. A grader that raises writes + `pass: null` with `errors`. A metric the task's `gate` names that the grader + did not produce is `null`. +- `oracle` writes the reference answer to `answer.txt` and leaves the + workspace alone. + +A review task with `pass: null` is scored, not gated: give it `score`. A `gate` +on its metrics is not applied (`success` is null), and without `score` the run +is a grader failure. A clean-diff control (no seeds) has `recall: null`, so it +takes no `score`; its gate is the default `["pass"]`. The manifest's +`entry_points.grade` is still required (`repo_manifest.v1`), even when every +task uses a built-in grader. + +Both review graders read the answer the same way. Its last parseable +` ```json ` fence (one nesting arrays or objects more than 100 deep counts +as unparseable, whatever `json.loads` accepts) must be an array of findings +`{"file", "line", "severity", "rule", "message"}`; entries that are not +objects are dropped. `file` is made repository-relative: a path is cut after +`/eval/run/workspace/`, then after the last `/repo/` (a standalone oneDAL +run's workspace; a relative `a/repo/b` becomes `b`, the seed's or thread's +`file` as well as the finding's), then leading `./` and `/` go. `line` is +read with Python's `int()` (`"6"` is 6, `5.9` is 5); a missing or falsy line +(`null`, `0`, `""`), or a value `int()` cannot read, is -99, near nothing. `severity` `blocker` or `major` counts toward +`n_severe`. An answer with no such array gives only `pass: false`, +`parse_error: true` and every recall 0 (`recall`; replay also `recall_all` +and `recall_addressed`; `null` when there is nothing to recall): the agent +failed the task, and on a task with `score` that is a failed run with score +0, not a grader failure. + +| `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 `[]`) | + +`manifest.json` is `{"seeds": [{id, class, file, lines, kw}], "decoys": +[{id, file, lines, kw}]}`; `file` `"*"` matches any file, and `kw` is a +regular expression searched case-insensitively in the finding's `rule`, +`message` and `evidence`. + +- A finding on a seed's file hits every seed whose `kw` matches. When none + matches, it hits the one seed (not `"*"`) whose `lines` are nearest, if + within 1 line. +- A finding that hits no seed is a false positive when it is on a decoy's + file and matches its `kw` or one of its `lines`; it is listed in + `decoys_flagged`. Otherwise it counts as `unmatched`. +- `recall` is seeds hit / seeds. + +`gold.json` is `[{id, file, line, kind, comment, addressed}]`: one human +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. +- `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 + thread, a lower bound on precision. Humans miss problems and raise + non-defects, so neither is a verdict. +- The oracle answer is one finding per thread, at its anchor. + ## 5. Aligning oneDAL `dev/agent_evals` (uxlfoundation/oneDAL#3818) -The harness stays in oneDAL and keeps working standalone. These changes make -the same content runnable by the framework. +oneDAL drops its standalone mode: the generic parts of its harness move into +dokimi (#41), and `dev/agent_evals` keeps only its tasks, its image and the +graders that know oneDAL. These changes make that content runnable by the +framework. | Today | Change | |---|---| | guidance globs in `config.py` | move to `repo-eval.yaml` `guidance.globs` | -| `task.json` keys | already match the core keys. Add `gate: ["strict_pass"]` to fix tasks, so a hidden-test pass that breaks a regression suite is not counted as success. Add `score: {metric: recall, max: 1.0}` to review tasks | +| `task.json` keys | already match the core keys. Add `gate: ["strict_pass"]` to fix tasks, so a hidden-test pass that breaks a regression suite is not counted as success. Add `score: {metric: recall, max: 1.0}` to review tasks with seeds or threads, not to clean-diff controls (§4.5) | | `base` is a 8–12 character SHA | expand to the full 40-character SHA (keep a trailing `^`) | | entry points call `dev/agent_evals/run.py` | call `{eval_dir}/run.py`; the graders must not assume they sit inside the workspace (§1.1) | -| `prep()` in `common.py` | unchanged for standalone use; under the framework the same steps run framework-side (§3) | +| `prep()` in `common.py` | goes: the framework prepares every workspace (§3) | | arms `none` / `main-raw` | become `guidance:none` / `guidance:raw`. `guidance:bridge` is framework-only (§3); oneDAL keeps no bridge arm | -| graders read history from `ONEDAL_EVAL_SRC` | read `{source_dir}` under the framework; the variable stays for standalone runs | -| `contract.py` derives the base when `meta.json` has none | diff from `meta.json` `start_sha`, and read `base_sha` / `base_rev` for the upstream commit (§4); derive only standalone | +| graders read history from `ONEDAL_EVAL_SRC` | read `{source_dir}`, the only full-history source; the variable goes | +| `contract.py` derives the base when `meta.json` has none | diff from `meta.json` `start_sha`, and read `base_sha` / `base_rev` for the upstream commit (§4); nothing derives it | | `build_make_gnu` pip-installs MKL and TBB | install them in the image, or give the task `egress: [pypi.org, files.pythonhosted.org]` | | Bazel install base in the container's `HOME` | put it on a declared cache (`bin/bazel --output_user_root`) | | `static_check.py --out` writes `files` / `counts` / `claims` | add `"schema": "repo_static.v1"`; the claim fields already fit §4.4 | -| `run.py grade ` writes a flat dict | add `--contract` mode that reads §4 inputs and writes `repo_grade.v1` (scalar fields move under `metrics`, lists and objects under `details`) | +| `run.py grade ` writes a flat dict | `run.py grade --contract` reads the §4 inputs and writes `repo_grade.v1` (scalar fields under `metrics`, lists and objects under `details`); it stays for the graders that are not built in: `build.py` and `make.py`, which know oneDAL, and the fix and mutation graders until `dokimi:hidden_tests` and `dokimi:mutation` exist (#41) | +| `graders/review.py` and `replay.py` (task `grader` `review`, `review_replay`) | review tasks name `dokimi:review_seeds` or `dokimi:review_replay` (§4.5) and keep `manifest.json`, `oracle.json` or `gold.json`; both files go. The built-in graders give the same metrics for the same answers | | graders read Claude stream-json via `traces.py` | read `events.jsonl` instead; `traces.py` becomes the Claude Code → events converter, which moves to the framework's harness profile | -| oracle logic spread across `run.py check` | expose it as the `oracle` entry point | +| oracle logic spread across `run.py check` | expose it as the `oracle` entry point; review tasks need none (§4.5) | | `bin/bazel` wrapper, shared caches, `MKLROOT` removal | expressed in the Dockerfile and `image.caches` / `image.env` | | clean-ancestor guard, no-background hook, per-run config dir | done by the framework for every harness | | fix and feature tasks have no review-level grading | add `rubric.json` per task ([mergeability](repo-evaluation-mergeability.md) §2), starting from the tasks mined from reviewed PRs; maintainers approve each rubric | @@ -829,7 +918,8 @@ python cli.py repo compare repo-runs/pilot --baseline guidance:none --metric roo on a rerun unless `--force`; a cell without one is reported as missing, not failed, and the next run of the batch fills it. A batch stays on the eval tree it started with: its revision, or for a working tree the digests of - its `eval_dir` and of the image build context. + its `eval_dir` and of the image build context. A batch with a task that + names a `dokimi:` grader (or a task directory that cannot be read) also stays on the built-in graders' sources. - The agent's container mounts the workspace, the per-run `home`, `config` and scratch directories (each `image.env` value under `{run_dir}/`), the caches, the harness executable (`--bin`, read-only at `/eval/harness`) and the @@ -868,10 +958,10 @@ python cli.py repo compare repo-runs/pilot --baseline guidance:none --metric roo arm and score-only in the other, or a measure (such as a metric) one arm lacks. A metric's `n_baseline`/`n_treatment` count the cells that report it. The output names the batches behind each selector and each batch's pin - (`tree`, `eval_dir`, `image` from `_eval/batch.json`) with its cells' + (`tree`, `eval_dir`, `image`, `graders` from `_eval/batch.json`) with its cells' harness versions, and warns when a bare `ARM` pools several batches, when a batch of a comparison has no pin, or when the two arms ran from different - `eval_dir` or `image` pins or harness versions. An image `repo run` tagged + `eval_dir`, `image` or `graders` pins or harness versions. An image `repo run` tagged from the tree (`dokimi/:`) follows the tree, so two revisions are not warned about it; an `--image` of its own is compared. `repo report` warns (`model_mismatch` in its summary) when a cell's `model_resolved` is another diff --git a/dokimi/dokimi/commands/repo_run.py b/dokimi/dokimi/commands/repo_run.py index 4c67b35f..15506ff4 100644 --- a/dokimi/dokimi/commands/repo_run.py +++ b/dokimi/dokimi/commands/repo_run.py @@ -15,6 +15,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + from dokimi.repos.manifest import RepoManifest from dokimi.repos.workspace import Arm @@ -29,6 +30,7 @@ def _context(args: argparse.Namespace, out: Path, pin: bool = False): from dokimi.repos import RepoSource, open_source from dokimi.repos.container import DockerRunner from dokimi.repos.grading import EvalContext + from dokimi.repos.graders import graders_digest from dokimi.repos.runner import slug from dokimi.repos.source import default_cache_dir from dokimi.repos.workspace import export_build_context, export_eval_dir, source_git_dir @@ -60,7 +62,10 @@ def _context(args: argparse.Namespace, out: Path, pin: bool = False): elif not runner.image_exists(image): raise ValueError(f"--image {image} does not exist") if pin: - _pin_batch(out, {"tree": tree.label, "eval_dir": context_digest(staged), "image": image}) + stamp = {"tree": tree.label, "eval_dir": context_digest(staged), "image": image} + if _uses_builtin_graders(staged, manifest): # else no score depends on them: an upgrade keeps the batch + stamp["graders"] = graders_digest() + _pin_batch(out, stamp) shutil.rmtree(content, ignore_errors=True) staged.rename(content) if not args.image and (args.rebuild_image or not runner.image_exists(image)): @@ -104,6 +109,18 @@ def context_digest(root: Path) -> str: return h.hexdigest() +def _uses_builtin_graders(content: Path, manifest: RepoManifest) -> bool: + """Whether a task of the exported eval dir names a ``dokimi:`` grader; an unreadable task.json counts.""" + from dokimi.repos.graders import builtin_name + from dokimi.repos.workspace import load_tasks + + try: + tasks = load_tasks(content, manifest) + except (OSError, ValueError): + return True + return any(isinstance(t.raw, dict) and builtin_name(t.raw.get("grader")) is not None for t in tasks) + + def _pin_batch(batch_dir: Path, pin: dict[str, str]) -> None: path = batch_dir / "_eval" / "batch.json" if path.is_file(): diff --git a/dokimi/dokimi/repos/compare.py b/dokimi/dokimi/repos/compare.py index 59b2db71..6a75a933 100644 --- a/dokimi/dokimi/repos/compare.py +++ b/dokimi/dokimi/repos/compare.py @@ -65,7 +65,7 @@ PRIMARY = ("gate", "score") #: Batch stamp keys that must agree between the arms of a comparison; ``tree`` is shown but may differ (another #: revision of the repository is a treatment of its own). -STAMP_KEYS = ("eval_dir", "image") +STAMP_KEYS = ("eval_dir", "image", "graders") class CompareError(ValueError): @@ -196,7 +196,7 @@ def _provenance_warnings(cells: list[dict[str, Any]], baseline: str, treatments: for b in batches if len(batches) > 1 else (): # the arms of one batch share its pin if b not in unstamped and not any(k in stamps[b] for k in STAMP_KEYS): unstamped.add(b) - out.append(f"batch {b} has no stamp (_eval/batch.json); its eval_dir and image are not compared") + out.append(f"batch {b} has no stamp (_eval/batch.json); its eval_dir, image and graders are not compared") for key in STAMP_KEYS: pinned = [b for b in batches if key in stamps[b]] if key == "image" and all(_built_from_tree(stamps[b]) for b in pinned): @@ -414,7 +414,7 @@ def _format_header(summary: dict[str, Any]) -> list[str]: for text, batches in summary["sources"].items(): lines.append(f" {text}: {', '.join(batches)}") for name, stamp in summary["stamps"].items(): - shown = [f"{k} {stamp[k][:12] if k == 'eval_dir' else stamp[k]}" for k in ("tree", *STAMP_KEYS) if k in stamp] + shown = [f"{k} {stamp[k][:12] if k in ('eval_dir', 'graders') else stamp[k]}" for k in ("tree", *STAMP_KEYS) if k in stamp] lines.append(f" batch {name}: " + ", ".join([*shown, "harness " + ", ".join(stamp["harness_versions"])])) return lines diff --git a/dokimi/dokimi/repos/container.py b/dokimi/dokimi/repos/container.py index 0efaae9a..736cbfc5 100644 --- a/dokimi/dokimi/repos/container.py +++ b/dokimi/dokimi/repos/container.py @@ -47,6 +47,7 @@ SOURCE_DIR = "/eval/source" HARNESS_DIR = "/eval/harness" SUPPORT_DIR = "/eval/support" +GRADERS_DIR = "/eval/graders" # the built-in graders, mounted read-only into the grading container only NETWORKS = ("none", "bridge", "host") # Predefined docker build args: passed by name when set, so a build behind a proxy works # and the values stay out of the image history. diff --git a/dokimi/dokimi/repos/graders/__init__.py b/dokimi/dokimi/repos/graders/__init__.py new file mode 100644 index 00000000..373cc322 --- /dev/null +++ b/dokimi/dokimi/repos/graders/__init__.py @@ -0,0 +1,46 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Built-in graders: ``task.json`` ``grader: "dokimi:"`` (contract §4.5). + +A task naming one is graded by framework code instead of the repository's +``grade`` and ``oracle`` entry points, so the task is data only. The modules +next to this file run inside the grading container: this directory is +mounted read-only at :data:`~dokimi.repos.container.GRADERS_DIR` and +:mod:`.main` runs on the image's ``python3``, with the standard library only. +This file is the host side and is never imported there. +""" + +from __future__ import annotations + +from pathlib import Path + +from .review import GRADERS + +PREFIX = "dokimi:" +NAMES = tuple(GRADERS) +# the data file each built-in grader reads next to task.json: the task key that renames it, and its default +DATA_FILES = {"review_seeds": ("manifest", "manifest.json"), "review_replay": ("gold", "gold.json")} +HOST_DIR = Path(__file__).resolve().parent +ENTRY = "main.py" + + +def builtin_name(grader: object) -> str | None: + """``review_seeds`` for ``"dokimi:review_seeds"``; ``None`` for a repository's grader name. + + The whole ``dokimi:`` namespace is the framework's: an unknown name in it is returned as well, + for the caller to refuse rather than hand to the repository's ``grade``.""" + if isinstance(grader, str) and grader.startswith(PREFIX): + return grader[len(PREFIX):] + return None + + +def graders_digest() -> str: + """sha256 over the built-in grader sources: what a batch's score depends on beyond its ``eval_dir``.""" + import hashlib + + h = hashlib.sha256() + for path in sorted(HOST_DIR.glob("*.py")): + h.update(f"{path.name}\0".encode()) + h.update(hashlib.sha256(path.read_bytes()).digest()) + return h.hexdigest() diff --git a/dokimi/dokimi/repos/graders/main.py b/dokimi/dokimi/repos/graders/main.py new file mode 100644 index 00000000..7d553644 --- /dev/null +++ b/dokimi/dokimi/repos/graders/main.py @@ -0,0 +1,111 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Run a built-in grader in the grading container (contract §4.5). + +``python3 -I main.py grade|oracle []`` + +- ``grade`` reads ``task.json`` and ``answer.txt`` from ``run_dir`` (default + ``/eval/run``) and the task's data from ``task_dir`` (its directory under + ``/eval/content``), and writes ``run_dir/grade.json`` as ``repo_grade.v1``: + numbers, booleans and null under ``metrics``; strings, lists and objects + under ``details``. A grader that raises writes ``pass: null`` with + ``errors``, which the framework counts as a grader failure, and exits 0. +- ``oracle`` writes the reference answer to ``run_dir/answer.txt``. + +The framework mounts this directory read-only and runs it with ``-I``, so +neither ``PYTHONPATH`` nor the workspace can put a module in front of the +standard library. Standard library only: it runs on the image's ``python3``. +""" + +from __future__ import annotations + +import importlib +import json +import os +import sys +import traceback +from pathlib import Path + +if __package__: + from . import review +else: # a script under -I: its own directory is not on sys.path + # By name, not `import review`: dependency hygiene (deptry) reads a bare import as a third-party package. + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + review = importlib.import_module("review") + +GRADERS = dict(review.GRADERS) +ORACLES = dict(review.ORACLES) +VERSIONS = {name: review.VERSION for name in review.GRADERS} +RUN_DIR = "/eval/run" + + +def split_result(g): + """``repo_grade.v1``: numbers, booleans and null under metrics; strings, lists and objects under details.""" + mets, det = {}, {} + for k, v in g.items(): + if k == "pass": + continue + if v is None or isinstance(v, (bool, int, float)): + mets[k] = v + else: + det[k] = v + return mets, det + + +def _read_task(run_dir): + with open(str(run_dir / "task.json"), encoding="utf-8") as fh: + return json.load(fh) + + +def gate_names(task): + """Metric names the task's ``gate`` reads (``pass`` is not a metric).""" + out = [] + for entry in task.get("gate", []): + name = entry.get("metric") if isinstance(entry, dict) else entry + if isinstance(name, str) and name != "pass": + out.append(name) + return out + + +def grade(name, task_dir, run_dir): + """Grade the run in ``run_dir`` and write its ``grade.json``; returns the document.""" + t = _read_task(run_dir) + out = {"schema": "repo_grade.v1", "task": t.get("id"), "grader": t.get("grader"), "pass": None, + "metrics": {}, "details": {}, "errors": []} + try: + answer = run_dir / "answer.txt" + text = answer.read_text(encoding="utf-8") if answer.exists() else "" + g = GRADERS[name](t, task_dir, text) + out["metrics"], out["details"] = split_result(g) + for k in gate_names(t): # a gate metric the grader did not reach is unmeasured, not failed + out["metrics"].setdefault(k, None) + out["pass"] = g.get("pass") + except Exception as e: # noqa: BLE001 # reported as not graded, never as a failure + out.update(metrics={}, details={}, errors=[f"{type(e).__name__}: {e}", traceback.format_exc(limit=5)]) + out["pass"] = None + out["details"]["grader_version"] = VERSIONS[name] + with open(str(run_dir / "grade.json"), "w", encoding="utf-8") as fh: + json.dump(out, fh, indent=1) + return out + + +def oracle(name, task_dir, run_dir): + """Write the reference answer to ``run_dir/answer.txt``; returns it.""" + text = ORACLES[name](_read_task(run_dir), task_dir) + if text: + (run_dir / "answer.txt").write_text(text, encoding="utf-8") + return text + + +def main(argv): + if len(argv) not in (3, 4) or argv[0] not in ("grade", "oracle") or argv[1] not in GRADERS: + sys.stderr.write("usage: main.py grade|oracle {%s} []\n" % "|".join(sorted(GRADERS))) + return 2 + run_dir = Path(argv[3] if len(argv) == 4 else RUN_DIR) + (grade if argv[0] == "grade" else oracle)(argv[1], Path(argv[2]), run_dir) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/dokimi/dokimi/repos/graders/review.py b/dokimi/dokimi/repos/graders/review.py new file mode 100644 index 00000000..721f2cfc --- /dev/null +++ b/dokimi/dokimi/repos/graders/review.py @@ -0,0 +1,266 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Review graders: the agent reviews the ``HEAD`` commit and ends its answer with findings as JSON. + +The answer's last parseable ```` ```json ```` fenced block must be an array of +findings, each an object ``{file, line, severity, rule, message}`` (``evidence`` +is read too). Anything else is ``pass: false`` and ``parse_error``, with +every recall 0 (``null`` where there is nothing to recall): the agent failed. A finding's ``file`` is made +repository-relative: it is cut after ``/eval/run/workspace/`` (the agent's +workspace), then after the last ``/repo/`` (a standalone oneDAL run's +workspace; a relative ``a/repo/b`` becomes ``b`` too, on a seed's or a +thread's file as on a finding's), then leading ``./`` and ``/`` are dropped. ``severity`` ``blocker`` or +``major`` counts toward ``n_severe``. + +``review_seeds``: ``manifest.json`` next to ``task.json`` (or ``task.json`` +``manifest``):: + + seeds [{id, class, file, lines, kw}] known problems. A finding hits a seed if its file matches ("*" = any) and + its rule/message/evidence matches the regex kw (case-insensitive), or + (no kw hit on any seed) its line is within 1 of the seed's lines; the + line fallback credits only the nearest such seed. + decoys [{id, file, lines, kw}] things that look wrong but are fine. A finding that hits no seed and + matches a decoy's kw or one of its lines is a false positive. + +A manifest with no seeds is a clean-diff control: ``pass`` is "no blocker or +major finding". Otherwise ``pass`` is null and ``recall`` is the score. The +oracle answer is ``oracle.json`` (or ``task.json`` ``oracle_answer``), a list +of findings, in a fence. + +``review_replay``: a merged PR replayed at the revision humans reviewed. +``gold.json`` (or ``task.json`` ``gold``):: + + [{id, file, line, kind, comment, addressed}] one human thread anchored on a line of the reviewed + revision's new file; kind is free (correctness, api, + tests, convention, docs, nit); addressed says whether + the code changed in response before merge. + +A thread is hit by a finding on the same file with ``|finding.line - +gold.line| <= window`` (``task.json`` ``window``, default 5). Matching is one +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. +``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. +``pass`` is null, so these tasks are scored (``score``), not gated. +The oracle answer is one finding per thread, at its anchor. + +Ported from oneDAL ``dev/agent_evals/graders/review.py`` and ``replay.py`` +with the same results for the same inputs. Standard library only: this file +runs on the image's ``python3``, which may be old. +""" + +from __future__ import annotations + +import json +import re + +VERSION = "1" # bump when a result for the same inputs changes; grade.json records it +SEVERE = ("blocker", "major") + + +MAX_DEPTH = 100 # json.loads' own limit is the interpreter's (3.14 parses what 3.13 refuses) + + +def _too_deep(block): + """True when ``block`` nests arrays/objects deeper than MAX_DEPTH (brackets inside strings ignored).""" + depth, in_str, esc = 0, False, False + for ch in block: + if in_str: + if esc: + esc = False + elif ch == "\\": + esc = True + elif ch == '"': + in_str = False + elif ch == '"': + in_str = True + elif ch in "[{": + depth += 1 + if depth > MAX_DEPTH: + return True + elif ch in "]}": + depth -= 1 + return False + + +def last_json(text): + """The last parseable ```` ```json ```` fenced block of ``text``, or None.""" + for block in reversed(re.findall(r"```json\s*(.*?)```", text or "", re.S)): + if _too_deep(block): + continue + try: + return json.loads(block) + except (ValueError, RecursionError): # a fence nested too deep is unreadable, like any other + continue + return None + + +def norm_path(p): + """Repository-relative path of a finding: cut after the workspace, then after the last ``/repo/``.""" + p = str(p or "").strip().replace("\\", "/") + if "/eval/run/workspace/" in p: # the agent's workspace under dokimi + p = p.rsplit("/eval/run/workspace/", 1)[1] + if "/repo/" in p: # oneDAL's rule, applied to every path alike so a seed's file and a finding's agree + p = p.rsplit("/repo/", 1)[1] + while p.startswith("./"): + p = p[2:] + return p.lstrip("/") + + +def finding_fields(f): + """``(path, text, line)`` of one finding; an unreadable line is -99, near nothing.""" + path = norm_path(f.get("file")) + text = " ".join(str(f.get(k, "")) for k in ("rule", "message", "evidence")) + try: + line = int(f.get("line") or -99) + except (TypeError, ValueError, OverflowError): + line = -99 + return path, text, line + + +def same_file(entry, path): + return entry["file"] == "*" or bool(path) and path == norm_path(entry["file"]) + + +def findings_of(answer): + """The answer's findings (objects only), or None when it has no JSON array.""" + findings = last_json(answer) + if not isinstance(findings, list): + return None + return [f for f in findings if isinstance(f, dict)] + + +def n_severe(findings): + return sum(str(f.get("severity", "")).lower() in SEVERE for f in findings) + + +def _read_json(task_dir, name): + with open(str(task_dir / name), encoding="utf-8") as fh: + return json.load(fh) + + +def _seed_hits(seeds, path, text, line): + """Seed ids one finding credits: every keyword match, else the nearest seed within one line.""" + matched = [s["id"] for s in seeds if same_file(s, path) and re.search(s["kw"], text, re.I)] + if matched: + return matched + near = [(min(abs(line - x) for x in s["lines"]), s["id"]) for s in seeds + if same_file(s, path) and s["file"] != "*" and s["lines"]] + near = [n for n in near if n[0] <= 1] + return [min(near, key=lambda n: (n[0], _id_key(n[1])))[1]] if near else [] + + +def _id_key(x): + """Sort key for an id: numbers in numeric order, then strings; a manifest may mix int and str ids.""" + return (isinstance(x, str), x) + + +def _ids(ids): + """Ids in a stable order.""" + return sorted(ids, key=_id_key) + + +def _per_group(entries, key, hit): + """``{group: "hit/total"}`` over ``entries`` grouped by ``key``, in first-seen order.""" + counts: dict = {} + for e in entries: + c = counts.setdefault(e[key], [0, 0]) + c[1] += 1 + c[0] += e["id"] in hit + return {k: f"{a}/{b}" for k, (a, b) in counts.items()} + + +def grade_seeds(task, task_dir, answer): + """``review_seeds``: the result dict (``pass`` and flat metrics, lists and objects for details).""" + man = _read_json(task_dir, task.get("manifest", "manifest.json")) + seeds, decoys = man.get("seeds", []), man.get("decoys", []) + findings = findings_of(answer) + if findings is None: # no findings to score: the run failed, and recall is 0, not unmeasured + return {"pass": False, "parse_error": True, "recall": 0.0 if seeds else None} + hit, fp, other = set(), set(), 0 + for f in findings: + path, text, line = finding_fields(f) + matched = _seed_hits(seeds, path, text, line) + if matched: + hit.update(matched) + continue + dec = [d["id"] for d in decoys + if same_file(d, path) and (re.search(d["kw"], text, re.I) or line in d["lines"])] + if dec: + fp.update(dec) + else: + other += 1 + severe = n_severe(findings) + out = {"pass": None, "n_findings": len(findings), "n_severe": severe, "seeds_hit": _ids(hit), + "decoys_flagged": _ids(fp), "n_decoys": len(decoys), "unmatched": other, + "recall": round(len(hit) / len(seeds), 3) if seeds else None, + "recall_by_class": _per_group(seeds, "class", hit)} + if not seeds: # clean-diff control: pass = nothing blocking reported + out["pass"] = severe == 0 + return out + + +def oracle_seeds(task, task_dir): + """The reference answer: ``oracle.json`` (a list of findings) in a fence.""" + with open(str(task_dir / task.get("oracle_answer", "oracle.json")), encoding="utf-8") as fh: + # ``` can only sit in a JSON string: escape it as oracle_replay does, or it closes the fence + return "```json\n" + fh.read().replace("```", "\\u0060\\u0060\\u0060") + "\n```" + + +def _ratio(a, b): + return round(a / b, 3) if b else None + + +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 = [], 0 + for i, f in enumerate(findings): + path, _, line = finding_fields(f) + close = [(abs(line - g["line"]), i, g["id"]) for g in gold + if same_file(g, path) and abs(line - g["line"]) <= window] + pairs += close + near += bool(close) + return pairs, near + + +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")) + findings = findings_of(answer) + if findings is None: # no findings to score: the run failed, and every recall is 0, not unmeasured + zero = _ratio(0, len(gold)) + 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() + # 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]))): + if i not in used and gid not in hit: + used.add(i) + hit.add(gid) + 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), + "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), + "precision_proxy": _ratio(near, len(findings)), "decoys_flagged": []} + + +def oracle_replay(task, task_dir): + """The reference answer: one finding per gold thread, at its anchor.""" + gold = _read_json(task_dir, task.get("gold", "gold.json")) + ans = [{"file": g["file"], "line": g["line"], "severity": "minor", "rule": g.get("kind", ""), + "message": ((g.get("comment") or "").splitlines() or [""])[0][:200]} for g in gold] + # a comment's ``` (a GitHub suggestion) would close the fence: JSON-escape it, same decoded text + return "```json\n" + json.dumps(ans, indent=1).replace("```", "\\u0060\\u0060\\u0060") + "\n```" + + +GRADERS = {"review_seeds": grade_seeds, "review_replay": grade_replay} +ORACLES = {"review_seeds": oracle_seeds, "review_replay": oracle_replay} diff --git a/dokimi/dokimi/repos/grading.py b/dokimi/dokimi/repos/grading.py index 103ef833..e37043f1 100644 --- a/dokimi/dokimi/repos/grading.py +++ b/dokimi/dokimi/repos/grading.py @@ -6,7 +6,10 @@ ``grade`` and ``oracle`` run in a container of their own after any agent has exited. It mounts the run directory read-write at ``/eval/run``, the exported eval dir read-only at ``/eval/content`` and, only for a template that names -``{source_dir}``, the full-history source read-only at ``/eval/source``. +``{source_dir}``, the full-history source read-only at ``/eval/source``. A task +whose ``grader`` is ``dokimi:`` runs a built-in grader +(:mod:`dokimi.repos.graders`, mounted read-only at ``/eval/graders``) in that +container instead of the manifest's entry point (contract §4.5). A grader failure is a failure of the run on every gated metric: ``grade`` exiting non-zero, timing out, writing no valid ``grade.json``, or writing @@ -30,6 +33,7 @@ from .container import ( CONTENT_DIR, + GRADERS_DIR, INFRA_EXIT, RUN_DIR, SOURCE_DIR, @@ -40,6 +44,11 @@ ContainerSpec, Mount, ) +from .graders import ENTRY as BUILTIN_ENTRY +from .graders import HOST_DIR as GRADERS_HOST +from .graders import NAMES as BUILTIN_NAMES +from .graders import PREFIX as BUILTIN_PREFIX +from .graders import builtin_name from .manifest import RepoManifest from .workspace import Task @@ -53,6 +62,8 @@ _PLAIN_GIT_CONFIG = "[core]\n\trepositoryformatversion = 0\n\tfilemode = true\n\tbare = false\n" # Run-dir files the framework reads or writes after a grader had the run dir read-write. HOST_FILES = ("meta.json", "task.json", "answer.txt", "events.jsonl", "result.json", "grade.log", "grade.err") +_NOT_FOUND_EXIT = 127 # docker run: the command (here the image's python3) is not on PATH +_PYTHON3_NOT_FOUND_SIG = "exec python3 failed" # tini's message under --init, which every container runs with @dataclass @@ -100,12 +111,28 @@ def render_entry(template: str, task: Task) -> list[str]: return [substitute(word, {**CONTAINER_PATHS, "task": task.id}) for word in shlex.split(template)] +def _python3_not_found_in_stderr(run_dir: Path, kind: str) -> bool: + """Whether stderr indicates python3 exec failed (not just any 127 exit). + + Exit 127 can mean other things; only treat it as \"python3 missing\" if stderr + contains the known exec failure signature.""" + err_file = run_dir / f"{kind}.err" + if not err_file.exists(): + return False + try: + return _PYTHON3_NOT_FOUND_SIG in err_file.read_text(encoding="utf-8", errors="replace") + except OSError: + return False + + def run_entry(ctx: EvalContext, kind: str, task: Task, run_dir: Path, network: str, name: str | None = None) -> ContainerResult: - """Run entry point ``kind`` (grade, oracle) for the run in ``run_dir``; logs go to ``.log``.""" - template = ctx.manifest.entry_points.get(kind) - if not template: - raise ContainerError(f"the manifest declares no `{kind}` entry point") + """Run entry point ``kind`` (grade, oracle) for the run in ``run_dir``; logs go to ``.log``. + + A task whose ``grader`` is ``dokimi:`` runs the framework's built-in grader instead of the + manifest's entry point, in the same container (contract §4.5).""" + builtin = builtin_name(task.raw.get("grader")) + argv, extra = _builtin_entry(ctx, kind, task, builtin) if builtin is not None else _repo_entry(ctx, kind, task) home = run_dir / GRADE_HOME shutil.rmtree(home, ignore_errors=True) home.mkdir() @@ -115,14 +142,43 @@ def run_entry(ctx: EvalContext, kind: str, task: Task, run_dir: Path, network: s for log in (f"{kind}.log", f"{kind}.err"): # an earlier entry point's link would be opened for writing (run_dir / log).unlink(missing_ok=True) mounts = [Mount(run_dir.resolve(), RUN_DIR), Mount(ctx.content_dir.resolve(), CONTENT_DIR, read_only=True)] - if "{source_dir}" in template: - if ctx.source_dir is None: - raise ContainerError(f"`{kind}` uses {{source_dir}} but no full-history source is available") - mounts.append(Mount(ctx.source_dir.resolve(), SOURCE_DIR, read_only=True)) - spec = ContainerSpec(image=ctx.image, argv=render_entry(template, task), workdir=WORKSPACE, - mounts=mounts + ctx.cache_mounts(), env=ctx.grade_env(task), network=network, name=name) - return ctx.runner.run(spec, stdout=run_dir / f"{kind}.log", stderr=run_dir / f"{kind}.err", - timeout_s=ctx.entry_timeout_s) + spec = ContainerSpec(image=ctx.image, argv=argv, workdir=WORKSPACE, mounts=mounts + extra + ctx.cache_mounts(), + env=ctx.grade_env(task), network=network, name=name) + result = ctx.runner.run(spec, stdout=run_dir / f"{kind}.log", stderr=run_dir / f"{kind}.err", + timeout_s=ctx.entry_timeout_s) + if builtin is not None and result.returncode == _NOT_FOUND_EXIT and _python3_not_found_in_stderr(run_dir, kind): + raise ContainerError(f"{task.raw['grader']} runs on the image's python3, and the image has none on PATH " + f"(`{kind}` exited {_NOT_FOUND_EXIT}; see {run_dir / f'{kind}.err'})") + return result + + +def _repo_entry(ctx: EvalContext, kind: str, task: Task) -> tuple[list[str], list[Mount]]: + """argv of the manifest's entry point ``kind``, and ``{source_dir}`` when it names it.""" + template = ctx.manifest.entry_points.get(kind) + if not template: + raise ContainerError(f"the manifest declares no `{kind}` entry point") + if "{source_dir}" not in template: + return render_entry(template, task), [] + if ctx.source_dir is None: + raise ContainerError(f"`{kind}` uses {{source_dir}} but no full-history source is available") + return render_entry(template, task), [Mount(ctx.source_dir.resolve(), SOURCE_DIR, read_only=True)] + + +def _builtin_entry(ctx: EvalContext, kind: str, task: Task, builtin: str) -> tuple[list[str], list[Mount]]: + """argv of built-in grader ``builtin`` for ``kind``, and the read-only mount of its code. + + ``-I``: no ``PYTHONPATH`` from ``image.env``, no user site and no script directory on ``sys.path``, + so nothing the agent wrote can stand in for a standard-library module.""" + if builtin not in BUILTIN_NAMES: + raise ContainerError(f"unknown built-in grader {task.raw['grader']!r}; known: " + + ", ".join(BUILTIN_PREFIX + n for n in BUILTIN_NAMES)) + try: + rel = task.dir.resolve().relative_to(ctx.content_dir.resolve()) + except ValueError: + raise ContainerError(f"task {task.id}: its directory {task.dir} is not in the exported eval dir") from None + argv = ["python3", "-I", "-B", f"{GRADERS_DIR}/{BUILTIN_ENTRY}", kind, builtin, + f"{CONTENT_DIR}/{rel.as_posix()}", RUN_DIR] + return argv, [Mount(GRADERS_HOST, GRADERS_DIR, read_only=True)] def _set_git_config_aside(run_dir: Path) -> None: @@ -210,23 +266,9 @@ def score_value(task: Task, grade: dict[str, Any] | None) -> float | None: def outcome(task: Task, run_dir: Path, result: ContainerResult, error: str | None = None) -> dict[str, Any]: """The framework's verdict on one ``grade`` run (contract §4.2); ``error`` when the run itself failed.""" grade, problems = read_grade(run_dir) - reasons = [] - if error: - reasons.append(error) - elif result.timed_out: - reasons.append("grade timed out") - elif result.returncode != 0: - reasons.append(f"grade exited {result.returncode}") - reasons += problems + reasons = _run_reasons(result, error) + problems reasons += [f"{name} is not a regular file after grading" for name in replaced_files(run_dir)] - if grade is not None and grade["pass"] is None and grade["errors"]: - reasons.append("grader error: " + str(grade["errors"][0]).strip().splitlines()[0][:300]) - elif grade is not None and grade["pass"] is None and not task.score: - # score-only is a property of the task, not of one grade: a gated task's grade must decide - reasons.append("grade.json is score-only (pass: null) but task.json has no `score`") - if not reasons and task.score and score_value(task, grade) is None: - # a missing score would leave the mean, while a grader failure counts as 0 - reasons.append(f"grade.json has no numeric metrics.{task.score['metric']} (task.json `score`)") + reasons += _grade_reasons(task, grade, bool(reasons)) failed = bool(reasons) score_only = not failed and grade is not None and grade["pass"] is None return { @@ -242,6 +284,29 @@ def outcome(task: Task, run_dir: Path, result: ContainerResult, error: str | Non } +def _run_reasons(result: ContainerResult, error: str | None) -> list[str]: + if error: + return [error] + if result.timed_out: + return ["grade timed out"] + return [f"grade exited {result.returncode}"] if result.returncode != 0 else [] + + +def _grade_reasons(task: Task, grade: dict[str, Any] | None, failed: bool) -> list[str]: + """Why a valid ``grade.json`` still makes a grader failure; ``failed``: the run already is one.""" + if grade is None: + return [] + if grade["pass"] is None and grade["errors"]: + return ["grader error: " + str(grade["errors"][0]).strip().splitlines()[0][:300]] + if grade["pass"] is None and not task.score: + # score-only is a property of the task, not of one grade: a gated task's grade must decide + return ["grade.json is score-only (pass: null) but task.json has no `score`"] + if not failed and task.score and score_value(task, grade) is None: + # a missing score would leave the mean, while a grader failure counts as 0 + return [f"grade.json has no numeric metrics.{task.score['metric']} (task.json `score`)"] + return [] + + def failed_outcome(reason: str) -> dict[str, Any]: """:func:`outcome` for a run that was never graded (the grade could not start, or the host failed around it): a grader failure that reads nothing from the run dir.""" diff --git a/dokimi/dokimi/repos/manifest.py b/dokimi/dokimi/repos/manifest.py index 9ab20dd3..28cfbc75 100644 --- a/dokimi/dokimi/repos/manifest.py +++ b/dokimi/dokimi/repos/manifest.py @@ -31,6 +31,7 @@ from dokimi.artifacts import schema_errors +from .graders import DATA_FILES, NAMES as BUILTIN_NAMES, PREFIX as BUILTIN_PREFIX from .host_spec import HostSpecError, parse_host from .source import RepoTree @@ -453,6 +454,9 @@ def _check_task(tree: RepoTree, task_dir: str, report: RepoCheckReport) -> None: problems.append(f"patch {patch!r}: {problem}") elif not tree.is_file(f"{task_dir}/{patch}"): problems.append(f"patch file does not exist: {patch}") + grader = raw.get("grader") + if isinstance(grader, str) and grader.startswith(BUILTIN_PREFIX): + problems += _builtin_problems(tree, task_dir, raw, grader[len(BUILTIN_PREFIX):]) base = raw.get("base") if isinstance(base, str) and base.strip(): @@ -522,6 +526,25 @@ def _positive(value: Any) -> bool: return _is_number(value) and math.isfinite(value) and value > 0 +def _builtin_problems(tree: RepoTree, task_dir: str, raw: dict[str, Any], name: str) -> list[str]: + """A ``dokimi:`` grader must be one the framework has, and its data file must be in the task directory.""" + if name not in BUILTIN_NAMES: + return [f"unknown built-in grader {BUILTIN_PREFIX + name!r}; known: " + + ", ".join(BUILTIN_PREFIX + n for n in BUILTIN_NAMES)] + key, default = DATA_FILES[name] + path = raw.get(key, default) + if not isinstance(path, str): + return [f"{key} must be a path"] + problem = _path_problem(path) + if problem: + return [f"{key} {path!r}: {problem}"] + if not tree.is_file(f"{task_dir}/{path}"): + return [f"{key} file does not exist: {path}"] + if name == "review_replay" and not _positive(raw.get("window", 5)): # review.py compares lines to it + return [f"window must be a positive number, not {raw.get('window')!r}"] + return [] + + def _path_fields(raw: dict[str, Any]) -> list[tuple[str, str]]: fields: list[tuple[str, str]] = [(key, raw[key]) for key in ("eval_dir", "tasks") if key in raw] if raw.get("explore_tasks"): diff --git a/dokimi/pyproject.toml b/dokimi/pyproject.toml index 70a69353..46cc2345 100644 --- a/dokimi/pyproject.toml +++ b/dokimi/pyproject.toml @@ -151,6 +151,8 @@ extend-exclude = ["benchmarks", "data"] [tool.ruff.per-file-target-version] "dokimi/repos/support/no_background.py" = "py310" "dokimi/harnesses/egress_proxy.py" = "py310" +# The built-in review graders run on the task image's python3 too, 3.8 or newer (contract §4.5). +"dokimi/repos/graders/*.py" = "py38" # A correctness gate: rules that flag code that misbehaves (a swallowed # KeyboardInterrupt, a naive datetime, a file handle never closed, an assert diff --git a/dokimi/tests/test_repo_builtin_graders.py b/dokimi/tests/test_repo_builtin_graders.py new file mode 100644 index 00000000..c2f3e7a3 --- /dev/null +++ b/dokimi/tests/test_repo_builtin_graders.py @@ -0,0 +1,585 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Built-in review graders ``dokimi:review_seeds`` and ``dokimi:review_replay`` (contract §4.5). No docker, no LLM.""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from dokimi.cli import build_parser +from dokimi.repos.container import CONTENT_DIR, GRADERS_DIR, SOURCE_DIR, ContainerError, ContainerResult +from dokimi.repos.graders import HOST_DIR, builtin_name, main, review +from dokimi.repos.grading import EvalContext, check_grade, run_entry +from dokimi.repos.manifest import parse_manifest +from dokimi.repos.workspace import Task +from repo_eval_fixture import MANIFEST, FakeRunner, fake_harness, git, make_repo, write + +SEEDS = {"seeds": [{"id": "s_op", "class": "correctness", "file": "src/calc.py", "lines": [5], "kw": "mul.*(add|plus|\\+)"}, + {"id": "s_name", "class": "convention", "file": "src/calc.py", "lines": [4], "kw": "naming"}], + "decoys": [{"id": "d_add", "file": "src/calc.py", "lines": [1], "kw": "add\\(\\) is unused"}]} +GOLD = [{"id": "t_op", "file": "src/calc.py", "line": 5, "kind": "correctness", + "comment": "mul adds instead of multiplying\nsecond line", "addressed": True}, + {"id": "t_doc", "file": "src/calc.py", "line": 4, "kind": "docs", "comment": "docstring?", "addressed": False}] +ORACLE = [{"file": "src/calc.py", "line": 5, "severity": "major", "rule": "wrong-op", "message": "mul adds"}, + {"file": "src/calc.py", "line": 4, "severity": "minor", "rule": "naming", "message": "name it multiply"}] +REVIEW = {"family": "review", "review_patch": "change.patch", "prompt": "Review HEAD."} +TASKS = { + "seeds_mul": {**REVIEW, "grader": "dokimi:review_seeds", "score": {"metric": "recall", "max": 1.0}}, + "replay_mul": {**REVIEW, "grader": "dokimi:review_replay", "score": {"metric": "recall", "max": 1.0}}, + "clean_mul": {**REVIEW, "grader": "dokimi:review_seeds"}, # clean-diff control: gate ["pass"] + "addressed_mul": {**REVIEW, "grader": "dokimi:review_replay", "score": {"metric": "recall_addressed", "max": 1.0}}, +} +DATA = {"seeds_mul": {"manifest.json": SEEDS, "oracle.json": ORACLE}, "replay_mul": {"gold.json": GOLD}, + "clean_mul": {"manifest.json": {"seeds": [], "decoys": []}, "oracle.json": []}, + "addressed_mul": {"gold.json": GOLD}} + + +def fence(findings) -> str: + return "Review done.\n```json\n" + json.dumps(findings) + "\n```\n" + + +def _task_dir(tmp: Path, task_id: str) -> Path: + for name, doc in DATA[task_id].items(): + write(tmp / task_id, name, json.dumps(doc)) + return tmp / task_id + + +def _f(line, file="src/calc.py", severity="minor", rule="", message=""): + return {"file": file, "line": line, "severity": severity, "rule": rule, "message": message} + + +# ------------------------------------------------------------------ semantics + + +def test_last_json_takes_the_last_parseable_fence(): + assert review.last_json("```json\n[1]\n``` and ```json\n[2]\n``` then ```json\n{broken\n```") == [2] + assert review.last_json("no fence") is None and review.last_json(None) is None + + +def test_a_fence_nested_too_deep_is_unreadable_not_a_grader_crash(tmp_path): + seeds, replay = _task_dir(tmp_path, "seeds_mul"), _task_dir(tmp_path, "replay_mul") + deep = "```json\n" + "[" * 100000 + "]" * 100000 + "\n```" + assert review.grade_seeds({}, seeds, deep) == {"pass": False, "parse_error": True, "recall": 0.0} + assert review.grade_replay({}, replay, deep)["parse_error"] is True + # an earlier readable fence is still the answer + assert review.grade_seeds({}, seeds, review.oracle_seeds({}, seeds) + deep)["recall"] == 1.0 + assert review.grade_replay({}, replay, review.oracle_replay({}, replay) + deep)["recall"] == 1.0 + + +def test_fence_nesting_cap_is_exactly_max_depth(): + def nested(n): + return "```json\n" + "[" * n + "]" * n + "\n```" + + cap = review.MAX_DEPTH + assert review._too_deep("[" * cap + "]" * cap) is False + assert review._too_deep("[" * (cap + 1) + "]" * (cap + 1)) is True + assert review.last_json(nested(cap)) is not None # just under the cap parses + assert review.last_json(nested(cap + 1)) is None # just over is unreadable + # brackets inside strings do not count toward the depth + assert review._too_deep('["' + "[" * (cap + 5) + '"]') is False + # an over-deep last fence falls back to the earlier readable one + assert review.last_json("```json\n[7]\n```" + nested(cap + 1)) == [7] + + +def test_the_nesting_cap_counts_open_brackets_not_brackets_seen(tmp_path): + d = _task_dir(tmp_path, "seeds_mul") + cap = review.MAX_DEPTH + many = ORACLE + [_f(90 + i, message=f"note {i}") for i in range(2 * cap)] # more objects than the cap + assert review.grade_seeds({}, d, fence(many))["recall"] == 1.0 + quoted = [{**f, "message": 'quotes "' + "[" * (2 * cap) + '"'} for f in ORACLE] # \" stays in the string + assert review.grade_seeds({}, d, fence(quoted))["recall"] == 1.0 + + +def test_norm_path_cuts_at_workspace_and_repo_and_drops_leading_dots_and_slashes(): + # workspace path (agent's working directory in container) + assert review.norm_path("/eval/run/workspace/src/calc.py") == "src/calc.py" + # /repo/ path (legacy, for backwards compatibility) + assert review.norm_path("/runs/r1/repo/src/calc.py") == "src/calc.py" + assert review.norm_path(" ./././src\\calc.py ") == "src/calc.py" + assert review.norm_path(None) == "" + + +def test_seeds_keyword_line_fallback_decoys_and_recall_by_class(tmp_path): + d = _task_dir(tmp_path, "seeds_mul") + findings = [ + _f(40, message="mul returns a + b"), # keyword hit far from the line + _f(1, message="add() is unused", severity="Major"), # a decoy, by keyword + _f(1, message="something"), # a decoy, by line + _f(30, file="other.py", severity="blocker"), # unmatched + "not an object", + ] + g = review.grade_seeds({}, d, fence(findings)) + assert g == {"pass": None, "n_findings": 4, "n_severe": 2, "seeds_hit": ["s_op"], "decoys_flagged": ["d_add"], + "n_decoys": 1, "unmatched": 1, "recall": 0.5, + "recall_by_class": {"correctness": "1/1", "convention": "0/1"}} + + +def test_seeds_with_workspace_paths(tmp_path): + """Regression: findings with workspace paths (/eval/run/workspace) must match seeds correctly.""" + d = _task_dir(tmp_path, "seeds_mul") + findings = [ + _f(5, file="/eval/run/workspace/src/calc.py", message="mul returns a + b"), # keyword hit with workspace path + _f(4, file="/eval/run/workspace/src/calc.py", message="naming issue"), # line fallback with workspace path + ] + g = review.grade_seeds({}, d, fence(findings)) + assert g["seeds_hit"] == ["s_name", "s_op"], f"workspace paths should match seeds; got {g}" + assert g["recall"] == 1.0 + + +def test_replay_with_workspace_paths(tmp_path): + """Regression: findings with workspace paths (/eval/run/workspace) must match gold threads correctly.""" + d = _task_dir(tmp_path, "replay_mul") + findings = [ + _f(5, file="/eval/run/workspace/src/calc.py"), + _f(4, file="/eval/run/workspace/src/calc.py"), + ] + g = review.grade_replay({}, d, fence(findings)) + assert g["threads_hit"] == ["t_doc", "t_op"], f"workspace paths should match threads; got {g}" + assert g["recall"] == 1.0 + + +def test_a_nested_repo_directory_matches_from_any_path_form(tmp_path): + seeds = {"seeds": [{"id": "s", "class": "c", "file": "src/repo/calc.py", "lines": [5], "kw": "nomatch"}]} + write(tmp_path / "t", "manifest.json", json.dumps(seeds)) + gold = [{"id": "g", "file": "src/repo/calc.py", "line": 5, "kind": "k", "comment": "c"}] + write(tmp_path / "t", "gold.json", json.dumps(gold)) + for path in ("src/repo/calc.py", "/eval/run/workspace/src/repo/calc.py", "/runs/r1/repo/src/repo/calc.py"): + assert review.grade_seeds({}, tmp_path / "t", fence([_f(5, file=path)]))["recall"] == 1.0, path + assert review.grade_replay({}, tmp_path / "t", fence([_f(5, file=path)]))["recall"] == 1.0, path + + +def test_line_fallback_credits_only_the_nearest_seed_within_one_line(tmp_path): + d = _task_dir(tmp_path, "seeds_mul") + assert review.grade_seeds({}, d, fence([_f(4)]))["seeds_hit"] == ["s_name"] # 0 from s_name, 1 from s_op + assert review.grade_seeds({}, d, fence([_f("6")]))["seeds_hit"] == ["s_op"] # a string line still counts + assert review.grade_seeds({}, d, fence([_f(7)]))["seeds_hit"] == [] + assert review.grade_seeds({}, d, fence([_f("x")]))["unmatched"] == 1 # unreadable line: near nothing + + +@pytest.mark.parametrize(("line", "read"), [("6", 6), (" 7 ", 7), (5.9, 5), (0, -99), (None, -99), ("6.0", -99)]) +def test_a_finding_line_is_read_as_the_contract_says(line, read): + assert review.finding_fields({"file": "a", "line": line})[2] == read + + +@pytest.mark.parametrize("line", ["Infinity", "-Infinity", "1e999"]) +def test_an_infinite_line_is_unreadable_not_a_grader_crash(tmp_path, line): + seeds, replay = _task_dir(tmp_path, "seeds_mul"), _task_dir(tmp_path, "replay_mul") + text = '```json\n[{"file": "src/calc.py", "line": ' + line + ', "message": "x"}]\n```' + assert review.grade_seeds({}, seeds, text)["unmatched"] == 1 + assert review.grade_replay({}, replay, text)["threads_hit"] == [] + + +def test_seeds_parse_error_and_clean_control(tmp_path): + seeds, clean = _task_dir(tmp_path, "seeds_mul"), _task_dir(tmp_path, "clean_mul") + unparsed = {"pass": False, "parse_error": True, "recall": 0.0} + assert review.grade_seeds({}, seeds, "") == unparsed + assert review.grade_seeds({}, seeds, "```json\n{\"file\": \"a\"}\n```") == unparsed + assert review.grade_seeds({}, clean, "") == {**unparsed, "recall": None} + assert review.grade_seeds({}, clean, fence([_f(5, severity="minor")]))["pass"] is True + g = review.grade_seeds({}, clean, fence([_f(5, severity="blocker")])) + assert g["pass"] is False and g["recall"] is None and g["unmatched"] == 1 + + +def test_replay_is_one_to_one_nearest_first_within_the_window(tmp_path): + d = _task_dir(tmp_path, "replay_mul") + # finding 0 is nearest to both threads' lines: it takes t_op (distance 0), finding 1 takes t_doc + g = review.grade_replay({}, d, fence([_f(5), _f(5), _f(20)])) + assert g["threads_hit"] == ["t_doc", "t_op"] and g["recall"] == g["recall_all"] == 1.0 + assert g["precision_proxy"] == 0.667 and g["recall_addressed"] == 1.0 and g["pass"] is None + assert g["recall_by_kind"] == {"correctness": "1/1", "docs": "1/1"} and g["decoys_flagged"] == [] + one = review.grade_replay({}, d, fence([_f(5)])) + assert one["threads_hit"] == ["t_op"] and one["recall"] == 0.5 and one["recall_addressed"] == 1.0 + assert review.grade_replay({"window": 2}, d, fence([_f(8)]))["threads_hit"] == [] + assert review.grade_replay({}, d, fence([_f(3)]))["threads_hit"] == ["t_doc"] # 1 from t_doc, 2 from t_op + assert review.grade_replay({}, d, fence([]))["precision_proxy"] is None + assert review.grade_replay({}, d, "none") == {"pass": False, "parse_error": True, "recall": 0.0, "recall_all": 0.0, + "recall_addressed": 0.0} + + +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.""" + seeds, replay = _task_dir(tmp_path, "seeds_mul"), _task_dir(tmp_path, "replay_mul") + other = "src/other.py" + assert review.grade_seeds({}, seeds, fence([_f(40, file=other, message="mul returns a + b")]))["seeds_hit"] == [] + assert review.grade_seeds({}, seeds, fence([_f(40, message="MUL RETURNS A + B")]))["seeds_hit"] == ["s_op"] + assert review.grade_seeds({}, seeds, fence([_f(1, file=other, message="add() is unused")]))["decoys_flagged"] == [] + assert review.grade_seeds({}, seeds, fence([_f(30, message="ADD() IS UNUSED")]))["decoys_flagged"] == ["d_add"] + assert review.grade_replay({}, replay, fence([_f(5, file=other)]))["threads_hit"] == [] + assert review.grade_replay({"window": 2}, replay, fence([_f(7)]))["threads_hit"] == ["t_op"] + assert review.grade_replay({}, replay, fence([_f(10)]))["threads_hit"] == ["t_op"] + assert review.grade_replay({}, replay, fence([_f(11)]))["threads_hit"] == [] + empty = {"id": "e", "class": "c", "file": "src/calc.py", "lines": [], "kw": "zzz"} + write(tmp_path / "e", "manifest.json", json.dumps({"seeds": [empty]})) + assert review.grade_seeds({}, tmp_path / "e", fence([_f(5)]))["seeds_hit"] == [] + write(tmp_path / "c", "gold.json", json.dumps([{"id": "c", "file": "a.py", "line": 1, "comment": "x" * 300}])) + assert review.last_json(review.oracle_replay({}, tmp_path / "c"))[0]["message"] == "x" * 200 + + +def test_oracles_reach_full_recall(tmp_path): + for task_id, grade, oracle in (("seeds_mul", review.grade_seeds, review.oracle_seeds), + ("replay_mul", review.grade_replay, review.oracle_replay)): + d = _task_dir(tmp_path, task_id) + assert grade({}, d, oracle({}, d))["recall"] == 1.0 + replay = review.last_json(review.oracle_replay({}, _task_dir(tmp_path, "replay_mul"))) + assert replay[0] == {"file": "src/calc.py", "line": 5, "severity": "minor", "rule": "correctness", + "message": "mul adds instead of multiplying"} + + +# ------------------------------------------------------------------ the entry script + + +def _script(tmp_path, kind, name, task_id, answer="", task=None): + rd = tmp_path / "run" + rd.mkdir(exist_ok=True) + (rd / "task.json").write_text(json.dumps(task or {"id": task_id, "grader": f"dokimi:{name}"})) + (rd / "answer.txt").write_text(answer) + # as the grading container runs it: -I, so the script's own directory is not on sys.path + proc = subprocess.run([sys.executable, "-I", "-B", str(HOST_DIR / "main.py"), kind, name, + str(_task_dir(tmp_path, task_id)), str(rd)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + return rd + + +def test_entry_script_writes_repo_grade_v1_with_scalars_under_metrics(tmp_path): + rd = _script(tmp_path, "grade", "review_seeds", "seeds_mul", fence(ORACLE)) + doc = json.loads((rd / "grade.json").read_text()) + assert check_grade(doc) == [] + assert doc["task"] == "seeds_mul" and doc["grader"] == "dokimi:review_seeds" and doc["pass"] is None + assert doc["metrics"] == {"n_findings": 2, "n_severe": 1, "n_decoys": 1, "unmatched": 0, "recall": 1.0} + assert doc["details"] == {"seeds_hit": ["s_name", "s_op"], "decoys_flagged": [], "grader_version": review.VERSION, + "recall_by_class": {"correctness": "1/1", "convention": "1/1"}} + + +def test_entry_script_oracle_writes_the_answer(tmp_path): + rd = _script(tmp_path, "oracle", "review_replay", "replay_mul") + assert review.last_json((rd / "answer.txt").read_text())[1]["line"] == 4 + + +def test_a_grader_that_raises_is_not_graded_and_an_unreached_gate_metric_is_null(tmp_path): + task = {"id": "seeds_mul", "grader": "dokimi:review_seeds", "manifest": "missing.json", + "gate": ["recall_ok", {"metric": "recall", "min": 0.5}, "pass"]} + doc = json.loads((_script(tmp_path, "grade", "review_seeds", "seeds_mul", task=task) / "grade.json").read_text()) + assert doc["pass"] is None and doc["errors"][0].startswith("FileNotFoundError") and doc["metrics"] == {} + task["manifest"] = "manifest.json" + doc = json.loads((_script(tmp_path, "grade", "review_seeds", "seeds_mul", "", task) / "grade.json").read_text()) + assert doc["metrics"] == {"parse_error": True, "recall": 0.0, "recall_ok": None} and doc["pass"] is False + + +def test_oracle_then_grade_in_process_and_a_missing_answer_is_empty(tmp_path): + for task_id, name in (("seeds_mul", "review_seeds"), ("replay_mul", "review_replay")): + rd = tmp_path / task_id / "run" + rd.mkdir(parents=True) + (rd / "task.json").write_text(json.dumps({"id": task_id, "grader": f"dokimi:{name}"})) + d = _task_dir(tmp_path / "data", task_id) + assert main.grade(name, d, rd)["metrics"]["parse_error"] is True # no answer.txt + assert main.oracle(name, d, rd) == (rd / "answer.txt").read_text() + doc = main.grade(name, d, rd) + assert doc["metrics"]["recall"] == 1.0 and json.loads((rd / "grade.json").read_text()) == doc + + +def test_entry_script_usage_and_split_result(): + assert main.main(["grade", "nope", "x"]) == 2 + assert main.split_result({"pass": True, "a": 1, "b": None, "c": [1], "d": "s", "e": {"x": 1}, "f": False}) == ( + {"a": 1, "b": None, "f": False}, {"c": [1], "d": "s", "e": {"x": 1}}) + + +def test_builtin_name(): + assert builtin_name("dokimi:review_seeds") == "review_seeds" and builtin_name("dokimi:nope") == "nope" + assert builtin_name("review") is None and builtin_name(None) is None + + +# ------------------------------------------------------------------ dispatch in run_entry + + +def _repo(tmp_path, extra=None): + tasks = {**TASKS, **(extra or {})} + root, _ = make_repo(tmp_path, tasks) + for task_id, files in DATA.items(): + for name, doc in files.items(): + write(root, f".agent-eval/tasks/{task_id}/{name}", json.dumps(doc)) + git(root, "add", "-A") + git(root, "commit", "-qm", "built-in review tasks") + return root + + +def _check(root, out, runner, monkeypatch, *tasks): + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, **k: runner) + argv = ["repo", "tasks", "check", f"{root}@HEAD", "--manifest", ".agent-eval/repo-eval.yaml", + "--out", str(out), "--cache-root", str(out / "caches")] + for t in tasks: + argv += ["--task", t] + args = build_parser().parse_args(argv) + try: + args.func(args) + except SystemExit: + pass + return {t["task"]: t for t in json.loads((out / "check/check.json").read_text())["tasks"]} + + +def test_tasks_check_runs_builtin_graders_not_the_repository_grade(tmp_path, monkeypatch): + runner = FakeRunner() + res = _check(_repo(tmp_path), tmp_path / "chk", runner, monkeypatch, *TASKS, "fix_add") + assert {k: v["verdict"] for k, v in res.items()} == dict.fromkeys([*TASKS, "fix_add"], "ok"), res + for task_id in ("seeds_mul", "replay_mul", "addressed_mul"): + null, oracle = res[task_id]["null"], res[task_id]["oracle"] + assert oracle["score"] == 1.0 and oracle["score_only"] + # an empty answer has no findings: a failed run with recall 0 (parse_error), not a grader failure + assert null["status"] == "graded" and null["pass"] is False and null["success"] is False + assert null["score"] == 0.0 + assert res["clean_mul"]["null"]["success"] is False and res["clean_mul"]["oracle"]["success"] is True + builtin = [s for s in runner.specs if f"{GRADERS_DIR}/main.py" in s.argv] + repo = [s for s in runner.specs if any(a.endswith("/grade.py") for a in s.argv)] + assert len(builtin) == 3 * len(TASKS) and len(repo) == 3 # each: null grade, oracle, oracle grade + for s in builtin: + assert s.argv[:3] == ["python3", "-I", "-B"] + kind, name, task_dir = s.argv[4:7] + assert kind in ("grade", "oracle") and name in ("review_seeds", "review_replay") + assert task_dir.startswith(f"{CONTENT_DIR}/tasks/") + mounts = {m.container: m for m in s.mounts} + assert mounts[GRADERS_DIR].read_only and mounts[GRADERS_DIR].host == HOST_DIR + assert mounts[CONTENT_DIR].read_only and SOURCE_DIR not in mounts # the repository's oracle names it + assert all(GRADERS_DIR not in {m.container for m in s.mounts} for s in repo) + + +def test_repo_validate_finds_a_builtin_task_without_its_data_file_or_a_wrong_name(tmp_path, capsys): + root = _repo(tmp_path, {"nodata": {**REVIEW, "grader": "dokimi:review_seeds"}, # no manifest.json + "typo": {**REVIEW, "grader": "dokimi:review_seed"}}) + args = build_parser().parse_args(["repo", "validate", f"{root}@HEAD", "--manifest", ".agent-eval/repo-eval.yaml", + "--cache-dir", str(tmp_path / "caches"), "--json"]) + with pytest.raises(SystemExit): + args.func(args) + errors = json.loads(capsys.readouterr().out)["errors"] + assert any("manifest file does not exist: manifest.json" in e and "nodata" in e for e in errors), errors + assert any("unknown built-in grader 'dokimi:review_seed'" in e for e in errors), errors + + +def test_an_unknown_builtin_grader_is_refused_not_given_to_the_repository(tmp_path, monkeypatch): + root = _repo(tmp_path, {"typo": {**REVIEW, "grader": "dokimi:review_seed"}}) + res = _check(root, tmp_path / "chk", FakeRunner(), monkeypatch, "typo") + assert res["typo"]["verdict"] == "broken" + assert "unknown built-in grader 'dokimi:review_seed'" in res["typo"]["problems"][0] + + +class NoPython(FakeRunner): + """An image without python3: docker run exits 127.""" + + def run(self, spec, *, stdout, stderr, timeout_s): + if spec.argv[0] == "python3": + self.specs.append(spec) + stdout.write_text("") + stderr.write_text("[FATAL tini (7)] exec python3 failed: No such file or directory\n") + return ContainerResult(returncode=127, timed_out=False, wall_s=0.0) + return super().run(spec, stdout=stdout, stderr=stderr, timeout_s=timeout_s) + + +def test_an_image_without_python3_is_a_grader_failure_with_the_reason(tmp_path, monkeypatch): + root = _repo(tmp_path) + res = _check(root, tmp_path / "chk", NoPython(), monkeypatch, "clean_mul") + assert res["clean_mul"]["verdict"] == "broken" + assert "dokimi:review_seeds runs on the image's python3, and the image has none on PATH" in \ + res["clean_mul"]["problems"][0] + + exe, profile = fake_harness(tmp_path) + monkeypatch.setenv("FAKE_ACTION", "fix") + no_python_runner = NoPython() # capture for verification after loop + for runner, check in ((no_python_runner, "none on PATH"), (FakeRunner(), None)): + out = tmp_path / ("runs-np" if check else "runs") + monkeypatch.setattr("dokimi.repos.container.DockerRunner", lambda *a, r=runner, **k: r) + args = build_parser().parse_args([ + "repo", "run", f"{root}@HEAD", "--manifest", ".agent-eval/repo-eval.yaml", "--harness", str(profile), + "--bin", str(exe), "--model", "m-1", "--batch", "b1", "--out", str(out), "--cache-root", + str(out / "caches"), "--agent-network", "host", "--arm", "guidance:raw", "--task", "clean_mul"]) + args.func(args) + cell = next((out / "b1").glob("clean_mul__*/result.json")).parent + grade = json.loads((cell / "result.json").read_text())["grade"] + if check: + assert grade["status"] == "grader_failure" and check in grade["reasons"][0] + else: # the fake agent's answer has no findings JSON: graded, and failed + assert grade["status"] == "graded" and grade["success"] is False and grade["pass"] is False + assert json.loads((cell / "grade.json").read_text())["metrics"]["parse_error"] is True + + # Verify that NoPython runner was called with python3 command (the 127-exit-code path) + assert any(s.argv[0] == "python3" for s in no_python_runner.specs), \ + "Built-in grader must invoke python3 to trigger 127-exit detection" + + +def test_builtin_grader_stderr_validation_for_127_exit(tmp_path): + """Only treat 127 exit as python3-not-found if stderr has expected signature.""" + from dokimi.repos.grading import _python3_not_found_in_stderr + + # Write stderr with python3 exec failure signature + (tmp_path / "grade.err").write_text("exec python3 failed: No such file or directory") + assert _python3_not_found_in_stderr(tmp_path, "grade") + + # the container runs with --init, so tini's message is the only one a missing python3 produces + (tmp_path / "grade.err").write_text("python3: not found") + assert not _python3_not_found_in_stderr(tmp_path, "grade") + + # Write stderr without signature (127 exit from grader itself) + (tmp_path / "grade.err").write_text("Grader exit 127 for other reason\n") + assert not _python3_not_found_in_stderr(tmp_path, "grade") + + # No stderr file (missing error logs) + (tmp_path / "grade.err").unlink() + assert not _python3_not_found_in_stderr(tmp_path, "grade") + + +def test_run_entry_needs_the_task_inside_the_exported_eval_dir(tmp_path): + ctx = EvalContext(manifest=parse_manifest(MANIFEST), git_dir=tmp_path, content_dir=tmp_path / "content", + image="img", runner=FakeRunner()) + (tmp_path / "content").mkdir() + task = Task(id="t", raw={"grader": "dokimi:review_seeds"}, dir=tmp_path / "elsewhere" / "t") + with pytest.raises(ContainerError, match="not in the exported eval dir"): + run_entry(ctx, "grade", task, tmp_path / "rd", "none") + + +# ------------------------------------------------------------------ review of 243c6a4 + + +def test_mixed_int_and_str_ids_do_not_crash_the_graders(tmp_path): + seeds = {"seeds": [{"id": 1, "class": "c", "file": "src/calc.py", "lines": [5], "kw": "mul"}, + {"id": "b", "class": "c", "file": "src/calc.py", "lines": [5], "kw": "mul"}]} + gold = [{"id": 1, "file": "src/calc.py", "line": 5, "kind": "k", "comment": "c"}, + {"id": "b", "file": "src/calc.py", "line": 5, "kind": "k", "comment": "c"}] + write(tmp_path / "t", "manifest.json", json.dumps(seeds)) + write(tmp_path / "t", "gold.json", json.dumps(gold)) + assert review.grade_seeds({}, tmp_path / "t", fence([_f(5, message="mul")]))["seeds_hit"] == [1, "b"] + # no keyword: the line fallback credits the nearest seed, and both are one line away + assert review.grade_seeds({}, tmp_path / "t", fence([_f(6)]))["seeds_hit"] == [1] + assert review.grade_replay({}, tmp_path / "t", fence([_f(5)]))["threads_hit"] == [1] + + +def test_the_replay_oracle_survives_an_empty_comment_and_a_missing_kind(tmp_path): + write(tmp_path / "t", "gold.json", json.dumps([{"id": "e", "file": "src/calc.py", "line": 5, "comment": ""}, + {"id": "k", "file": "src/calc.py", "line": 7, "kind": "docs"}])) + ans = review.last_json(review.oracle_replay({}, tmp_path / "t")) + assert ans[0] == {"file": "src/calc.py", "line": 5, "severity": "minor", "rule": "", "message": ""} + assert ans[1]["rule"] == "docs" + + +def test_the_replay_oracle_reaches_full_recall_on_a_suggestion_comment(tmp_path): + write(tmp_path / "t", "gold.json", json.dumps([{"id": "s", "file": "src/calc.py", "line": 5, "kind": "k", + "comment": "```suggestion\n return a * b\n```"}])) + oracle = review.oracle_replay({}, tmp_path / "t") + assert review.grade_replay({}, tmp_path / "t", oracle)["recall"] == 1.0 + assert review.last_json(oracle)[0]["message"] == "```suggestion" + + +def test_the_seeds_oracle_reaches_full_recall_when_oracle_json_quotes_a_fence(tmp_path): + d = _task_dir(tmp_path, "seeds_mul") + quoted = [{**f, "message": f["message"] + " ```suggestion x``` naming"} for f in ORACLE] + write(d, "oracle.json", json.dumps(quoted)) + oracle = review.oracle_seeds({}, d) + assert review.grade_seeds({}, d, oracle)["recall"] == 1.0 + assert review.last_json(oracle) == quoted + write(d, "oracle.json", json.dumps(ORACLE)) # no fence in it: the text is the file's, unchanged + assert review.oracle_seeds({}, d) == "```json\n" + json.dumps(ORACLE) + "\n```" + + +def test_wildcard_seeds_and_absolute_paths_match_as_pinned(tmp_path): + seeds = {"seeds": [{"id": "w", "class": "c", "file": "*", "lines": [5], "kw": "zzz"}, + {"id": "s", "class": "c", "file": "src/calc.py", "lines": [9], "kw": "mul"}]} + write(tmp_path / "t", "manifest.json", json.dumps(seeds)) + d = tmp_path / "t" + # a "*" seed takes a keyword hit only: a line alone does not credit it + assert review.grade_seeds({}, d, fence([_f(5, message="nothing")]))["seeds_hit"] == [] + assert review.grade_seeds({}, d, fence([_f(5, file="other.py", message="zzz")]))["seeds_hit"] == ["w"] + # a leading slash is not part of the repository-relative path + assert review.grade_seeds({}, d, fence([_f(9, file="/src/calc.py", message="x")]))["seeds_hit"] == ["s"] + + +def test_a_repository_grader_exiting_127_is_not_the_python3_failure(tmp_path): + """Only a built-in grader's 127 is reported as a missing python3; a repository's grade stays a plain exit.""" + ctx = EvalContext(manifest=parse_manifest(MANIFEST), git_dir=tmp_path, content_dir=tmp_path / "content", + image="img", runner=NoPython(), caches={"build-cache": tmp_path}) + (tmp_path / "content").mkdir() + rd = tmp_path / "rd" + rd.mkdir() + task = Task(id="t", raw={"grader": "repo_review"}, dir=tmp_path / "content" / "t") + result = run_entry(ctx, "grade", task, rd, "none") + assert result.returncode == 127 and ctx.runner.specs[0].argv[0] == "python3" + + +def test_int_ids_break_ties_in_numeric_order_as_onedal_does(tmp_path): + seeds = {"seeds": [{"id": 10, "class": "c", "file": "src/calc.py", "lines": [4], "kw": "zzz"}, + {"id": 2, "class": "c", "file": "src/calc.py", "lines": [6], "kw": "zzz"}]} + gold = [{"id": 10, "file": "src/calc.py", "line": 4, "kind": "k", "comment": "c"}, + {"id": 2, "file": "src/calc.py", "line": 6, "kind": "k", "comment": "c"}] + write(tmp_path / "t", "manifest.json", json.dumps(seeds)) + write(tmp_path / "t", "gold.json", json.dumps(gold)) + # one finding at line 5 is one line from both seeds and both threads: the lower int id wins + assert review.grade_seeds({}, tmp_path / "t", fence([_f(5)]))["seeds_hit"] == [2] + assert review.grade_replay({}, tmp_path / "t", fence([_f(5)]))["threads_hit"] == [2] + # ids that all match by keyword are listed in numeric order, not as strings + assert review.grade_seeds({}, tmp_path / "t", fence([_f(4, message="zzz"), _f(6, message="zzz")]))[ + "seeds_hit"] == [2, 10] + + +def test_a_builtin_grader_exiting_127_for_another_reason_is_a_plain_exit(tmp_path): + """Only a missing python3 (tini's message on stderr) is the image failure; any other 127 is the grader's.""" + class OtherExit127(FakeRunner): + def run(self, spec, *, stdout, stderr, timeout_s): + if spec.argv[0] == "python3": + self.specs.append(spec) + stdout.write_text("") + stderr.write_text("Grader exit 127 for other reason\n") + return ContainerResult(returncode=127, timed_out=False, wall_s=0.0) + return super().run(spec, stdout=stdout, stderr=stderr, timeout_s=timeout_s) + + ctx = EvalContext(manifest=parse_manifest(MANIFEST), git_dir=tmp_path, content_dir=tmp_path / "content", + image="img", runner=OtherExit127(), caches={"build-cache": tmp_path}) + (tmp_path / "content").mkdir() + rd = tmp_path / "rd" + rd.mkdir() + task = Task(id="t", raw={"grader": "dokimi:review_seeds"}, dir=tmp_path / "content" / "t") + result = run_entry(ctx, "grade", task, rd, "none") # no ContainerError: not the python3 failure + assert result.returncode == 127 and ctx.runner.specs[0].argv[0] == "python3" + + +def test_a_replay_window_that_is_not_a_positive_number_is_an_authoring_error(tmp_path, capsys): + root = _repo(tmp_path, {"replay_mul": {**TASKS["replay_mul"], "window": "5"}}) # its gold.json is in DATA + args = build_parser().parse_args(["repo", "validate", f"{root}@HEAD", "--manifest", ".agent-eval/repo-eval.yaml", + "--cache-dir", str(tmp_path / "caches"), "--json"]) + with pytest.raises(SystemExit): + args.func(args) + errors = json.loads(capsys.readouterr().out)["errors"] + assert any("replay_mul" in e and "window must be a positive number" in e for e in errors), errors + + +def test_a_finding_without_a_file_never_matches_an_empty_seed_path(tmp_path): + # same_file must not treat "no file" and an empty/root seed path as the same file + seeds = {"seeds": [{"id": "e", "class": "c", "file": "", "lines": [5], "kw": "zzz"}, + {"id": "r", "class": "c", "file": "/", "lines": [7], "kw": "zzz"}]} + write(tmp_path / "t", "manifest.json", json.dumps(seeds)) + d = tmp_path / "t" + no_file = {"line": 5, "severity": "minor", "rule": "", "message": "nothing"} + assert review.grade_seeds({}, d, fence([no_file]))["seeds_hit"] == [] + assert review.grade_seeds({}, d, fence([_f(5, file="", message="nothing")]))["seeds_hit"] == [] + assert review.grade_seeds({}, d, fence([_f(7, file="/", message="nothing")]))["seeds_hit"] == [] + + +def test_a_replay_window_is_checked_only_for_replay_and_defaults_to_five(tmp_path, capsys): + root = _repo(tmp_path, {"replay_nowin": {**REVIEW, "grader": "dokimi:review_replay"}, + "replay_win3": {**REVIEW, "grader": "dokimi:review_replay", "window": 3}, + "seeds_strwin": {**REVIEW, "grader": "dokimi:review_seeds", "window": "x"}}) + for task_id in ("replay_nowin", "replay_win3", "seeds_strwin"): + write(root, f".agent-eval/tasks/{task_id}/gold.json" if task_id.startswith("replay") + else f".agent-eval/tasks/{task_id}/manifest.json", + json.dumps(GOLD if task_id.startswith("replay") else {"seeds": [], "decoys": []})) + git(root, "add", "-A") + git(root, "commit", "-qm", "window cases") + args = build_parser().parse_args(["repo", "validate", f"{root}@HEAD", "--manifest", ".agent-eval/repo-eval.yaml", + "--cache-dir", str(tmp_path / "caches"), "--json"]) + try: + args.func(args) + except SystemExit: + pass + out = capsys.readouterr().out + errors = json.loads(out).get("errors", []) if out.strip() else [] + assert not any(t in e for e in errors for t in ("replay_nowin", "replay_win3", "seeds_strwin")), errors diff --git a/dokimi/tests/test_repo_compare.py b/dokimi/tests/test_repo_compare.py index 1291bf35..a63b9c74 100644 --- a/dokimi/tests/test_repo_compare.py +++ b/dokimi/tests/test_repo_compare.py @@ -558,7 +558,7 @@ def test_an_unstamped_batch_beside_a_stamped_one_is_named(tmp_path): _cell(old, "fix_a", "guidance:raw", 1) _cell(new, "fix_a", "guidance:raw", 1) s = compare([old, new], "old/guidance:raw", ["new/guidance:raw"]) - assert s["warnings"] == ["batch new has no stamp (_eval/batch.json); its eval_dir and image are not compared"] + assert s["warnings"] == ["batch new has no stamp (_eval/batch.json); its eval_dir, image and graders are not compared"] def test_images_built_from_each_tree_follow_the_tree(tmp_path): @@ -679,3 +679,13 @@ def test_an_unreadable_batch_stamp_is_no_stamp(tmp_path): _cell(b, "fix_a", "guidance:none", 1) _cell(b, "fix_a", "guidance:raw", 1) assert compare([b], "guidance:none")["stamps"]["b1"] == {"harness_versions": ["1.0"], "model_resolved": []} + + +def test_arms_from_batches_with_different_built_in_graders_warn(tmp_path): + old, new = tmp_path / "old", tmp_path / "new" + _stamp(old, tree="repo@abc", eval_dir="e1", image="img:1", graders="g1") + _stamp(new, tree="repo@abc", eval_dir="e1", image="img:1", graders="g2") + _cell(old, "fix_a", "guidance:raw", 1) + _cell(new, "fix_a", "guidance:raw", 1) + s = compare([old, new], "old/guidance:raw", ["new/guidance:raw"]) + assert s["warnings"] == ["new/guidance:raw vs old/guidance:raw: the batches ran from different graders (g1, g2)"] diff --git a/dokimi/tests/test_repo_run.py b/dokimi/tests/test_repo_run.py index 9cba295d..43161e66 100644 --- a/dokimi/tests/test_repo_run.py +++ b/dokimi/tests/test_repo_run.py @@ -613,3 +613,69 @@ def failing(path, data): assert "0 ran, 0 already done, 1 crashed" in out.out and "$0.0100" in out.out assert "cannot write result.json" in out.out + out.err + + +def test_a_batch_refuses_resume_after_a_built_in_grader_changes_only_if_a_task_names_one(setup, monkeypatch, + capsys): + _run(setup, "--arm", "guidance:raw", "--task", "fix_add") + monkeypatch.setattr("dokimi.repos.graders.graders_digest", lambda: "edited") + _run(setup, "--arm", "guidance:none", "--task", "fix_add") # no task names one: no score depends on them + assert "graders" not in json.loads((setup["out"] / "b1/_eval/batch.json").read_text()) + root = setup["root"] + shutil.copytree(root / ".agent-eval/tasks/review_add", root / ".agent-eval/tasks/seeds_add") + task = root / ".agent-eval/tasks/seeds_add/task.json" + task.write_text(json.dumps({**json.loads(task.read_text()), "grader": "dokimi:review_seeds"})) + (task.parent / "manifest.json").write_text(json.dumps({"seeds": [], "decoys": []})) + git(root, "add", "-A") + git(root, "commit", "-qm", "a built-in task") + _run(setup, "--arm", "guidance:raw", "--task", "fix_add", "--batch", "b2") + assert json.loads((setup["out"] / "b2/_eval/batch.json").read_text())["graders"] == "edited" + monkeypatch.setattr("dokimi.repos.graders.graders_digest", lambda: "edited again") + with pytest.raises(SystemExit): + _run(setup, "--arm", "guidance:none", "--task", "fix_add", "--batch", "b2") + err = capsys.readouterr().err + assert "graders" in err and "use another --batch" in err + assert not list((setup["out"] / "b2").glob("fix_add__guidance-none__*")) + + +@pytest.mark.parametrize(("text", "pinned"), [("{", True), ("[]", False)]) +def test_a_task_json_the_pin_cannot_read_pins_the_graders_and_a_non_object_one_does_not(setup, text, pinned): + d = setup["root"] / ".agent-eval/tasks/zz_other" + d.mkdir() + (d / "task.json").write_text(text) + git(setup["root"], "add", "-A") + git(setup["root"], "commit", "-qm", "an unreadable task the batch does not run") + _run(setup, "--arm", "guidance:raw", "--task", "fix_add") + assert ("graders" in json.loads((setup["out"] / "b1/_eval/batch.json").read_text())) is pinned + + +def test_graders_digest_follows_the_grader_sources(tmp_path, monkeypatch): + from dokimi.repos import graders + + (tmp_path / "review.py").write_text("VERSION = '1'\n") + monkeypatch.setattr(graders, "HOST_DIR", tmp_path) + before = graders.graders_digest() + (tmp_path / "review.py").write_text("VERSION = '2'\n") + assert graders.graders_digest() != before + + +def test_graders_digest_covers_main_and_pairs_names_with_contents(tmp_path, monkeypatch): + from dokimi.repos import graders + + monkeypatch.setattr(graders, "HOST_DIR", tmp_path) + (tmp_path / "main.py").write_text("ENTRY = 1\n") + (tmp_path / "review.py").write_text("VERSION = 1\n") + base = graders.graders_digest() + assert graders.graders_digest() == base # deterministic + (tmp_path / "main.py").write_text("ENTRY = 2\n") # the entry point is part of the score + edited = graders.graders_digest() + assert edited != base + (tmp_path / "main.py").write_text("VERSION = 1\n") # swap the contents between the files + (tmp_path / "review.py").write_text("ENTRY = 1\n") + swapped = graders.graders_digest() + assert swapped not in (base, edited) + (tmp_path / "main.py").write_text("ENTRY = 1\n") # same contents, names swapped back and forth + (tmp_path / "review.py").write_text("VERSION = 1\n") + assert graders.graders_digest() == base + (tmp_path / "main.py").rename(tmp_path / "other.py") # same bytes under another name + assert graders.graders_digest() != base