diff --git a/README.md b/README.md index 04eefe8..352e5de 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,10 @@ # skill-eval-loop `skill-eval-loop` is a self-contained Python 3 Agent Skill that measures -whether access to one local skill changes task outcomes. It runs the same task -under a no-skill control and an exact-hash treatment, then retains the raw -evidence and a comparison report. +whether explicitly applying one local skill changes task outcomes. The control +receives the original task. The treatment receives the exact hashed skill's +`SKILL.md` instructions in its prompt, with the installed payload available for +referenced files. The runner retains the raw evidence and a comparison report. ## Install @@ -49,30 +50,59 @@ Run a side-effect-free plan before a live invocation: Verify the printed hashes and invocation counts, obtain authorization for the live calls, then run the same command without `--dry-run`. -For rubric tasks, also pass `--judge-model` with a different exact model -identifier. The runner judges each condition only after deterministic gates -pass. A valid same-provider judgment is `provisional_non_independent`; a -timeout, failed gate, malformed response, or identity mismatch is `unknown`. -A missing trace-reported model is unattested, not a quality unknown. +### Public reference benchmark + +The checked-in development benchmark evaluates Vercel's +`vercel-react-best-practices` skill against the no-skill control: + +- repository: `https://github.com/vercel-labs/agent-skills.git` +- revision: `b8caa260a420a73042e35521de4b5c8baf6446cc` +- skill path: `skills/react-best-practices` +- tasks: `tasks/react-best-practices-v1.jsonl` +- expected evaluator payload SHA-256: + `5cbdbd8d9acc6913b8f4e0c7151830e88417872421a5975b86fa4b3eba5c36d3` +- expected task SHA-256: + `621a609cfcdb82756ebe6870a0fad16c6ef12f6186f6c75abb213195b4333c92` + +Fetch that exact revision into a controlled local directory and pass the +absolute skill subpath plus the checked-in task file to `run --dry-run`. Reject +the plan if the revision or payload hash differs. The public task file is +development evidence, not a secret client holdout. -The runner invokes Codex sequentially in read-only mode. Odd trials run -control first; even trials run treatment first. It retains `run.json`, the +For rubric tasks, also pass `--judge-model` with a different exact model +identifier and `--calibration /absolute/path/to/calibration.json` from an +accepted calibrate run. The runner judges each condition only after +deterministic gates pass. A valid same-provider judgment is +`provisional_non_independent`; a timeout, failed gate, malformed response, or +identity mismatch is `unknown`. A missing trace-reported model is unattested, +not a quality unknown. Omitting `--calibration` is allowed, but a rubric run +then remains quality-incomplete and cannot exit `0`. + +The runner invokes Codex sequentially in read-only mode, emitting invocation +progress to stderr. Odd trials run control first; even trials run treatment +first. The evaluator injects the exact `SKILL.md` text itself, so treatment +exposure does not depend on model-side discovery. Target, judge, and calibration +invocations share one lifecycle that uses cleaned OS-temporary workspaces outside +the evaluator repository. It retains `run.json`, the planned configuration, tasks, condition responses, traces, stderr, and a JSON/Markdown report for every pair. -`runner_valid` means the runner held its declared variables and isolation -checks. It is not a general quality claim. Read both transcripts before +`runner_valid` means the runner held its declared variables, isolation checks, +and treatment activation. It is not a general quality claim. Read both transcripts before interpreting `treatment_only`, `both_pass`, `control_only`, or `both_fail`. -JSON and Markdown reports also expose activation (currently unknown), -calibration (`not_run`), every judged dimension, `quality_status`, and +JSON and Markdown reports expose evaluator-recorded instruction delivery plus +optional trace telemetry when Codex also reads the installed skill, rolled-up timing and token usage, +calibration (`not_run`, or `accepted` plus `fixtures_sha256` when a bound +calibration is supplied), every judged dimension, `quality_status`, and `quality_outcome`. Deterministic-only reports say semantic quality was not judged. An overall pairwise winner is not a quality pass when any dimension is -unknown or disagrees with that winner. +unknown or favors the opposing condition. A tied dimension is compatible with +an otherwise coherent winner. -Live exit status is `0` when quality evidence is complete, `1` when the runner -is valid but quality is unknown or was not judged, and `2` when the runner is -invalid. +Live exit status is `0` when quality evidence is complete, which for rubric +runs requires a bound accepted calibration, `1` when the runner is valid but +quality is unknown or was not judged, and `2` when the runner is invalid. Calibrate the pairwise judge against versioned human-labeled `known-better`, `known-worse`, and `tie` cases before a live quality pilot: @@ -98,8 +128,18 @@ same-provider rubric judge, blinded pairwise comparison, and human-labeled calibration fixtures. It does not provide independent judging, pricing, parallel execution, provider discovery, or adapters for other harnesses. +Live evaluation is a trusted local-operator workflow. The configured harness +and Codex executable can read the run-local Codex credentials and therefore +must be trusted. This project does not sandbox hostile executables. Keep raw +run directories local and inspect them before sharing any evidence. + ## Development +Pull-request and push CI verifies evaluator mechanics with deterministic tests +and fake harnesses. It makes no live model calls, receives no model credentials, +and uploads no evaluation evidence. Authorized operators run live evaluations +locally; humans inspect the retained evidence and own promotion decisions. + Run the Python test suite and package healthcheck: ```bash diff --git a/docs/minimum-eval-contract.md b/docs/minimum-eval-contract.md index 2b70bf7..a14f8f7 100644 --- a/docs/minimum-eval-contract.md +++ b/docs/minimum-eval-contract.md @@ -48,7 +48,7 @@ Each non-empty JSONL line is one task: Required fields are: -- `id`: a unique, non-empty string; +- `id`: a unique, non-empty, path-safe string; - `prompt`: a non-empty string; - `graders`: a non-empty list of supported graders. @@ -92,29 +92,57 @@ This is a suite-bootstrap mechanism, not proof that tasks represent real use. Use independently sourced task data, blinded judging, and human calibration for skill-quality claims. +## Development versus promotion + +Repository pull-request and push CI verifies evaluator mechanics with +deterministic tests and fake harnesses only. It makes no live model calls, +receives no model credentials, and publishes no raw evaluation evidence. +Authorized operators run live development and promotion evaluations locally. +Humans inspect the retained evidence and own the promotion decision. + +The default `run` role is `development`. Development suites may be visible to +the skill author and optimization loop. They are useful for debugging and +regression detection, but repeated hill-climbing turns them into training data. + +`run --promotion` is a stricter execution guardrail. It requires: + +- an explicit `--tasks` path controlled outside the target skill; +- at least three trials; +- accepted calibration when the task set contains a rubric. + +The retained configuration records `evaluation_role` as `development` or +`promotion`. The flag cannot prove that a task set was independently authored, +kept hidden, representative of real use, or labeled by humans. Those remain +operator evidence requirements. A second model or provider does not replace a +human-labeled holdout. + ## Paired execution Every task trial runs twice: -- `control`: the target skill is unavailable; -- `treatment`: the exact hashed skill payload is available. +- `control`: the target skill is unavailable and Codex receives the original + task prompt; +- `treatment`: the exact hashed skill payload is available and the evaluator + injects its exact `SKILL.md` text before the original task prompt. -Prompt, harness, model, timeout, fixture, and tool posture remain fixed. Runs -are sequential. Condition order alternates by trial to reduce a fixed-order -confound. Trials never retry silently. +The original task, harness, model, timeout, fixture, and tool posture remain +fixed. The instruction injection is part of the treatment. Runs are sequential. +Condition order alternates by trial to reduce a fixed-order confound. Trials +never retry silently. The CLI emits invocation progress to stderr and stops +before repeating a detected network or transport failure. -Each condition starts in an empty read-only workspace. The minimum runner does -not seed a repository or fixture tree. Consequently, repository-editing tasks +Each condition starts in an empty OS-temporary read-only workspace outside the +evaluator repository, preventing ancestor project instructions from entering +the trial. The minimum runner does not seed a repository or fixture tree. Consequently, repository-editing tasks and claims about executed project tests are not reproducible under this contract; use self-contained response tasks until a separately justified workspace-fixture capability exists. -The intervention is availability of the exact hashed skill payload, not a -required execution path. Trace evidence about skill access is diagnostic when -available. Its absence does not invalidate the paired outcome comparison and -must not be scored as output quality. Results may claim only that access to the -skill changed measured outcomes under the retained configuration, not that the -model definitely read or followed the skill. +The intervention is evaluator-owned injection of the exact hashed skill's main +instructions. This guarantees treatment exposure without depending on the +model to discover or open `SKILL.md`; the installed payload remains available +for referenced files. Delivery does not prove faithful compliance, so human +transcript review remains required. The first Codex implementation is deliberately direct. A shared harness abstraction is not justified until a second real harness demonstrates common @@ -122,22 +150,29 @@ behavior. ## Codex home isolation -The experiment Codex home is not the user's `~/.codex`. A live run creates +The experiment Codex home is not the user's `~/.codex`. A live run temporarily creates `$output/codex-home` and sets `CODEX_HOME` to that directory for control, treatment, and judge. Place it under the output directory, not the OS temp directory: some Codex builds refuse a temp-dir home. If `~/.codex/auth.json` exists, copy only that file into the run-local home. Do not copy skills, sessions, or `config.toml`. Copied credentials are -runtime-only. They are not retained evidence and must not appear in reports. -Dry-run and fake-harness runs must not require an authenticated host Codex -home. +runtime-only and the runner removes the entire run-local home when it exits. The runner +does not intentionally serialize credentials into evidence. Because the +configured executable can read the copied file, the local operator must trust +the harness and inspect raw artifacts before sharing them. Dry-run and +fake-harness runs must not require an authenticated host Codex home. The treatment skill remains a workspace payload at `.agents/skills/`. Host `CODEX_HOME/skills` is not the intervention and is not consulted. A same-name skill in the user's Codex home is not a runner gate once the experiment uses a run-local home. +This isolation protects the experiment from ambient Codex configuration; it is +not a security sandbox for hostile executables. Strong isolation of untrusted +harnesses requires a separate OS or broker boundary and is outside this +project. + ## Dry-run accounting Dry-run validates consumed inputs and prints the complete plan without creating @@ -186,7 +221,9 @@ Rubric judging begins only after both condition runs satisfy runner isolation and execution checks and every deterministic grader passes. A failed gate produces quality status `unknown` and makes no judge call. -Each qualifying condition is judged separately in a fresh read-only workspace. +Each qualifying condition is judged separately in a fresh OS-temporary read-only +workspace outside the evaluator repository. Target, judge, and calibration +roles share the same workspace, environment, process, trace, and cleanup lifecycle. The prompt presents the task, untrusted candidate response, and locked rubric, but no control or treatment label. For every dimension, the judge must return concrete response evidence and exactly one declared level. The runner retains @@ -237,11 +274,14 @@ content is unchanged. ## Reports and exit status -Pair reports separate runner validity, activation, deterministic comparison, +Pair reports separate runner validity, evaluator-recorded activation, deterministic comparison, per-output rubric status, pairwise status, quality completeness, quality outcome, and calibration. `run.json` repeats the rolled-up runner validity and -quality status. Activation is `unknown` with reason `telemetry_unavailable` -until a later telemetry source exists. Calibration is `accepted` only when a +quality status and rolled-up timing/token usage. Activation is `observed` when +the evaluator injects the hashed treatment instructions. `trace_skill_read` +separately records whether Codex opened the installed main file; it is telemetry, +not a validity gate. +Calibration is `accepted` only when a validated binding is supplied. Without `--calibration`, a rubric run records `not_run`, quality remains `unknown`, and the runner cannot exit `0`. @@ -249,8 +289,9 @@ validated binding is supplied. Without `--calibration`, a rubric run records any required judgment is unknown, and `provisional_non_independent` when every required judgment succeeded. `quality_outcome` lists every dimension through `dimension_results` and is never a restored winner when a pairwise dimension -disagrees with the overall winner (`inconsistent`) or when quality is unknown -or not judged. Deterministic-only Markdown reports state that semantic quality +favors the condition opposing the overall winner (`inconsistent`) or when +quality is unknown or not judged. A tied dimension is compatible with an +otherwise coherent winner. Deterministic-only Markdown reports state that semantic quality was not judged. Process exit status distinguishes those cases: @@ -275,6 +316,15 @@ distribution, repeated trials, fair graders, and human review of transcripts. Improvement, regression, and no difference are all legitimate outcomes of a valid run. +A promotion claim additionally requires an independently controlled holdout, +human labels for the rubric or preference decisions, and measured agreement +between those labels and any automated judge. A visible development suite must +not be relabeled as a holdout after it has guided changes. + +The remaining real-promotion gate is external to this runner: independently +control the holdout, obtain human labels, repeat trials, and review the retained +transcripts before making a promotion claim. + A one-task pilot can establish runner acceptance. It cannot establish that a skill is generally effective. Capability suites should contain enough realistic, unsaturated tasks to reveal meaningful differences; regression diff --git a/skills/skill-eval-loop/SKILL.md b/skills/skill-eval-loop/SKILL.md index b73280b..f06f118 100644 --- a/skills/skill-eval-loop/SKILL.md +++ b/skills/skill-eval-loop/SKILL.md @@ -40,8 +40,10 @@ requirements. Use `file_exists` and `json_equal` only when the configured harness can create the stated workspace artifact. Keep unknown task metadata for human review; it does not affect execution. -The current runner starts each condition in an empty read-only workspace. Use -response-only tasks unless the prompt itself contains all required material. +The current runner starts each target, judge, and calibration invocation in an +empty OS-temporary read-only workspace outside the evaluator repository. Those +roles share one process and cleanup lifecycle. Use response-only tasks unless +the prompt itself contains all required material. Do not use repository-editing or test-running tasks as quality evidence: there is no seeded repository for the agent to change or verify. @@ -98,6 +100,35 @@ OpenAI model is explicitly same-provider evidence, not an independent judgment. A recommended OpenAI-only pair is `--model gpt-5.6-terra --judge-model gpt-5.6-sol`. +The default evaluation role is `development`. Treat any suite visible to the +skill author or repeatedly used during hill-climbing as development or +regression evidence, even when it is locked and hash-bound. + +For a promotion run, use an independently controlled task file, accepted +calibration, and repeated trials: + +```bash +"$EVALUATOR" run \ + --skill /absolute/path/to/target-skill \ + --tasks /absolute/path/to/operator-controlled-holdout.jsonl \ + --output /absolute/path/to/fresh-promotion-run \ + --harness codex \ + --harness-bin /absolute/path/to/codex \ + --model exact-model-id \ + --judge-model exact-judge-model-id \ + --calibration /absolute/path/to/fresh-calibration/calibration.json \ + --trials 3 \ + --timeout-seconds 300 \ + --promotion \ + --dry-run +``` + +`--promotion` rejects target-owned tasks, fewer than three trials, and rubric +runs without accepted calibration. It records the promotion role; it does not +prove task independence, representativeness, human labeling, or judge +agreement. Retain that evidence separately and keep the holdout unavailable to +the hill-climbing agent. + ## Calibrate the pairwise judge Score the judge against versioned human-labeled cases before a live quality @@ -123,8 +154,8 @@ calibration without both orientations cannot bind a rubric run. Disagreements keep the human rationale. Exit `0` if accepted, `1` if the runner is valid but below threshold, and `2` if a judgment is invalid. -For Task 8, the operator-controlled `calibration.json` and its original -absolute fixture path are the binding trust root. The runner validates their +The operator-controlled `calibration.json` and its original absolute fixture +path are the binding trust root. The runner validates their internal consistency, models, labels, agreement threshold, assignment orientations, and fixture hash. It does not authenticate the origin of the raw judge artifacts. Keep the calibration directory and fixture under controlled @@ -138,11 +169,14 @@ pilot until calibration is accepted and a human reviews disagreements. Run the identical command without `--dry-run`. The runner: -- uses a no-skill control and an exact-hash treatment; +- gives the control the original task and injects the exact-hash treatment's + `SKILL.md` instructions before that task; - runs sequentially, alternating control-first and treatment-first by trial; +- emits invocation progress to stderr and stops after a detected infrastructure failure; - invokes Codex in read-only mode; - retains response, trace, stderr, execution metadata, and reports; -- runs deterministic gates before any rubric judge; +- records treatment instruction delivery and requires deterministic gates before + any rubric judge; - asks the judge for concrete evidence and one locked level per dimension; - never retries silently. @@ -163,10 +197,13 @@ quality evidence, not runner validity. `quality_status` is evidence completeness. `quality_outcome` is `not_judged` when there is no rubric, `unknown` when any required judgment is unknown, `tie` when the restored winner is a tie, `inconsistent` when a pairwise -dimension disagrees with the overall winner, or the restored winner condition. +dimension favors the condition opposing the overall winner, or the restored +winner condition. A tied dimension does not contradict an overall winner. An overall winner is never a quality pass when a dimension is unknown or -disagrees. Activation is reported as unknown because Codex telemetry is not -scored. A bound accepted calibration records `calibration_status: accepted` +disagrees. Evaluator-owned treatment injection is `observed`; optional trace +telemetry records whether Codex also opened the installed `SKILL.md`. Delivery +proves exposure, not faithful compliance. +A bound accepted calibration records `calibration_status: accepted` and `fixtures_sha256` in `run.json` and every pair report. Without a binding, calibration remains `not_run` and rubric quality remains `unknown`. @@ -183,15 +220,17 @@ contains the source hash, and any trace-reported model identity agrees with the requested model. A live run creates `$output/codex-home` and points Codex at that directory. -If `~/.codex/auth.json` exists, it is copied there for the process and removed -afterward. Do not treat that file as retained evidence. Host Codex skills are -not part of the intervention. - -Treat access to the exact hashed payload as the intervention. A trace may help -explain how Codex used that access, but missing activation telemetry does not -invalidate the outcome comparison or become a quality score. Phrase the result -as the measured effect of skill access under the recorded configuration; do not -claim that Codex definitely read or followed the skill. +If `~/.codex/auth.json` exists, it is copied there for the process. The entire +run-local Codex home is removed afterward; it is not retained evidence. Use only a trusted +harness: it can read the run-local credential file, and this evaluator is not +a sandbox for hostile executables. Keep raw runs local and inspect them before +sharing. Host Codex skills are not part of the intervention. + +Treat injection of the exact hashed payload's `SKILL.md` instructions as the +intervention. The control receives the original task; the treatment receives +those instructions before that task and can access the installed payload for +references. Injection proves exposure, not faithful compliance, so inspect the +response before making a quality claim. Do not claim broad skill quality from one pilot or from same-provider judging. Use realistic unsaturated tasks, repeated trials, deterministic outcomes, diff --git a/skills/skill-eval-loop/references/eval-authoring.md b/skills/skill-eval-loop/references/eval-authoring.md index a87a3f6..88d8f19 100644 --- a/skills/skill-eval-loop/references/eval-authoring.md +++ b/skills/skill-eval-loop/references/eval-authoring.md @@ -30,4 +30,19 @@ payload fixed after this point. Run the evaluator's dry-run to validate the JSONL before authorizing live calls. This boundary prevents conversational leakage, not filesystem access. Treat the -post-authoring diff audit as required evidence. +post-authoring diff audit as required evidence. The resulting visible suite is +a development-suite bootstrap, not a promotion holdout. + +For an externally grounded public benchmark, keep the task file outside the +target and pass it with `--tasks`. The author receives the task contract and +allowed authoritative sources, but must not inspect the target skill, candidate +outputs, or prior reports. Record source URLs as inert task metadata. Once the +benchmark is checked in or used for optimization, classify it as development +evidence even if its initial authoring was independent. + +Promotion tasks must be controlled independently of the skill author and the +hill-climbing loop. Keep them outside the target skill, provide them through an +explicit `--tasks` path, attach human labels and rationales under operator +custody, and use `run --promotion`. Do not inspect or revise the holdout in +response to model outputs. If the tasks become visible during optimization, +reclassify them as development evidence and replace the holdout. diff --git a/skills/skill-eval-loop/scripts/skill_eval_loop.py b/skills/skill-eval-loop/scripts/skill_eval_loop.py index 80cc6e7..2e4ece0 100644 --- a/skills/skill-eval-loop/scripts/skill_eval_loop.py +++ b/skills/skill-eval-loop/scripts/skill_eval_loop.py @@ -13,7 +13,9 @@ import shutil import subprocess import sys +import tempfile import time +import unicodedata from typing import Any @@ -38,6 +40,10 @@ def error(message: str) -> None: print(f"ERROR: {message}", file=sys.stderr) +def progress(message: str) -> None: + print(f"PROGRESS: {message}", file=sys.stderr, flush=True) + + def absolute_path(value: str, label: str) -> Path: path = Path(value) if not path.is_absolute(): @@ -136,7 +142,9 @@ def load_tasks(path: Path) -> list[dict[str, Any]]: if not isinstance(raw, dict): raise ValueError(f"line {line_number}: task must be an object") task_id = required_string(raw.get("id"), f"line {line_number} field id") - if task_id in seen: + safe_task_id(task_id) + task_key = normalized_id(task_id) + if task_key in seen: raise ValueError(f'task "{task_id}" field id: duplicate value') prompt = required_string(raw.get("prompt"), f'task "{task_id}" field prompt') raw_graders = raw.get("graders") @@ -155,13 +163,14 @@ def load_tasks(path: Path) -> list[dict[str, Any]]: task = dict(raw) task.update({"id": task_id, "prompt": prompt, "graders": graders}) tasks.append(task) - seen.add(task_id) + seen.add(task_key) if not tasks: raise ValueError("tasks: at least one task is required") return tasks REQUIRED_CALIBRATION_CASES = ("known-better", "known-worse", "tie") +INTERVENTION = "injected_skill_instructions" def load_calibration(path: Path) -> dict[str, Any]: @@ -186,7 +195,8 @@ def load_calibration(path: Path) -> dict[str, Any]: raise ValueError(f"{label}: must be an object") case_id = required_string(value.get("id"), f"{label} field id") safe_task_id(case_id) - if case_id in seen: + case_key = normalized_id(case_id) + if case_key in seen: raise ValueError(f"{label} field id: duplicate value {case_id!r}") human_winner = required_string(value.get("human_winner"), f"{label} field human_winner") if human_winner not in {"better", "other", "tie"}: @@ -202,7 +212,7 @@ def load_calibration(path: Path) -> dict[str, Any]: "rationale": required_string(value.get("rationale"), f"{label} field rationale"), } ) - seen.add(case_id) + seen.add(case_key) missing = [case_id for case_id in REQUIRED_CALIBRATION_CASES if case_id not in seen] if missing: raise ValueError( @@ -317,7 +327,7 @@ def load_calibration_binding(path: Path, runner_model: str, judge_model: str) -> def payload_files(root: Path) -> list[Path]: - excluded = {"evals", "tests", "__pycache__", ".DS_Store"} + excluded = {"evals", "tests", "__pycache__", ".DS_Store", ".tink-source.json"} files: list[Path] = [] for path in root.rglob("*"): relative = path.relative_to(root) @@ -325,7 +335,7 @@ def payload_files(root: Path) -> list[Path]: continue if path.is_symlink(): raise ValueError(f"symlinked skill payload entry is not allowed: {path}") - if path.is_file() and path.suffix != ".pyc": + if path.is_file(): files.append(path) return sorted(files) @@ -370,22 +380,43 @@ def resolve_tasks_path(skill: Path, value: str | None) -> Path: return owned_suite +def reject_tasks_inside_skill(skill: Path, tasks_path: Path, promotion: bool) -> None: + if not promotion: + return + skill_root = skill.resolve() + resolved_tasks = tasks_path.resolve() + try: + resolved_tasks.relative_to(skill_root) + except ValueError: + return + raise ValueError( + "promotion tasks path must be independently controlled and outside the target skill" + ) + + def build_plan(arguments: argparse.Namespace) -> dict[str, Any]: if arguments.harness != "codex": raise ValueError("harness must be codex") if not arguments.model or arguments.trials < 1 or arguments.timeout_seconds < 1: raise ValueError("model, positive trials, and positive timeout-seconds are required") + if arguments.promotion and arguments.tasks is None: + raise ValueError("promotion runs require an explicit independently controlled tasks path") + if arguments.promotion and arguments.trials < 3: + raise ValueError("promotion runs require at least 3 trials") skill = absolute_path(arguments.skill, "skill") output = absolute_path(arguments.output, "output") if not (skill / "SKILL.md").is_file(): raise ValueError("skill path must contain SKILL.md") tasks_path = resolve_tasks_path(skill, arguments.tasks) + reject_tasks_inside_skill(skill, tasks_path, arguments.promotion) tasks = load_tasks(tasks_path) rubrics = sum( 1 for task in tasks for grader in task["graders"] if grader["type"] == "rubric" ) if rubrics and not arguments.judge_model: raise ValueError("judge-model is required when rubric graders are present") + if arguments.promotion and rubrics and arguments.calibration is None: + raise ValueError("promotion runs with rubric graders require accepted calibration") calibration: dict[str, Any] | None = None if arguments.calibration is not None: try: @@ -412,6 +443,8 @@ def build_plan(arguments: argparse.Namespace) -> dict[str, Any]: "harness_version": version, "model": arguments.model, "judge_model": arguments.judge_model, + "evaluation_role": "promotion" if arguments.promotion else "development", + "intervention": INTERVENTION, "trials": arguments.trials, "timeout_seconds": arguments.timeout_seconds, "output_dir": str(output), @@ -446,8 +479,17 @@ def write_json(path: Path, value: dict[str, Any]) -> None: def safe_task_id(task_id: str) -> None: - if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]*", task_id): + if not task_id or task_id in {".", ".."} or not task_id[0].isalnum(): raise ValueError(f'task "{task_id}" field id: must be path-safe') + if any( + not (char.isalnum() or unicodedata.category(char).startswith("M") or char in "._-") + for char in task_id + ): + raise ValueError(f'task "{task_id}" field id: must be path-safe') + + +def normalized_id(value: str) -> str: + return unicodedata.normalize("NFC", value).casefold() def copy_skill_payload(source: Path, destination: Path) -> None: @@ -469,10 +511,9 @@ def prepare_run_codex_home(output: Path) -> Path: return home -def discard_runtime_auth(home: Path) -> None: - target = home / "auth.json" - if target.is_file(): - target.unlink() +def discard_runtime_home(home: Path) -> None: + if home.exists(): + shutil.rmtree(home) def trace_value(event: Any, *keys: str) -> Any: @@ -484,11 +525,13 @@ def trace_value(event: Any, *keys: str) -> Any: return current -def parse_trace(path: Path) -> dict[str, Any]: +def parse_trace(path: Path, skill_name: str = "") -> dict[str, Any]: observed: dict[str, Any] = { "response": "", "actual_model": "", "session_id": "", + "skill_accessed": False, + "failure_message": "", "input_tokens": None, "output_tokens": None, "total_tokens": None, @@ -506,8 +549,24 @@ def parse_trace(path: Path) -> dict[str, Any]: elif event.get("type") == "thread.started": observed["session_id"] = trace_value(event, "thread_id") or "" elif event.get("type") == "item.completed": - if trace_value(event, "item", "type") == "agent_message": + item_type = trace_value(event, "item", "type") + if item_type == "agent_message": observed["response"] = str(trace_value(event, "item", "text") or "").strip() + elif item_type == "command_execution" and skill_name: + command = str(trace_value(event, "item", "command") or "") + output = str(trace_value(event, "item", "aggregated_output") or "") + skill_path = f".agents/skills/{skill_name}/SKILL.md" + skill_frontmatter = re.search( + rf"(?m)^name:\s*{re.escape(skill_name)}\s*$", output + ) + if ( + skill_path in command + and ( + trace_value(event, "item", "exit_code") == 0 + or skill_frontmatter is not None + ) + ): + observed["skill_accessed"] = True elif event.get("type") == "turn.completed": input_tokens = trace_value(event, "usage", "input_tokens") output_tokens = trace_value(event, "usage", "output_tokens") @@ -517,9 +576,27 @@ def parse_trace(path: Path) -> dict[str, Any]: observed["output_tokens"] = output_tokens if observed["input_tokens"] is not None and observed["output_tokens"] is not None: observed["total_tokens"] = observed["input_tokens"] + observed["output_tokens"] + elif event.get("type") == "turn.failed": + observed["failure_message"] = str(trace_value(event, "error", "message") or "") + elif event.get("type") == "error": + observed["failure_message"] = str(event.get("message") or "") return observed +def is_infrastructure_failure(message: str) -> bool: + lowered = message.casefold() + return any( + marker in lowered + for marker in ( + "failed to lookup address information", + "error sending request", + "connection refused", + "connection reset", + "network is unreachable", + ) + ) + + def workspace_target(workspace: Path, relative: str) -> Path: root = workspace.resolve() target = (root / relative).resolve() @@ -602,109 +679,244 @@ def grade(task: dict[str, Any], workspace: Path, response: str) -> dict[str, Any } -def run_condition( - *, - condition: str, - pair_dir: Path, - skill: Path, - skill_hash: str, - skill_name: str, - codex_directory: Path, - configuration: dict[str, Any], - task: dict[str, Any], -) -> tuple[dict[str, Any], dict[str, bool]]: - condition_dir = pair_dir / condition - workspace = condition_dir / "workspace" - workspace.mkdir(parents=True) - (condition_dir / "home").mkdir() - installed_skill = workspace / ".agents" / "skills" / skill_name - if installed_skill.exists(): - raise ValueError(f"fixture exposes target skill in {condition}") - isolation = {"control_skill_absent": condition == "control", "treatment_skill_present": False, "treatment_hash_matches": False} - if condition == "treatment": - copy_skill_payload(skill, installed_skill) - if hash_skill(installed_skill) != skill_hash: - raise ValueError("installed skill hash does not match source") - isolation["treatment_skill_present"] = True - isolation["treatment_hash_matches"] = True - trace_path = condition_dir / "trace.jsonl" - stderr_path = condition_dir / "stderr.txt" - environment = os.environ.copy() - environment.update( - { - "HOME": str(condition_dir / "home"), - "CODEX_HOME": str(codex_directory), - "SKILL_EVAL_SKILL_NAME": skill_name, - } - ) - arguments = [ - configuration["harness_executable"], - "exec", - "--json", - "--ephemeral", - "--skip-git-repo-check", - "--ignore-user-config", - "--ignore-rules", - "--sandbox", - "read-only", - "--model", - configuration["model"], - task["prompt"], - ] - started = time.monotonic() - timed_out = False - try: - with trace_path.open("w", encoding="utf-8") as trace, stderr_path.open("w", encoding="utf-8") as stderr: - completed = subprocess.run( - arguments, - cwd=workspace, - env=environment, - stdout=trace, - stderr=stderr, - timeout=configuration["timeout_seconds"], - check=False, - ) - exit_code = completed.returncode - except subprocess.TimeoutExpired: - timed_out = True - exit_code = -1 - duration_ms = round((time.monotonic() - started) * 1000) - observed = parse_trace(trace_path) - response_path = condition_dir / "response.md" - response_path.write_text(observed["response"], encoding="utf-8") - deterministic = grade(task, workspace, observed["response"]) - actual_model = observed["actual_model"] - model_matches = actual_model == configuration["model"] if actual_model else None - model_requirement_satisfied = bool(configuration["model"]) and (not actual_model or model_matches) - status = "timed_out" if timed_out else ("completed" if exit_code == 0 else "failed") - return ( - { - "name": condition, +class CodexRuntime: + """Own the shared Codex process, workspace, and evidence lifecycle.""" + + def __init__(self, codex_directory: Path, configuration: dict[str, Any]) -> None: + self.codex_directory = codex_directory + self.configuration = configuration + + def _invoke( + self, + *, + invocation_dir: Path, + workspace: Path, + prompt: str, + role: str, + display_name: str, + skill_name: str = "", + ) -> dict[str, Any]: + target_role = role in {"control", "treatment"} + model = ( + self.configuration["model"] + if target_role + else self.configuration["judge_model"] + ) + invocation_dir.mkdir(parents=True) + (invocation_dir / "home").mkdir() + if not target_role: + (invocation_dir / "prompt.txt").write_text(prompt, encoding="utf-8") + trace_path = invocation_dir / "trace.jsonl" + stderr_path = invocation_dir / "stderr.txt" + response_name = "response.md" if target_role else "response.txt" + response_path = invocation_dir / response_name + environment = os.environ.copy() + environment.pop("OPENAI_API_KEY", None) + environment.update( + { + "HOME": str(invocation_dir / "home"), + "CODEX_HOME": str(self.codex_directory), + } + ) + if target_role: + environment["SKILL_EVAL_SKILL_NAME"] = skill_name + else: + environment["SKILL_EVAL_ROLE"] = role + arguments = [ + self.configuration["harness_executable"], + "exec", + "--json", + "--ephemeral", + "--skip-git-repo-check", + "--ignore-user-config", + "--ignore-rules", + "--sandbox", + "read-only", + "--model", + model, + prompt, + ] + started = time.monotonic() + timed_out = False + progress(f"starting {display_name}") + try: + with trace_path.open("w", encoding="utf-8") as trace, stderr_path.open( + "w", encoding="utf-8" + ) as stderr: + completed = subprocess.run( + arguments, + cwd=workspace, + env=environment, + stdout=trace, + stderr=stderr, + timeout=self.configuration["timeout_seconds"], + check=False, + ) + exit_code = completed.returncode + except subprocess.TimeoutExpired: + timed_out = True + exit_code = -1 + duration_ms = round((time.monotonic() - started) * 1000) + observed = parse_trace( + trace_path, + skill_name if role == "treatment" else "", + ) + response_path.write_text(observed["response"], encoding="utf-8") + reported_model = observed["actual_model"] + model_matches = reported_model == model if reported_model else None + status = "timed_out" if timed_out else ("completed" if exit_code == 0 else "failed") + failure_reason = ( + "infrastructure_failed" + if exit_code != 0 and is_infrastructure_failure(observed["failure_message"]) + else "" + ) + progress(f"finished {display_name}: {status} in {duration_ms} ms") + return { "response": observed["response"], - "deterministic_status": deterministic["status"], - "pending_rubrics": deterministic["pending_rubrics"], - "graders": deterministic["results"], + "skill_accessed": observed["skill_accessed"], + "failure_reason": failure_reason, + "timed_out": timed_out, "execution": { "status": status, "exit_code": exit_code, "duration_ms": duration_ms, - "requested_model": configuration["model"], - "trace_reported_model": actual_model, - "model_identity_source": "trace_reported" if actual_model else "cli_configured", + "requested_model": model, + "trace_reported_model": reported_model, + "model_identity_source": ( + "trace_reported" if reported_model else "cli_configured" + ), "model_matches_requested": model_matches, - "model_requirement_satisfied": model_requirement_satisfied, "input_tokens": observed["input_tokens"], "output_tokens": observed["output_tokens"], "total_tokens": observed["total_tokens"], }, + "artifact_names": { + **({"prompt": "prompt.txt"} if not target_role else {}), + "response": response_name, + "trace": "trace.jsonl", + "stderr": "stderr.txt", + }, + } + + def run_condition( + self, + *, + condition: str, + pair_dir: Path, + skill: Path, + skill_hash: str, + skill_name: str, + task: dict[str, Any], + ) -> tuple[dict[str, Any], dict[str, bool]]: + condition_dir = pair_dir / condition + with tempfile.TemporaryDirectory(prefix=f"skill-eval-{condition}-") as temporary: + workspace = Path(temporary) + installed_skill = workspace / ".agents" / "skills" / skill_name + if installed_skill.exists(): + raise ValueError(f"fixture exposes target skill in {condition}") + isolation = { + "control_skill_absent": condition == "control", + "treatment_skill_present": False, + "treatment_hash_matches": False, + } + prompt = task["prompt"] + if condition == "treatment": + copy_skill_payload(skill, installed_skill) + if hash_skill(installed_skill) != skill_hash: + raise ValueError("installed skill hash does not match source") + isolation["treatment_skill_present"] = True + isolation["treatment_hash_matches"] = True + skill_instructions = (installed_skill / "SKILL.md").read_text( + encoding="utf-8" + ) + prompt = ( + "Apply the following skill instructions to the task. The exact hashed " + f"skill package is available at .agents/skills/{skill_name}/ for any " + "referenced files.\n\n" + f"\n" + f"{skill_instructions}\n" + "\n\n" + f"\n{task['prompt']}\n" + ) + invocation = self._invoke( + invocation_dir=condition_dir, + workspace=workspace, + prompt=prompt, + role=condition, + display_name=f"target {task['id']} {condition}", + skill_name=skill_name, + ) + deterministic = grade(task, workspace, invocation["response"]) + execution = dict(invocation["execution"]) + reported_model = execution["trace_reported_model"] + execution["model_requirement_satisfied"] = bool(self.configuration["model"]) and ( + not reported_model or execution["model_matches_requested"] + ) + execution["failure_reason"] = invocation["failure_reason"] + return ( + { + "name": condition, + "response": invocation["response"], + "activation": { + "status": "observed" if condition == "treatment" else "unknown", + "reason": ( + "skill_instructions_injected" + if condition == "treatment" + else "no_skill_in_control" + ), + "trace_skill_read": invocation["skill_accessed"], + }, + "deterministic_status": deterministic["status"], + "pending_rubrics": deterministic["pending_rubrics"], + "graders": deterministic["results"], + "execution": execution, + "artifacts": { + label: f"{condition}/{name}" + for label, name in invocation["artifact_names"].items() + }, + }, + isolation, + ) + + def invoke_judge( + self, + *, + judge_dir: Path, + artifact_root: Path, + prompt: str, + role: str, + ) -> tuple[dict[str, Any], str]: + artifact_prefix = judge_dir.relative_to(artifact_root).as_posix() + with tempfile.TemporaryDirectory(prefix=f"skill-eval-{role}-") as temporary: + invocation = self._invoke( + invocation_dir=judge_dir, + workspace=Path(temporary), + prompt=prompt, + role=role, + display_name=f"{role} {artifact_prefix}", + ) + execution = invocation["execution"] + result: dict[str, Any] = { + "status": "unknown", + "reason": "", + "dimensions": [], + "execution": execution, "artifacts": { - "response": f"{condition}/response.md", - "trace": f"{condition}/trace.jsonl", - "stderr": f"{condition}/stderr.txt", + label: f"{artifact_prefix}/{name}" + for label, name in invocation["artifact_names"].items() }, - }, - isolation, - ) + } + if invocation["timed_out"]: + result["reason"] = "timed_out" + elif execution["exit_code"] != 0: + result["reason"] = ( + "infrastructure_failed" + if invocation["failure_reason"] == "infrastructure_failed" + else "judge_failed" + ) + elif execution["trace_reported_model"] and not execution["model_matches_requested"]: + result["reason"] = "model_identity_mismatch" + return result, invocation["response"] def deterministic_comparison(control: str, treatment: str) -> str: @@ -729,6 +941,7 @@ def runner_is_valid(conditions: dict[str, dict[str, Any]], isolation: dict[str, and isolation["control_skill_absent"] and isolation["treatment_skill_present"] and isolation["treatment_hash_matches"] + and treatment["activation"]["status"] == "observed" ) @@ -857,99 +1070,6 @@ def unknown_pairwise(reason: str, judge_model: str) -> dict[str, Any]: return unknown_judgment(reason, judge_model) -def invoke_judge( - *, - judge_dir: Path, - codex_directory: Path, - configuration: dict[str, Any], - prompt: str, - role: str, -) -> tuple[dict[str, Any], str]: - workspace = judge_dir / "workspace" - workspace.mkdir(parents=True) - (judge_dir / "home").mkdir() - (judge_dir / "prompt.txt").write_text(prompt, encoding="utf-8") - trace_path = judge_dir / "trace.jsonl" - stderr_path = judge_dir / "stderr.txt" - response_path = judge_dir / "response.txt" - environment = os.environ.copy() - environment.update( - { - "HOME": str(judge_dir / "home"), - "CODEX_HOME": str(codex_directory), - "SKILL_EVAL_ROLE": role, - } - ) - arguments = [ - configuration["harness_executable"], - "exec", - "--json", - "--ephemeral", - "--skip-git-repo-check", - "--ignore-user-config", - "--ignore-rules", - "--sandbox", - "read-only", - "--model", - configuration["judge_model"], - prompt, - ] - started = time.monotonic() - timed_out = False - try: - with trace_path.open("w", encoding="utf-8") as trace, stderr_path.open( - "w", encoding="utf-8" - ) as stderr: - completed = subprocess.run( - arguments, - cwd=workspace, - env=environment, - stdout=trace, - stderr=stderr, - timeout=configuration["timeout_seconds"], - check=False, - ) - exit_code = completed.returncode - except subprocess.TimeoutExpired: - timed_out = True - exit_code = -1 - duration_ms = round((time.monotonic() - started) * 1000) - observed = parse_trace(trace_path) - response_path.write_text(observed["response"], encoding="utf-8") - reported_model = observed["actual_model"] - model_matches = reported_model == configuration["judge_model"] if reported_model else None - result: dict[str, Any] = { - "status": "unknown", - "reason": "", - "dimensions": [], - "execution": { - "status": "timed_out" if timed_out else ("completed" if exit_code == 0 else "failed"), - "exit_code": exit_code, - "duration_ms": duration_ms, - "requested_model": configuration["judge_model"], - "trace_reported_model": reported_model, - "model_identity_source": "trace_reported" if reported_model else "cli_configured", - "model_matches_requested": model_matches, - "input_tokens": observed["input_tokens"], - "output_tokens": observed["output_tokens"], - "total_tokens": observed["total_tokens"], - }, - "artifacts": { - "prompt": f"{judge_dir.name}/prompt.txt", - "response": f"{judge_dir.name}/response.txt", - "trace": f"{judge_dir.name}/trace.jsonl", - "stderr": f"{judge_dir.name}/stderr.txt", - }, - } - if timed_out: - result["reason"] = "timed_out" - elif exit_code != 0: - result["reason"] = "judge_failed" - elif reported_model and not model_matches: - result["reason"] = "model_identity_mismatch" - return result, observed["response"] - - def mark_provisional(result: dict[str, Any]) -> dict[str, Any]: result["status"] = "provisional_non_independent" result["reason"] = "same_provider_family" @@ -958,18 +1078,17 @@ def mark_provisional(result: dict[str, Any]) -> dict[str, Any]: def run_rubric_judge( *, + runtime: CodexRuntime, + pair_dir: Path, condition_dir: Path, - codex_directory: Path, - configuration: dict[str, Any], task: dict[str, Any], response: str, rubric: dict[str, Any], rubric_index: int, ) -> dict[str, Any]: - result, raw = invoke_judge( + result, raw = runtime.invoke_judge( judge_dir=condition_dir / f"judge-{rubric_index:03d}", - codex_directory=codex_directory, - configuration=configuration, + artifact_root=pair_dir, prompt=judge_prompt(task, response, rubric), role="judge", ) @@ -985,9 +1104,8 @@ def run_rubric_judge( def run_pairwise_judge( *, + runtime: CodexRuntime, pair_dir: Path, - codex_directory: Path, - configuration: dict[str, Any], task: dict[str, Any], conditions: dict[str, dict[str, Any]], rubric: dict[str, Any], @@ -998,10 +1116,9 @@ def run_pairwise_judge( candidates = { label: conditions[condition]["response"] for label, condition in mapping.items() } - result, raw = invoke_judge( + result, raw = runtime.invoke_judge( judge_dir=pair_dir / f"pairwise-{rubric_index:03d}", - codex_directory=codex_directory, - configuration=configuration, + artifact_root=pair_dir, prompt=pairwise_prompt(task, candidates, rubric), role="pairwise", ) @@ -1021,8 +1138,8 @@ def run_pairwise_judge( def judge_conditions( *, + runtime: CodexRuntime, pair_dir: Path, - codex_directory: Path, configuration: dict[str, Any], task: dict[str, Any], conditions: dict[str, dict[str, Any]], @@ -1048,9 +1165,9 @@ def judge_conditions( for condition_name, condition in conditions.items(): condition["rubric_judgments"] = [ run_rubric_judge( + runtime=runtime, + pair_dir=pair_dir, condition_dir=pair_dir / condition_name, - codex_directory=codex_directory, - configuration=configuration, task=task, response=condition["response"], rubric=rubric, @@ -1062,9 +1179,8 @@ def judge_conditions( return [unknown_pairwise("per_output_unknown", configuration["judge_model"]) for _ in rubrics] return [ run_pairwise_judge( + runtime=runtime, pair_dir=pair_dir, - codex_directory=codex_directory, - configuration=configuration, task=task, conditions=conditions, rubric=rubric, @@ -1083,6 +1199,39 @@ def all_rubric_judgments(conditions: dict[str, dict[str, Any]]) -> list[dict[str ] +def pair_executions( + conditions: dict[str, dict[str, Any]], pairwise: list[dict[str, Any]] +) -> list[dict[str, Any]]: + executions = [condition["execution"] for condition in conditions.values()] + executions.extend(judgment["execution"] for judgment in all_rubric_judgments(conditions)) + executions.extend(judgment["execution"] for judgment in pairwise) + return executions + + +def summarize_usage(executions: list[dict[str, Any]]) -> dict[str, Any]: + invoked = [execution for execution in executions if execution.get("status") != "not_run"] + measured = [execution for execution in invoked if isinstance(execution.get("total_tokens"), int)] + return { + "status": ( + "complete" + if invoked and len(measured) == len(invoked) + else "partial" if measured else "unknown" + ), + "invocations": len(invoked), + "measured_invocations": len(measured), + "duration_ms": sum( + execution.get("duration_ms", 0) + for execution in invoked + if isinstance(execution.get("duration_ms"), int) + ), + "input_tokens": sum(execution.get("input_tokens", 0) for execution in measured), + "output_tokens": sum(execution.get("output_tokens", 0) for execution in measured), + "total_tokens": sum(execution["total_tokens"] for execution in measured), + "cost": None, + "cost_status": "unknown", + } + + def evidence_status(judgments: list[dict[str, Any]]) -> str: if not judgments: return "not_required" @@ -1179,7 +1328,8 @@ def quality_outcome_for(pairwise: list[dict[str, Any]], quality_status: str) -> inconsistent = True overall = overall or winner for dimension in judgment["dimensions"]: - if restored_condition(judgment, dimension["winner"]) != winner: + dimension_winner = restored_condition(judgment, dimension["winner"]) + if winner != "tie" and dimension_winner not in {"tie", winner}: inconsistent = True if inconsistent: return "inconsistent" @@ -1238,10 +1388,11 @@ def write_pair_report( dimensions = dimension_results(conditions, pairwise) report = { "runner_valid": runner_valid, + "intervention": INTERVENTION, "task": {"id": task["id"], "prompt": task["prompt"], "graders": task["graders"]}, "trial": trial, "execution_order": execution_order, - "activation": {"status": "unknown", "reason": "telemetry_unavailable"}, + "activation": conditions["treatment"]["activation"], "deterministic_comparison": deterministic_comparison( control["deterministic_status"], treatment["deterministic_status"] ), @@ -1253,6 +1404,7 @@ def write_pair_report( "calibration_status": calibration_status, "fixtures_sha256": fixtures_sha256, "dimension_results": dimensions, + "usage": summarize_usage(pair_executions(conditions, pairwise)), "pairwise": pairwise, "skill": {"name": skill_name, "sha256": skill_hash}, "isolation": { @@ -1273,12 +1425,23 @@ def write_pair_report( if dimensions else ["- Semantic quality was not judged."] ) + artifact_links: list[str] = [] + for condition in (control, treatment): + for label, relative in condition["artifacts"].items(): + artifact_links.append(f"- [{condition['name']} {label}]({relative})") + for judgment in condition.get("rubric_judgments", []): + for label, relative in judgment.get("artifacts", {}).items(): + artifact_links.append(f"- [{condition['name']} judge {label}]({relative})") + for index, judgment in enumerate(pairwise, start=1): + for label, relative in judgment.get("artifacts", {}).items(): + artifact_links.append(f"- [pairwise {index} {label}]({relative})") markdown_path.write_text( "\n".join( [ f"# {task['id']} trial {trial}", "", f"Runner valid: {runner_valid}", + f"Intervention: {INTERVENTION}", f"Activation: {report['activation']['status']} ({report['activation']['reason']})", f"Deterministic comparison: {report['deterministic_comparison']}", f"Rubric status: {report['rubric_status']}", @@ -1291,6 +1454,9 @@ def write_pair_report( "Dimensions:", *dimension_lines, "", + "Artifacts (relative to this report):", + *artifact_links, + "", "Inspect the JSON report and condition artifacts for authoritative evidence.", "", ] @@ -1306,8 +1472,6 @@ def run_live(plan: dict[str, Any]) -> dict[str, Any]: tasks_path = Path(configuration["tasks_path"]) output = Path(configuration["output_dir"]) tasks = load_tasks(tasks_path) - for task in tasks: - safe_task_id(task["id"]) if hash_file(tasks_path) != configuration["tasks_sha256"]: raise ValueError("tasks changed after dry-run planning") if hash_skill(skill) != configuration["skill_sha256"]: @@ -1327,8 +1491,10 @@ def run_live(plan: dict[str, Any]) -> dict[str, Any]: raise ValueError(f"output directory already exists: {output}") skill_name = skill.name output.mkdir(parents=True) - codex_directory = prepare_run_codex_home(output) + codex_directory = output / "codex-home" try: + codex_directory = prepare_run_codex_home(output) + runtime = CodexRuntime(codex_directory, configuration) write_json( output / "config.json", {"mode": "live", "configuration": configuration, "counts": plan["counts"]}, @@ -1340,13 +1506,14 @@ def run_live(plan: dict[str, Any]) -> dict[str, Any]: "output_dir": str(output), "configuration": configuration, "counts": plan["counts"], - "activation": {"status": "unknown", "reason": "telemetry_unavailable"}, + "activation": {"status": "unknown", "reason": "pending"}, "calibration_status": calibration_status, "fixtures_sha256": fixtures_sha256, "quality_status": "not_required", "pairs": [], } quality_statuses: list[str] = [] + executions: list[dict[str, Any]] = [] for task in tasks: for trial in range(1, configuration["trials"] + 1): pair_dir = output / f"task-{task['id']}" / f"trial-{trial:03d}" @@ -1355,28 +1522,43 @@ def run_live(plan: dict[str, Any]) -> dict[str, Any]: conditions: dict[str, dict[str, Any]] = {} isolation = {"control_skill_absent": False, "treatment_skill_present": False, "treatment_hash_matches": False} for condition in execution_order: - condition_result, current_isolation = run_condition( + condition_result, current_isolation = runtime.run_condition( condition=condition, pair_dir=pair_dir, skill=skill, skill_hash=configuration["skill_sha256"], skill_name=skill_name, - codex_directory=codex_directory, - configuration=configuration, task=task, ) conditions[condition] = condition_result + executions.append(condition_result["execution"]) for key, value in current_isolation.items(): isolation[key] = isolation[key] or value + if condition_result["execution"]["failure_reason"] == "infrastructure_failed": + result["valid"] = False + result["quality_status"] = "unknown" + result["failure"] = { + "reason": "infrastructure_failed", + "task_id": task["id"], + "trial": trial, + "condition": condition, + } + result["usage"] = summarize_usage(executions) + write_json(output / "run.json", result) + return result pairwise = judge_conditions( + runtime=runtime, pair_dir=pair_dir, - codex_directory=codex_directory, configuration=configuration, task=task, conditions=conditions, isolation=isolation, trial=trial, ) + executions.extend( + judgment["execution"] for judgment in all_rubric_judgments(conditions) + ) + executions.extend(judgment["execution"] for judgment in pairwise) report, report_path, markdown_path = write_pair_report( pair_dir, task, @@ -1400,16 +1582,36 @@ def run_live(plan: dict[str, Any]) -> dict[str, Any]: "runner_valid": report["runner_valid"], "quality_status": report["quality_status"], "quality_outcome": report["quality_outcome"], + "activation": report["activation"], "execution_order": execution_order, "report_json": str(report_path.relative_to(output).as_posix()), "report_markdown": str(markdown_path.relative_to(output).as_posix()), } ) result["quality_status"] = rollup_quality_status(quality_statuses) + result["usage"] = summarize_usage(executions) + observed_activations = sum( + pair["activation"]["status"] == "observed" for pair in result["pairs"] + ) + if observed_activations == len(result["pairs"]): + result["activation"] = { + "status": "observed", + "reason": "all_treatments_received_skill_instructions", + } + elif observed_activations: + result["activation"] = { + "status": "partial", + "reason": "some_treatments_received_skill_instructions", + } + else: + result["activation"] = { + "status": "unknown", + "reason": "no_treatment_skill_instructions_delivered", + } write_json(output / "run.json", result) return result finally: - discard_runtime_auth(codex_directory) + discard_runtime_home(codex_directory) def build_calibration_plan(arguments: argparse.Namespace) -> dict[str, Any]: @@ -1459,19 +1661,17 @@ def build_calibration_plan(arguments: argparse.Namespace) -> dict[str, Any]: def run_calibration_case( *, + runtime: CodexRuntime, output: Path, - codex_directory: Path, - configuration: dict[str, Any], suite: dict[str, Any], case: dict[str, Any], seed: int, ) -> dict[str, Any]: mapping = calibration_mapping(seed) candidates = {label: case[slot] for label, slot in mapping.items()} - result, raw = invoke_judge( + result, raw = runtime.invoke_judge( judge_dir=output / case["id"], - codex_directory=codex_directory, - configuration=configuration, + artifact_root=output, prompt=pairwise_prompt( {"prompt": suite["prompt"]}, candidates, @@ -1510,8 +1710,10 @@ def run_calibrate(plan: dict[str, Any]) -> dict[str, Any]: if output.exists(): raise ValueError(f"output directory already exists: {output}") output.mkdir(parents=True) - codex_directory = prepare_run_codex_home(output) + codex_directory = output / "codex-home" try: + codex_directory = prepare_run_codex_home(output) + runtime = CodexRuntime(codex_directory, configuration) write_json( output / "config.json", {"mode": "calibrate", "configuration": configuration, "counts": plan["counts"]}, @@ -1529,9 +1731,8 @@ def run_calibrate(plan: dict[str, Any]) -> dict[str, Any]: } for index, case in enumerate(suite["cases"], start=1): judged = run_calibration_case( + runtime=runtime, output=output, - codex_directory=codex_directory, - configuration=configuration, suite=suite, case=case, seed=index, @@ -1539,6 +1740,8 @@ def run_calibrate(plan: dict[str, Any]) -> dict[str, Any]: result["cases"].append(judged) if judged["status"] == "unknown": result["valid"] = False + if judged["reason"] == "infrastructure_failed": + break continue if judged["agrees"]: result["agreements"] += 1 @@ -1554,10 +1757,11 @@ def run_calibrate(plan: dict[str, Any]) -> dict[str, Any]: result["accepted"] = ( result["valid"] and result["agreements"] >= suite["minimum_agreements"] ) + result["usage"] = summarize_usage([case["execution"] for case in result["cases"]]) write_json(output / "calibration.json", result) return result finally: - discard_runtime_auth(codex_directory) + discard_runtime_home(codex_directory) def calibration_exit_code(result: dict[str, Any]) -> int: @@ -1620,6 +1824,11 @@ def parser() -> argparse.ArgumentParser: run_parser.add_argument("--timeout-seconds", type=int, default=120) run_parser.add_argument("--judge-model", default="") run_parser.add_argument("--calibration") + run_parser.add_argument( + "--promotion", + action="store_true", + help="require an explicit task set, accepted rubric calibration, and at least 3 trials", + ) run_parser.add_argument("--dry-run", action="store_true") run_parser.set_defaults(handler=run) calibrate_parser = commands.add_parser( diff --git a/tasks/plan.md b/tasks/plan.md index 04d0a24..40cee37 100644 --- a/tasks/plan.md +++ b/tasks/plan.md @@ -23,9 +23,9 @@ control/treatment baseline, semantic grading, multi-dimensional rubrics, - Use the existing Codex authentication for the first semantic path, label all OpenAI-to-OpenAI results provisional and non-independent, and do not claim independence until a different provider or human calibration supplies it. -- Treat availability of the exact hashed skill payload as the intervention. - Activation telemetry is optional diagnostic evidence, not a quality score or - a gate on the outcome comparison. +- Treat evaluator-owned injection of the exact hashed skill's `SKILL.md` as the + intervention. Keep the control task untouched and the installed treatment + payload available for referenced files. - Give each live run a private Codex home under `$output/codex-home`. Copy only `~/.codex/auth.json` when present. Do not reuse the user's Codex home as the experiment environment. @@ -81,22 +81,23 @@ outcomes. ## Task 2: Confirm intervention semantics **Description:** Confirm what the paired experiment changes and what it may -claim. The intervention is access to the exact hashed skill payload. Activation -telemetry can diagnose how Codex used that access, but is not required for an -outcome comparison and must not become a path-based quality metric. +claim. The intervention is injection of the exact hashed skill's main +instructions: the control receives the original task and treatment receives +those instructions plus access to the installed payload. **Acceptance criteria:** -- [x] Control absence, treatment presence, and treatment/source hash equality - define the isolated intervention. -- [x] Missing activation telemetry does not invalidate an outcome comparison. -- [x] Claims are limited to the measured effect of skill access under the - retained configuration. +- [x] Control absence, treatment presence, treatment/source hash equality, and + treatment-only instruction injection define the intervention. +- [x] Evaluator-owned delivery is recorded independently of optional skill-read + trace telemetry. +- [x] Claims are limited to the measured effect of injected skill instructions + under the retained configuration. **Verification:** - [x] A controlled fixture asserts control absence, treatment presence, and treatment/source hash equality. -- [x] Manual check: Codex CLI 0.147.0 treatment trace exposes no activation - event; this remains diagnostic rather than a gate. +- [x] A fake-harness regression proves treatment delivery does not depend on a + model-side file read. - [x] Human review accepted the outcome-based evidence definition. **Dependencies:** None. @@ -106,12 +107,13 @@ outcome comparison and must not become a path-based quality metric. - `skills/skill-eval-loop/SKILL.md` - `docs/minimum-eval-contract.md` -**Result:** Resolved without activation machinery. +**Result:** Treatment instruction delivery and payload isolation are part of +runner validity; model-side reads are retained as optional telemetry. ### Checkpoint: Evidence contract - [x] Tasks 1 and 2 are complete. -- [x] Deterministic validity, exact skill availability, and quality evidence +- [x] Deterministic validity, injected skill exposure, and quality evidence remain separate concepts. - [x] Human approves `gpt-5.6-sol` as the provisional judge for `gpt-5.6-terra` runs, without an independence claim. @@ -137,13 +139,12 @@ and raw output, and label same-provider results as non-independent. - [x] Tests pass: `python3 -m unittest discover -s tests -v`. - [x] Focused fake-adapter tests cover success, malformed response, timeout, identity mismatch, and deterministic short-circuiting. -- [ ] Manual check: inspect a retained live judge trace with the selected +- [x] Manual check: inspect a retained live judge trace with the selected provider after separate authorization. **Dependencies:** Tasks 1 and 2; human approval of the provisional pairing. -**Result:** Implementation complete with fake-adapter evidence. Live provider -verification remains part of the later authorized pilot. +**Result:** Implementation and authorized live-provider verification complete. **Files likely touched:** - `skills/skill-eval-loop/scripts/skill_eval_loop.py` @@ -173,13 +174,14 @@ host Codex home. Land this before any authorized live Codex run. - [x] Tests pass: `python3 -m unittest discover -s tests -v`. - [x] Focused tests assert the subprocess `CODEX_HOME` path and that fake runs no longer need `CODEX_HOME` in the caller environment. -- [ ] Manual check: one authorized live exec with only copied `auth.json` - remains deferred to the later pilot. +- [x] Manual check: an authorized live exec used the runtime credential and + removed the entire run-local Codex home afterward. **Dependencies:** Task 3. -**Result:** Implementation complete with fake-adapter evidence. Live provider -verification remains part of the later authorized pilot. +**Result:** Implementation and authorized live-provider verification complete. +Target, judge, and calibration roles now share one `CodexRuntime` process and +cleaned OS-temporary workspace lifecycle, preventing role-specific isolation drift. **Files likely touched:** - `skills/skill-eval-loop/scripts/skill_eval_loop.py` @@ -205,13 +207,12 @@ report. **Verification:** - [x] Tests pass: `python3 -m unittest discover -s tests -v`. - [x] Focused tests prove condition labels cannot enter the judge payload. -- [ ] Manual check: compare the retained blind prompt, raw judgment, and +- [x] Manual check: compare the retained blind prompt, raw judgment, and restored report. **Dependencies:** Tasks 1 and 3. -**Result:** Implementation complete with fake-adapter evidence. Live prompt and -restored-report review remains part of the later authorized pilot. +**Result:** Implementation and authorized live prompt/report review complete. **Files likely touched:** - `skills/skill-eval-loop/scripts/skill_eval_loop.py` @@ -248,13 +249,12 @@ critical failed or unknown dimension. **Verification:** - [x] Tests pass: `python3 -m unittest discover -s tests -v`. - [x] Focused report fixtures cover pass, tie, failed critical dimension, - unavailable judge, and activation unknown. -- [ ] Manual check: inspect JSON and Markdown reports for the same pair. + unavailable judge, and runner-invalid isolation failures. +- [x] Manual check: inspect JSON and Markdown reports for the same pair. **Dependencies:** Tasks 2, 3, and 4. -**Result:** Implementation complete with fake-adapter evidence. Live report -inspection remains part of the later authorized pilot. +**Result:** Implementation and authorized live report inspection complete. **Files likely touched:** - `skills/skill-eval-loop/scripts/skill_eval_loop.py` @@ -289,10 +289,11 @@ then run one real paired pilot only if calibration accepts the chosen judge. **Result:** Calibration command and v1 fixtures are complete. Live `gpt-5.6-sol` calibration against the locked cases accepted 3/3 with no disagreements. Codex 0.147.0 `exec --json` traces do not report a model; missing identity is -unattested CLI configuration, not a failed judgment. One paired live pilot -reported runner validity, activation unknown, deterministic both_pass, per- -dimension rubric scores, and a blinded pairwise tie. That is not a skill- -quality claim. Judge evidence remains same-provider and non-independent. + unattested CLI configuration, not a failed judgment. One paired live pilot + under the superseded availability-only intervention reported activation + unknown, deterministic both_pass, per-dimension rubric scores, and a blinded + pairwise tie. It is retained as historical evidence, not an applied-skill + quality claim. Judge evidence remains same-provider and non-independent. **Files likely touched:** - `tests/fixtures/` @@ -316,57 +317,66 @@ quality claim. Judge evidence remains same-provider and non-independent. | Risk | Impact | Mitigation | |---|---|---| -| Codex exposes no activation telemetry | High | Stop at Task 2 and narrow the claim rather than infer use. | +| Codex does not open the installed `SKILL.md` | Low | Inject the exact main instructions in treatment; retain file-read events only as telemetry. | | No independent judge is available | High | Label OpenAI-only evidence provisional and require human calibration before broader claims. | | Judge prompt leaks condition labels | High | Build prompt from anonymized candidates and test the raw payload. | | Rubric is gamed or too vague | High | Lock it before runs and calibrate against human-labeled cases. | | Pilot is saturated or too small | Medium | Report tie/no-signal and expand only after calibration. | | Host Codex home leaks extra skills | High | Use a run-local `$output/codex-home` and copy only `auth.json`. | -| Copied `auth.json` is published as evidence | High | Treat it as runtime-only; keep it out of reports and condition artifacts. | +| A hostile harness reads or echoes `auth.json` | High | Live runs are trusted local-operator workflows; hostile-process isolation is a separate system. Keep raw artifacts local and inspect before sharing. | ## Open questions - Which provider or human calibration process will supply independent evidence beyond the provisional OpenAI judge? -- What activation evidence can current Codex emit, if any? +- Do referenced multi-file skills require an additional fixture before promotion use? - Which human-approved threshold should calibration meet before a pilot result is considered quality evidence? -## Phase 2: Karpathy hill climb (next agent) +## Phase 2: Validate one public skill -**Baseline:** branch `python-core-redesign`, no upstream. Tasks 1–6 code and -docs are committed. `python3 -m unittest discover -s tests -v` is green (22 -tests). Live artifacts under `.eval-runs/` are gitignored: `calibrate-v1b` -accepted 3/3 (still `provisional_non_independent`); `pilot-v1` was a saturated -toy (“Choose Blue.” / pairwise tie). Codex CLI 0.147.0 traces omit model -identity; missing identity is unattested, not fail-closed. +**Objective:** Demonstrate the evaluator on one frozen public skill versus the +existing no-skill control. Use an independently authored, externally grounded +benchmark; do not select a weak opponent or change the evaluator into a +multi-skill comparison framework. -**Objective:** Make `skill-eval-loop` a CI-gated hill climb on one locked -non-toy skill suite: `run` consumes an accepted `calibrate` fixture hash; live -calibration A/B-flips so `A` is not always the known-better seed; a live paired -`calibrate` then `run` exits `0` with complete quality evidence -(`quality_outcome` never a restored winner when a dimension is unknown or -inconsistent). Same-provider judging stays `provisional_non_independent`. Out -of scope: modularizing the evaluator script, installing GSD/NTT123, and any -quality-winner claim on the toy pilot. +**Selected target:** Vercel's `vercel-react-best-practices` skill. -**Do not start by splitting `skills/skill-eval-loop/scripts/skill_eval_loop.py`.** -The bottleneck is eval validity, not file size. +- repository: `https://github.com/vercel-labs/agent-skills.git` +- revision: `b8caa260a420a73042e35521de4b5c8baf6446cc` +- subpath: `skills/react-best-practices` +- evaluator payload SHA-256: + `5cbdbd8d9acc6913b8f4e0c7151830e88417872421a5975b86fa4b3eba5c36d3` +- declared skill license: MIT -**Stop and ask** if the first target skill is unnamed, if the user wants a -second-provider judge before the CI gate, or if transcripts are still -`human_transcript_review_required`. +The repository does not vendor the target. An operator fetches the exact +revision into a temporary or controlled source directory, verifies the revision, +then passes the absolute skill subpath to the evaluator. `run.json` binds the +actual payload hash. -### Task 7: Lock a non-toy skill suite +### Task 7: Lock a public benchmark -**Description:** Replace the toy Blue prompt with one real skill directory and -a locked JSONL suite that can fail. Do not invent the skill; ask. +**Description:** Replace the prior skill-specific suite with response-only +React/Next review tasks authored without inspecting the target skill. Ground +task metadata in official React and Next.js documentation. **Acceptance criteria:** -- [ ] Named skill path and task file are recorded here and used by later tasks. -- [ ] Tasks are not saturated at baseline (not every row `both_pass` by design). - -**Dependencies:** User names the skill. No code until that answer exists. +- [x] The public target identity, immutable revision, subpath, license, and + evaluator payload hash are recorded. +- [x] `tasks/react-best-practices-v1.jsonl` contains realistic positive, + negative-control, ambiguous, and false-positive-sensitive cases. +- [x] The suite passes an evaluator dry-run against the frozen target. + +The checked-in suite is a public development benchmark, not a secret promotion +holdout. A client promotion claim still requires an independently controlled +task file that was unavailable to the skill-authoring and hill-climbing loop. + +**Result:** A fresh-context author created ten response-only tasks using only +official React and Next.js documentation. The suite includes two explicit +negative controls. Dry-run against the exact target revision is valid with +target hash `5cbdbd8d9acc6913b8f4e0c7151830e88417872421a5975b86fa4b3eba5c36d3`, +task hash `621a609cfcdb82756ebe6870a0fad16c6ef12f6186f6c75abb213195b4333c92`, +10 paired trials, 50 planned invocations, zero provider calls, and no artifacts. ### Task 8: Bind calibration into live `run` @@ -384,32 +394,63 @@ drifts. - [x] `python3 -m unittest discover -s tests -v` - [x] Focused tests cover hash bind, missing calibration, and A/B flip. -**Dependencies:** Task 7 for the live suite; tests can land first. +**Dependencies:** The calibration implementation is independent of the selected +public target. **Result:** Production calibration alternates both candidate orientations. Rubric runs bind a validated accepted calibration and fixture hash; missing calibration cannot complete quality evidence, and malformed or drifted supplied bindings are runner-invalid. Unit and fake-harness verification is complete; -no external Codex run was added. The Task 8 trust root is the operator-controlled -`calibration.json` plus its original absolute fixture path. Task 9 must keep one -stable CI path or separately approve a portable content-addressed design. +no external Codex run was added. The trust root is the operator-controlled +`calibration.json` plus its original absolute fixture path. -### Task 9: CI as the product UI +### Task 9: CI protects evaluator mechanics -**Description:** Add a CI job that runs unit tests and, when secrets exist, the -locked calibrate-then-run pair. The gate is complete hash-bound quality -evidence, not a skill-quality winner. +**Description:** Keep CI deterministic and credential-free. CI tests evaluator +mechanics with fake harnesses; authorized operators run live evaluations +locally. Complete hash-bound quality evidence is a local evaluation property, +not a CI or skill-quality claim. **Acceptance criteria:** -- [ ] CI fails on unittest failure or runner-invalid (`exit 2`). -- [ ] Rubric runs without bound accepted calibration cannot look like a quality +- [x] CI fails on unittest failure or runner-invalid (`exit 2`). +- [x] Rubric runs without bound accepted calibration cannot look like a quality pass. **Dependencies:** Task 8. -### Task 10: Independent judge or holdout +**Result:** Pull-request and push CI run the Python tests, healthcheck, packaging +checks, and fake-harness coverage. CI has no live model invocation, model +credential, calibration run, or raw evidence upload. Public benchmark and live +calibration runs remain local, explicit operator actions. Real promotion +evidence remains external and incomplete; no treatment winner is claimed. + +### Task 10: Human-labeled holdout and judge validation -**Description:** Only after Tasks 8–9. A second provider or a held-out human -set. Same-provider evidence stays provisional until then. +**Description:** Separate visible development/regression cases from promotion +evidence. Promotion uses an independently controlled, human-labeled holdout, +repeated trials, and measured agreement between human labels and any automated +judge. A second provider is useful corroboration, not a substitute for the +holdout or human agreement. -**Dependencies:** Tasks 8 and 9. User approval before adding a provider. +**Acceptance criteria:** +- [x] `run --promotion` requires an explicit task path, at least three trials, + and accepted calibration for rubric tasks. +- [x] The public React suite is classified as development evidence; live runs + are local and explicitly authorized. +- [ ] An independently controlled holdout covers positive, negative, + ambiguous, near-tie, and adversarial cases from the intended use + distribution. +- [ ] At least two humans label the holdout; disagreements and rationales are + retained. +- [ ] Automated-judge agreement with the retained human labels is measured + before a promotion claim. +- [ ] A repeated-trial promotion run is transcript-reviewed and reports + per-dimension outcomes, regressions, variance, usage, and cost. + +**Result so far:** The evaluator now distinguishes `development` and +`promotion` roles and rejects underpowered or uncalibrated rubric promotion +runs. No holdout content was invented in this repository: independence and +human labels remain the next evidence gate. + +**Dependencies:** Tasks 8 and 9. User approval before adding a provider or +making paid calls. diff --git a/tasks/react-best-practices-v1.jsonl b/tasks/react-best-practices-v1.jsonl new file mode 100644 index 0000000..5da4f5f --- /dev/null +++ b/tasks/react-best-practices-v1.jsonl @@ -0,0 +1,10 @@ +{"id":"parallel-dashboard-fetches","prompt":"You are reviewing a Next.js App Router page for a dashboard. The team reports that server response time grew after adding the activity panel. Review the snippet and return a concise, prioritized performance review. Do not edit files. For each finding, explain the user-visible impact and show a concrete replacement snippet.\n\n```tsx\n// app/dashboard/page.tsx\nimport { getAccount, getNotifications, getRecentActivity } from '@/lib/data'\n\nexport default async function DashboardPage() {\n const account = await getAccount()\n const notifications = await getNotifications()\n const activity = await getRecentActivity()\n\n return (\n
\n

