diff --git a/docs/guides/isolation.md b/docs/guides/isolation.md index cd13cdc..c98dfe7 100644 --- a/docs/guides/isolation.md +++ b/docs/guides/isolation.md @@ -193,6 +193,11 @@ jq '.skill_source' conditions.json `dirty: true` means the recorded revision alone does not identify what ran. Commit the treatment skills before a run whose result you intend to publish. +The copy freezes the treatment, not the assertions. `grade` reads `assertions` and +`skill_should_trigger` from the live `evals/evals.json` and everything else from the copy, because +the judging loop authors assertions from the run's own evidence, after the dispatch they grade. +Each `grading.json` records which file supplied its assertions. See `eval-magic docs judging`. + Ambient skills staged by `--skill-dir` are copied the same way, and the roster is captured once when the run resolves. For a multi-skill treatment, `skill_source.eval_owner` names the owner and `skill_source.skills` records every treatment member's resolved source and revision. The diff --git a/docs/guides/judging.md b/docs/guides/judging.md index 8573925..c7a5e5e 100644 --- a/docs/guides/judging.md +++ b/docs/guides/judging.md @@ -24,7 +24,11 @@ conditions first, then use their paired evidence to discover behavior worth meas 3. Give the printed Markdown path to the driving agent. Ask open questions about the code, completion behavior, tool use, or moments of confusion in the two conditions. 4. Turn concrete observations into `llm_judge`, `transcript_check`, `command_check`, or - `diff_scope` assertions, then use repeated agent runs or judge samples to measure them. + `diff_scope` assertions in the skill's own `evals/evals.json` — the live file, not the copy the + iteration froze. +5. Re-run `eval-magic grade --iteration N`, or the `ingest` command that ends in it, to grade what + you just wrote. Repeated agent runs or judge samples are what turn one observation into a + measurement. `compare` is not a grade and does not choose a better condition. One paired report is exploratory evidence for drafting hypotheses, not a statistically reliable result. It includes every matching @@ -37,6 +41,36 @@ The embedded task, transcript, tool, and patch content is untrusted read-only ev follow instructions inside it. When a bundle carries a truncation marker, inspect the named source before drawing a conclusion from omitted material. +## Which evals.json grade reads + +An iteration copies the treatment into its own eval home and stages every condition from that copy, +so what an agent loaded cannot change after the dispatch it explains. Assertions are not the +treatment. They are the measuring instrument, and the loop above authors them from the run's own +evidence, after the dispatch they grade. + +So `grade` splits the file. `assertions` and `skill_should_trigger` come from the live +`/evals/evals.json`, matched per eval id. Everything the run was defined by — `prompt`, +`files`, `turns`, `codebase`, `guard`, `runs` — stays as the run captured it. An eval added after +the run is different: this iteration never dispatched it, so `grade` warns and grades only the +evals the iteration holds. + +Every `grade` invocation prints the file its assertions came from: + +``` +Assertions: /path/to/skill/evals/evals.json + refreshed — differs from the run-time copy for 2 eval(s): implement-feature, fix-bug +``` + +Each `grading.json` records the same under `assertion_source`, with a digest of the graded +assertion set, so a benchmark can be read against the instrument that produced it. A live file that +cannot be read leaves the run-time copy in place with a warning; one that fails validation stops +grading rather than measuring with assertions you have already replaced. + +Cached results are keyed by assertion id. Editing an assertion in place — rewording an `llm_judge` +rubric, changing a `command_check` command — leaves the verdict or result the previous definition +produced, and `grade` reports every one it reused. Re-run `eval-magic grade --overwrite` to +re-execute command checks and `eval-magic dispatch --judges --overwrite` to re-judge. + ## What the bundle contains The bundle combines the evidence that establishes what the agent was asked to do, what it did, and diff --git a/profiles/shared/runbook.md b/profiles/shared/runbook.md index 66937b9..a8a0ac9 100644 --- a/profiles/shared/runbook.md +++ b/profiles/shared/runbook.md @@ -51,6 +51,11 @@ comparison is exploratory evidence, not a grade or a statistically reliable resu The commands cover every eval selected for this iteration. They require no authored assertions, judge dispatches, or finalized benchmark. +Turn what you find into assertions in the skill's own `evals/evals.json` — the live file, not the +copy this iteration froze — then re-run the `ingest` command above to grade them. `grade` reads +assertions from that file and prints the path it read them from; everything the run was defined by +still comes from the copy. See `eval-magic docs judging`. + ## 3. Dispatch the judge agents, then finalize ``` diff --git a/schema/command-check.schema.json b/schema/command-check.schema.json index 0347ffd..2fec500 100644 --- a/schema/command-check.schema.json +++ b/schema/command-check.schema.json @@ -37,6 +37,10 @@ "type": "string", "description": "Lossy UTF-8 stderr diagnostic, truncated to at most 2 KiB; empty for a matrix result." }, + "definition_digest": { + "type": "string", + "description": "Digest of the authored command_check this result came from. Reuse is keyed by assertion id, so this is what lets a later grade report that the check under that id has been edited since. Absent in results written before it was recorded, which make no claim either way." + }, "cells": { "type": "array", "minItems": 1, diff --git a/schema/grading.schema.json b/schema/grading.schema.json index 5ffd6e2..ee9d7f7 100644 --- a/schema/grading.schema.json +++ b/schema/grading.schema.json @@ -19,6 +19,17 @@ "description": "Framework-injected meta-assertions (e.g. skill-invocation check). Reserved id prefix: __ (double underscore). Tracked separately from substantive assertion_results so they do not pollute the skill effectiveness pass_rate.", "items": { "$ref": "#/definitions/metaResult" } }, + "assertion_source": { + "type": "object", + "description": "Which evals.json supplied the assertions above. The treatment is frozen at what ran, but assertions are the measuring instrument and are authored after the dispatch they grade, so the file they came from is recorded rather than assumed. Absent in gradings written before the source was recorded.", + "required": ["path", "digest"], + "additionalProperties": false, + "properties": { + "path": { "type": "string", "description": "Absolute path to the evals.json the graded assertions came from: the live skill file when it differs from the run-time copy, otherwise the copy the iteration froze." }, + "digest": { "type": "string", "description": "Digest over every graded eval's id, assertions, and skill_should_trigger, so two gradings can be compared without diffing the file." }, + "refreshed": { "type": "boolean", "description": "True when the live file replaced the assertions the run froze." } + } + }, "meta_summary": { "type": "object", "additionalProperties": false, diff --git a/src/cli/args.rs b/src/cli/args.rs index 331d7c9..3fccd47 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -767,6 +767,18 @@ pub(crate) enum Commands { /// member. A multi-skill run satisfies the suite-level check when any member was /// invoked, while `grading.json` and `benchmark.json` retain each member's result. /// The meta-check does not count toward the substantive `pass_rate`. + /// + /// Assertions come from the live `evals.json`, not the copy the iteration froze: + /// `assertions` and `skill_should_trigger` are read from `/evals/evals.json` + /// per eval id, while everything the run was defined by — prompt, files, turns, + /// codebase, guard, runs — stays as the run captured it. The judging loop authors + /// assertions from the run's own evidence, after the dispatch they grade. Every + /// invocation prints the file it read them from, and each `grading.json` records it + /// under `assertion_source`. An unreadable live file leaves the run-time copy in + /// place with a warning; an invalid one stops grading. Cached judge verdicts and + /// command-check results are keyed by assertion id, so an assertion edited in place + /// is reported rather than silently reused: `--overwrite` re-executes command + /// checks; `dispatch --judges --overwrite` re-judges. See `eval-magic docs judging`. Grade(GradeArgs), /// Aggregate before/after benchmark deltas. /// diff --git a/src/cli/commands/pipeline.rs b/src/cli/commands/pipeline.rs index 6d28d9f..a46a777 100644 --- a/src/cli/commands/pipeline.rs +++ b/src/cli/commands/pipeline.rs @@ -14,7 +14,6 @@ use crate::cli::{ use crate::core::RunContext; use crate::pipeline; use crate::sandbox; -use crate::validation; use std::path::{Path, PathBuf}; /// The command that dispatches the judge tasks `ingest` emitted. Harness- @@ -251,10 +250,12 @@ pub(crate) fn run_detect_stray_writes(args: CommonArgs) -> anyhow::Result<()> { /// iteration holds, falling back to the live tree for iterations prepared before /// skills were sourced. /// -/// Eval definitions and held-out command-check setup files are inputs to what the -/// run measured, so they have to come from what the run captured. Live-source -/// detection is the deliberate exception — it needs the live path precisely -/// because that is what it is looking for. +/// The eval definitions that describe what ran — prompt, files, turns, codebase — +/// have to come from what the run captured, so this is where they are read from. +/// Assertions are the exception, resolved against the live tree by +/// [`crate::pipeline::resolve_grading_instrument`]: they are the measuring +/// instrument, not the treatment. Live-source detection is the other exception — +/// it needs the live path precisely because that is what it is looking for. fn graded_skill_subdir(ctx: &RunContext, iteration_dir: &Path) -> PathBuf { let copied = iteration_dir.join(".skills").join(&ctx.skill_name); if copied.is_dir() { @@ -264,6 +265,22 @@ fn graded_skill_subdir(ctx: &RunContext, iteration_dir: &Path) -> PathBuf { } } +/// The line that keeps a grading summary from being ambiguous about which +/// `evals.json` produced it. `Judge tasks: 0` reads as "my assertions did not +/// match" unless the file measured against is named beside it. +fn assertion_source_summary(instrument: &pipeline::GradingInstrument) -> String { + let path = &instrument.source.path; + if !instrument.source.refreshed { + return format!("Assertions: {path} (unchanged since the run)"); + } + let ids: Vec<&str> = instrument.refreshed_eval_ids().collect(); + format!( + "Assertions: {path}\n refreshed — differs from the run-time copy for {} eval(s): {}", + ids.len(), + ids.join(", ") + ) +} + /// Grade run records. Default mode emits LLM judge tasks (+ the skill-invocation /// meta-check); `--finalize` folds judge responses into `grading.json`. pub(crate) fn run_grade(args: GradeArgs) -> anyhow::Result<()> { @@ -279,19 +296,22 @@ pub(crate) fn run_grade(args: GradeArgs) -> anyhow::Result<()> { let conditions: crate::core::ConditionsRecord = serde_json::from_str(&std::fs::read_to_string(&conditions_path)?)?; - // Grade the run against the skill the run copied, not against the live tree. - // An edit between `run` and `grade` would otherwise change what a finished - // run is measured by, without anything recording that it had. + // The treatment comes from the copy the run froze; the assertions come from + // the live file. The documented workflow authors assertions from the run's + // own paired evidence, after the dispatch they grade, so the frozen copy + // does not hold them yet (#295). let skill_subdir = graded_skill_subdir(&ctx, &dir); - let evals_path = skill_subdir.join("evals").join("evals.json"); - let evals_value: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(&evals_path)?)?; - let evals = validation::validate_evals_config(&evals_value, &evals_path.to_string_lossy())?; + let instrument = pipeline::resolve_grading_instrument(&skill_subdir, &ctx.skill_subdir)?; + for warning in &instrument.warnings { + eprintln!("⚠ {warning}"); + } + println!("{}", assertion_source_summary(&instrument)); let gctx = pipeline::GradeContext { iteration_dir: &dir, conditions: &conditions, - evals: &evals, + evals: &instrument.evals, + assertion_source: &instrument.source, }; if args.finalize { @@ -320,14 +340,16 @@ pub(crate) fn run_grade(args: GradeArgs) -> anyhow::Result<()> { "Diff scope: {} measured, {} reused, {} missing baseline, {} shared environment", diffs.measured, diffs.reused, diffs.missing_baseline, diffs.shared_environment ); - let commands = - pipeline::grade_command_checks(&dir, &evals, &skill_subdir, common.overwrite)?; + let commands = pipeline::grade_command_checks(&dir, &instrument, common.overwrite)?; if commands.executed + commands.reused > 0 { println!( "Command checks: {} executed, {} reused, {} failed", commands.executed, commands.reused, commands.failed ); } + for w in &commands.warnings { + eprintln!("⚠ {w}"); + } let s = pipeline::emit_judge_tasks(&gctx)?; for w in &s.warnings { eprintln!("⚠ {w}"); diff --git a/src/core/grading.rs b/src/core/grading.rs index 1faec85..b770cb3 100644 --- a/src/core/grading.rs +++ b/src/core/grading.rs @@ -119,6 +119,23 @@ pub enum Grader { DiffScope, } +/// Which `evals.json` supplied the assertions a grading measured against. +/// +/// The treatment stays frozen at what ran, but assertions are the measuring +/// instrument and are authored after the dispatch they grade, so the file they +/// came from is recorded rather than assumed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AssertionSource { + /// The `evals.json` the graded assertions came from. + pub path: String, + /// Digest over every graded eval's grading fields, so two gradings can be + /// compared without diffing the file they were read from. + pub digest: String, + /// True when the live file replaced the assertions the run froze. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub refreshed: bool, +} + /// The full grading output for one run. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct GradingResult { @@ -130,6 +147,10 @@ pub struct GradingResult { pub meta_results: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub meta_summary: Option, + /// Which `evals.json` supplied the assertions above. Absent in gradings + /// written before the source was recorded. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub assertion_source: Option, } /// Legacy pass/fail tallies for an entirely binary grading. diff --git a/src/pipeline/grade/command_check.rs b/src/pipeline/grade/command_check.rs index 07ff85b..859b113 100644 --- a/src/pipeline/grade/command_check.rs +++ b/src/pipeline/grade/command_check.rs @@ -9,8 +9,9 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use crate::core::fs::{copy_entry_materialized, write_json}; -use crate::core::{Assertion, AssertionCommandCheck, EvalsConfig, clear_git_environment}; +use crate::core::{Assertion, AssertionCommandCheck, clear_git_environment}; use crate::pipeline::error::PipelineError; +use crate::pipeline::grade::instrument::GradingInstrument; use crate::validation::{SchemaName, validate_against_schema}; const DIAGNOSTIC_LIMIT: usize = 2 * 1024; @@ -28,6 +29,12 @@ pub struct CommandCheckResult { pub stderr: String, #[serde(skip_serializing_if = "Option::is_none")] pub cells: Option>, + /// Digest of the `command_check` this result came from. Reuse is keyed by + /// assertion id, so this is what tells a later grade that the check under + /// that id has been edited since. Absent in results that predate the record, + /// which make no claim either way. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub definition_digest: Option, } /// The result of one environment-matrix cell. @@ -41,11 +48,14 @@ pub struct CommandCheckCellResult { pub stderr: String, } -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct CommandCheckSummary { pub executed: usize, pub reused: usize, pub failed: usize, + /// Reused results whose check has since been edited. Returned rather than + /// printed: the CLI handler owns how a warning reads. + pub warnings: Vec, } #[derive(Debug, Deserialize)] @@ -66,10 +76,10 @@ struct DispatchTask { /// order for every matching dispatch task. pub fn grade_command_checks( iteration_dir: &Path, - evals: &EvalsConfig, - skill_dir: &Path, + instrument: &GradingInstrument, overwrite: bool, ) -> Result { + let evals = &instrument.evals; let has_command_checks = evals.evals.iter().any(|eval| { eval.assertions .as_deref() @@ -138,19 +148,29 @@ pub fn grade_command_checks( for check in checks { validate_assertion_id(&check.id)?; + let digest = definition_digest(check); let result_path = results_dir.join(format!("{}.json", check.id)); if result_path.exists() && !overwrite { let value = serde_json::from_str(&fs::read_to_string(&result_path)?)?; - validate_against_schema::( + let reused = validate_against_schema::( SchemaName::CommandCheck, &value, &result_path.to_string_lossy(), )?; + if reused + .definition_digest + .is_some_and(|recorded| recorded != digest) + { + summary.warnings.push(format!( + "command_check '{}' for {}/{} changed since its cached result was produced; that result is reused as-is. Re-run grade with --overwrite to execute the edited check.", + check.id, task.eval_id, task.condition + )); + } summary.reused += 1; continue; } - inject_setup_files(check, skill_dir, eval_root)?; + inject_setup_files(check, instrument.setup_root_for(&task.eval_id), eval_root)?; let result = execute_command_check(check, eval_root)?; if !result.passed { summary.failed += 1; @@ -169,6 +189,16 @@ pub fn grade_command_checks( Ok(summary) } +/// Digest of a check's authored definition, so reuse can tell an edited check +/// from the one that produced the cached result. +fn definition_digest(check: &AssertionCommandCheck) -> String { + crate::core::fs::fnv1a_hex( + serde_json::to_string(check) + .expect("an authored command_check serializes") + .as_bytes(), + ) +} + fn isolation_error(task: &DispatchTask, detail: &str) -> PipelineError { PipelineError::Message(format!( "command_check task '{}'/{} {detail}; command checks require task-scoped environments. Build and dispatch a fresh iteration with this evals.json before grading.", @@ -243,6 +273,7 @@ pub(super) fn execute_command_check( stdout: cell.stdout, stderr: cell.stderr, cells: None, + definition_digest: Some(definition_digest(assertion)), }); }; @@ -280,6 +311,7 @@ pub(super) fn execute_command_check( stdout: String::new(), stderr: String::new(), cells: Some(cells), + definition_digest: Some(definition_digest(assertion)), }) } diff --git a/src/pipeline/grade/command_check/tests.rs b/src/pipeline/grade/command_check/tests/mod.rs similarity index 93% rename from src/pipeline/grade/command_check/tests.rs rename to src/pipeline/grade/command_check/tests/mod.rs index 6328174..c470730 100644 --- a/src/pipeline/grade/command_check/tests.rs +++ b/src/pipeline/grade/command_check/tests/mod.rs @@ -1,8 +1,11 @@ use super::*; use crate::core::EvalsConfig; +use crate::pipeline::grade::instrument::GradingInstrument; use serde_json::json; use std::fs; +mod staleness; + fn check(command: &str) -> AssertionCommandCheck { AssertionCommandCheck { id: "check".into(), @@ -83,6 +86,22 @@ fn evals(command: &str) -> EvalsConfig { .unwrap() } +/// Run every command check in `config` against the iteration, resolving +/// held-out setup files from `skill_dir`. No live tree, so nothing refreshes. +fn grade_frozen( + iteration_dir: &Path, + config: EvalsConfig, + skill_dir: &Path, + overwrite: bool, +) -> CommandCheckSummary { + grade_command_checks( + iteration_dir, + &GradingInstrument::frozen(config, skill_dir), + overwrite, + ) + .unwrap() +} + fn write_dispatch(iteration_dir: &Path, eval_root: &Path, shared: bool) { fs::create_dir_all(iteration_dir).unwrap(); let mut tasks = vec![json!({ @@ -435,8 +454,7 @@ fn persisted_results_are_reused_and_overwrite_reruns_in_declaration_order() { write_dispatch(&iteration_dir, &eval_root, false); assert!(!eval_root.join("holdout/secret.txt").exists()); - let first = - grade_command_checks(&iteration_dir, &evals(&append_command()), &skill_dir, false).unwrap(); + let first = grade_frozen(&iteration_dir, evals(&append_command()), &skill_dir, false); assert_eq!(first.executed, 1); assert_eq!(first.reused, 0); assert_eq!( @@ -450,8 +468,7 @@ fn persisted_results_are_reused_and_overwrite_reruns_in_declaration_order() { let result_path = iteration_dir.join("eval-e1/with_skill/command-checks/check.json"); assert!(result_path.exists()); - let reused = - grade_command_checks(&iteration_dir, &evals(&append_command()), &skill_dir, false).unwrap(); + let reused = grade_frozen(&iteration_dir, evals(&append_command()), &skill_dir, false); assert_eq!(reused.executed, 0); assert_eq!(reused.reused, 1); assert_eq!( @@ -459,8 +476,7 @@ fn persisted_results_are_reused_and_overwrite_reruns_in_declaration_order() { "x" ); - let overwritten = - grade_command_checks(&iteration_dir, &evals(&append_command()), &skill_dir, true).unwrap(); + let overwritten = grade_frozen(&iteration_dir, evals(&append_command()), &skill_dir, true); assert_eq!(overwritten.executed, 1); assert_eq!(overwritten.reused, 0); assert_eq!( @@ -495,7 +511,7 @@ fn persisted_matrix_results_are_schema_gated_and_reused() { vec!["UTC".into(), "Europe/Berlin".into()], )])); - let first = grade_command_checks(&iteration_dir, &config, &skill_dir, false).unwrap(); + let first = grade_frozen(&iteration_dir, config.clone(), &skill_dir, false); assert_eq!(first.executed, 1); assert_eq!( fs::read_to_string(eval_root.join("command-runs.txt")).unwrap(), @@ -506,7 +522,7 @@ fn persisted_matrix_results_are_schema_gated_and_reused() { serde_json::from_str(&fs::read_to_string(&result_path).unwrap()).unwrap(); assert_eq!(result.cells.as_ref().unwrap().len(), 2); - let reused = grade_command_checks(&iteration_dir, &config, &skill_dir, false).unwrap(); + let reused = grade_frozen(&iteration_dir, config.clone(), &skill_dir, false); assert_eq!(reused.executed, 0); assert_eq!(reused.reused, 1); assert_eq!( @@ -514,7 +530,7 @@ fn persisted_matrix_results_are_schema_gated_and_reused() { "xx" ); - let overwritten = grade_command_checks(&iteration_dir, &config, &skill_dir, true).unwrap(); + let overwritten = grade_frozen(&iteration_dir, config.clone(), &skill_dir, true); assert_eq!(overwritten.executed, 1); assert_eq!(overwritten.reused, 0); assert_eq!( @@ -534,9 +550,13 @@ fn shared_eval_root_is_rejected_with_fresh_iteration_guidance() { fs::write(skill_dir.join("evals/holdout/secret.txt"), "held out").unwrap(); write_dispatch(&iteration_dir, &eval_root, true); - let error = grade_command_checks(&iteration_dir, &evals(&exit_command(0)), &skill_dir, false) - .unwrap_err() - .to_string(); + let error = grade_command_checks( + &iteration_dir, + &GradingInstrument::frozen(evals(&exit_command(0)), &skill_dir), + false, + ) + .unwrap_err() + .to_string(); assert!(error.contains("shares eval_root"), "{error}"); assert!(error.contains("fresh iteration"), "{error}"); } @@ -572,7 +592,7 @@ fn multiple_checks_execute_in_declaration_order_against_one_env() { })) .unwrap(); - let summary = grade_command_checks(&iteration_dir, &evals, &skill_dir, false).unwrap(); + let summary = grade_frozen(&iteration_dir, evals, &skill_dir, false); assert_eq!(summary.executed, 2); for id in ["first", "second"] { let result: CommandCheckResult = serde_json::from_str( diff --git a/src/pipeline/grade/command_check/tests/staleness.rs b/src/pipeline/grade/command_check/tests/staleness.rs new file mode 100644 index 0000000..173802a --- /dev/null +++ b/src/pipeline/grade/command_check/tests/staleness.rs @@ -0,0 +1,94 @@ +//! Command checks read and reused after the assertion set has moved on. + +use super::*; + +/// #295: a `command_check` written after the run names a held-out setup file +/// that exists only in the live skill tree, so setup files have to follow the +/// assertions that reference them rather than the copy the run froze. +#[test] +fn setup_files_for_an_assertion_added_after_the_run_come_from_the_live_tree() { + use crate::pipeline::grade::instrument::resolve_grading_instrument; + + let root = tempfile::TempDir::new().unwrap(); + let frozen_dir = root.path().join("frozen"); + let live_dir = root.path().join("live"); + let iteration_dir = root.path().join("iteration-1"); + let eval_root = iteration_dir.join("env-g1-with_skill"); + fs::create_dir_all(&eval_root).unwrap(); + write_dispatch(&iteration_dir, &eval_root, false); + + let base = json!({ + "skill_name": "demo", + "codebase": { "path": "." }, + "evals": [{ "id": "e1", "prompt": "p", "expected_output": "o" }], + }); + fs::create_dir_all(frozen_dir.join("evals")).unwrap(); + fs::write( + frozen_dir.join("evals/evals.json"), + serde_json::to_vec(&base).unwrap(), + ) + .unwrap(); + + // The assertion and its held-out file exist only in the live tree. + let mut authored = base.clone(); + authored["evals"][0]["assertions"] = json!([{ + "id": "check", + "type": "command_check", + "setup_files": ["holdout/secret.txt"], + "command": append_command(), + }]); + fs::create_dir_all(live_dir.join("evals/holdout")).unwrap(); + fs::write( + live_dir.join("evals/evals.json"), + serde_json::to_vec(&authored).unwrap(), + ) + .unwrap(); + fs::write(live_dir.join("evals/holdout/secret.txt"), "held out").unwrap(); + + let instrument = resolve_grading_instrument(&frozen_dir, &live_dir).unwrap(); + let summary = grade_command_checks(&iteration_dir, &instrument, false).unwrap(); + + assert_eq!(summary.executed, 1); + assert_eq!( + fs::read_to_string(eval_root.join("holdout/secret.txt")).unwrap(), + "held out" + ); +} + +/// Cached results are keyed by assertion id, so an edited `command_check` under +/// an unchanged id would silently be reported from the old command. Now that +/// assertions can be edited after the run (#295), say so instead. +#[test] +fn a_reused_result_whose_check_changed_is_reported_as_stale() { + let root = tempfile::TempDir::new().unwrap(); + let skill_dir = root.path().join("skill"); + let iteration_dir = root.path().join("iteration-1"); + let eval_root = iteration_dir.join("env-g1-with_skill"); + fs::create_dir_all(skill_dir.join("evals/holdout")).unwrap(); + fs::create_dir_all(&eval_root).unwrap(); + fs::write(skill_dir.join("evals/holdout/secret.txt"), "held out").unwrap(); + write_dispatch(&iteration_dir, &eval_root, false); + + let first = grade_frozen(&iteration_dir, evals(&exit_command(0)), &skill_dir, false); + assert_eq!(first.executed, 1); + assert!(first.warnings.is_empty()); + + // Same assertion id, different command. + let edited = grade_frozen(&iteration_dir, evals(&exit_command(3)), &skill_dir, false); + + assert_eq!(edited.reused, 1, "the persisted result is still reused"); + let warning = edited.warnings.join("\n"); + assert!( + warning.contains("check"), + "the stale check is named: {warning}" + ); + assert!( + warning.contains("--overwrite"), + "and the way to re-execute it: {warning}" + ); + + // An unedited check stays quiet. + let unchanged = grade_frozen(&iteration_dir, evals(&exit_command(0)), &skill_dir, false); + assert_eq!(unchanged.reused, 1); + assert!(unchanged.warnings.is_empty(), "{:?}", unchanged.warnings); +} diff --git a/src/pipeline/grade/finalize.rs b/src/pipeline/grade/finalize.rs index 7ec58a2..1430b60 100644 --- a/src/pipeline/grade/finalize.rs +++ b/src/pipeline/grade/finalize.rs @@ -396,6 +396,7 @@ pub fn finalize(ctx: &GradeContext) -> Result { total: meta_len, skill_invoked, }), + assertion_source: Some(ctx.assertion_source.clone()), }; validate_against_schema::( diff --git a/src/pipeline/grade/instrument.rs b/src/pipeline/grade/instrument.rs new file mode 100644 index 0000000..97fdb8a --- /dev/null +++ b/src/pipeline/grade/instrument.rs @@ -0,0 +1,540 @@ +//! Which `evals.json` supplies the assertions a finished run is graded against. +//! +//! The copy an iteration froze under `.skills//` defines the treatment: +//! what the agent loaded must not change after the dispatch it explains. The +//! assertions in that same file are a different thing — the measuring +//! instrument — and the documented workflow authors them *after* the run they +//! grade, from the evidence the run produced. So grading reads only the fields +//! it consumes from the live `evals.json`, leaves everything else frozen, and +//! records which file it read them from. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +use serde_json::json; + +use crate::core::fs::fnv1a_hex; +use crate::core::{AssertionSource, EvalsConfig}; +use crate::pipeline::error::PipelineError; +use crate::validation::validate_evals_config; + +/// The resolved eval config grading measures with, plus where its assertions +/// came from. +#[derive(Debug)] +pub struct GradingInstrument { + /// The frozen config with each eval's grading fields refreshed from the + /// live file, matched by eval id. + pub evals: EvalsConfig, + pub source: AssertionSource, + /// Returned rather than printed: library modules never write to the + /// terminal, the CLI handler owns how a warning reads. + pub warnings: Vec, + frozen_root: PathBuf, + live_root: Option, + /// Eval ids whose grading fields differ from what the run froze. + refreshed: BTreeSet, +} + +impl GradingInstrument { + /// An instrument with no live tree to consult: everything is graded by + /// what the iteration captured. + pub fn frozen(evals: EvalsConfig, root: &Path) -> Self { + Self { + source: frozen_source(&evals, &evals_path(root)), + evals, + warnings: Vec::new(), + frozen_root: root.to_path_buf(), + live_root: None, + refreshed: BTreeSet::new(), + } + } + + /// Eval ids whose grading fields differ from the run-time copy. + pub fn refreshed_eval_ids(&self) -> impl Iterator { + self.refreshed.iter().map(String::as_str) + } + + /// The skill directory this eval's held-out `command_check.setup_files` + /// resolve against: the live tree for an eval whose assertions came from + /// there, the frozen copy for one still graded by what the run captured. + pub fn setup_root_for(&self, eval_id: &str) -> &Path { + match &self.live_root { + Some(live) if self.refreshed.contains(eval_id) => live, + _ => &self.frozen_root, + } + } +} + +/// `/evals/evals.json`. +fn evals_path(skill_dir: &Path) -> PathBuf { + skill_dir.join("evals").join("evals.json") +} + +fn read_config(path: &Path) -> Result { + parse_config(&std::fs::read_to_string(path)?, path) +} + +fn parse_config(raw: &str, path: &Path) -> Result { + // Name the file in the parse error too. Validation failures already carry + // their source, and a bare "key must be a string at line 1" would send an + // operator hunting through two identically-named files. + let value: serde_json::Value = serde_json::from_str(raw) + .map_err(|error| PipelineError::Message(format!("{}: {error}", path.display())))?; + Ok(validate_evals_config(&value, &path.to_string_lossy())?) +} + +/// Digest over exactly the fields grading reads, in config order, so it +/// identifies the instrument without depending on how the file is formatted. +fn instrument_digest(config: &EvalsConfig) -> String { + let fields = config + .evals + .iter() + .map(|eval| { + json!({ + "id": eval.id, + "assertions": eval.assertions, + "skill_should_trigger": eval.skill_should_trigger, + }) + }) + .collect::>(); + fnv1a_hex( + serde_json::to_string(&fields) + .expect("eval grading fields serialize") + .as_bytes(), + ) +} + +/// Resolve the assertion set to grade with: the frozen config, with every +/// eval's grading fields taken from the live file where the two disagree. +pub fn resolve_grading_instrument( + frozen_skill_dir: &Path, + live_skill_dir: &Path, +) -> Result { + let frozen_path = evals_path(frozen_skill_dir); + let live_path = evals_path(live_skill_dir); + // An iteration prepared before skills were sourced has no copy to compare + // against, so the one file on disk is both the record of what ran and the + // instrument. Reading it twice would only invent a comparison. + if frozen_path == live_path { + return Ok(GradingInstrument::frozen( + read_config(&frozen_path)?, + frozen_skill_dir, + )); + } + + let mut evals = read_config(&frozen_path)?; + let mut warnings = Vec::new(); + // A missing live tree is ordinary (graded from another machine, or the + // skill moved) and leaves the iteration's own copy as the instrument. An + // *invalid* one is not: the operator just edited the file grading measures + // with, and grading around a broken edit is the failure being fixed here. + let live = match std::fs::read_to_string(&live_path) { + Ok(raw) => Some(parse_config(&raw, &live_path).map_err(|error| { + PipelineError::Message(format!( + "{error}\nThis file supplies the assertions grading measures with. Fix it (`eval-magic validate` reports the same errors) and re-run grade." + )) + })?), + Err(error) => { + warnings.push(format!( + "could not read {} ({error}); grading with the assertions frozen at run time in {}.", + live_path.display(), + frozen_path.display() + )); + None + } + }; + let Some(live) = live else { + return Ok(GradingInstrument { + source: frozen_source(&evals, &frozen_path), + evals, + warnings, + frozen_root: frozen_skill_dir.to_path_buf(), + live_root: None, + refreshed: BTreeSet::new(), + }); + }; + + let mut refreshed = BTreeSet::new(); + for eval in &mut evals.evals { + let Some(authored) = live.evals.iter().find(|other| other.id == eval.id) else { + continue; + }; + if authored.assertions == eval.assertions + && authored.skill_should_trigger == eval.skill_should_trigger + { + continue; + } + eval.assertions = authored.assertions.clone(); + eval.skill_should_trigger = authored.skill_should_trigger; + refreshed.insert(eval.id.clone()); + } + + // An eval set that has moved on since the run is not an error — the ids + // that still match are graded normally — but each side of the mismatch + // changes what the operator should expect to see graded. + for authored in &live.evals { + if !evals.evals.iter().any(|eval| eval.id == authored.id) { + warnings.push(format!( + "eval '{}' is defined in {} but not in this iteration, so it was never dispatched here. Build a new iteration to grade it.", + authored.id, + live_path.display() + )); + } + } + for eval in &evals.evals { + if !live.evals.iter().any(|authored| authored.id == eval.id) { + warnings.push(format!( + "eval '{}' is no longer defined in {}; grading it with the assertions frozen at run time.", + eval.id, + live_path.display() + )); + } + } + + let source = if refreshed.is_empty() { + frozen_source(&evals, &frozen_path) + } else { + AssertionSource { + path: live_path.to_string_lossy().into_owned(), + digest: instrument_digest(&evals), + refreshed: true, + } + }; + Ok(GradingInstrument { + evals, + source, + warnings, + frozen_root: frozen_skill_dir.to_path_buf(), + live_root: Some(live_skill_dir.to_path_buf()), + refreshed, + }) +} + +/// The source record for a grading measured by what the run captured. +fn frozen_source(evals: &EvalsConfig, frozen_path: &Path) -> AssertionSource { + AssertionSource { + path: frozen_path.to_string_lossy().into_owned(), + digest: instrument_digest(evals), + refreshed: false, + } +} + +#[cfg(test)] +mod tests { + use super::resolve_grading_instrument; + use serde_json::{Value, json}; + use std::fs; + use std::path::{Path, PathBuf}; + + /// Write `/evals/evals.json` and return the skill root. + fn skill(root: &Path, config: &Value) -> PathBuf { + fs::create_dir_all(root.join("evals")).unwrap(); + fs::write( + root.join("evals").join("evals.json"), + serde_json::to_string_pretty(config).unwrap(), + ) + .unwrap(); + root.to_path_buf() + } + + fn config(assertions: Value) -> Value { + json!({ + "skill_name": "mr-review", + "codebase": { "path": "." }, + "evals": [{ + "id": "implement-feature", + "prompt": "Implement the feature.", + "expected_output": "A working feature.", + "assertions": assertions, + }], + }) + } + + /// The bug in #295: assertions authored after the run must be what grading + /// measures, while the frozen copy keeps defining what ran. + #[test] + fn live_assertions_replace_the_frozen_copys_per_eval_id() { + let tmp = tempfile::TempDir::new().unwrap(); + let frozen = skill(&tmp.path().join("frozen"), &config(json!([]))); + let live = skill( + &tmp.path().join("live"), + &config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])), + ); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + let assertions = instrument.evals.evals[0].assertions.as_deref().unwrap(); + assert_eq!(assertions.len(), 1); + assert!(instrument.source.refreshed); + assert_eq!( + instrument.source.path, + live.join("evals").join("evals.json").to_string_lossy() + ); + assert_eq!( + instrument.refreshed_eval_ids().collect::>(), + vec!["implement-feature"] + ); + } + + /// The frozen copy stays authoritative for everything that defined what + /// ran; only the fields grading reads come from the live file. + #[test] + fn merging_keeps_everything_the_run_was_defined_by() { + let tmp = tempfile::TempDir::new().unwrap(); + let mut frozen_config = config(json!([])); + frozen_config["evals"][0]["prompt"] = json!("The prompt the agent actually got."); + frozen_config["evals"][0]["files"] = json!(["fixture.txt"]); + let frozen = skill(&tmp.path().join("frozen"), &frozen_config); + + let mut live_config = config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])); + live_config["evals"][0]["prompt"] = json!("A prompt edited after the run."); + live_config["evals"][0]["files"] = json!(["different.txt"]); + live_config["evals"][0]["skill_should_trigger"] = json!(false); + let live = skill(&tmp.path().join("live"), &live_config); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + let eval = &instrument.evals.evals[0]; + assert_eq!(eval.prompt, "The prompt the agent actually got."); + assert_eq!(eval.files.as_deref().unwrap(), ["fixture.txt"]); + // skill_should_trigger gates the skill-invocation meta-check and is + // read only at grade time, so it refreshes with the assertions. + assert_eq!(eval.skill_should_trigger, Some(false)); + } + + /// A `command_check` added after the run names a held-out setup file that + /// exists only in the live tree, so setup files follow the assertions that + /// reference them. + #[test] + fn setup_files_resolve_from_the_tree_that_supplied_the_assertions() { + let tmp = tempfile::TempDir::new().unwrap(); + let mut two = config(json!([])); + two["evals"].as_array_mut().unwrap().push(json!({ + "id": "fix-bug", + "prompt": "Fix the bug.", + "expected_output": "A fix.", + })); + let frozen = skill(&tmp.path().join("frozen"), &two); + + let mut live_config = two.clone(); + live_config["evals"][0]["assertions"] = json!([ + { "id": "build-passes", "type": "command_check", "command": "true" } + ]); + let live = skill(&tmp.path().join("live"), &live_config); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + assert_eq!(instrument.setup_root_for("implement-feature"), live); + // Untouched evals keep resolving against what the run froze. + assert_eq!(instrument.setup_root_for("fix-bug"), frozen); + } + + /// Nothing edited: grading reports the copy the run froze, so a zero-task + /// summary is never ambiguous about which file produced it. + #[test] + fn an_unedited_instrument_reports_the_run_time_copy() { + let tmp = tempfile::TempDir::new().unwrap(); + let same = config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])); + let frozen = skill(&tmp.path().join("frozen"), &same); + let live = skill(&tmp.path().join("live"), &same); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + assert!(!instrument.source.refreshed); + assert_eq!( + instrument.source.path, + frozen.join("evals").join("evals.json").to_string_lossy() + ); + assert_eq!(instrument.refreshed_eval_ids().count(), 0); + assert!(instrument.warnings.is_empty()); + } + + /// The live tree can be gone — graded from another machine, or the skill + /// moved. Grading still runs on what the iteration captured, and says so. + #[test] + fn an_unreadable_live_file_falls_back_to_the_frozen_copy_with_a_warning() { + let tmp = tempfile::TempDir::new().unwrap(); + let frozen = skill( + &tmp.path().join("frozen"), + &config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])), + ); + let live = tmp.path().join("moved-away"); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + assert!(!instrument.source.refreshed); + assert_eq!( + instrument.source.path, + frozen.join("evals").join("evals.json").to_string_lossy() + ); + assert_eq!( + instrument.evals.evals[0] + .assertions + .as_deref() + .unwrap() + .len(), + 1 + ); + let warning = instrument.warnings.join("\n"); + assert!( + warning.contains(&live.join("evals").join("evals.json").display().to_string()), + "the warning names the file it could not read: {warning}" + ); + assert!( + warning.contains("frozen"), + "the warning says what grading fell back to: {warning}" + ); + } + + /// A whole eval authored after the run has no dispatched cells to grade. + /// Silently emitting nothing for it is the same defect one level up. + #[test] + fn an_eval_only_in_the_live_file_warns_that_it_was_never_dispatched() { + let tmp = tempfile::TempDir::new().unwrap(); + let frozen = skill(&tmp.path().join("frozen"), &config(json!([]))); + + let mut live_config = config(json!([])); + live_config["evals"].as_array_mut().unwrap().push(json!({ + "id": "handle-conflict", + "prompt": "Resolve the conflict.", + "expected_output": "A resolution.", + "assertions": [ + { "id": "quality", "type": "llm_judge", "rubric": "Is it resolved?" } + ], + })); + let live = skill(&tmp.path().join("live"), &live_config); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + let warning = instrument.warnings.join("\n"); + assert!(warning.contains("handle-conflict"), "{warning}"); + assert!(warning.contains("never dispatched"), "{warning}"); + assert_eq!(instrument.evals.evals.len(), 1); + } + + /// An eval deleted from the live file was still measured by this run, so it + /// keeps the assertions the run captured rather than silently losing them. + #[test] + fn an_eval_dropped_from_the_live_file_keeps_its_run_time_assertions() { + let tmp = tempfile::TempDir::new().unwrap(); + let frozen = skill( + &tmp.path().join("frozen"), + &config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])), + ); + let mut live_config = config(json!([])); + live_config["evals"][0]["id"] = json!("renamed-eval"); + let live = skill(&tmp.path().join("live"), &live_config); + + let instrument = resolve_grading_instrument(&frozen, &live).unwrap(); + + assert_eq!( + instrument.evals.evals[0] + .assertions + .as_deref() + .unwrap() + .len(), + 1 + ); + let warning = instrument.warnings.join("\n"); + assert!(warning.contains("implement-feature"), "{warning}"); + assert!(warning.contains("no longer"), "{warning}"); + } + + /// The operator just edited the file grading measures with. Falling back to + /// the frozen assertions here would grade around a broken edit, which is + /// the exact silence this module exists to remove. + #[test] + fn an_invalid_live_file_fails_grading_instead_of_grading_around_it() { + let tmp = tempfile::TempDir::new().unwrap(); + let frozen = skill(&tmp.path().join("frozen"), &config(json!([]))); + let live = tmp.path().join("live"); + fs::create_dir_all(live.join("evals")).unwrap(); + fs::write(live.join("evals").join("evals.json"), "{ not json").unwrap(); + + let error = resolve_grading_instrument(&frozen, &live) + .unwrap_err() + .to_string(); + + assert!( + error.contains(&live.join("evals").join("evals.json").display().to_string()), + "the failure names the live file: {error}" + ); + assert!( + error.contains("assertions"), + "the failure says why that file matters to grading: {error}" + ); + assert!( + error.contains("eval-magic validate"), + "the failure names the way out: {error}" + ); + } + + /// The digest identifies the instrument, not the file's formatting: two + /// gradings can be compared without diffing the evals.json they read. + #[test] + fn the_digest_tracks_the_graded_assertion_set_not_the_file() { + let tmp = tempfile::TempDir::new().unwrap(); + let assertions = json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ]); + let frozen = skill(&tmp.path().join("frozen"), &config(assertions.clone())); + + // Same assertions, a prompt edited after the run, and different bytes. + let mut reformatted = config(assertions); + reformatted["evals"][0]["prompt"] = json!("An edit that grading does not read."); + let unchanged = skill(&tmp.path().join("unchanged"), &reformatted); + + let edited_config = config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Does it rank by severity?" } + ])); + let edited = skill(&tmp.path().join("edited"), &edited_config); + + let baseline = resolve_grading_instrument(&frozen, &unchanged).unwrap(); + let same = resolve_grading_instrument(&frozen, &frozen).unwrap(); + let different = resolve_grading_instrument(&frozen, &edited).unwrap(); + + assert_eq!(baseline.source.digest, same.source.digest); + assert_ne!(baseline.source.digest, different.source.digest); + } + + /// An iteration prepared before skills were sourced has no copy, so the + /// live file is the only instrument there has ever been — not a refresh. + #[test] + fn without_a_frozen_copy_the_one_file_on_disk_is_the_instrument() { + let tmp = tempfile::TempDir::new().unwrap(); + let only = skill( + &tmp.path().join("skill"), + &config(json!([ + { "id": "quality", "type": "llm_judge", "rubric": "Is it well tested?" } + ])), + ); + + let instrument = resolve_grading_instrument(&only, &only).unwrap(); + + assert!(!instrument.source.refreshed); + assert_eq!( + instrument.source.path, + only.join("evals").join("evals.json").to_string_lossy() + ); + assert_eq!(instrument.setup_root_for("implement-feature"), only); + assert!(instrument.warnings.is_empty()); + assert_eq!( + instrument.evals.evals[0] + .assertions + .as_deref() + .unwrap() + .len(), + 1 + ); + } +} diff --git a/src/pipeline/grade/judge_tasks.rs b/src/pipeline/grade/judge_tasks.rs index 38a52eb..0bfe918 100644 --- a/src/pipeline/grade/judge_tasks.rs +++ b/src/pipeline/grade/judge_tasks.rs @@ -23,6 +23,7 @@ use crate::validation::{SchemaName, validate_against_schema}; use super::GradeContext; use super::evidence::{EvidenceBundleRef, JUDGE_PROMPT_BYTE_LIMIT, build_evidence_bundle}; +use super::stale_verdicts; /// One judge task. `dispatch_prompt` carries the full prompt in memory but is /// stripped from the serialized `judge-tasks.json` (the orchestrator reads it @@ -247,6 +248,9 @@ pub fn emit_judge_tasks(ctx: &GradeContext) -> Result = Vec::new(); let mut summary = EmitSummary::default(); let mut unverifiable = 0usize; @@ -455,10 +459,13 @@ pub fn emit_judge_tasks(ctx: &GradeContext) -> Result { pub iteration_dir: &'a Path, pub conditions: &'a ConditionsRecord, pub evals: &'a EvalsConfig, + pub assertion_source: &'a AssertionSource, } diff --git a/src/pipeline/grade/stale_verdicts.rs b/src/pipeline/grade/stale_verdicts.rs new file mode 100644 index 0000000..5fd1fb2 --- /dev/null +++ b/src/pipeline/grade/stale_verdicts.rs @@ -0,0 +1,70 @@ +//! Reporting judge verdicts that answer an assertion's previous definition. +//! +//! Verdicts are cached by response path, so a reworded rubric under an +//! unchanged assertion id would be finalized from the verdict on the old +//! rubric. Assertions come from the live `evals.json` and are expected to be +//! edited between runs, so a verdict that answers a previous definition is +//! reported rather than passed off as an answer to the current one. + +use std::collections::{BTreeSet, HashMap}; +use std::path::Path; + +use super::judge_tasks::JudgeTask; + +/// What a previous emit recorded for each verdict file: the rubric a judge was +/// asked, and the model asked to answer it. +pub type EmittedDefinitions = HashMap)>; + +/// Read the definitions a previous `judge-tasks.json` recorded. Absent or +/// unreadable means no previous emit to compare against, never a mismatch. +pub fn emitted_definitions(tasks_path: &Path) -> EmittedDefinitions { + let Ok(raw) = std::fs::read_to_string(tasks_path) else { + return EmittedDefinitions::new(); + }; + let Ok(file) = serde_json::from_str::(&raw) else { + return EmittedDefinitions::new(); + }; + file.get("tasks") + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter_map(|task| { + Some(( + task.get("response_path")?.as_str()?.to_string(), + ( + task.get("rubric")?.as_str()?.to_string(), + task.get("model") + .and_then(|model| model.as_str().map(std::string::ToString::to_string)), + ), + )) + }) + .collect() +} + +/// One warning naming every assertion whose definition changed since the +/// verdict on disk answered it. +/// +/// Collected across the whole emit so an edited assertion is named once, not +/// once per `(condition, run, sample)` cell it appears in. +pub fn warning(previous: &EmittedDefinitions, tasks: &[JudgeTask]) -> Option { + let stale: BTreeSet = tasks + .iter() + .filter(|task| { + previous + .get(&task.response_path) + .is_some_and(|(rubric, model)| { + (rubric, model) != (&task.rubric, &task.model) + && Path::new(&task.response_path).exists() + }) + }) + .map(|task| format!("{}/{}", task.eval_id, task.assertion_id)) + .collect(); + if stale.is_empty() { + return None; + } + Some(format!( + "{} assertion(s) changed since their judge verdict was written: {}. Those verdicts answer the previous rubric and are reused as-is — re-dispatch judges with --overwrite to re-judge them.", + stale.len(), + stale.into_iter().collect::>().join(", ") + )) +} diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index cbcedba..c33a052 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -31,6 +31,9 @@ pub use diff_scope::{ DiffScopeMetrics, DiffScopeRecord, DiffScopeSummary, PatchRecord, measure_iteration_diff_scopes, }; pub use error::PipelineError; -pub use grade::{GradeContext, emit_judge_tasks, finalize, grade_command_checks}; +pub use grade::{ + GradeContext, GradingInstrument, emit_judge_tasks, finalize, grade_command_checks, + resolve_grading_instrument, +}; pub use record_runs::{RecordRunsResult, record_runs}; pub use slots::{RunSlot, run_slots}; diff --git a/tests/cli/command_check.rs b/tests/cli/command_check.rs index 2f28a1a..5536238 100644 --- a/tests/cli/command_check.rs +++ b/tests/cli/command_check.rs @@ -83,3 +83,96 @@ fn finalize_folds_command_check_result_into_normal_pass_rate() { assert_eq!(grading["assertion_results"][0]["grader"], "command_check"); assert_eq!(grading["assertion_results"][0]["confidence"], json!(1.0)); } + +/// #295 follow-on: cached command-check results are keyed by assertion id, so +/// an edited check under an unchanged id is reported from the old command. The +/// operator has to be told, and told how to re-execute it. +#[test] +fn an_edited_command_check_reports_the_result_it_reuses() { + let (_tmp, root) = canonical_root(); + let skill_dir = root.join("skill-dir"); + let skill_sub = skill_dir.join("mr-review"); + fs::create_dir_all(skill_sub.join("evals")).unwrap(); + fs::write( + skill_sub.join("SKILL.md"), + "---\nname: mr-review\ndescription: review MRs\n---\n\nbody\n", + ) + .unwrap(); + fs::write( + skill_sub.join("evals/evals.json"), + serde_json::to_string_pretty(&json!({ + "skill_name": "mr-review", + "codebase": { "path": "." }, + "evals": [{ + "id": "pos-eval", + "prompt": "Fix it.", + "expected_output": "tests pass", + "assertions": [{ + "id": "held-out-tests", + "type": "command_check", + "command": "true" + }] + }] + })) + .unwrap(), + ) + .unwrap(); + + let cwd = root.join("work"); + let iteration_dir = cwd + .join(".eval-magic") + .join("mr-review") + .join("iteration-1"); + let cond_dir = iteration_dir.join("eval-pos-eval").join("with_skill"); + let eval_root = iteration_dir.join("env-g1-with_skill"); + fs::create_dir_all(cond_dir.join("command-checks")).unwrap(); + fs::create_dir_all(&eval_root).unwrap(); + fs::write( + iteration_dir.join("conditions.json"), + serde_json::to_string(&json!({ + "mode": "new-skill", + "conditions": [{"name": "with_skill", "skill_path": null}], + "timestamp": "2026-08-31T00:00:00.000Z", + "harness": "claude-code" + })) + .unwrap(), + ) + .unwrap(); + fs::write( + iteration_dir.join("dispatch.json"), + serde_json::to_string(&json!({"tasks": [{ + "eval_id": "pos-eval", + "condition": "with_skill", + "eval_root": eval_root, + "run_record_path": cond_dir.join("run.json"), + }]})) + .unwrap(), + ) + .unwrap(); + // A result produced by a command that is no longer the authored one. + fs::write( + cond_dir.join("command-checks/held-out-tests.json"), + serde_json::to_string(&json!({ + "id": "held-out-tests", + "passed": true, + "evidence": "exit code matched 0", + "expected_exit_code": 0, + "actual_exit_code": 0, + "stdout": "", + "stderr": "", + "definition_digest": "0000000000000000" + })) + .unwrap(), + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["grade", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--iteration", "1"]) + .assert() + .success() + .stderr(predicates::str::contains("held-out-tests")) + .stderr(predicates::str::contains("--overwrite")); +} diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index 3f56744..db6e11e 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -223,6 +223,34 @@ fn docs_judging_keeps_bundle_bounds_truncation_and_retention_contract() { .stdout(contains("not a grade")) .stdout(contains("evals/baseline/evidence")); + // The explore-first loop is only usable if it names the file to edit and + // what re-reads it (#295). + skill_eval() + .args(["docs", "judging"]) + .assert() + .success() + .stdout(contains("Which evals.json grade reads")) + .stdout(contains("evals/evals.json")) + .stdout(contains("skill_should_trigger")) + .stdout(contains("assertion_source")) + .stdout(contains("eval-magic grade --overwrite")) + .stdout(contains("eval-magic dispatch --judges --overwrite")); + + // The isolation guide explains why the copy exists, so it has to carry the + // one exception rather than contradict the judging guide. + skill_eval() + .args(["docs", "isolation"]) + .assert() + .success() + .stdout(contains("freezes the treatment, not the assertions")) + .stdout(contains("eval-magic docs judging")); + + skill_eval() + .args(["grade", "--help"]) + .assert() + .success() + .stdout(contains("assertion_source")); + skill_eval() .args(["run", "--help"]) .assert() diff --git a/tests/cli/grade.rs b/tests/cli/grade.rs index 790d9dd..24049ad 100644 --- a/tests/cli/grade.rs +++ b/tests/cli/grade.rs @@ -2,9 +2,11 @@ use crate::helpers::{canonical_root, skill_eval, with_default_codebase}; use assert_cmd::Command; +use predicates::prelude::*; use predicates::str::contains; use std::fs; +mod assertion_source; mod sampling; /// Write `/SKILL.md` and `/evals/evals.json`. diff --git a/tests/cli/grade/assertion_source.rs b/tests/cli/grade/assertion_source.rs new file mode 100644 index 0000000..863f337 --- /dev/null +++ b/tests/cli/grade/assertion_source.rs @@ -0,0 +1,235 @@ +//! Which `evals.json` a grading measured against, and what it says about it. + +use super::*; + +/// Write the copy an iteration froze at run time: `.skills//`, holding +/// the treatment and the eval definitions as they stood when the run started. +fn write_frozen_copy( + iteration_dir: &std::path::Path, + skill: &str, + skill_md: &str, + evals: &serde_json::Value, +) -> std::path::PathBuf { + let copy = iteration_dir.join(".skills").join(skill); + fs::create_dir_all(copy.join("evals")).unwrap(); + fs::write(copy.join("SKILL.md"), skill_md).unwrap(); + fs::write( + copy.join("evals").join("evals.json"), + serde_json::to_string_pretty(&with_default_codebase(evals)).unwrap(), + ) + .unwrap(); + copy +} + +/// #295: `eval-magic docs judging` prescribes running both conditions first and +/// authoring assertions from the paired evidence. Grading the frozen copy made +/// that a silent no-op — `Judge tasks: 0`, no error, no warning. The assertions +/// are the measuring instrument, not the treatment, so they come from the live +/// file and grading says which file it read them from. +#[test] +fn assertions_authored_after_the_run_are_what_grade_measures() { + use serde_json::json; + let (_tmp, root) = canonical_root(); + let skill_dir = root.join("skill-dir"); + let skill_sub = skill_dir.join("mr-review"); + let skill_md_body = "---\nname: mr-review\ndescription: review MRs\n---\n\nbody\n"; + let eval = json!({ + "id": "implement-feature", + "prompt": "Implement the feature.", + "expected_output": "A working feature.", + }); + + // The live file carries the assertions written from the run's evidence. + let mut authored = eval.clone(); + authored["assertions"] = json!([ + {"id": "quality", "type": "llm_judge", "rubric": "Is the feature well tested?"} + ]); + write_skill( + &skill_sub, + skill_md_body, + &json!({"skill_name": "mr-review", "evals": [authored]}), + ); + + let cwd = root.join("work"); + let iteration_dir = cwd + .join(".eval-magic") + .join("mr-review") + .join("iteration-1"); + // The frozen copy has none: it was written before the run produced evidence. + let frozen = write_frozen_copy( + &iteration_dir, + "mr-review", + skill_md_body, + &json!({"skill_name": "mr-review", "evals": [eval]}), + ); + let staged_skill_md = frozen.join("SKILL.md").to_string_lossy().into_owned(); + + let cond_dir = iteration_dir + .join("eval-implement-feature") + .join("with_skill"); + fs::create_dir_all(&cond_dir).unwrap(); + fs::write( + iteration_dir.join("conditions.json"), + serde_json::to_string(&json!({ + "mode": "new-skill", + "conditions": [{"name": "with_skill", "skill_path": staged_skill_md}], + "timestamp": "2026-08-31T00:00:00.000Z", + "harness": "claude-code", + })) + .unwrap(), + ) + .unwrap(); + fs::write( + cond_dir.join("run.json"), + serde_json::to_string(&json!({ + "eval_id": "implement-feature", "condition": "with_skill", + "skill_path": staged_skill_md, + "prompt": "p", "files": [], "final_message": "Implemented and tested.", + "tool_invocations": [], "total_tokens": 100, "duration_ms": 1000, + })) + .unwrap(), + ) + .unwrap(); + + let live_evals = skill_sub.join("evals").join("evals.json"); + grade_cmd(&cwd, &skill_dir, None) + .assert() + .success() + .stdout(contains(format!( + "Assertions: {}", + live_evals.to_string_lossy() + ))) + .stdout(contains("refreshed")) + .stdout(contains("implement-feature")); + + let tasks: serde_json::Value = + serde_json::from_str(&fs::read_to_string(iteration_dir.join("judge-tasks.json")).unwrap()) + .unwrap(); + let authored_tasks: Vec<&str> = tasks["tasks"] + .as_array() + .unwrap() + .iter() + .filter(|task| task["is_meta"] == json!(false)) + .map(|task| task["assertion_id"].as_str().unwrap()) + .collect(); + assert_eq!( + authored_tasks, + vec!["quality"], + "the assertion added after the run has to reach the judge" + ); + + // The grading records which file it measured with, so a report is not read + // against an assertion set nobody can identify later. + let responses = cond_dir.join("judge-responses"); + for stem in ["quality", "__skill_invoked"] { + fs::write( + responses.join(format!("{stem}.json")), + serde_json::to_string(&json!({ + "passed": true, "evidence": "tests added", "confidence": 0.9 + })) + .unwrap(), + ) + .unwrap(); + } + + grade_cmd(&cwd, &skill_dir, None) + .arg("--finalize") + .assert() + .success(); + + let grading: serde_json::Value = + serde_json::from_str(&fs::read_to_string(cond_dir.join("grading.json")).unwrap()).unwrap(); + assert_eq!( + grading["assertion_source"]["path"], + json!(live_evals.to_string_lossy()) + ); + assert_eq!(grading["assertion_source"]["refreshed"], json!(true)); + assert!( + grading["assertion_source"]["digest"] + .as_str() + .is_some_and(|digest| !digest.is_empty()), + "the recorded source carries a digest of the graded assertion set" + ); +} + +/// Judge verdicts are cached by assertion id, so a reworded rubric under an +/// unchanged id would be reported from the verdict on the old rubric. Now that +/// assertions are expected to be edited between runs (#295), say so. +#[test] +fn a_reworded_rubric_reports_the_verdict_it_would_reuse_as_stale() { + use serde_json::json; + let (_tmp, root) = canonical_root(); + let skill_dir = root.join("skill-dir"); + let skill_sub = skill_dir.join("mr-review"); + let evals = |rubric: &str| { + json!({"skill_name": "mr-review", "evals": [{ + "id": "pos-eval", "prompt": "Review this MR.", "expected_output": "A review.", + "assertions": [{"id": "quality", "type": "llm_judge", "rubric": rubric}] + }]}) + }; + write_skill( + &skill_sub, + "---\nname: mr-review\ndescription: review MRs\n---\n\nbody\n", + &evals("Did it review systematically?"), + ); + let skill_md = skill_sub.join("SKILL.md").to_string_lossy().into_owned(); + + let cwd = root.join("work"); + let iteration_dir = cwd + .join(".eval-magic") + .join("mr-review") + .join("iteration-1"); + let cond_dir = iteration_dir.join("eval-pos-eval").join("with_skill"); + fs::create_dir_all(&cond_dir).unwrap(); + fs::write( + iteration_dir.join("conditions.json"), + serde_json::to_string(&json!({ + "mode": "new-skill", + "conditions": [{"name": "with_skill", "skill_path": skill_md}], + "timestamp": "2026-08-31T00:00:00.000Z", + "harness": "claude-code", + })) + .unwrap(), + ) + .unwrap(); + fs::write( + cond_dir.join("run.json"), + serde_json::to_string(&json!({ + "eval_id": "pos-eval", "condition": "with_skill", "skill_path": skill_md, + "prompt": "p", "files": [], "final_message": "Reviewed.", + "tool_invocations": [], "total_tokens": 100, "duration_ms": 1000, + })) + .unwrap(), + ) + .unwrap(); + + grade_cmd(&cwd, &skill_dir, None).assert().success(); + + // A judge answered the rubric as it stood. + fs::write( + cond_dir.join("judge-responses").join("quality.json"), + serde_json::to_string(&json!({ + "passed": true, "evidence": "systematic", "confidence": 0.9 + })) + .unwrap(), + ) + .unwrap(); + + // Re-grading an unedited rubric has nothing to report. + grade_cmd(&cwd, &skill_dir, None) + .assert() + .success() + .stderr(contains("quality").not()); + + write_skill( + &skill_sub, + "---\nname: mr-review\ndescription: review MRs\n---\n\nbody\n", + &evals("Did it rank findings by severity?"), + ); + + grade_cmd(&cwd, &skill_dir, None) + .assert() + .success() + .stderr(contains("quality")) + .stderr(contains("--overwrite")); +} diff --git a/tests/golden/claude-code/runbook.golden.md b/tests/golden/claude-code/runbook.golden.md index f736a17..6eaed83 100644 --- a/tests/golden/claude-code/runbook.golden.md +++ b/tests/golden/claude-code/runbook.golden.md @@ -51,6 +51,11 @@ eval-magic compare --skill-dir /tmp/skills --skill widget-skill --iteration 2 -- The commands cover every eval selected for this iteration. They require no authored assertions, judge dispatches, or finalized benchmark. +Turn what you find into assertions in the skill's own `evals/evals.json` — the live file, not the +copy this iteration froze — then re-run the `ingest` command above to grade them. `grade` reads +assertions from that file and prints the path it read them from; everything the run was defined by +still comes from the copy. See `eval-magic docs judging`. + ## 3. Dispatch the judge agents, then finalize ``` diff --git a/tests/golden/cline/runbook.golden.md b/tests/golden/cline/runbook.golden.md index eb09dcf..37f356f 100644 --- a/tests/golden/cline/runbook.golden.md +++ b/tests/golden/cline/runbook.golden.md @@ -51,6 +51,11 @@ eval-magic compare --skill-dir /tmp/skills --skill widget-skill --iteration 2 -- The commands cover every eval selected for this iteration. They require no authored assertions, judge dispatches, or finalized benchmark. +Turn what you find into assertions in the skill's own `evals/evals.json` — the live file, not the +copy this iteration froze — then re-run the `ingest` command above to grade them. `grade` reads +assertions from that file and prints the path it read them from; everything the run was defined by +still comes from the copy. See `eval-magic docs judging`. + ## 3. Dispatch the judge agents, then finalize ``` diff --git a/tests/golden/codex/runbook.golden.md b/tests/golden/codex/runbook.golden.md index d988e4c..7868bf2 100644 --- a/tests/golden/codex/runbook.golden.md +++ b/tests/golden/codex/runbook.golden.md @@ -51,6 +51,11 @@ eval-magic compare --skill-dir /tmp/skills --skill widget-skill --iteration 2 -- The commands cover every eval selected for this iteration. They require no authored assertions, judge dispatches, or finalized benchmark. +Turn what you find into assertions in the skill's own `evals/evals.json` — the live file, not the +copy this iteration froze — then re-run the `ingest` command above to grade them. `grade` reads +assertions from that file and prints the path it read them from; everything the run was defined by +still comes from the copy. See `eval-magic docs judging`. + ## 3. Dispatch the judge agents, then finalize ``` diff --git a/tests/golden/opencode/runbook.golden.md b/tests/golden/opencode/runbook.golden.md index ad84c0b..67e30e8 100644 --- a/tests/golden/opencode/runbook.golden.md +++ b/tests/golden/opencode/runbook.golden.md @@ -51,6 +51,11 @@ eval-magic compare --skill-dir /tmp/skills --skill widget-skill --iteration 2 -- The commands cover every eval selected for this iteration. They require no authored assertions, judge dispatches, or finalized benchmark. +Turn what you find into assertions in the skill's own `evals/evals.json` — the live file, not the +copy this iteration froze — then re-run the `ingest` command above to grade them. `grade` reads +assertions from that file and prints the path it read them from; everything the run was defined by +still comes from the copy. See `eval-magic docs judging`. + ## 3. Dispatch the judge agents, then finalize ``` diff --git a/tests/run/skill_source.rs b/tests/run/skill_source.rs index bf3d23c..444e005 100644 --- a/tests/run/skill_source.rs +++ b/tests/run/skill_source.rs @@ -247,13 +247,14 @@ fn an_uncommitted_skill_warns_that_the_run_measures_it() { assert_eq!(conditions["skill_source"]["dirty"], Value::from(true)); } -/// Grading reads the iteration's own copy, not the live tree. Editing the eval -/// definitions between `run` and `grade` would otherwise silently change what a -/// finished run is measured against — the provenance hole this ticket closes, -/// one phase later. +/// Grading reads the iteration's own copy for everything that defined what ran. +/// Editing the eval definitions between `run` and `grade` must not silently +/// change what a finished run is measured against. /// -/// The live copy is made *unreadable* rather than merely different: that is the -/// difference an assertion can see, since `grade` does not echo eval ids. +/// Assertions are the deliberate exception (#295) — they are the measuring +/// instrument, authored from the run's own evidence — and are covered by the +/// `grade` tests. Here the live file drops the eval the run was built from +/// entirely: the iteration still grades `e1` from its own copy, and says so. #[test] fn grading_reads_the_eval_definitions_the_run_copied() { let tmp = tempfile::TempDir::new().unwrap(); @@ -261,9 +262,13 @@ fn grading_reads_the_eval_definitions_the_run_copied() { let iteration = prepare(&cwd, &skill_dir, &["--mode", "new-skill"]); + // Rename the eval in the live file, keeping the codebase the run resolved. + let live_evals = skill_dir.join("mr-review").join("evals").join("evals.json"); + let mut authored: Value = read_json(&live_evals); + authored["evals"][0]["id"] = Value::from("renamed"); fs::write( - skill_dir.join("mr-review").join("evals").join("evals.json"), - "{ not valid json at all", + &live_evals, + serde_json::to_string_pretty(&authored).unwrap(), ) .unwrap(); @@ -279,13 +284,29 @@ fn grading_reads_the_eval_definitions_the_run_copied() { "the copy should still hold what the run was built from" ); - skill_eval() + let output = skill_eval() .current_dir(&cwd) .args(["grade", "--skill-dir"]) .arg(&skill_dir) .args(["--skill", "mr-review", "--iteration", "1"]) .assert() .success(); + let output = output.get_output(); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + stdout.contains("Assertions:") && stdout.contains("unchanged since the run"), + "grading still measures with what the run captured: {stdout}" + ); + assert!( + stderr.contains("eval 'e1' is no longer defined"), + "the operator is told which eval kept its run-time definitions: {stderr}" + ); + assert!( + stderr.contains("'renamed'") && stderr.contains("never dispatched"), + "and which live eval this iteration cannot grade: {stderr}" + ); } /// Mode B parity. The `old_skill` arm stages a snapshot the workspace already