From 02d57693a3633ef193c1d5e7bda931ed5b346a97 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 24 Aug 2026 03:14:23 -0400 Subject: [PATCH] feat(evals): support multi-skill treatments Stage ordered skill rosters as one treatment while preserving scalar artifact compatibility. Grade invocation per member and carry complete treatment provenance through reports and promotion. --- README.md | 4 + docs/guides/isolation.md | 61 +- docs/guides/judging.md | 10 +- schema/benchmark.schema.json | 42 +- schema/evals.schema.json | 12 +- schema/grading.schema.json | 20 +- schema/judge-tasks.schema.json | 4 + schema/run-record.schema.json | 42 ++ src/adapters/skill_shadow/grouping.rs | 22 +- src/cli/args.rs | 38 +- src/cli/commands/workspace.rs | 56 +- src/cli/run/dispatch.rs | 93 +-- src/cli/run/dispatch/prompt_components.rs | 130 ++++ src/cli/run/orchestrate/build.rs | 60 +- src/cli/run/orchestrate/build/roster.rs | 23 + src/cli/run/orchestrate/mod.rs | 101 ++- src/cli/run/orchestrate/resolve.rs | 192 ++++-- src/cli/run/orchestrate/shadow_preflight.rs | 117 ++-- src/cli/run/orchestrate/skill.rs | 59 ++ src/cli/run/orchestrate/stage.rs | 115 +++- src/cli/run/staging/mod.rs | 14 +- src/core/grading.rs | 18 +- src/core/types.rs | 78 ++- src/pipeline/aggregate.rs | 45 +- src/pipeline/detect_stray_writes.rs | 34 +- src/pipeline/grade/finalize.rs | 106 ++-- src/pipeline/grade/judge_tasks.rs | 207 +++--- src/pipeline/record_runs.rs | 7 +- src/validation/evals.rs | 3 + src/validation/evals/multi_skill_tests.rs | 34 + src/workspace/mod.rs | 2 +- src/workspace/promote.rs | 7 + src/workspace/promote/source_row.rs | 39 ++ src/workspace/promote/tests.rs | 59 ++ src/workspace/snapshot.rs | 67 +- tests/run/main.rs | 1 + tests/run/multi_skill.rs | 661 ++++++++++++++++++++ 37 files changed, 2154 insertions(+), 429 deletions(-) create mode 100644 src/cli/run/dispatch/prompt_components.rs create mode 100644 src/cli/run/orchestrate/build/roster.rs create mode 100644 src/cli/run/orchestrate/skill.rs create mode 100644 src/validation/evals/multi_skill_tests.rs create mode 100644 src/workspace/promote/source_row.rs create mode 100644 tests/run/multi_skill.rs diff --git a/README.md b/README.md index d3a647d..71f4438 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,10 @@ eval-magic run --mode revision The command help and generated runbook describe baseline selection and the rest of the workflow. +An eval can treat coordinated skills as one treatment by setting `skill_name` to an ordered list. +Pass one listed member with `--skill`; it remains the eval owner and supplies fixtures. See +`eval-magic docs isolation` for the complete configuration, Mode A/B behavior, and provenance. + ## How it works Each eval case runs once per condition and repetition in its own clean Git repository. The two arms diff --git a/docs/guides/isolation.md b/docs/guides/isolation.md index 46c24e1..07a8d09 100644 --- a/docs/guides/isolation.md +++ b/docs/guides/isolation.md @@ -144,29 +144,52 @@ then use `isolates_live_sources` to record the operator assertion. dispatch's setting-source selection. A plugin can appear there and remain absent from the dispatch, or the reverse. Use the dispatch's init event. -## The skill under test is a copy - -Every skill an eval stages is copied into the eval home before any dispatch runs, and -each condition stages from that copy. Nothing the agent can reach is read from your own -skill directory, so editing a skill mid-campaign cannot change what a prepared iteration -measures. +## Treatment skills are copies + +`skill_name` in the `evals/evals.json` file accepts either one skill name or an ordered, +non-empty list: + +```json +{ + "skill_name": ["review-workflow", "review-verification"], + "evals": [ + { + "id": "review-change", + "prompt": "Review this change.", + "expected_output": "A prioritized review." + } + ] +} +``` -The copy is the working tree as it sits on disk, not a checkout of a commit — -evaluating an uncommitted revision is the ordinary case, and in a `--mode revision` run -the edit under test is uncommitted by definition. What the run measured is recorded rather than inferred, in -`conditions.json`, each `run.json`, `benchmark.json`, and the `BASELINE.md` written by +With a list, `--skill` selects the eval owner: the member whose `evals/` directory supplies the +definitions and fixtures, and whose name owns the workspace and promotion destination. The owner +must appear in the list. `--stage-name` is unavailable because one override cannot name several +staged skills. + +Every treatment member is copied into the eval home before any dispatch runs, and each condition +stages from those copies. Mode A stages all treatment members in `with_skill` and none in +`without_skill`. Other siblings from `--skill-dir` remain ambient in both arms. Mode B snapshots +and stages the complete set in both revisions. A scalar `skill_name` retains the existing +single-skill paths and artifacts. + +Each copy is the working tree as it sits on disk, not a checkout of a commit. Evaluating an +uncommitted revision is the ordinary case, and in a `--mode revision` run the edit under test is +uncommitted by definition. What the run measured is recorded rather than inferred in +`conditions.json`, each `run.json`, `benchmark.json`, and the `BASELINE.md` file written by `promote-baseline`: ```sh jq '.skill_source' conditions.json ``` -`dirty: true` means the recorded revision alone does not identify what ran. Commit the -skill before a run whose result you intend to publish. +`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. -Sibling skills staged by `--skill-dir` are copied the same way, and the roster is -captured once when the run resolves. The `siblings` field names exactly what every -environment received. +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 +`siblings` field, when present, names ambient skills staged in both arms. The eval home sits outside the skill's own repository: under `$XDG_DATA_HOME/eval-magic` (or `~/.local/share/eval-magic`), in a directory named for the skill directory it serves. @@ -174,10 +197,10 @@ The eval home sits outside the skill's own repository: under `$XDG_DATA_HOME/eva so there is nothing to remember. `EVAL_MAGIC_WORKSPACE_DIR` moves the default; `--workspace-dir` overrides both. -Copying does not remove the live directory from the machine, so a dispatch can still read -it by absolute path. `detect-stray-writes` reports that as a live-source read, and -`aggregate` carries it into `validity_warnings` for the same reason a discoverable -plugin copy is carried there: the arm may not be comparing what it claims to. +Copying does not remove the live directories from the machine, so a dispatch can still read one by +absolute path. `detect-stray-writes` checks every treatment source and reports that as a live-source +read. `aggregate` carries it into `validity_warnings` for the same reason a discoverable plugin +copy is carried there: the arm may not be comparing what it claims to. ## The task repository is a separate boundary diff --git a/docs/guides/judging.md b/docs/guides/judging.md index af045c2..8573925 100644 --- a/docs/guides/judging.md +++ b/docs/guides/judging.md @@ -73,7 +73,15 @@ An authored `llm_judge` assertion can request several independent verdicts for t Use `run --judge-samples N` to set a campaign-wide default. An assertion's `samples` field takes precedence over that default. The effective count must be at least one. The framework-injected -`__skill_invoked` meta-check is not substantive grading and remains single-shot. +`__skill_invoked` checks are not substantive grading and remain single-shot per treatment member. + +For a multi-skill treatment, deterministic transcript grading checks every staged slug separately. +The response files use `__skill_invoked__skill-N.json`, and each meta result names its +`skill_name`, so partial and complete invocation are distinguishable. Harness descriptors supply +the tool and argument signature; a harness without deterministic invocation events receives one +LLM fallback task per member. The suite-level `meta_summary.skill_invoked` value is true when any +treatment member was invoked. The `benchmark.json` file retains the suite rate and adds per-skill +counts and rates. A scalar treatment keeps the `__skill_invoked.json` filename and artifact shape. Each sample is a separate judge task and response, but every sample for a run receives the exact same bounded `judge-evidence.md`. The agent is not rerun, and eval-magic does not rebuild or expand diff --git a/schema/benchmark.schema.json b/schema/benchmark.schema.json index 90e5ecb..6f318ed 100644 --- a/schema/benchmark.schema.json +++ b/schema/benchmark.schema.json @@ -166,9 +166,36 @@ "type": "array", "items": { "type": "string" }, "description": "Sibling skills staged alongside the skill under test, as the roster was captured at resolution." + }, + "eval_owner": { + "type": "string", + "description": "CLI-selected skill that owns the eval definitions and workspace namespace. Present for multi-skill treatments." + }, + "skills": { + "type": "array", + "minItems": 1, + "items": { "$ref": "#/definitions/skillSourceEntry" }, + "description": "Ordered resolved source and revision for every multi-skill treatment member." } } }, + "skillSourceEntry": { + "type": "object", + "required": ["name", "kind", "source", "branch"], + "additionalProperties": false, + "properties": { + "name": { "type": "string" }, + "kind": { "type": "string", "enum": ["git", "path"] }, + "source": { "type": "string" }, + "resolved_path": { "type": "string" }, + "ref": { "type": "string" }, + "revision": { "type": "string" }, + "origin_url": { "type": "string" }, + "branch": { "type": "string" }, + "host_local": { "type": "boolean" }, + "dirty": { "type": "boolean" } + } + }, "assertionCount": { "type": "object", "required": ["passed", "n"], @@ -234,7 +261,20 @@ "duration_ms": { "$ref": "#/definitions/stats" }, "total_tokens": { "$ref": "#/definitions/stats" }, "skill_invocation_n": { "type": "integer" }, - "skill_invocation_rate": { "type": ["number", "null"] } + "skill_invocation_rate": { "type": ["number", "null"] }, + "skill_invocations": { + "type": "object", + "description": "Per-treatment-member invocation counts and rates for multi-skill evals.", + "additionalProperties": { + "type": "object", + "required": ["n", "rate"], + "additionalProperties": false, + "properties": { + "n": { "type": "integer", "minimum": 0 }, + "rate": { "type": "number", "minimum": 0, "maximum": 1 } + } + } + } } }, "diffScopeRun": { diff --git a/schema/evals.schema.json b/schema/evals.schema.json index 4759fe7..56eae83 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -8,8 +8,16 @@ "additionalProperties": false, "properties": { "skill_name": { - "type": "string", - "description": "Name of the skill being evaluated. Should match the skill directory name." + "description": "Name of the skill being evaluated, or an ordered non-empty set of coordinated skills. The CLI-selected eval owner must be a member of a set.", + "oneOf": [ + { "type": "string", "minLength": 1 }, + { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 } + } + ] }, "codebase": { "$ref": "#/definitions/codebase", diff --git a/schema/grading.schema.json b/schema/grading.schema.json index 34d785a..5ffd6e2 100644 --- a/schema/grading.schema.json +++ b/schema/grading.schema.json @@ -17,7 +17,7 @@ "meta_results": { "type": "array", "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/binaryAssertionResult" } + "items": { "$ref": "#/definitions/metaResult" } }, "meta_summary": { "type": "object", @@ -27,13 +27,29 @@ "failed": { "type": "integer", "minimum": 0 }, "total": { "type": "integer", "minimum": 0 }, "skill_invoked": { - "description": "True when the skill-invocation meta-check passed; false when the judge found no evidence the skill influenced behavior; null when no skill was loaded for this run.", + "description": "True when at least one treatment member's invocation meta-check passed; false when none did; null when no treatment skill was loaded for this run.", "type": ["boolean", "null"] } } } }, "definitions": { + "metaResult": { + "type": "object", + "required": ["id", "passed", "evidence"], + "additionalProperties": false, + "properties": { + "id": { "type": "string" }, + "skill_name": { "type": "string", "description": "Treatment member checked; absent for scalar legacy artifacts." }, + "passed": { "type": "boolean" }, + "evidence": { "type": "string" }, + "confidence": { "type": "number", "minimum": 0, "maximum": 1 }, + "grader": { + "type": "string", + "enum": ["transcript_check", "llm_judge", "command_check", "diff_scope"] + } + } + }, "assertionResult": { "oneOf": [ { "$ref": "#/definitions/binaryAssertionResult" }, diff --git a/schema/judge-tasks.schema.json b/schema/judge-tasks.schema.json index 2557604..0e2eefb 100644 --- a/schema/judge-tasks.schema.json +++ b/schema/judge-tasks.schema.json @@ -54,6 +54,10 @@ "description": "1-based run index within a multi-run (eval, condition) cell; absent for single-run cells." }, "assertion_id": { "type": "string" }, + "skill_name": { + "type": "string", + "description": "Treatment member checked by a multi-skill invocation meta task." + }, "sample_index": { "type": "integer", "minimum": 1, diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index a1819d9..441540c 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -27,6 +27,11 @@ "type": ["string", "null"], "description": "Absolute path to the SKILL.md the subagent could load, or null if no skill was provided (without_skill condition)." }, + "skills": { + "type": "array", + "description": "Ordered treatment roster for list-authored evals. Empty in the control arm and absent in scalar legacy records.", + "items": { "$ref": "#/definitions/conditionSkill" } + }, "prompt": { "type": "string", "description": "The user prompt as dispatched to the subagent." @@ -95,6 +100,16 @@ } }, "definitions": { + "conditionSkill": { + "type": "object", + "required": ["name", "skill_path", "staged_skill_slug"], + "additionalProperties": false, + "properties": { + "name": { "type": "string" }, + "skill_path": { "type": "string" }, + "staged_skill_slug": { "type": ["string", "null"] } + } + }, "responderOutcome": { "type": "object", "required": ["ending"], @@ -321,9 +336,36 @@ "type": "array", "items": { "type": "string" }, "description": "Sibling skills staged alongside the skill under test, as the roster was captured at resolution." + }, + "eval_owner": { + "type": "string", + "description": "CLI-selected skill that owns the eval definitions and workspace namespace. Present for multi-skill treatments." + }, + "skills": { + "type": "array", + "minItems": 1, + "items": { "$ref": "#/definitions/skillSourceEntry" }, + "description": "Ordered resolved source and revision for every multi-skill treatment member." } } }, + "skillSourceEntry": { + "type": "object", + "required": ["name", "kind", "source", "branch"], + "additionalProperties": false, + "properties": { + "name": { "type": "string" }, + "kind": { "type": "string", "enum": ["git", "path"] }, + "source": { "type": "string" }, + "resolved_path": { "type": "string" }, + "ref": { "type": "string" }, + "revision": { "type": "string" }, + "origin_url": { "type": "string" }, + "branch": { "type": "string" }, + "host_local": { "type": "boolean" }, + "dirty": { "type": "boolean" } + } + }, "conversationTool": { "type": "object", "required": ["type", "ordinal", "round", "name"], diff --git a/src/adapters/skill_shadow/grouping.rs b/src/adapters/skill_shadow/grouping.rs index 6ddc06c..fb77691 100644 --- a/src/adapters/skill_shadow/grouping.rs +++ b/src/adapters/skill_shadow/grouping.rs @@ -48,10 +48,10 @@ impl PluginShadowReport { subject_skill_name: &str, expected_cells: &[(String, String)], ) -> Self { - Self::from_observed_sources_with_class( + Self::from_observed_sources_for_subjects_with_class( config_dir, sources, - subject_skill_name, + &[subject_skill_name], expected_cells, ShadowFindingClass::OperatorEnvironment, ) @@ -63,6 +63,22 @@ impl PluginShadowReport { subject_skill_name: &str, expected_cells: &[(String, String)], class: ShadowFindingClass, + ) -> Self { + Self::from_observed_sources_for_subjects_with_class( + config_dir, + sources, + &[subject_skill_name], + expected_cells, + class, + ) + } + + pub(crate) fn from_observed_sources_for_subjects_with_class( + config_dir: impl Into, + sources: Vec, + subject_skill_names: &[&str], + expected_cells: &[(String, String)], + class: ShadowFindingClass, ) -> Self { let mut merged = Vec::::new(); for mut source in sources { @@ -88,7 +104,7 @@ impl PluginShadowReport { } for finding in &mut report.findings { finding.class = class; - finding.role = if finding.skill_name == subject_skill_name { + finding.role = if subject_skill_names.contains(&finding.skill_name.as_str()) { ShadowSkillRole::Subject } else { ShadowSkillRole::Sibling diff --git a/src/cli/args.rs b/src/cli/args.rs index 7918772..b877b02 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -62,7 +62,7 @@ pub struct CommonArgs { /// disagree. Omit it for the default single-skill isolated run. #[arg(long)] pub skill_dir: Option, - /// Skill under evaluation. + /// Eval owner and, for a multi-skill treatment, one member of the set. /// /// With `--skill-dir`, this is the child folder name, inferred when the /// directory contains exactly one skill. Without `--skill-dir`, this is a @@ -76,11 +76,10 @@ pub struct CommonArgs { /// Comparison mode: `new-skill` (default, with vs. without) or `revision` /// (old vs. new). /// - /// Mode A (`new-skill`) validates a brand-new skill against baseline behavior - /// with no skill loaded. Mode B (`revision`) tests a language change to an - /// existing skill: snapshot the old `SKILL.md` (see `snapshot`), then run both - /// variants against the same prompts. `revision` defaults `--baseline` to - /// `baseline`. + /// Mode A (`new-skill`) validates the skill or ordered skill set declared by + /// `skill_name` against baseline behavior with none of those treatment skills + /// loaded. Mode B (`revision`) snapshots and compares every treatment member. + /// `revision` defaults `--baseline` to `baseline`. #[arg(long)] pub mode: Option, /// Target harness: `claude-code` (default), `cline`, `codex`, or `opencode`. @@ -497,8 +496,10 @@ pub struct RunArgs { /// Stage the skill-under-test under this verbatim name instead of the /// conspicuous `slow-powers-eval-…` slug. /// - /// For name-confound experiments. Single-staging-condition modes only; refuses - /// to clobber an existing dir; registered for next-run cleanup. + /// For name-confound experiments. A scalar `skill_name` and one staging + /// condition are required; a multi-skill treatment is rejected because one + /// override cannot name every member. Refuses to clobber an existing dir and + /// registers the staged name for next-run cleanup. #[arg(long)] pub stage_name: Option, /// Inject the shared plan-mode profile as an operating-context layer. @@ -612,7 +613,7 @@ pub struct DispatchArgs { pub(crate) enum Commands { /// Build dispatches and run evals (the default action). /// - /// Builds the iteration workspace, snapshots the `SKILL.md`, stages skills, and + /// Builds the iteration workspace, copies and stages the treatment skill set, and /// emits `dispatch.json` (machine-readable) alongside `dispatch-manifest.md` /// (human-readable). It prepares the run but does not dispatch agents — /// `eval-magic dispatch` does. After setup, read `RUNBOOK.md` end to end; that @@ -671,9 +672,10 @@ pub(crate) enum Commands { Dispatch(DispatchArgs), /// Snapshot a workspace baseline. /// - /// Snapshots the skill as a Mode B baseline under - /// `//snapshots/", - ] - .join("\n") - } else if !staged_skills.is_empty() || is_truthy(opts.bootstrap_content) { - // Skill-absent arm in a realistic environment: stay silent. The - // available-skills block already omits the skill-under-test, so any - // commentary here would only announce the eval. - String::new() - } else { - "No skill is loaded. Respond as you naturally would.".to_string() - }; + let skill_block = render_skill_block( + opts, + skill_path.as_deref(), + staged_skill_path.as_deref(), + &staged_skills, + )?; - let fixtures_block = if opts.fixtures.is_empty() { - "Available fixture files: none".to_string() - } else { - format!( - "Available fixture files:\n{}", - opts.fixtures - .iter() - .map(|f| format!(" - {f}")) - .collect::>() - .join("\n") - ) - }; + let fixtures_block = render_fixtures_block(&opts.fixtures); // A condition that does not load the skill-under-test must carry zero // reference to it: the available-skills block auto-omits it, and a // user-supplied bootstrap that names it in prose is redacted here. - let skill_absent = skill_path.is_none() && opts.staged_skill_slug.is_none(); - let effective_bootstrap: Option = match opts.bootstrap_content { - Some(b) if !b.is_empty() => Some(if skill_absent { - redact_skill_from_bootstrap(b, opts.skill_name) - } else { - b.to_string() - }), - _ => None, - }; + let skill_absent = opts.skills.map_or_else( + || skill_path.is_none() && opts.staged_skill_slug.is_none(), + <[ConditionSkill]>::is_empty, + ); + let effective_bootstrap = effective_bootstrap(opts, skill_absent); let mut sections: Vec = Vec::new(); if let Some(boot) = &effective_bootstrap { @@ -292,6 +257,8 @@ pub fn build_dispatch_task(opts: &DispatchTaskOpts) -> Result::to_vec), + available_skills: opts.skills.map(|_| staged_skills), user_prompt: opts.user_prompt.to_string(), fixtures: opts.fixtures.clone(), run_record_path: artifact_path(&cond_dir.join("run.json")), diff --git a/src/cli/run/dispatch/prompt_components.rs b/src/cli/run/dispatch/prompt_components.rs new file mode 100644 index 0000000..4b49830 --- /dev/null +++ b/src/cli/run/dispatch/prompt_components.rs @@ -0,0 +1,130 @@ +use std::fs; +use std::path::Path; + +use crate::adapters::adapter_for; +use crate::core::AvailableSkill; + +use super::{DispatchTaskOpts, RunError, is_truthy, redact_skill_from_bootstrap}; + +pub(super) fn render_skill_block( + opts: &DispatchTaskOpts<'_>, + skill_path: Option<&str>, + staged_skill_path: Option<&str>, + staged_skills: &[AvailableSkill], +) -> Result { + if let Some(skills) = opts.skills { + if skills.is_empty() { + return Ok(String::new()); + } + if skills.iter().any(|skill| skill.staged_skill_slug.is_some()) { + let surface = adapter_for(opts.harness).skill_surface_phrase(); + return Ok(skills + .iter() + .map(|skill| { + let slug = skill + .staged_skill_slug + .as_deref() + .unwrap_or(skill.name.as_str()); + format!( + "The `{}` skill is registered under the identifier `{slug}` and is discoverable {surface}. If you invoke it, use that identifier.", + skill.name + ) + }) + .collect::>() + .join("\n")); + } + + let mut sections = vec![ + "The following skills are loaded into your operating guidelines. Apply them where relevant to the user's request." + .to_string(), + ]; + for skill in skills { + let content = fs::read_to_string(&skill.skill_path)?; + sections.push(format!( + "\n\n{}\n", + skill.name, + content.trim() + )); + } + return Ok(sections.join("\n")); + } + + if let Some(slug) = opts.staged_skill_slug { + // The identifier disambiguates the staged copy without implying that it + // is a globally installed plugin. + let adapter = adapter_for(opts.harness); + let surface = adapter.skill_surface_phrase(); + let mut lines = vec![format!( + "The `{}` skill is registered under the identifier `{slug}` and is discoverable {surface}. If you invoke it, use that identifier.", + opts.skill_name + )]; + if let Some(staged_path) = staged_skill_path { + let cannot_resolve = adapter.skill_unresolved_phrase(); + lines.push(format!( + "{cannot_resolve}, read the skill from `{staged_path}` instead." + )); + } + return Ok(lines.join("\n")); + } + + if let Some(skill_path) = skill_path { + let content = fs::read_to_string(skill_path)?; + let dir_name = Path::new(skill_path) + .parent() + .and_then(Path::file_name) + .map(|s| s.to_string_lossy().into_owned()) + .unwrap_or_default(); + return Ok([ + "The following skill is loaded into your operating guidelines. Apply it where relevant to the user's request.", + "", + &format!(""), + content.trim(), + "", + ] + .join("\n")); + } + + if !staged_skills.is_empty() || is_truthy(opts.bootstrap_content) { + // In a realistic skill-absent arm, the available-skills block already + // communicates the environment; extra commentary would announce the eval. + Ok(String::new()) + } else { + Ok("No skill is loaded. Respond as you naturally would.".to_string()) + } +} + +pub(super) fn render_fixtures_block(fixtures: &[String]) -> String { + if fixtures.is_empty() { + "Available fixture files: none".to_string() + } else { + format!( + "Available fixture files:\n{}", + fixtures + .iter() + .map(|fixture| format!(" - {fixture}")) + .collect::>() + .join("\n") + ) + } +} + +pub(super) fn effective_bootstrap( + opts: &DispatchTaskOpts<'_>, + skill_absent: bool, +) -> Option { + match opts.bootstrap_content { + Some(content) if !content.is_empty() => Some(if skill_absent { + opts.treatment_names.map_or_else( + || redact_skill_from_bootstrap(content, opts.skill_name), + |names| { + names.iter().fold(content.to_string(), |bootstrap, name| { + redact_skill_from_bootstrap(&bootstrap, name) + }) + }, + ) + } else { + content.to_string() + }), + _ => None, + } +} diff --git a/src/cli/run/orchestrate/build.rs b/src/cli/run/orchestrate/build.rs index 838f384..b7493c4 100644 --- a/src/cli/run/orchestrate/build.rs +++ b/src/cli/run/orchestrate/build.rs @@ -10,7 +10,7 @@ use std::path::Path; use serde_json::{Value, json}; use crate::adapters::adapter_for; -use crate::core::{AvailableSkill, ConditionEntry, ConditionsRecord, RunContext}; +use crate::core::{AvailableSkill, ConditionEntry, ConditionSkill, ConditionsRecord, RunContext}; use crate::pipeline::io::now_iso8601; use super::super::RunError; @@ -26,6 +26,10 @@ use super::{Resolved, RunOptions, Staged}; use crate::cli::command_target_args; use crate::core::fs::{artifact_path, write_json}; +mod roster; + +use roster::condition_roster; + /// Build every `(eval, condition)` dispatch task and write `conditions.json`, /// `dispatch-manifest.md`, the per-task prompt files, and `dispatch.json`. /// Returns the number of dispatch tasks. @@ -41,6 +45,15 @@ pub(super) fn write_dispatch( let condition_skill_path = |path: &Option| -> Option { path.as_deref().map(|p| artifact_path(Path::new(p))) }; + let multi_skill = r.skill.multi; + let treatment_names = r + .skill + .treatments + .iter() + .map(|skill| skill.name.clone()) + .collect::>(); + let cond_a_roster = condition_roster(&r.skill_paths_a, &staged.cond_a_skills); + let cond_b_roster = condition_roster(&r.skill_paths_b, &staged.cond_b_skills); let conditions = ConditionsRecord { mode: r.mode, baseline: r.baseline.clone(), @@ -49,11 +62,13 @@ pub(super) fn write_dispatch( name: r.cond_a.to_string(), skill_path: condition_skill_path(&r.skill_path_a), staged_skill_slug: Some(staged.cond_a_slug.clone()), + skills: multi_skill.then(|| cond_a_roster.clone()), }, ConditionEntry { name: r.cond_b.to_string(), skill_path: condition_skill_path(&r.skill_path_b), staged_skill_slug: Some(staged.cond_b_slug.clone()), + skills: multi_skill.then(|| cond_b_roster.clone()), }, ], timestamp: now_iso8601(), @@ -88,7 +103,8 @@ pub(super) fn write_dispatch( // skill-under-test when that condition loads it. Paths are task-env-specific. let available_skills_for = |env_root: &Path, cond_skill_path: Option<&str>, - cond_slug: Option<&str>| + cond_slug: Option<&str>, + roster: &[ConditionSkill]| -> Vec { if opts.no_stage { return Vec::new(); @@ -106,7 +122,25 @@ pub(super) fn write_dispatch( description: description.clone(), }) .collect(); - if let Some(csp) = cond_skill_path { + if multi_skill { + for treatment in roster { + let name = match treatment.staged_skill_slug.as_deref() { + Some(slug) if adapter_for(ctx.harness).advertises_staged_slug_name() => { + slug.to_string() + } + _ => treatment.name.clone(), + }; + skills.push(AvailableSkill { + name, + path: treatment + .staged_skill_slug + .as_deref() + .and_then(|slug| staged_skill_path_for(env_root, Some(slug))) + .unwrap_or_else(|| treatment.skill_path.clone()), + description: get_skill_description(Path::new(&treatment.skill_path)), + }); + } + } else if let Some(csp) = cond_skill_path { let name = match cond_slug { Some(slug) if adapter_for(ctx.harness).advertises_staged_slug_name() => { slug.to_string() @@ -141,16 +175,18 @@ pub(super) fn write_dispatch( let mut tasks = Vec::new(); // Build tasks CONDITION-outer, GROUP-inner. A single group collapses this to // the legacy condition-outer order. - for (cond_name, cond_skill_path, cond_slug) in [ + for (cond_name, cond_skill_path, cond_slug, condition_roster) in [ ( r.cond_a, r.skill_path_a.as_deref(), staged.cond_a_slug.as_deref(), + cond_a_roster.as_slice(), ), ( r.cond_b, r.skill_path_b.as_deref(), staged.cond_b_slug.as_deref(), + cond_b_roster.as_slice(), ), ] { for group in &r.groups { @@ -189,8 +225,12 @@ pub(super) fn write_dispatch( ); let env_root_str = env_root.to_string_lossy().into_owned(); let staged_path = staged_skill_path_for(&env_root, cond_slug); - let available_skills = - available_skills_for(&env_root, cond_skill_path, cond_slug); + let available_skills = available_skills_for( + &env_root, + cond_skill_path, + cond_slug, + condition_roster, + ); // Create the per-run meta dir (run.json / timing.json), which // lives above the env. fs::create_dir_all(&run_dir)?; @@ -218,6 +258,8 @@ pub(super) fn write_dispatch( skill_path: cond_skill_path, staged_skill_slug: cond_slug, staged_skill_path: staged_path.as_deref(), + skills: multi_skill.then_some(condition_roster), + treatment_names: multi_skill.then_some(treatment_names.as_slice()), user_prompt: &ev.prompt, fixtures, turns: ev.turns.as_deref(), @@ -276,7 +318,11 @@ pub(super) fn write_dispatch( let dispatch_json_path = r.iteration_dir.join("dispatch.json"); let mut dispatch_json = json!({ - "skill_name": ctx.skill_name, + "skill_name": if multi_skill { + json!(r.skill.treatments.iter().map(|skill| &skill.name).collect::>()) + } else { + json!(ctx.skill_name) + }, "iteration": r.iteration, "run_nonce": r.run_nonce, "iteration_dir": artifact_path(&r.iteration_dir), diff --git a/src/cli/run/orchestrate/build/roster.rs b/src/cli/run/orchestrate/build/roster.rs new file mode 100644 index 0000000..c5bd7cb --- /dev/null +++ b/src/cli/run/orchestrate/build/roster.rs @@ -0,0 +1,23 @@ +use std::path::Path; + +use crate::core::ConditionSkill; +use crate::core::fs::artifact_path; + +use super::super::StagedTreatmentSkill; + +pub(super) fn condition_roster( + paths: &[(String, String)], + staged_skills: &[StagedTreatmentSkill], +) -> Vec { + paths + .iter() + .map(|(name, path)| ConditionSkill { + name: name.clone(), + skill_path: artifact_path(Path::new(path)), + staged_skill_slug: staged_skills + .iter() + .find(|skill| &skill.name == name) + .and_then(|skill| skill.slug.clone()), + }) + .collect() +} diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index 6cc0eae..45d30fc 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -20,7 +20,7 @@ use crate::cli::command_target_args; use crate::core::fs::artifact_path; use crate::core::{ Assertion, CodebaseRecord, CodebaseSource, CodebaseUse, Eval, GuardPolicyConfig, Mode, - RunContext, SkillSource, SourceKind, SourceRecord, + RunContext, SourceKind, SourceRecord, }; use crate::source::ResolvedSource; @@ -34,8 +34,11 @@ mod git; mod resolve; mod shadow_preflight; mod shell; +mod skill; mod stage; +use skill::{RunSkill, TreatmentSkill}; + /// Run options parsed from the `run` subcommand flags (everything beyond the /// shared skill/workspace/harness context, which lives in [`RunContext`]). #[derive(Debug, Clone, Default)] @@ -95,6 +98,8 @@ struct Resolved { cond_b: &'static str, skill_path_a: Option, skill_path_b: Option, + skill_paths_a: Vec<(String, String)>, + skill_paths_b: Vec<(String, String)>, selected_evals: Vec, total_evals: usize, /// Task-scoped groups computed from the selected evals in config order. @@ -121,40 +126,6 @@ pub(super) fn skills_copy_root(iteration_dir: &Path) -> PathBuf { iteration_dir.join(".skills") } -/// The resolved skill under test and the sibling roster staged with it. -struct RunSkill { - source: ResolvedSource, - /// Captured at resolution. Staging copies exactly these names, so what the - /// record claims and what the environments hold cannot drift apart. - siblings: Vec, -} - -impl RunSkill { - fn record(&self) -> SkillSource { - SkillSource { - source: SourceRecord { - // A skill is named by a path on this host; there is no url form. - kind: SourceKind::Path, - source: self.source.source.clone(), - resolved_path: self - .source - .resolved_path - .as_deref() - .map(|path| artifact_path(Path::new(path))), - reference: self.source.reference.clone(), - revision: self.source.revision.clone(), - origin_url: self.source.origin_url.clone(), - branch: self.source.branch.clone(), - host_local: self.source.host_local, - // The copy is the working tree as it sits, so an uncommitted edit - // is in what ran and the revision alone does not name it. - dirty: self.source.dirty, - }, - siblings: self.siblings.clone(), - } - } -} - impl RunCodebase { /// The artifact form, shared by every provenance surface so a reader never /// has to reconcile two spellings of the same resolution. @@ -227,6 +198,8 @@ impl Resolved { struct Staged { cond_a_slug: Option, cond_b_slug: Option, + cond_a_skills: Vec, + cond_b_skills: Vec, /// Sibling skills' `(name, description)` — env-independent. `build` resolves /// the on-disk path for each private task environment. sibling_meta: Vec<(String, String)>, @@ -238,6 +211,12 @@ struct Staged { codebase_shadow_sources: std::collections::HashMap>, } +#[derive(Clone)] +struct StagedTreatmentSkill { + name: String, + slug: Option, +} + /// Build the iteration workspace and dispatch plan for a run. pub fn command_run(ctx: &RunContext, opts: &RunOptions) -> Result<(), RunError> { // Git is a hard runtime dependency for task-repository isolation. Probe it @@ -314,28 +293,40 @@ fn print_run_plan(ctx: &RunContext, opts: &RunOptions, r: &Resolved) { r.iteration, mode_str(r.mode) ); - println!( - " {}: {}", - r.cond_a, - r.skill_path_a.as_deref().unwrap_or("(no skill)") - ); - println!( - " {}: {}", - r.cond_b, - r.skill_path_b.as_deref().unwrap_or("(no skill)") - ); + let render_paths = |paths: &[(String, String)]| { + if paths.is_empty() { + "(no skill)".to_string() + } else if !r.skill.multi { + paths[0].1.clone() + } else { + paths + .iter() + .map(|(name, path)| format!("{name}: {path}")) + .collect::>() + .join(", ") + } + }; + println!(" {}: {}", r.cond_a, render_paths(&r.skill_paths_a)); + println!(" {}: {}", r.cond_b, render_paths(&r.skill_paths_b)); // The conditions above name the copy; this names where the copy came from, // which is what a reader of the report has to be able to find again. - let source = &r.skill.source; - let revision = match (source.revision.as_deref(), source.dirty) { - (Some(sha), true) => format!(" ({}, uncommitted changes)", &sha[..7.min(sha.len())]), - (Some(sha), false) => format!(" ({})", &sha[..7.min(sha.len())]), - (None, _) => String::new(), - }; - println!( - " skill source: {}{revision}", - source.resolved_path.as_deref().unwrap_or(&source.source) - ); + for treatment in &r.skill.treatments { + let source = &treatment.source; + let revision = match (source.revision.as_deref(), source.dirty) { + (Some(sha), true) => format!(" ({}, uncommitted changes)", &sha[..7.min(sha.len())]), + (Some(sha), false) => format!(" ({})", &sha[..7.min(sha.len())]), + (None, _) => String::new(), + }; + println!( + " skill source{}: {}{revision}", + if !r.skill.multi { + String::new() + } else { + format!(" ({})", treatment.name) + }, + source.resolved_path.as_deref().unwrap_or(&source.source) + ); + } // The codebases the environments are built from, in the same shape as the // skill source line — and the one-checkout-per-iteration fact the caching // makes true. diff --git a/src/cli/run/orchestrate/resolve.rs b/src/cli/run/orchestrate/resolve.rs index 21e4ccc..4272da9 100644 --- a/src/cli/run/orchestrate/resolve.rs +++ b/src/cli/run/orchestrate/resolve.rs @@ -2,6 +2,7 @@ //! per-condition skill paths, before any directory is created. use std::fs; +use std::path::{Component, Path}; use serde_json::Value; @@ -85,36 +86,6 @@ pub(super) fn resolve_request(ctx: &RunContext, opts: &RunOptions) -> Result Result Result, Option) = match mode { - Mode::NewSkill => (Some(copied_skill_md.clone()), None), - Mode::Revision => { - let baseline = baseline.as_deref().expect("revision baseline set above"); - let baseline_skill = workspace_skill_dir - .join("snapshots") - .join(baseline) - .join("SKILL.md"); - if !baseline_skill.exists() { - let target_args = command_target_args(ctx); - return Err(RunError::msg(format!( - "baseline snapshot not found: {}\n Run: eval-magic snapshot{target_args} --label {} (before editing)", - baseline_skill.display(), - baseline - ))); - } + let copied_skill_paths = skill + .treatments + .iter() + .map(|treatment| { ( - Some(baseline_skill.to_string_lossy().into_owned()), - Some(copied_skill_md.clone()), + treatment.name.clone(), + skills_copy_root(&iteration_dir) + .join(&treatment.name) + .join("SKILL.md") + .to_string_lossy() + .into_owned(), ) + }) + .collect::>(); + let copied_owner_skill_md = copied_skill_paths + .iter() + .find(|(name, _)| name == &ctx.skill_name) + .map(|(_, path)| path.clone()) + .expect("the eval owner belongs to the treatment"); + let (skill_paths_a, skill_paths_b) = match mode { + Mode::NewSkill => (copied_skill_paths.clone(), Vec::new()), + Mode::Revision => { + let baseline = baseline.as_deref().expect("revision baseline set above"); + let baseline_root = workspace_skill_dir.join("snapshots").join(baseline); + let baseline_paths = skill + .treatments + .iter() + .map(|treatment| { + let path = if skill.multi { + baseline_root + .join("skills") + .join(&treatment.name) + .join("SKILL.md") + } else { + baseline_root.join("SKILL.md") + }; + if !path.exists() { + let target_args = command_target_args(ctx); + return Err(RunError::msg(format!( + "baseline snapshot not found: {}\n Run: eval-magic snapshot{target_args} --label {} (before editing)", + path.display(), + baseline + ))); + } + Ok((treatment.name.clone(), path.to_string_lossy().into_owned())) + }) + .collect::, RunError>>()?; + (baseline_paths, copied_skill_paths.clone()) } }; + let owner_path = |paths: &[(String, String)]| { + paths + .iter() + .find(|(name, _)| name == &ctx.skill_name) + .map(|(_, path)| path.clone()) + }; + let skill_path_a = owner_path(&skill_paths_a); + let skill_path_b = owner_path(&skill_paths_b); + if mode == Mode::NewSkill { + debug_assert_eq!( + skill_path_a.as_deref(), + Some(copied_owner_skill_md.as_str()) + ); + } // The mirror image of the codebase warning: a skill is copied as it sits, so // uncommitted work is in what ran, and the recorded revision alone does not // name it. - if skill.source.dirty { + for treatment in skill.treatments.iter().filter(|skill| skill.source.dirty) { eprintln!( "⚠ skill '{}' has uncommitted changes; the run measures them, so its recorded \ revision alone does not identify what was evaluated", - ctx.skill_name + treatment.name ); } @@ -256,6 +334,8 @@ pub(super) fn resolve_request(ctx: &RunContext, opts: &RunOptions) -> Result Result<(), RunError> { - let mut names: Vec<&str> = vec![ctx.skill_name.as_str()]; - names.extend(ctx.sibling_skill_names.iter().map(String::as_str)); + let mut names = r + .skill + .treatments + .iter() + .map(|skill| skill.name.as_str()) + .collect::>(); + names.extend(r.skill.siblings.iter().map(String::as_str)); let adapter = adapter_for(ctx.harness); let expected_cells = targets .iter() @@ -210,19 +215,45 @@ pub(super) fn run( codebase_scans, &codebase_shadowed_names, ); - let operator_report = PluginShadowReport::from_observed_sources( - config_dir.clone().unwrap_or_default(), - operator_sources, - &ctx.skill_name, - &expected_cells, - ); - let codebase_report = PluginShadowReport::from_observed_sources_with_class( - config_dir.clone().unwrap_or_default(), - codebase_sources, - &ctx.skill_name, - &expected_cells, - ShadowFindingClass::CodebaseSourced, - ); + let subject_names = r + .skill + .treatments + .iter() + .map(|skill| skill.name.as_str()) + .collect::>(); + let operator_report = if subject_names.len() == 1 { + PluginShadowReport::from_observed_sources( + config_dir.clone().unwrap_or_default(), + operator_sources, + subject_names[0], + &expected_cells, + ) + } else { + PluginShadowReport::from_observed_sources_for_subjects_with_class( + config_dir.clone().unwrap_or_default(), + operator_sources, + &subject_names, + &expected_cells, + ShadowFindingClass::OperatorEnvironment, + ) + }; + let codebase_report = if subject_names.len() == 1 { + PluginShadowReport::from_observed_sources_with_class( + config_dir.clone().unwrap_or_default(), + codebase_sources, + subject_names[0], + &expected_cells, + ShadowFindingClass::CodebaseSourced, + ) + } else { + PluginShadowReport::from_observed_sources_for_subjects_with_class( + config_dir.clone().unwrap_or_default(), + codebase_sources, + &subject_names, + &expected_cells, + ShadowFindingClass::CodebaseSourced, + ) + }; let mut findings = operator_report.findings.clone(); findings.extend(codebase_report.findings.clone()); findings.sort_by(|a, b| { @@ -283,23 +314,24 @@ fn collect_observed_sources( continue; }; if !opts.no_stage { - let (condition, condition_skill_path) = &target.conditions[0]; - let condition_slug = condition_slug(r, staged, condition); - if condition_skill_path.is_some() - && shadowed_names.contains(&ctx.skill_name) - && let Some(slug) = condition_slug - { - let mut source = ShadowSource::staged( - &ctx.skill_name, - slug, - &skills_dir.join(slug), - ShadowRoot::staged(&skills_dir), - ); - source.add_appearance(appearance.clone()); - sources.push(source); + let (condition, _condition_skill_path) = &target.conditions[0]; + for skill in condition_skills(r, staged, condition) { + if !shadowed_names.contains(&skill.name) { + continue; + } + if let Some(slug) = &skill.slug { + let mut source = ShadowSource::staged( + &skill.name, + slug, + &skills_dir.join(slug), + ShadowRoot::staged(&skills_dir), + ); + source.add_appearance(appearance.clone()); + sources.push(source); + } } if ctx.stage_siblings { - for sibling in &ctx.sibling_skill_names { + for sibling in &r.skill.siblings { if !shadowed_names.contains(sibling) { continue; } @@ -320,13 +352,17 @@ fn collect_observed_sources( observed } -fn condition_slug<'a>(r: &Resolved, staged: &'a Staged, condition: &str) -> Option<&'a str> { +fn condition_skills<'a>( + r: &Resolved, + staged: &'a Staged, + condition: &str, +) -> &'a [super::StagedTreatmentSkill] { if condition == r.cond_a { - staged.cond_a_slug.as_deref() + &staged.cond_a_skills } else if condition == r.cond_b { - staged.cond_b_slug.as_deref() + &staged.cond_b_skills } else { - None + &[] } } @@ -348,15 +384,16 @@ fn omit_sources_displaced_by_staging( return; }; let mut displaced = BTreeSet::new(); - let (condition, condition_skill_path) = &target.conditions[0]; - if condition_skill_path.is_some() - && let Some(slug) = condition_slug(r, staged, condition) - { - displaced.insert(artifact_path(&skills_dir.join(slug))); + let (condition, _condition_skill_path) = &target.conditions[0]; + for skill in condition_skills(r, staged, condition) { + if let Some(slug) = &skill.slug { + displaced.insert(artifact_path(&skills_dir.join(slug))); + } } if ctx.stage_siblings { displaced.extend( - ctx.sibling_skill_names + r.skill + .siblings .iter() .map(|name| artifact_path(&skills_dir.join(name))), ); diff --git a/src/cli/run/orchestrate/skill.rs b/src/cli/run/orchestrate/skill.rs new file mode 100644 index 0000000..20a6a5f --- /dev/null +++ b/src/cli/run/orchestrate/skill.rs @@ -0,0 +1,59 @@ +use std::path::Path; + +use crate::core::fs::artifact_path; +use crate::core::{SkillSource, SkillSourceEntry, SourceKind, SourceRecord}; +use crate::source::ResolvedSource; + +/// The resolved treatment and ambient roster staged with it. +pub(super) struct RunSkill { + pub(super) eval_owner: String, + /// Whether `evals.json` authored the treatment as a list. This is distinct + /// from roster length so a one-member list still uses the list artifact form. + pub(super) multi: bool, + pub(super) source: ResolvedSource, + pub(super) treatments: Vec, + /// Captured at resolution. Staging copies exactly these names, so what the + /// record claims and what the environments hold cannot drift apart. + pub(super) siblings: Vec, +} + +pub(super) struct TreatmentSkill { + pub(super) name: String, + pub(super) source: ResolvedSource, +} + +impl RunSkill { + pub(super) fn record(&self) -> SkillSource { + SkillSource { + source: skill_source_record(&self.source), + siblings: self.siblings.clone(), + eval_owner: self.multi.then(|| self.eval_owner.clone()), + skills: self.multi.then(|| { + self.treatments + .iter() + .map(|skill| SkillSourceEntry { + name: skill.name.clone(), + source: skill_source_record(&skill.source), + }) + .collect() + }), + } + } +} + +fn skill_source_record(source: &ResolvedSource) -> SourceRecord { + SourceRecord { + kind: SourceKind::Path, + source: source.source.clone(), + resolved_path: source + .resolved_path + .as_deref() + .map(|path| artifact_path(Path::new(path))), + reference: source.reference.clone(), + revision: source.revision.clone(), + origin_url: source.origin_url.clone(), + branch: source.branch.clone(), + host_local: source.host_local, + dirty: source.dirty, + } +} diff --git a/src/cli/run/orchestrate/stage.rs b/src/cli/run/orchestrate/stage.rs index b9be06c..c9e8c18 100644 --- a/src/cli/run/orchestrate/stage.rs +++ b/src/cli/run/orchestrate/stage.rs @@ -17,7 +17,7 @@ use super::super::fixtures::{FixtureClaims, copy_fixtures}; use super::super::staging::{ StageSiblingOpts, StageSkillOpts, cleanup_staged_skills, exclude_codebase_skill_sources, register_staged_skill_for_cleanup, skills_dir_for_harness, stage_sibling_skills, - stage_skill_for_harness, + stage_sibling_skills_excluding, stage_skill_for_harness, }; use super::super::util::{harness_label, resolve_plan_mode_profile}; use super::envs::{EnvLayoutInput, env_targets}; @@ -33,10 +33,12 @@ pub(super) fn stage_conditions( // even under `--no-stage`, where the dispatch prompt inlines the skill body // by reading the same path. let skills = materialize_skills(ctx, r)?; - fs::copy( - skills.join(&ctx.skill_name).join("SKILL.md"), - r.iteration_dir.join("skill-snapshot.md"), - )?; + if !r.skill.multi { + fs::copy( + skills.join(&ctx.skill_name).join("SKILL.md"), + r.iteration_dir.join("skill-snapshot.md"), + )?; + } let bootstrap_content = match &ctx.bootstrap_path { Some(path) => Some(fs::read_to_string(path)?), @@ -71,6 +73,12 @@ pub(super) fn stage_conditions( .collect() }; + if opts.stage_name.is_some() && r.skill.multi { + return Err(RunError::msg( + "--stage-name is only supported for a single skill under test", + )); + } + // --stage-name overrides the conspicuous slug with a verbatim name; it targets // the single staging condition, so reject the both-stage case up front. if let Some(_stage_name) = opts.stage_name @@ -96,13 +104,19 @@ pub(super) fn stage_conditions( let mut cond_a_slug = None; let mut cond_b_slug = None; + let mut cond_a_skills = Vec::new(); + let mut cond_b_skills = Vec::new(); // Distinct codebases materialized so far this iteration, by key. Every // environment sharing a codebase is provisioned from one materialization. let mut materialized: HashMap = HashMap::new(); let mut guard_policies = HashMap::new(); let mut codebase_shadow_sources = HashMap::new(); - let evaluated_names = std::iter::once(ctx.skill_name.as_str()) - .chain(ctx.sibling_skill_names.iter().map(String::as_str)) + let evaluated_names = r + .skill + .treatments + .iter() + .map(|skill| skill.name.as_str()) + .chain(r.skill.siblings.iter().map(String::as_str)) .collect::>(); for target in &targets { @@ -152,19 +166,35 @@ pub(super) fn stage_conditions( } if !opts.no_stage && ctx.stage_siblings { - stage_sibling_skills(&StageSiblingOpts { + let treatment_names = r + .skill + .treatments + .iter() + .map(|skill| skill.name.clone()) + .collect::>(); + let sibling_opts = StageSiblingOpts { skill_under_test: &ctx.skill_name, skills_source_dir: &skills, repo_root: &target.root, harness: ctx.harness, - })?; + }; + if treatment_names.len() == 1 { + stage_sibling_skills(&sibling_opts)?; + } else { + stage_sibling_skills_excluding(&sibling_opts, &treatment_names)?; + } } - for (cond_name, cond_skill_path) in &target.conditions { + for (cond_name, _cond_skill_path) in &target.conditions { + let condition_skills = if *cond_name == r.cond_a { + &r.skill_paths_a + } else { + &r.skill_paths_b + }; // Refuse to clobber a pre-existing --stage-name dir in this env. if let Some(stage_name) = opts.stage_name && !opts.no_stage - && cond_skill_path.is_some() + && !condition_skills.is_empty() { let dir = skills_dir_for_harness(&target.root, ctx.harness).join(stage_name); if dir.exists() { @@ -175,25 +205,39 @@ pub(super) fn stage_conditions( } } - if let Some(slug) = stage_for( - ctx, - opts, - r, - cond_name, - cond_skill_path.as_deref(), - &target.root, - )? { - if *cond_name == r.cond_a { - cond_a_slug = Some(slug.clone()); - } - if *cond_name == r.cond_b { - cond_b_slug = Some(slug.clone()); - } - // A custom-named dir isn't caught by the prefix scan; record it in - // this env's manifest so cleanup removes it. - if opts.stage_name == Some(slug.as_str()) { - register_staged_skill_for_cleanup(&target.root, &slug, ctx.harness)?; + let mut staged_condition = Vec::new(); + for (skill_name, skill_path) in condition_skills { + let slug = stage_for( + ctx, + opts, + r, + cond_name, + skill_name, + Some(skill_path), + &target.root, + )?; + if let Some(slug) = &slug + && opts.stage_name == Some(slug.as_str()) + { + register_staged_skill_for_cleanup(&target.root, slug, ctx.harness)?; } + staged_condition.push(super::StagedTreatmentSkill { + name: skill_name.clone(), + slug, + }); + } + if *cond_name == r.cond_a { + cond_a_skills = staged_condition; + cond_a_slug = cond_a_skills + .iter() + .find(|skill| skill.name == ctx.skill_name) + .and_then(|skill| skill.slug.clone()); + } else { + cond_b_skills = staged_condition; + cond_b_slug = cond_b_skills + .iter() + .find(|skill| skill.name == ctx.skill_name) + .and_then(|skill| skill.slug.clone()); } } @@ -229,6 +273,8 @@ pub(super) fn stage_conditions( Ok(Staged { cond_a_slug, cond_b_slug, + cond_a_skills, + cond_b_skills, sibling_meta, bootstrap_content, plan_mode_content, @@ -250,7 +296,13 @@ fn materialize_skills(ctx: &RunContext, r: &Resolved) -> Result, root: &Path, ) -> Result, RunError> { @@ -324,7 +377,7 @@ fn stage_for( content: &content, iteration: r.iteration, condition: cond_name, - skill_name: &ctx.skill_name, + skill_name, repo_root: root, assets_dir: Path::new(path).parent(), stage_name_override: opts.stage_name, diff --git a/src/cli/run/staging/mod.rs b/src/cli/run/staging/mod.rs index f518e22..bbd82ce 100644 --- a/src/cli/run/staging/mod.rs +++ b/src/cli/run/staging/mod.rs @@ -250,8 +250,18 @@ pub fn register_staged_skill_for_cleanup( /// its `evals/`) into the harness skills dir, backing up any colliding /// pre-existing entry, and write the manifest. pub fn stage_sibling_skills(opts: &StageSiblingOpts) -> Result { + stage_sibling_skills_excluding(opts, &[opts.skill_under_test.to_string()]) +} + +/// Stage ambient skills while excluding a complete coordinated treatment set. +/// The scalar wrapper above preserves the established public helper contract. +pub fn stage_sibling_skills_excluding( + opts: &StageSiblingOpts, + skills_under_test: &[String], +) -> Result { let skills_dir = skills_dir_for_harness(opts.repo_root, opts.harness); - let mut manifest = load_or_create_manifest(&skills_dir, opts.skill_under_test)?; + let staged_label = skills_under_test.join(","); + let mut manifest = load_or_create_manifest(&skills_dir, &staged_label)?; fs::create_dir_all(&skills_dir)?; write_json(&skills_dir.join(STAGED_SIBLING_MANIFEST), &manifest)?; @@ -259,7 +269,7 @@ pub fn stage_sibling_skills(opts: &StageSiblingOpts) -> Result, } +/// A framework-injected binary result. Multi-skill invocation checks name the +/// treatment member; scalar artifacts omit the field and retain their legacy +/// shape. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct MetaResult { + pub id: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skill_name: Option, + pub passed: bool, + pub evidence: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub confidence: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub grader: Option, +} + /// One verdict inside a multi-sample LLM assertion result. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct JudgeSampleResult { @@ -111,7 +127,7 @@ pub struct GradingResult { // grading.json reads as "the verdict, then the validity check on it". pub summary: GradingSummary, #[serde(skip_serializing_if = "Option::is_none")] - pub meta_results: Option>, + pub meta_results: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub meta_summary: Option, } diff --git a/src/core/types.rs b/src/core/types.rs index eb62d42..d503bec 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -300,6 +300,20 @@ pub struct SkillSource { pub source: SourceRecord, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub siblings: Vec, + /// Eval owner and complete treatment provenance for the multi-skill form. + /// Absent for scalar legacy records, whose flattened source remains the + /// authoritative single-skill record. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub eval_owner: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skills: Option>, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SkillSourceEntry { + pub name: String, + #[serde(flatten)] + pub source: SourceRecord, } /// One resolved codebase plus the evals built from it. `conditions.json` and @@ -320,10 +334,50 @@ pub struct CodebaseUse { pub evals: Vec, } -/// The parsed `evals.json` for one skill. +/// One skill name or an ordered set of coordinated skills under test. +/// +/// The scalar form remains the wire representation for existing evals. The +/// list form is deliberately ordered: artifacts, prompts, and per-skill grading +/// use the authored order so readers can join those surfaces without sorting. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum SkillNames { + One(String), + Many(Vec), +} + +impl SkillNames { + pub fn as_slice(&self) -> &[String] { + match self { + Self::One(name) => std::slice::from_ref(name), + Self::Many(names) => names, + } + } + + pub fn is_multi(&self) -> bool { + matches!(self, Self::Many(_)) + } +} + +impl std::fmt::Display for SkillNames { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::One(name) => formatter.write_str(name), + Self::Many(names) => formatter.write_str(&names.join(", ")), + } + } +} + +impl PartialEq<&str> for SkillNames { + fn eq(&self, other: &&str) -> bool { + matches!(self, Self::One(name) if name == other) + } +} + +/// The parsed `evals.json` for one skill or coordinated skill set. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct EvalsConfig { - pub skill_name: String, + pub skill_name: SkillNames, /// Default codebase for every eval in this config; a per-eval `codebase` /// overrides it. Mirrors how `runs` defaults and is overridden. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -335,6 +389,10 @@ pub struct EvalsConfig { } impl EvalsConfig { + pub fn skill_names(&self) -> &[String] { + self.skill_name.as_slice() + } + /// Return the authored policy effective for `eval`. A per-eval block is a /// complete replacement, including when it is empty. pub fn guard_for<'a>(&'a self, eval: &'a Eval) -> Option<&'a GuardPolicyConfig> { @@ -351,6 +409,14 @@ pub struct AvailableSkill { pub description: String, } +/// One member of the treatment roster in a condition, dispatch task, or run. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ConditionSkill { + pub name: String, + pub skill_path: String, + pub staged_skill_slug: Option, +} + /// One condition in a comparison run. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ConditionEntry { @@ -363,6 +429,10 @@ pub struct ConditionEntry { deserialize_with = "deserialize_present_key" )] pub staged_skill_slug: Option>, + /// Present for list-authored evals, including an empty list in the control + /// arm. Absent for scalar artifacts so their established shape is stable. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skills: Option>, } /// Tri-state field deserializer: a present key — even an explicit `null` — @@ -458,6 +528,8 @@ pub struct RunRecord { pub eval_id: String, pub condition: String, pub skill_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skills: Option>, pub prompt: String, pub files: Vec, pub final_message: String, @@ -776,6 +848,7 @@ mod tests { eval_id: "e".into(), condition: "with-skill".into(), skill_path: None, + skills: None, prompt: "p".into(), files: vec![], final_message: "done".into(), @@ -815,6 +888,7 @@ mod tests { name: "c".into(), skill_path: Some("/p".into()), staged_skill_slug: slug, + skills: None, }; // Absent → key omitted. let absent = serde_json::to_value(base(None)).unwrap(); diff --git a/src/pipeline/aggregate.rs b/src/pipeline/aggregate.rs index 561e94f..244dcfd 100644 --- a/src/pipeline/aggregate.rs +++ b/src/pipeline/aggregate.rs @@ -11,7 +11,7 @@ mod assertions; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; use std::path::Path; @@ -92,6 +92,16 @@ struct ConditionSummary { /// Present (possibly `null`) only when the skill was loaded. #[serde(skip_serializing_if = "Option::is_none")] skill_invocation_rate: Option>, + /// Per-treatment-member invocation rollup. Present only for multi-skill + /// artifacts; the suite-level fields above retain their established shape. + #[serde(skip_serializing_if = "Option::is_none")] + skill_invocations: Option>, +} + +#[derive(Debug, Clone, Serialize)] +struct SkillInvocationSummary { + n: usize, + rate: f64, } /// The `a - b` differences between the two compared conditions. @@ -160,6 +170,7 @@ struct Bucket { durations: Vec, tokens: Vec, skill_invoked: Vec, + skill_invoked_by_skill: HashMap>, had_skill_loaded: bool, } @@ -217,7 +228,10 @@ pub fn aggregate( by_condition.insert( c.name.clone(), Bucket { - had_skill_loaded: c.skill_path.is_some(), + had_skill_loaded: c + .skills + .as_ref() + .map_or_else(|| c.skill_path.is_some(), |skills| !skills.is_empty()), ..Bucket::default() }, ); @@ -308,6 +322,17 @@ pub fn aggregate( { bucket.skill_invoked.push(invoked); } + if let Some(results) = &grading.meta_results { + for result in results { + if let Some(skill_name) = &result.skill_name { + bucket + .skill_invoked_by_skill + .entry(skill_name.clone()) + .or_default() + .push(result.passed); + } + } + } if timing_path.exists() { let timing: TimingRecord = @@ -353,6 +378,22 @@ pub fn aggregate( total_tokens: stats(&bucket.tokens, 0), skill_invocation_n, skill_invocation_rate, + skill_invocations: (!bucket.skill_invoked_by_skill.is_empty()).then(|| { + bucket + .skill_invoked_by_skill + .iter() + .map(|(name, results)| { + let passed = results.iter().filter(|&&invoked| invoked).count(); + ( + name.clone(), + SkillInvocationSummary { + n: results.len(), + rate: round(passed as f64 / results.len() as f64, 3), + }, + ) + }) + .collect() + }), }; run_summary.insert(cond.clone(), serde_json::to_value(&summary)?); summaries.insert(cond.clone(), summary); diff --git a/src/pipeline/detect_stray_writes.rs b/src/pipeline/detect_stray_writes.rs index 68fe1b6..67e7939 100644 --- a/src/pipeline/detect_stray_writes.rs +++ b/src/pipeline/detect_stray_writes.rs @@ -15,7 +15,7 @@ //! into the separate schema-gated `guard-denials.json` artifact, even when a //! task has no `run.json`. -use std::path::Path; +use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -270,6 +270,26 @@ pub fn detect_stray_writes_report( } let conditions: ConditionsRecord = serde_json::from_str(&std::fs::read_to_string(&conditions_path)?)?; + let live_skill_dirs = conditions + .skill_source + .as_ref() + .and_then(|source| source.skills.as_ref()) + .map(|skills| { + skills + .iter() + .map(|skill| { + PathBuf::from( + skill + .source + .resolved_path + .as_deref() + .unwrap_or(&skill.source.source), + ) + }) + .collect::>() + }) + .filter(|skills| !skills.is_empty()) + .unwrap_or_else(|| vec![live_skill_dir.to_path_buf()]); let condition_names: Vec = conditions .conditions .iter() @@ -339,8 +359,16 @@ pub fn detect_stray_writes_report( RunFindings::default() } }; - let live_reads = - detect_live_source_reads(&run.tool_invocations, live_skill_dir, repo_root); + let mut live_reads = Vec::new(); + for live_skill_dir in &live_skill_dirs { + for finding in + detect_live_source_reads(&run.tool_invocations, live_skill_dir, repo_root) + { + if !live_reads.contains(&finding) { + live_reads.push(finding); + } + } + } totals.violations += findings.violations.len(); totals.warnings += findings.warnings.len(); diff --git a/src/pipeline/grade/finalize.rs b/src/pipeline/grade/finalize.rs index 0ac7f7f..7ec58a2 100644 --- a/src/pipeline/grade/finalize.rs +++ b/src/pipeline/grade/finalize.rs @@ -15,8 +15,8 @@ use crate::adapters::adapter_for; use crate::core::fs::write_json; use crate::core::{ Assertion, AssertionResult, BinaryGradingSummary, GradedAssertionResult, Grader, GradingResult, - GradingSummary, JudgeSampleResult, JudgeVotes, MetaSummary, RunRecord, SKILL_INVOKED_META_ID, - SampledAssertionResult, SampledGradingSummary, ToolInvocation, + GradingSummary, JudgeSampleResult, JudgeVotes, MetaResult, MetaSummary, RunRecord, + SKILL_INVOKED_META_ID, SampledAssertionResult, SampledGradingSummary, ToolInvocation, }; use crate::pipeline::DiffScopeMetrics; use crate::pipeline::error::PipelineError; @@ -26,6 +26,7 @@ use crate::validation::{SchemaName, validate_against_schema}; use super::GradeContext; use super::command_check::CommandCheckResult; use super::diff_scope::grade_diff_scope; +use super::judge_tasks::meta_response_stem; use super::transcript_check::grade_transcript_check_with_context; /// What finalize graded, for the CLI summary. @@ -58,11 +59,24 @@ struct JudgeResponse { /// Fold runner checks and judge responses into a `grading.json` per /// `(eval, condition)`. See the module docs for the per-assertion behavior. pub fn finalize(ctx: &GradeContext) -> Result { - let conds: Vec<(String, Option)> = ctx + let default_skill_name = ctx.evals.skill_names().first().cloned().unwrap_or_default(); + let conds: Vec<(String, Vec, bool)> = ctx .conditions .conditions .iter() - .map(|c| (c.name.clone(), c.skill_path.clone())) + .map(|c| { + let is_multi = c.skills.is_some(); + let skills = c.skills.as_ref().map_or_else( + || { + c.skill_path + .as_ref() + .map(|_| vec![default_skill_name.clone()]) + .unwrap_or_default() + }, + |skills| skills.iter().map(|skill| skill.name.clone()).collect(), + ); + (c.name.clone(), skills, is_multi) + }) .collect(); let mut summary = FinalizeSummary::default(); @@ -76,7 +90,7 @@ pub fn finalize(ctx: &GradeContext) -> Result { let assertions = ev.assertions.as_deref().unwrap_or(&[]); let has_assertions = !assertions.is_empty(); - for (cond, cond_skill_path) in &conds { + for (cond, condition_skills, multi_skill) in &conds { let cond_dir = ctx.iteration_dir.join(format!("eval-{}", ev.id)).join(cond); if !cond_dir.exists() { continue; @@ -269,40 +283,51 @@ pub fn finalize(ctx: &GradeContext) -> Result { } // Mirror the emit gate: negative evals carry no meta-check. - let mut meta_results: Vec = Vec::new(); - if cond_skill_path.is_some() && ev.skill_should_trigger != Some(false) { - let response_path = - judge_responses_dir.join(format!("{SKILL_INVOKED_META_ID}.json")); - if response_path.exists() { - let response: JudgeResponse = - serde_json::from_str(&fs::read_to_string(&response_path)?)?; - let passed = response.passed; - meta_results.push(AssertionResult { - id: SKILL_INVOKED_META_ID.to_string(), - passed, - evidence: response.evidence.unwrap_or_default(), - confidence: Some(response.confidence.unwrap_or(0.0)), - grader: Some(response.grader.unwrap_or(Grader::LlmJudge)), - }); - summary.total_meta_graded += 1; - if !passed { - summary.meta_failures += 1; + let mut meta_results: Vec = Vec::new(); + let mut scalar_meta_failed = false; + if !condition_skills.is_empty() && ev.skill_should_trigger != Some(false) { + for (index, skill_name) in condition_skills.iter().enumerate() { + let stem = meta_response_stem(index, *multi_skill); + let response_path = judge_responses_dir.join(format!("{stem}.json")); + if response_path.exists() { + let response: JudgeResponse = + serde_json::from_str(&fs::read_to_string(&response_path)?)?; + let passed = response.passed; + meta_results.push(MetaResult { + id: SKILL_INVOKED_META_ID.to_string(), + skill_name: (*multi_skill).then(|| skill_name.clone()), + passed, + evidence: response.evidence.unwrap_or_default(), + confidence: Some(response.confidence.unwrap_or(0.0)), + grader: Some(response.grader.unwrap_or(Grader::LlmJudge)), + }); + summary.total_meta_graded += 1; + scalar_meta_failed |= !*multi_skill && !passed; + } else { + summary.warnings.push(if *multi_skill { + format!( + "missing skill-invocation meta response for '{}': {}", + skill_name, + response_path.display() + ) + } else { + format!( + "missing skill-invocation meta response: {}", + response_path.display() + ) + }); + meta_results.push(MetaResult { + id: SKILL_INVOKED_META_ID.to_string(), + skill_name: (*multi_skill).then(|| skill_name.clone()), + passed: false, + evidence: format!( + "meta judge response missing at {}", + response_path.display() + ), + confidence: Some(0.0), + grader: Some(Grader::LlmJudge), + }); } - } else { - summary.warnings.push(format!( - "missing skill-invocation meta response: {}", - response_path.display() - )); - meta_results.push(AssertionResult { - id: SKILL_INVOKED_META_ID.to_string(), - passed: false, - evidence: format!( - "meta judge response missing at {}", - response_path.display() - ), - confidence: Some(0.0), - grader: Some(Grader::LlmJudge), - }); } } @@ -310,7 +335,10 @@ pub fn finalize(ctx: &GradeContext) -> Result { let meta_len = meta_results.len() as u32; let meta_passed = meta_results.iter().filter(|r| r.passed).count() as u32; let has_meta = !meta_results.is_empty(); - let skill_invoked = has_meta.then(|| meta_results.iter().all(|r| r.passed)); + let skill_invoked = has_meta.then(|| meta_results.iter().any(|r| r.passed)); + if (*multi_skill && skill_invoked == Some(false)) || scalar_meta_failed { + summary.meta_failures += 1; + } let has_sampled = assertion_results .iter() diff --git a/src/pipeline/grade/judge_tasks.rs b/src/pipeline/grade/judge_tasks.rs index c152cd4..38a52eb 100644 --- a/src/pipeline/grade/judge_tasks.rs +++ b/src/pipeline/grade/judge_tasks.rs @@ -15,7 +15,7 @@ use serde::Serialize; use serde_json::json; use crate::core::fs::{artifact_path, write_json}; -use crate::core::{Assertion, RunRecord, SKILL_INVOKED_META_ID, ToolInvocation}; +use crate::core::{Assertion, ConditionSkill, RunRecord, SKILL_INVOKED_META_ID, ToolInvocation}; use crate::pipeline::error::PipelineError; use crate::pipeline::io::now_iso8601; use crate::pipeline::slots::run_slots; @@ -36,6 +36,10 @@ pub struct JudgeTask { #[serde(skip_serializing_if = "Option::is_none")] pub run_index: Option, pub assertion_id: String, + /// Treatment member for a multi-skill meta task. Absent for authored + /// assertions and scalar invocation checks. + #[serde(skip_serializing_if = "Option::is_none")] + pub skill_name: Option, /// 1-based verdict index when this assertion requests more than one sample. #[serde(skip_serializing_if = "Option::is_none")] pub sample_index: Option, @@ -105,6 +109,14 @@ pub fn check_skill_invoked_from_transcript( }) } +pub(super) fn meta_response_stem(index: usize, multi_skill: bool) -> String { + if multi_skill { + format!("{SKILL_INVOKED_META_ID}__skill-{}", index + 1) + } else { + SKILL_INVOKED_META_ID.to_string() + } +} + /// The meta-check rubric asking a judge whether the agent actually applied the /// skill (separate from correctness). fn skill_invoked_rubric(skill_name: &str, skill_content: Option<&str>) -> String { @@ -203,16 +215,26 @@ fn build_judge_prompt( /// Emit judge tasks + prompt files for the iteration, writing `judge-tasks.json`. /// See the module docs for the per-assertion and meta-check behavior. pub fn emit_judge_tasks(ctx: &GradeContext) -> Result { - let conds: Vec<(String, Option, Option)> = ctx + let default_skill_name = ctx.evals.skill_names().first().cloned().unwrap_or_default(); + let conds: Vec<(String, Vec, bool)> = ctx .conditions .conditions .iter() .map(|c| { - ( - c.name.clone(), - c.skill_path.clone(), - c.staged_skill_slug.clone().flatten(), - ) + let is_multi = c.skills.is_some(); + let skills = c.skills.clone().unwrap_or_else(|| { + c.skill_path + .as_ref() + .map(|path| { + vec![ConditionSkill { + name: default_skill_name.clone(), + skill_path: path.clone(), + staged_skill_slug: c.staged_skill_slug.clone().flatten(), + }] + }) + .unwrap_or_default() + }); + (c.name.clone(), skills, is_multi) }) .collect(); // The deterministic `__skill_invoked` code check needs a transcript that @@ -233,7 +255,7 @@ pub fn emit_judge_tasks(ctx: &GradeContext) -> Result Result 1).then_some(sample_count), rubric: j.rubric.clone(), @@ -339,73 +362,93 @@ pub fn emit_judge_tasks(ctx: &GradeContext) -> Result, skill_path: Option, + #[serde(default)] + skills: Option>, user_prompt: String, fixtures: Vec, outputs_dir: String, @@ -315,6 +317,7 @@ pub fn record_runs( eval_id: task.eval_id.clone(), condition: task.condition.clone(), skill_path: task.skill_path.clone(), + skills: task.skills.clone(), prompt: task.user_prompt.clone(), files: task.fixtures.clone(), final_message, diff --git a/src/validation/evals.rs b/src/validation/evals.rs index c596744..f849af8 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -963,3 +963,6 @@ mod tests { assert!(error.contains("ref"), "error was: {error}"); } } + +#[cfg(test)] +mod multi_skill_tests; diff --git a/src/validation/evals/multi_skill_tests.rs b/src/validation/evals/multi_skill_tests.rs new file mode 100644 index 0000000..4c3ac13 --- /dev/null +++ b/src/validation/evals/multi_skill_tests.rs @@ -0,0 +1,34 @@ +use serde_json::{Value, json}; + +use super::validate_evals_config; + +fn base() -> Value { + json!({ + "skill_name": "demo", + "evals": [{ + "id": "e1", + "prompt": "do the thing", + "expected_output": "the thing is done" + }] + }) +} + +#[test] +fn accepts_an_ordered_set_of_skills_under_test() { + let mut config = base(); + config["skill_name"] = json!(["demo", "helper"]); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!(parsed.skill_names(), ["demo", "helper"]); +} + +#[test] +fn rejects_an_empty_or_duplicate_skill_set() { + for names in [json!([]), json!(["demo", "demo"])] { + let mut config = base(); + config["skill_name"] = names; + + assert!(validate_evals_config(&config, "evals.json").is_err()); + } +} diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs index e9cf1da..e717fb1 100644 --- a/src/workspace/mod.rs +++ b/src/workspace/mod.rs @@ -8,7 +8,7 @@ pub mod snapshot; pub mod teardown; pub use promote::{NotesStatus, PromoteOptions, PromoteResult, promote_baseline}; -pub use snapshot::snapshot; +pub use snapshot::{snapshot, snapshot_set}; pub use teardown::{ KeptIteration, PROMOTED_MARKER, SNAPSHOT_META, WorkspaceCleanupSummary, cleanup_workspace, }; diff --git a/src/workspace/promote.rs b/src/workspace/promote.rs index 405ed84..e8d91a7 100644 --- a/src/workspace/promote.rs +++ b/src/workspace/promote.rs @@ -8,6 +8,8 @@ //! (dispatch/timing/run records, produced outputs, transcripts) is intentionally //! left behind. +mod source_row; + use std::fs; use std::path::{Path, PathBuf}; @@ -20,6 +22,8 @@ use crate::pipeline::run_slots; use crate::workspace::teardown::PROMOTED_MARKER; use crate::workspace::{WorkspaceError, now_iso8601}; +use source_row::multi_skill_source_row; + /// Inputs for [`promote_baseline`]. Borrowed for the duration of the call. pub struct PromoteOptions<'a> { pub workspace_root: &'a Path, @@ -324,6 +328,9 @@ fn skill_source_row(conditions: Option<&ConditionsRecord>) -> String { let Some(skill) = conditions.and_then(|c| c.skill_source.as_ref()) else { return String::new(); }; + if let Some(row) = multi_skill_source_row(skill) { + return row; + } let source = &skill.source; let mut cell = source .resolved_path diff --git a/src/workspace/promote/source_row.rs b/src/workspace/promote/source_row.rs new file mode 100644 index 0000000..7a71ad6 --- /dev/null +++ b/src/workspace/promote/source_row.rs @@ -0,0 +1,39 @@ +use crate::core::SkillSource; + +pub(super) fn multi_skill_source_row(skill_source: &SkillSource) -> Option { + let skills = skill_source.skills.as_ref()?; + let members = skills + .iter() + .map(|skill| { + let source = &skill.source; + let mut item = format!( + "{}: {}", + skill.name, + source + .resolved_path + .clone() + .unwrap_or_else(|| source.source.clone()) + ); + if let Some(revision) = &source.revision { + item.push_str(&format!( + " ({})", + revision.chars().take(7).collect::() + )); + } + if source.dirty { + item.push_str(" — uncommitted changes were in what ran"); + } + if let Some(origin) = &source.origin_url { + item.push_str(&format!("; origin {origin}")); + } + item + }) + .collect::>() + .join("
"); + let ambient = if skill_source.siblings.is_empty() { + String::new() + } else { + format!("; ambient skills: {}", skill_source.siblings.join(", ")) + }; + Some(format!("| Skill sources | {members}{ambient} |")) +} diff --git a/src/workspace/promote/tests.rs b/src/workspace/promote/tests.rs index 18b4df5..7b368c7 100644 --- a/src/workspace/promote/tests.rs +++ b/src/workspace/promote/tests.rs @@ -287,6 +287,65 @@ fn provenance_names_the_skill_source_and_its_uncommitted_state() { ); } +#[test] +fn provenance_names_every_multi_skill_source() { + let f = fixture(1); + let mut conditions: Value = serde_json::from_str(CONDITIONS_WITH_PROVENANCE).unwrap(); + let owner_path = f.skill_subdir.to_string_lossy(); + conditions["skill_source"] = serde_json::json!({ + "kind": "path", + "source": owner_path, + "resolved_path": owner_path, + "branch": "main", + "host_local": true, + "dirty": false, + "eval_owner": "mr-review", + "skills": [ + { + "name": "mr-review", + "kind": "path", + "source": owner_path, + "resolved_path": owner_path, + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "branch": "main", + "host_local": true, + "dirty": false + }, + { + "name": "review-verification", + "kind": "path", + "source": "/skills/review-verification", + "resolved_path": "/skills/review-verification", + "revision": "b2c3d4e5f60718293a4b5c6d7e8f90123456789a", + "branch": "main", + "host_local": true, + "dirty": true + } + ], + "siblings": ["ambient-helper"] + }); + write( + &f.iteration_dir.join("conditions.json"), + &serde_json::to_string(&conditions).unwrap(), + ); + write( + &f.iteration_dir.join("benchmark.json"), + r#"{"delta":{"pass_rate":0}}"#, + ); + + promote_baseline(&opts(&f, 1)).unwrap(); + + let provenance = fs::read_to_string(f.skill_subdir.join("evals/baseline/BASELINE.md")).unwrap(); + assert!(provenance.contains("mr-review:"), "{provenance}"); + assert!(provenance.contains("a1b2c3d"), "{provenance}"); + assert!(provenance.contains("review-verification:"), "{provenance}"); + assert!(provenance.contains("b2c3d4e"), "{provenance}"); + assert!( + provenance.contains("ambient skills: ambient-helper"), + "{provenance}" + ); +} + /// The baseline belongs to the skill the *run* measured. Deriving it from the /// operator's current selection instead would write into whichever skill they /// happen to be pointing at now. diff --git a/src/workspace/snapshot.rs b/src/workspace/snapshot.rs index 4f33c06..4dc1395 100644 --- a/src/workspace/snapshot.rs +++ b/src/workspace/snapshot.rs @@ -8,7 +8,8 @@ //! source so `teardown` knows whether the snapshot is reproducible. use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; use serde_json::json; @@ -46,6 +47,70 @@ pub fn snapshot( Ok(dest_dir) } +/// Atomically snapshot an ordered treatment set under +/// `snapshots/