Welcome, {account.name}

\n \n \n
\n )\n}\n```\n\nContext: the three functions call independent internal services; none needs another function's result. Preserve the rendered content and error semantics.","source":{"urls":["https://nextjs.org/docs/app/getting-started/fetching-data"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"waterfall_diagnosis","levels":[{"name":"not_met","description":"Misses the sequential request waterfall or incorrectly treats the requests as dependent."},{"name":"met","description":"Correctly identifies that serial awaits add avoidable latency because the three requests are independent, and prioritizes this as the main performance issue."}]},{"name":"concrete_parallelization","levels":[{"name":"not_met","description":"Offers only general advice or a replacement that still starts the requests sequentially or changes behavior."},{"name":"met","description":"Shows a valid replacement that initiates the independent work together, such as Promise.all over the three calls, while preserving the rendered data and failure behavior."}]},{"name":"impact_explanation","levels":[{"name":"not_met","description":"Does not connect the change to request latency or makes unsupported claims about caching or bundle size."},{"name":"met","description":"Explains that total blocking time approaches the slowest concurrent request instead of the sum of three serial waits, without inventing unprovided measurements."}]}]}]} +{"id":"search-effect-race","prompt":"Review this Client Component from a Next.js App Router application. Users report that quickly typing and then navigating Back can sometimes show results for an older query. Return a prioritized code-review response with the smallest safe correction and any relevant architectural alternative. Do not edit files.\n\n```tsx\n'use client'\n\nimport { useEffect, useState } from 'react'\n\ntype Result = { id: string; title: string }\n\nexport function SearchResults({ query }: { query: string }) {\n const [results, setResults] = useState([])\n const [loading, setLoading] = useState(false)\n\n useEffect(() => {\n setLoading(true)\n fetch(`/api/search?q=${encodeURIComponent(query)}`)\n .then((response) => response.json())\n .then((data) => {\n setResults(data.results)\n setLoading(false)\n })\n }, [query])\n\n if (loading) return

Searching…

\n return \n}\n```\n\nContext: requests are cacheable GETs, the URL is the source of truth for `query`, and changing the API is out of scope.","source":{"urls":["https://react.dev/reference/react/useEffect","https://react.dev/learn/you-might-not-need-an-effect","https://nextjs.org/docs/app/getting-started/fetching-data"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"race_and_lifecycle","levels":[{"name":"not_met","description":"Misses the stale-response race and unmount lifecycle risk, or attributes the behavior only to rendering speed."},{"name":"met","description":"Explains that an earlier request can resolve after a later one or after unmount and overwrite current state, and treats request cleanup or stale-result suppression as the immediate correctness fix."}]},{"name":"safe_local_fix","levels":[{"name":"not_met","description":"Proposes a debounce alone, omits cleanup, or provides a fix that can still commit stale results or leave loading stuck."},{"name":"met","description":"Provides a coherent Effect implementation using AbortController or an ignore flag with cleanup, error or abort handling, and loading state that belongs to the active request."}]},{"name":"next_architecture_judgment","levels":[{"name":"not_met","description":"Mandates a rewrite without considering the stated URL-driven context, or gives no relevant alternative."},{"name":"met","description":"Notes that App Router server data fetching, streaming, or a client data library can provide lifecycle and caching benefits, while clearly separating that optional architecture choice from the minimal local correction."}]}]}]} +{"id":"derived-cart-total-effect","prompt":"Perform a React performance and correctness review of this cart summary. The parent may replace `items` after applying a coupon. Return only actionable findings, ordered by impact, with corrected code where appropriate. Do not edit files.\n\n```tsx\n'use client'\n\nimport { useEffect, useState } from 'react'\n\ntype Item = { id: string; price: number; quantity: number }\n\nexport function CartSummary({ items }: { items: Item[] }) {\n const [subtotal, setSubtotal] = useState(0)\n const [total, setTotal] = useState(0)\n\n useEffect(() => {\n setSubtotal(items.reduce((sum, item) => sum + item.price * item.quantity, 0))\n }, [items])\n\n useEffect(() => {\n setTotal(subtotal * 1.0825)\n }, [subtotal])\n\n return

Total: ${total.toFixed(2)}

\n}\n```\n\nContext: the item list is normally under 30 entries, tax is a fixed 8.25%, and no external system needs to observe subtotal or total.","source":{"urls":["https://react.dev/learn/you-might-not-need-an-effect","https://react.dev/reference/react/useMemo"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"derived_state_diagnosis","levels":[{"name":"not_met","description":"Treats the Effect chain as necessary or focuses only on dependency-array syntax."},{"name":"met","description":"Identifies subtotal and total as values derivable during render and explains that storing them in state creates extra render passes and transient stale output."}]},{"name":"minimal_refactor","levels":[{"name":"not_met","description":"Adds more state, Effects, or unconditional memoization, or fails to preserve the calculation."},{"name":"met","description":"Shows the values calculated directly from items during render, preserving the subtotal and 8.25 percent tax computation without Effects or redundant state."}]},{"name":"memoization_judgment","levels":[{"name":"not_met","description":"Claims useMemo is required for correctness or recommends it without regard to the small stated workload."},{"name":"met","description":"Treats useMemo as optional and measurement-driven for this small list, while allowing it if profiling later shows the reduction is expensive or item identity is stable enough to benefit."}]}]}]} +{"id":"resize-listener-subscription","prompt":"Review this React component after a report that window resizing becomes increasingly sluggish when users switch between compact and expanded modes. Provide a concise diagnosis and corrected implementation. Do not edit files.\n\n```tsx\n'use client'\n\nimport { useEffect, useState } from 'react'\n\nexport function ViewportLabel({ compact }: { compact: boolean }) {\n const [width, setWidth] = useState(0)\n\n useEffect(() => {\n function handleResize() {\n setWidth(window.innerWidth)\n analytics.track('viewport_resize', { compact, width: window.innerWidth })\n }\n\n window.addEventListener('resize', handleResize)\n handleResize()\n }, [compact])\n\n return {compact ? 'Compact' : 'Expanded'} at {width}px\n}\n```\n\nContext: `analytics.track` is a stable imported singleton, tracking should use the current `compact` value, and the component can mount and unmount repeatedly.","source":{"urls":["https://react.dev/reference/react/useEffect","https://react.dev/learn/synchronizing-with-effects"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"subscription_leak","levels":[{"name":"not_met","description":"Misses that listeners accumulate or incorrectly blames React Strict Mode as the production cause."},{"name":"met","description":"Identifies the missing removeEventListener cleanup as the cause of accumulating handlers across dependency changes and unmounts."}]},{"name":"correct_effect_lifecycle","levels":[{"name":"not_met","description":"Provides cleanup with a different function identity, omits compact from synchronization, or does not initialize width."},{"name":"met","description":"Shows an Effect that registers one handler, invokes it for initial synchronization, removes that same handler in cleanup, and continues to track the current compact value."}]},{"name":"strict_mode_reasoning","levels":[{"name":"not_met","description":"Recommends suppressing duplicate development execution or removing dependencies to hide the symptom."},{"name":"met","description":"Explains that React's development setup-cleanup cycle exposes lifecycle bugs and that symmetric cleanup makes the component safe rather than requiring suppression."}]}]}]} +{"id":"hydration-personalized-header","prompt":"A Next.js App Router page intermittently logs a hydration mismatch, especially for signed-in users. Review the component and propose the smallest robust design that keeps the first server and client render consistent. Do not edit files. Include a replacement snippet and explain any user-experience tradeoff.\n\n```tsx\n'use client'\n\nexport function HeaderGreeting() {\n const name = typeof window === 'undefined'\n ? 'Guest'\n : window.localStorage.getItem('displayName') ?? 'Guest'\n const generatedAt = new Date().toLocaleTimeString()\n\n return (\n
\n Hello, {name}\n Rendered at {generatedAt}\n
\n )\n}\n```\n\nContext: the server does not have the display name, the timestamp is decorative, and disabling server rendering for the entire page is not acceptable.","source":{"urls":["https://nextjs.org/docs/messages/react-hydration-error","https://nextjs.org/docs/app/getting-started/server-and-client-components","https://react.dev/reference/react/useEffect"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"mismatch_causes","levels":[{"name":"not_met","description":"Finds only one nondeterministic value or suggests the mismatch is caused by use client itself."},{"name":"met","description":"Identifies both the localStorage or window-dependent first render and the time-dependent Date output as values that can differ between server HTML and the first client render."}]},{"name":"consistent_initial_render","levels":[{"name":"not_met","description":"Reads browser state during initial render, disables SSR for the whole page, or relies on suppressHydrationWarning as the primary blanket fix."},{"name":"met","description":"Provides a design with deterministic server and initial client markup, then reads localStorage after hydration in an Effect or passes server-known data as props, while isolating or deferring the decorative timestamp."}]},{"name":"tradeoff_and_scope","levels":[{"name":"not_met","description":"Does not mention the temporary fallback or visual transition, or recommends broad client-only rendering without justification."},{"name":"met","description":"Explains the brief Guest or placeholder state and limits any client-only or warning-suppression escape hatch to the smallest inherently nondeterministic element."}]}]}]} +{"id":"client-boundary-product-page","prompt":"Review the module boundaries in this Next.js App Router product page. The team wants to reduce shipped JavaScript without losing the Add to cart interaction. Return a concrete refactor sketch and explain which modules execute on the server versus the client. Do not edit files.\n\n```tsx\n// app/products/[id]/page.tsx\n'use client'\n\nimport { useState } from 'react'\nimport { marked } from 'marked'\nimport { getProduct } from '@/lib/products'\nimport { SiteFooter } from '@/components/site-footer'\n\nexport default async function ProductPage({ params }: { params: Promise<{ id: string }> }) {\n const { id } = await params\n const product = await getProduct(id)\n const [quantity, setQuantity] = useState(1)\n\n return (\n <>\n
\n

{product.name}

\n
\n \n \n
\n \n \n )\n}\n```\n\nContext: `getProduct` accesses a server-only database module, `marked` is needed only to render stored product copy, and `SiteFooter` is static.","source":{"urls":["https://nextjs.org/docs/app/getting-started/server-and-client-components"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"boundary_diagnosis","levels":[{"name":"not_met","description":"Leaves the whole page in the client graph or suggests client-side database access."},{"name":"met","description":"Explains that the use client directive creates a client boundary for imports and descendants, conflicting with server-only data access and unnecessarily pulling static or rendering work toward the client bundle."}]},{"name":"server_client_split","levels":[{"name":"not_met","description":"The proposed split loses interactivity, passes non-serializable server values, or keeps marked and the footer behind the client boundary."},{"name":"met","description":"Sketches an async Server Component page that fetches and renders product content and the static footer, plus a small Client Component island that owns quantity state and click handlers using serializable props such as productId."}]},{"name":"performance_outcome","levels":[{"name":"not_met","description":"Makes only correctness claims or promises exact bundle savings without evidence."},{"name":"met","description":"Connects the narrower client boundary to less JavaScript shipped and hydrated while preserving server-side data access and the required cart interaction, without inventing measurements."}]}]}]} +{"id":"lazy-admin-chart","prompt":"Review this Next.js Client Component for initial-load performance. Most users never open the analytics panel, and field data shows elevated JavaScript execution time on the route. Return a prioritized recommendation with an implementation sketch and a measurement plan. Do not edit files.\n\n```tsx\n'use client'\n\nimport { useState } from 'react'\nimport { AnalyticsChart } from '@/components/analytics-chart'\nimport { buildChartSeries } from '@/lib/chart-series'\n\nexport function AdminToolbar({ rows }: { rows: ReportRow[] }) {\n const [open, setOpen] = useState(false)\n const series = buildChartSeries(rows)\n\n return (\n
\n \n {open ? : null}\n
\n )\n}\n```\n\nContext: `AnalyticsChart` imports a large browser-only charting library, `buildChartSeries` is expensive for large reports, and the panel is not needed for indexing or the initial view.","source":{"urls":["https://nextjs.org/docs/app/guides/lazy-loading","https://nextjs.org/docs/app/guides/production-checklist","https://react.dev/reference/react/useMemo"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"defer_code_and_work","levels":[{"name":"not_met","description":"Only conditionally renders the already statically imported chart or memoizes the eager import, leaving its code in the initial client graph."},{"name":"met","description":"Recommends a dynamic import for the browser-only chart and defers buildChartSeries until the panel is opened, so both download and expensive computation are avoided on the common closed path."}]},{"name":"implementation_quality","levels":[{"name":"not_met","description":"Uses an invalid dynamic import pattern, performs expensive work on every closed render, or ignores an appropriate loading state."},{"name":"met","description":"Provides a plausible next/dynamic or React lazy implementation with a loading fallback and computes the series only for the open panel, optionally memoizing it when open if repeated renders justify that."}]},{"name":"measurement_plan","levels":[{"name":"not_met","description":"Claims improvement without proposing production-oriented verification."},{"name":"met","description":"Proposes comparing production bundle analysis and route performance before and after, including the client import chain or bundle contribution and an execution or user-centric metric."}]}]}]} +{"id":"responsive-hero-image","prompt":"Review this above-the-fold hero in a Next.js App Router page for loading performance and layout stability. Return the highest-impact findings, corrected code, and what you would verify after the change. Do not edit files.\n\n```tsx\nexport function Hero() {\n return (\n
\n \n

Plan the launch with confidence

\n
\n )\n}\n```\n\n```css\n.heroImage {\n display: block;\n width: 100%;\n height: auto;\n}\n```\n\nContext: the source image is 2400 by 1350 pixels, it is the route's likely largest-contentful-paint element, and it spans the viewport up to a 1200-pixel content maximum.","source":{"urls":["https://nextjs.org/docs/app/getting-started/images","https://nextjs.org/docs/app/api-reference/components/image"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"image_optimization","levels":[{"name":"not_met","description":"Keeps an unqualified raw img or recommends only compressing the source with no responsive delivery strategy."},{"name":"met","description":"Recommends Next.js Image with intrinsic dimensions or a correctly constrained fill container, an accurate sizes value for the responsive layout, and appropriate priority or preload treatment for the likely LCP image."}]},{"name":"layout_and_bandwidth","levels":[{"name":"not_met","description":"Omits dimensions and layout reservation or asserts that CSS width alone prevents layout shift and oversized delivery."},{"name":"met","description":"Explains how intrinsic dimensions preserve aspect ratio and reserve space, and how responsive image selection avoids sending the 2400-pixel source unnecessarily to smaller viewports."}]},{"name":"verification","levels":[{"name":"not_met","description":"Provides no verification or relies only on development-mode impressions."},{"name":"met","description":"Calls for a production-like check of LCP, layout shift, and the selected image resource or transfer size across representative viewport widths."}]}]}]} +{"id":"negative-parallel-server-page","prompt":"Perform a performance-focused review of this Next.js App Router page. The team has no reported regression; this is a pre-merge review. State whether a code change is justified from the supplied evidence, identify any confirmed issue, and list any measurement you would request before suggesting speculative optimization. Do not edit files.\n\n```tsx\nimport { Suspense } from 'react'\nimport { getProduct, getReviews } from '@/lib/data'\n\nexport default async function ProductPage({ params }: { params: Promise<{ id: string }> }) {\n const { id } = await params\n const productPromise = getProduct(id)\n const reviewsPromise = getReviews(id)\n const [product, reviews] = await Promise.all([productPromise, reviewsPromise])\n\n return (\n
\n

{product.name}

\n Loading reviews…

}>\n \n
\n
\n )\n}\n```\n\nContext: both data functions are server-only and independent, the arrays are serializable, production traces and bundle reports were not provided, and `ReviewList` is synchronous once it receives `reviews`.","source":{"urls":["https://nextjs.org/docs/app/getting-started/fetching-data","https://nextjs.org/docs/app/getting-started/server-and-client-components"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"restraint","levels":[{"name":"not_met","description":"Invents a confirmed performance defect, mandates memoization or client fetching, or claims a Suspense streaming benefit that the already-resolved reviews prop cannot provide."},{"name":"met","description":"States that the independent fetches are already started in parallel and that no performance change is justified by the supplied evidence; it may note that the current Suspense boundary does not stream the already-awaited reviews."}]},{"name":"evidence_request","levels":[{"name":"not_met","description":"Requests vague optimization work or proposes changes before identifying a measured bottleneck."},{"name":"met","description":"Requests targeted production evidence such as server timing for each fetch, route latency, bundle analysis, or a trace before recommending further optimization."}]},{"name":"safe_optional_observation","levels":[{"name":"not_met","description":"Treats an optional restructuring as required or changes data ownership without a demonstrated need."},{"name":"met","description":"If discussing streaming, clearly labels it optional and explains that the reviews await would need to move behind an async component or promise-consuming boundary for the fallback to become meaningful."}]}]}]} +{"id":"negative-trivial-memoization","prompt":"A reviewer proposed wrapping every component in `memo` and every calculation in `useMemo`. Assess that proposal for this component and return a go/no-go recommendation. Explain what evidence would change your decision and mention any actual correctness concern visible in the snippet. Do not edit files.\n\n```tsx\n'use client'\n\nimport { useState } from 'react'\n\nexport function GreetingCard({ name }: { name: string }) {\n const [expanded, setExpanded] = useState(false)\n const greeting = `Hello, ${name}!`\n\n return (\n
\n

{greeting}

\n \n {expanded ?

Thanks for visiting.

: null}\n
\n )\n}\n```\n\nContext: no lag or excessive render count has been observed, the parent behavior is not shown, and the application may enable React Compiler later.","source":{"urls":["https://react.dev/reference/react/memo","https://react.dev/reference/react/useMemo","https://react.dev/reference/react-compiler/directives/use-memo"]},"graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"no_go_judgment","levels":[{"name":"not_met","description":"Recommends blanket memo or useMemo, or presents memoization as required for correctness."},{"name":"met","description":"Rejects the blanket proposal for this trivial component because no expensive repeated work or lag is established and treats memoization as a performance optimization rather than a semantic requirement."}]},{"name":"evidence_threshold","levels":[{"name":"not_met","description":"Offers no condition under which memoization would be warranted or relies on intuition alone."},{"name":"met","description":"Would reconsider after production profiling shows frequent costly renders with stable props or expensive recalculation, and notes that parent prop stability and compiler configuration affect the decision."}]},{"name":"correctness_scope","levels":[{"name":"not_met","description":"Invents a correctness bug or proposes unrelated refactors."},{"name":"met","description":"States that no correctness defect is visible in the supplied snippet and preserves the current implementation absent contrary evidence."}]}]}]} diff --git a/tasks/todo.md b/tasks/todo.md index de6f71f..6b5379d 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,16 +1,19 @@ # Trustworthy paired evaluation tasks - [x] Task 1: Lock the qualitative task contract and target-owned suite source. -- [x] Task 2: Confirm exact skill availability as the intervention. +- [x] Task 2: Confirm evaluator-owned target-skill instruction injection as the intervention. - [x] Checkpoint: approve evidence contract and provisional OpenAI judge choice. - [x] Task 3: Add a provisional Codex judge path. - [x] Task 3b: Isolate the live Codex home from the host user directory. - [x] Task 4: Add blinded pairwise comparison. - [x] Checkpoint: review the first raw judge artifact. - [x] Task 5: Make the report and exit status quality-aware. -- [x] Task 6: Calibrate with known outcomes; one-task live pilot recorded. +- [x] Task 6: Calibrate with known outcomes; live development pilots recorded. - [ ] Checkpoint: verify all six capabilities (independent judge still open). -- [ ] Task 7: Lock a non-toy skill suite (ask before inventing one). +- [x] Task 7: Freeze Vercel's public React skill and an independently authored, + externally grounded development benchmark. - [x] Task 8: Force live calibrate A/B flips; bind accepted fixture hash into `run`. -- [ ] Task 9: CI gate on complete, hash-bound, non-toy quality evidence. -- [ ] Task 10: Independent judge or holdout only after the CI gate is honest. +- [x] Task 9: Deterministic, fake-harness CI protects evaluator mechanics; + authorized local runs produce development or promotion evidence. +- [ ] Task 10: Validate a repeated-trial promotion run on an independently + controlled, human-labeled holdout (promotion guardrails implemented). diff --git a/tests/fixtures/simple-fake-codex b/tests/fixtures/simple-fake-codex index 0e020e5..f6c4a3b 100755 --- a/tests/fixtures/simple-fake-codex +++ b/tests/fixtures/simple-fake-codex @@ -28,6 +28,12 @@ fi if [ -n "${SIMPLE_FAKE_INVOCATION_LOG:-}" ]; then printf '%s\n' "$role" >> "$SIMPLE_FAKE_INVOCATION_LOG" fi +if [ "$role" = "runner" ] && [ -n "${SIMPLE_FAKE_CWD_LOG:-}" ]; then + printf '%s\n' "$PWD" >> "$SIMPLE_FAKE_CWD_LOG" +fi +if [ -n "${SIMPLE_FAKE_ROLE_CWD_LOG:-}" ]; then + printf '%s\t%s\n' "$role" "$PWD" >> "$SIMPLE_FAKE_ROLE_CWD_LOG" +fi if [ -n "${SIMPLE_FAKE_AUTH_LOG:-}" ]; then if [ -f "${CODEX_HOME:-}/auth.json" ]; then printf 'present\n' >> "$SIMPLE_FAKE_AUTH_LOG" @@ -36,6 +42,12 @@ if [ -n "${SIMPLE_FAKE_AUTH_LOG:-}" ]; then fi fi +if [ -n "${SIMPLE_FAKE_INFRA_FAILURE:-}" ]; then + printf '%s\n' '{"type":"turn.failed","error":{"message":"error sending request: failed to lookup address information"}}' + printf '%s\n' 'failed to lookup address information' >&2 + exit 1 +fi + if [ "$judge_like" -eq 1 ]; then if [ -n "${SIMPLE_FAKE_JUDGE_SLEEP_SECONDS:-}" ]; then sleep "$SIMPLE_FAKE_JUDGE_SLEEP_SECONDS" @@ -80,6 +92,9 @@ else if [ -f "$PWD/.agents/skills/$skill_name/SKILL.md" ]; then response=${SIMPLE_FAKE_TREATMENT_RESPONSE:-Blue} thread=treatment-thread + if [ -z "${SIMPLE_FAKE_SKIP_SKILL_READ:-}" ]; then + python3 -c 'import json, sys; print(json.dumps({"type":"item.completed","item":{"type":"command_execution","command":sys.argv[1],"exit_code":0}}))' "sed -n '1,200p' .agents/skills/$skill_name/SKILL.md" + fi fi reported_model=${SIMPLE_FAKE_REPORTED_MODEL:-$model} fi diff --git a/tests/test_skill_eval_loop.py b/tests/test_skill_eval_loop.py index 5af1da5..e875002 100644 --- a/tests/test_skill_eval_loop.py +++ b/tests/test_skill_eval_loop.py @@ -205,6 +205,7 @@ def test_dry_run_validates_inputs_without_creating_output(self) -> None: plan = json.loads(result.stdout) self.assertTrue(plan["valid"]) self.assertFalse(plan["created_artifacts"]) + self.assertEqual(plan["configuration"]["intervention"], "injected_skill_instructions") self.assertEqual(plan["counts"]["total_invocations"], 15) self.assertEqual( plan["task_snapshot"][0]["graders"][1]["dimensions"][0]["name"], @@ -244,6 +245,36 @@ def test_dry_run_rejects_rubric_without_response_preflight(self) -> None: self.assertEqual(result.returncode, 1) self.assertIn("require a response_not_empty preflight", result.stderr) + def test_dry_run_rejects_a_path_unsafe_task_id(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + tasks = root / "tasks.jsonl" + tasks.write_text( + '{"id":"../escape","prompt":"Choose Blue.","graders":[{"type":"regex","pattern":"Blue"}]}\n', + encoding="utf-8", + ) + + result = self.run_cli( + "run", + "--skill", + str(skill), + "--tasks", + str(tasks), + "--output", + str(root / "new-run"), + "--harness", + "codex", + "--harness-bin", + str(FAKE_CODEX), + "--model", + "test-model", + "--dry-run", + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("must be path-safe", result.stderr) + def test_dry_run_rejects_invalid_rubric_dimensions(self) -> None: cases = [ ( @@ -323,6 +354,127 @@ def test_dry_run_uses_target_owned_tasks_when_tasks_are_omitted(self) -> None: self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(json.loads(result.stdout)["configuration"]["tasks_path"], str(tasks)) + def test_promotion_requires_explicit_tasks_and_repeated_trials(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + evals = skill / "evals" + evals.mkdir() + (evals / "tasks.jsonl").write_text( + '{"id":"choice","prompt":"Choose Blue.","graders":[{"type":"regex","pattern":"Blue"}]}\n', + encoding="utf-8", + ) + + missing_tasks = self.run_cli( + "run", + "--skill", + str(skill), + "--output", + str(root / "missing-tasks"), + "--harness", + "codex", + "--harness-bin", + str(FAKE_CODEX), + "--model", + "test-model", + "--trials", + "3", + "--promotion", + "--dry-run", + ) + + self.assertEqual(missing_tasks.returncode, 1) + self.assertIn("explicit independently controlled tasks path", missing_tasks.stderr) + + tasks = evals / "tasks.jsonl" + too_few_trials = self.run_cli( + "run", + "--skill", + str(skill), + "--tasks", + str(tasks), + "--output", + str(root / "too-few-trials"), + "--harness", + "codex", + "--harness-bin", + str(FAKE_CODEX), + "--model", + "test-model", + "--trials", + "2", + "--promotion", + "--dry-run", + ) + + self.assertEqual(too_few_trials.returncode, 1) + self.assertIn("at least 3 trials", too_few_trials.stderr) + + def test_promotion_plan_records_role_and_requires_rubric_calibration(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + deterministic_tasks = root / "deterministic.jsonl" + deterministic_tasks.write_text( + '{"id":"choice","prompt":"Choose Blue.","graders":[{"type":"regex","pattern":"Blue"}]}\n', + encoding="utf-8", + ) + + result = self.run_cli( + "run", + "--skill", + str(skill), + "--tasks", + str(deterministic_tasks), + "--output", + str(root / "promotion"), + "--harness", + "codex", + "--harness-bin", + str(FAKE_CODEX), + "--model", + "test-model", + "--trials", + "3", + "--promotion", + "--dry-run", + ) + + self.assertEqual(result.returncode, 0, result.stderr) + plan = json.loads(result.stdout) + self.assertEqual(plan["configuration"]["evaluation_role"], "promotion") + self.assertEqual(plan["counts"]["paired_trials"], 3) + + rubric_tasks = root / "rubric.jsonl" + rubric_tasks.write_text( + '{"id":"choice","prompt":"Choose Blue.","graders":[{"type":"response_not_empty"},{"type":"rubric","dimensions":[{"name":"choice","levels":[{"name":"not_met","description":"Does not choose Blue."},{"name":"met","description":"Chooses Blue."}]}]}]}\n', + encoding="utf-8", + ) + uncalibrated = self.run_cli( + "run", + "--skill", + str(skill), + "--tasks", + str(rubric_tasks), + "--output", + str(root / "uncalibrated-promotion"), + "--harness", + "codex", + "--harness-bin", + str(FAKE_CODEX), + "--model", + "runner-model", + "--judge-model", + "judge-model", + "--trials", + "3", + "--promotion", + "--dry-run", + ) + + self.assertEqual(uncalibrated.returncode, 1) + self.assertIn("require accepted calibration", uncalibrated.stderr) + def test_dry_run_requires_explicit_or_target_owned_tasks_before_harness_resolution(self) -> None: with tempfile.TemporaryDirectory() as temporary: root = Path(temporary) @@ -389,6 +541,7 @@ def test_live_run_retains_control_and_treatment_evidence(self) -> None: encoding="utf-8", ) output = root / "run" + cwd_log = root / "runner-cwds.txt" host_skill = root / "user-home" / ".codex" / "skills" / "target-skill" host_skill.mkdir(parents=True) (host_skill / "SKILL.md").write_text("---\nname: target-skill\n---\n", encoding="utf-8") @@ -418,14 +571,14 @@ def test_live_run_retains_control_and_treatment_evidence(self) -> None: text=True, capture_output=True, check=False, - env=self.isolated_env(root), + env=self.isolated_env(root, {"SIMPLE_FAKE_CWD_LOG": str(cwd_log)}), ) self.assertEqual(result.returncode, 1, result.stderr) report = json.loads(result.stdout) self.assertTrue(report["valid"]) self.assertEqual(report["quality_status"], "not_required") - self.assertEqual(report["activation"]["status"], "unknown") + self.assertEqual(report["activation"]["status"], "observed") self.assertEqual(report["calibration_status"], "not_run") self.assertEqual(len(report["pairs"]), 2) self.assertEqual(report["pairs"][0]["execution_order"], ["control", "treatment"]) @@ -436,9 +589,10 @@ def test_live_run_retains_control_and_treatment_evidence(self) -> None: self.assertTrue((first_pair / "treatment" / "response.md").is_file()) pair_report = json.loads((first_pair / "report.json").read_text(encoding="utf-8")) self.assertTrue(pair_report["runner_valid"]) + self.assertEqual(pair_report["intervention"], "injected_skill_instructions") self.assertEqual(pair_report["quality_status"], "not_required") self.assertEqual(pair_report["quality_outcome"], "not_judged") - self.assertEqual(pair_report["activation"]["status"], "unknown") + self.assertEqual(pair_report["activation"]["status"], "observed") self.assertEqual(pair_report["calibration_status"], "not_run") self.assertEqual(pair_report["deterministic_comparison"], "treatment_only") self.assertTrue(pair_report["isolation"]["control_skill_absent"]) @@ -446,12 +600,24 @@ def test_live_run_retains_control_and_treatment_evidence(self) -> None: self.assertTrue( pair_report["isolation"]["treatment_installed_source_hash_match"] ) - self.assertTrue((output / "codex-home").is_dir()) - self.assertFalse((output / "codex-home" / "auth.json").exists()) + self.assertFalse((output / "codex-home").exists()) self.assertNotIn("auth.json", (first_pair / "report.json").read_text(encoding="utf-8")) markdown = (first_pair / "report.md").read_text(encoding="utf-8") + self.assertIn("Intervention: injected_skill_instructions", markdown) self.assertIn("Semantic quality was not judged.", markdown) - self.assertIn("Activation: unknown (telemetry_unavailable)", markdown) + self.assertIn("Activation: observed (skill_instructions_injected)", markdown) + control_stderr = (first_pair / "control" / "stderr.txt").read_text(encoding="utf-8") + treatment_stderr = (first_pair / "treatment" / "stderr.txt").read_text( + encoding="utf-8" + ) + self.assertNotIn("\nChoose Blue.\n", treatment_stderr) + runner_cwds = [Path(item) for item in cwd_log.read_text(encoding="utf-8").splitlines()] + self.assertEqual(len(runner_cwds), 4) + self.assertTrue(all(ROOT not in path.parents for path in runner_cwds)) + self.assertTrue(all(output not in path.parents for path in runner_cwds)) + self.assertTrue(all(not path.exists() for path in runner_cwds)) def test_live_run_copies_host_auth_json_only_during_the_run(self) -> None: with tempfile.TemporaryDirectory() as temporary: @@ -494,8 +660,7 @@ def test_live_run_copies_host_auth_json_only_during_the_run(self) -> None: self.assertEqual(result.returncode, 1, result.stderr) self.assertEqual(set(auth_log.read_text(encoding="utf-8").splitlines()), {"present"}) - self.assertTrue((output / "codex-home").is_dir()) - self.assertFalse((output / "codex-home" / "auth.json").exists()) + self.assertFalse((output / "codex-home").exists()) report_text = (output / "task-choice" / "trial-001" / "report.json").read_text(encoding="utf-8") self.assertNotIn("secret", report_text) self.assertNotIn("auth.json", report_text) @@ -562,8 +727,7 @@ def fail_task_copy(source: Path, destination: Path) -> None: with self.assertRaisesRegex(OSError, "task copy failed"): evaluator.run_live(plan) - self.assertTrue((output / "codex-home").is_dir()) - self.assertFalse((output / "codex-home" / "auth.json").exists()) + self.assertFalse((output / "codex-home").exists()) def test_calibrate_discards_auth_when_config_initialization_fails(self) -> None: with tempfile.TemporaryDirectory() as temporary: @@ -606,8 +770,7 @@ def test_calibrate_discards_auth_when_config_initialization_fails(self) -> None: with self.assertRaisesRegex(OSError, "config write failed"): evaluator.run_calibrate(plan) - self.assertTrue((output / "codex-home").is_dir()) - self.assertFalse((output / "codex-home" / "auth.json").exists()) + self.assertFalse((output / "codex-home").exists()) def test_live_run_marks_model_mismatch_invalid_and_preserves_evidence(self) -> None: with tempfile.TemporaryDirectory() as temporary: @@ -660,12 +823,15 @@ def test_live_rubric_judge_retains_structured_evidence_and_identity(self) -> Non summary = json.loads(result.stdout) self.assertTrue(summary["valid"]) self.assertEqual(summary["quality_status"], "provisional_non_independent") + self.assertEqual(summary["usage"]["measured_invocations"], 5) + self.assertEqual(summary["usage"]["total_tokens"], 65) report = json.loads(report_path.read_text(encoding="utf-8")) + self.assertEqual(report["usage"], summary["usage"]) self.assertEqual(report["rubric_status"], "provisional_non_independent") self.assertEqual(report["pairwise_status"], "provisional_non_independent") self.assertEqual(report["quality_status"], "provisional_non_independent") self.assertEqual(report["quality_outcome"], report["pairwise"][0]["winner_condition"]) - self.assertEqual(report["activation"]["status"], "unknown") + self.assertEqual(report["activation"]["status"], "observed") self.assertEqual(report["calibration_status"], "accepted") self.assertIsNotNone(report["fixtures_sha256"]) names = {item["name"] for item in report["dimension_results"]} @@ -689,6 +855,7 @@ def test_live_rubric_judge_retains_structured_evidence_and_identity(self) -> Non set(payload), {"task_prompt", "candidate_A", "candidate_B", "dimensions"}, ) + pair_dir = output / "task-choice" / "trial-001" for condition in report["conditions"]: judgment = condition["rubric_judgments"][0] self.assertEqual(judgment["status"], "provisional_non_independent") @@ -696,9 +863,14 @@ def test_live_rubric_judge_retains_structured_evidence_and_identity(self) -> Non self.assertEqual(judgment["execution"]["requested_model"], "gpt-5.6-sol") self.assertEqual(judgment["execution"]["trace_reported_model"], "gpt-5.6-sol") self.assertEqual(judgment["execution"]["model_identity_source"], "trace_reported") - judge_dir = output / "task-choice" / "trial-001" / condition["name"] / "judge-001" - self.assertTrue((judge_dir / "trace.jsonl").is_file()) - self.assertTrue((judge_dir / "response.txt").is_file()) + self.assertEqual( + judgment["artifacts"]["prompt"], + f"{condition['name']}/judge-001/prompt.txt", + ) + for relative in judgment["artifacts"].values(): + self.assertTrue((pair_dir / relative).is_file(), relative) + for relative in pairwise["artifacts"].values(): + self.assertTrue((pair_dir / relative).is_file(), relative) def test_live_rubric_judge_keeps_missing_trace_model_unattested(self) -> None: with tempfile.TemporaryDirectory() as temporary: @@ -772,7 +944,123 @@ def test_live_rubric_judge_is_skipped_when_deterministic_preflight_fails(self) - report["conditions"][0]["rubric_judgments"][0]["reason"], "deterministic_gate_failed", ) - self.assertEqual(invocation_log.read_text(encoding="utf-8").splitlines(), ["runner", "runner"]) + + def test_live_rubric_judge_runs_when_injected_skill_needs_no_trace_read(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + invocation_log = root / "invocations.txt" + result, _, report_path = self.run_live_rubric( + root, + extra_env={ + "SIMPLE_FAKE_INVOCATION_LOG": str(invocation_log), + "SIMPLE_FAKE_SKIP_SKILL_READ": "1", + }, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + report = json.loads(report_path.read_text(encoding="utf-8")) + self.assertTrue(report["runner_valid"]) + self.assertEqual(report["activation"]["status"], "observed") + treatment = next( + condition for condition in report["conditions"] if condition["name"] == "treatment" + ) + self.assertFalse(treatment["activation"]["trace_skill_read"]) + self.assertEqual(report["quality_status"], "provisional_non_independent") + self.assertEqual( + invocation_log.read_text(encoding="utf-8").splitlines(), + ["runner", "runner", "judge", "judge", "pairwise"], + ) + + def test_all_codex_roles_use_cleaned_workspaces_outside_retained_output(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + cwd_log = root / "role-cwds.txt" + + result, output, _ = self.run_live_rubric( + root, + extra_env={"SIMPLE_FAKE_ROLE_CWD_LOG": str(cwd_log)}, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + entries = [line.split("\t", 1) for line in cwd_log.read_text(encoding="utf-8").splitlines()] + self.assertEqual( + [role for role, _ in entries], + ["runner", "runner", "judge", "judge", "pairwise"], + ) + workspaces = [Path(path).resolve() for _, path in entries] + self.assertTrue(all(ROOT.resolve() not in workspace.parents for workspace in workspaces)) + self.assertTrue(all(output.resolve() not in workspace.parents for workspace in workspaces)) + self.assertTrue(all(not workspace.exists() for workspace in workspaces)) + + def test_codex_runtime_is_the_shared_target_and_judge_test_surface(self) -> None: + spec = importlib.util.spec_from_file_location("skill_eval_loop_runtime", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + pair_dir = root / "retained" / "task-choice" / "trial-001" + codex_home = root / "codex-home" + codex_home.mkdir() + cwd_log = root / "runtime-cwds.txt" + runtime = evaluator.CodexRuntime( + codex_home, + { + "harness_executable": str(FAKE_CODEX), + "model": "runner-model", + "judge_model": "judge-model", + "timeout_seconds": 1, + }, + ) + task = { + "id": "choice", + "prompt": "Choose Blue.", + "graders": [{"type": "response_not_empty"}], + } + + with patch.dict( + os.environ, + {"SIMPLE_FAKE_ROLE_CWD_LOG": str(cwd_log)}, + clear=False, + ): + control, control_isolation = runtime.run_condition( + condition="control", + pair_dir=pair_dir, + skill=skill, + skill_hash=evaluator.hash_skill(skill), + skill_name=skill.name, + task=task, + ) + treatment, treatment_isolation = runtime.run_condition( + condition="treatment", + pair_dir=pair_dir, + skill=skill, + skill_hash=evaluator.hash_skill(skill), + skill_name=skill.name, + task=task, + ) + judgment, _ = runtime.invoke_judge( + judge_dir=pair_dir / "judge-001", + artifact_root=pair_dir, + prompt="Judge this response.", + role="judge", + ) + + self.assertEqual(control["execution"]["status"], "completed") + self.assertEqual(treatment["execution"]["status"], "completed") + self.assertTrue(control_isolation["control_skill_absent"]) + self.assertTrue(treatment_isolation["treatment_hash_matches"]) + self.assertEqual(judgment["reason"], "") + self.assertEqual( + judgment["artifacts"]["prompt"], + "judge-001/prompt.txt", + ) + entries = [line.split("\t", 1) for line in cwd_log.read_text(encoding="utf-8").splitlines()] + self.assertEqual([role for role, _ in entries], ["runner", "runner", "judge"]) + self.assertTrue(all(not Path(path).exists() for _, path in entries)) def test_pairwise_judge_is_skipped_when_per_output_judgment_is_unknown(self) -> None: with tempfile.TemporaryDirectory() as temporary: @@ -855,6 +1143,120 @@ def test_pairwise_dimension_disagreement_blocks_aggregate_winner(self) -> None: self.assertIn("Quality outcome: inconsistent", markdown) self.assertIn("pairwise / safe choice: B", markdown) + def test_pairwise_tied_dimension_is_compatible_with_aggregate_winner(self) -> None: + spec = importlib.util.spec_from_file_location("skill_eval_loop_outcome", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + + outcome = evaluator.quality_outcome_for( + [ + { + "winner_condition": "control", + "mapping": {"A": "control", "B": "treatment"}, + "dimensions": [ + {"winner": "tie"}, + {"winner": "A"}, + ], + } + ], + "provisional_non_independent", + ) + + self.assertEqual(outcome, "control") + + def test_trace_records_successful_target_skill_read_as_activation(self) -> None: + spec = importlib.util.spec_from_file_location("skill_eval_loop_activation", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + + with tempfile.TemporaryDirectory() as temporary: + trace = Path(temporary) / "trace.jsonl" + trace.write_text( + json.dumps( + { + "type": "item.completed", + "item": { + "type": "command_execution", + "command": "sed -n '1,200p' .agents/skills/target-skill/SKILL.md", + "exit_code": 0, + }, + } + ) + + "\n", + encoding="utf-8", + ) + + observed = evaluator.parse_trace(trace, skill_name="target-skill") + + self.assertTrue(observed["skill_accessed"]) + + def test_trace_records_skill_read_when_later_compound_command_fails(self) -> None: + spec = importlib.util.spec_from_file_location("skill_eval_loop_activation", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + + with tempfile.TemporaryDirectory() as temporary: + trace = Path(temporary) / "trace.jsonl" + trace.write_text( + json.dumps( + { + "type": "item.completed", + "item": { + "type": "command_execution", + "command": ( + "sed -n '1,200p' .agents/skills/target-skill/SKILL.md " + "&& sed -n '1,200p' missing.md" + ), + "aggregated_output": ( + "---\nname: target-skill\ndescription: Test skill.\n---\n" + "sed: missing.md: No such file or directory\n" + ), + "exit_code": 1, + }, + } + ) + + "\n", + encoding="utf-8", + ) + + observed = evaluator.parse_trace(trace, skill_name="target-skill") + + self.assertTrue(observed["skill_accessed"]) + + def test_trace_does_not_treat_skill_directory_listing_as_activation(self) -> None: + spec = importlib.util.spec_from_file_location("skill_eval_loop_activation", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + + with tempfile.TemporaryDirectory() as temporary: + trace = Path(temporary) / "trace.jsonl" + trace.write_text( + json.dumps( + { + "type": "item.completed", + "item": { + "type": "command_execution", + "command": "find .agents/skills/target-skill -maxdepth 1 -type f", + "exit_code": 0, + }, + } + ) + + "\n", + encoding="utf-8", + ) + + observed = evaluator.parse_trace(trace, skill_name="target-skill") + + self.assertFalse(observed["skill_accessed"]) + def test_rubric_run_without_calibration_stays_quality_unknown(self) -> None: with tempfile.TemporaryDirectory() as temporary: result, _, report_path = self.run_live_rubric( @@ -1143,8 +1545,28 @@ def test_calibrate_accepts_when_judge_matches_locked_labels(self) -> None: self.assertTrue(summary["accepted"]) self.assertEqual(summary["agreements"], 3) self.assertEqual(summary["disagreements"], []) + self.assertEqual(summary["usage"]["measured_invocations"], 3) + self.assertEqual(summary["usage"]["total_tokens"], 39) retained = json.loads((output / "calibration.json").read_text(encoding="utf-8")) self.assertEqual(retained["accepted"], True) + + def test_calibrate_fails_fast_after_infrastructure_failure(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + invocation_log = root / "invocations.txt" + result, output = self.run_calibrate( + root, + extra_env={ + "SIMPLE_FAKE_INFRA_FAILURE": "1", + "SIMPLE_FAKE_INVOCATION_LOG": str(invocation_log), + }, + ) + + self.assertEqual(result.returncode, 2, result.stderr) + self.assertEqual(invocation_log.read_text(encoding="utf-8").splitlines(), ["pairwise"]) + self.assertIn("PROGRESS:", result.stderr) + retained = json.loads((output / "calibration.json").read_text(encoding="utf-8")) + self.assertEqual(retained["cases"][0]["reason"], "infrastructure_failed") self.assertTrue((output / "known-better" / "prompt.txt").is_file()) prompt = (output / "known-better" / "prompt.txt").read_text(encoding="utf-8") self.assertNotIn("better", prompt.split("\n\n", 1)[0]) @@ -1169,6 +1591,78 @@ def test_calibrate_reports_disagreements_below_threshold(self) -> None: retained = json.loads((output / "calibration.json").read_text(encoding="utf-8")) self.assertFalse(retained["accepted"]) + def test_promotion_rejects_tasks_equal_or_beneath_skill_through_symlink(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + tasks = skill / "holdout.jsonl" + tasks.write_text('{"id":"choice","prompt":"Choose.","graders":[{"type":"regex","pattern":"Blue"}]}\n') + alias = root / "alias" + alias.symlink_to(skill, target_is_directory=True) + result = self.run_cli("run", "--skill", str(skill), "--tasks", str(alias / tasks.name), "--output", str(root / "out"), "--harness", "codex", "--harness-bin", str(FAKE_CODEX), "--model", "m", "--trials", "3", "--promotion", "--dry-run") + self.assertEqual(result.returncode, 1) + self.assertIn("outside the target skill", result.stderr) + + def test_task_ids_collide_after_unicode_normalization_and_casefold(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + tasks = root / "tasks.jsonl" + tasks.write_text("\n".join([ + '{"id":"Café","prompt":"One.","graders":[{"type":"regex","pattern":"x"}]}', + '{"id":"café","prompt":"Two.","graders":[{"type":"regex","pattern":"x"}]}', + ]) + "\n", encoding="utf-8") + result = self.run_cli("run", "--skill", str(skill), "--tasks", str(tasks), "--output", str(root / "out"), "--harness", "codex", "--harness-bin", str(FAKE_CODEX), "--model", "m", "--dry-run") + self.assertEqual(result.returncode, 1) + self.assertIn("duplicate value", result.stderr) + + def test_markdown_artifact_links_resolve_from_pair_report_root(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + result, output, report_path = self.run_live_rubric(Path(temporary)) + self.assertEqual(result.returncode, 0, result.stderr) + pair_dir = report_path.parent + markdown = (pair_dir / "report.md").read_text(encoding="utf-8") + import re + links = re.findall(r"\]\(([^)]+)\)", markdown) + self.assertTrue(links) + self.assertTrue(all((pair_dir / link).is_file() for link in links), links) + + def test_tink_source_receipt_is_not_payload_hash_or_treatment_copy(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + spec = importlib.util.spec_from_file_location("skill_eval_loop_receipt", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + before = evaluator.hash_skill(skill) + (skill / ".tink-source.json").write_text('{"managed":true}\n', encoding="utf-8") + self.assertEqual(before, evaluator.hash_skill(skill)) + destination = root / "copied" + evaluator.copy_skill_payload(skill, destination) + self.assertFalse((destination / ".tink-source.json").exists()) + + def test_evals_and_tests_are_not_payload_hash_or_treatment_copy(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + skill = self.make_skill(root) + spec = importlib.util.spec_from_file_location("skill_eval_loop_payload", EVALUATOR) + self.assertIsNotNone(spec) + self.assertIsNotNone(spec.loader) + evaluator = importlib.util.module_from_spec(spec) + spec.loader.exec_module(evaluator) + before = evaluator.hash_skill(skill) + for directory in ("evals", "tests"): + excluded = skill / directory + excluded.mkdir() + (excluded / "extra.txt").write_text("ignored\n", encoding="utf-8") + self.assertEqual(before, evaluator.hash_skill(skill)) + destination = root / "copied" + evaluator.copy_skill_payload(skill, destination) + self.assertFalse((destination / "evals").exists()) + self.assertFalse((destination / "tests").exists()) + if __name__ == "__main__": unittest.main()