diff --git a/docs/guides/byoh.md b/docs/guides/byoh.md index e8af18c..ea18b71 100644 --- a/docs/guides/byoh.md +++ b/docs/guides/byoh.md @@ -139,7 +139,8 @@ Multi-turn evals — scripted `turns` and `responder` alike — require `[conversation].resume_exec_template` plus transcript extraction of ordered assistant messages and the native session ID. There is no fresh-session fallback: `run` rejects the case when the harness cannot preserve the conversation. The responder itself needs nothing further from a descriptor; it -reads the agent's message as Markdown, so it works on any harness that can resume. See +reads the agent's message out of the transcript and consults its own model through the dispatch +template you already declared, so it works on any harness that can resume. See `eval-magic docs conversations`. When a shadow preflight reports a live copy, isolate every initial and resumed eval-agent dispatch diff --git a/docs/guides/conversations.md b/docs/guides/conversations.md index fbbedba..dc190b1 100644 --- a/docs/guides/conversations.md +++ b/docs/guides/conversations.md @@ -29,100 +29,110 @@ front when the selected harness cannot. `eval-magic harness list` names the "id": "add-request-caching", "prompt": "Requests to the pricing API are slow. Can you add caching?", "expected_output": "A working cache with the pricing endpoint under 100ms.", - "responder": { "type": "heuristic", "max_turns": 8 } + "responder": { "type": "llm", "max_turns": 8 } } ``` -- **`type`** is required. `heuristic` is the only responder today. It is - deterministic and costs nothing: it reads the agent's message and applies - fixed rules, with no second model involved. +- **`type`** is required. `llm` is the only responder: a small model, consulted + once after every round through the same harness as the agent under test. + Choose it with `eval-magic run --responder-model`; omit that flag and the + consultation runs on the harness's default model. - **`max_turns`** bounds how many follow-ups the responder may synthesize. The opening prompt is not one of them. It defaults to 8. Every turn the responder produces is recorded in the run's `conversation.json` -with an `origin` naming the rule that produced it, so you can audit whether the -responder distorted the run instead of taking the transcript on trust. The -eval's own opening prompt carries no `origin` — that absence is how you tell an -authored turn from a derived one. +with an `origin` naming the responder and, when it offered one, its one-line +reason for answering that way. The eval's own opening prompt carries no +`origin` — that absence is how you tell an authored turn from a derived one. -## What the heuristic answers +The full prompt and verdict of every consultation are kept on disk under the +run's `responder/turn-/`, so you can audit what the responder was shown and +what it wrote without rerunning anything. -The heuristic reads the last message of each round as Markdown and answers -exactly one shape of question: **a list of options introduced by a question.** +## What the responder is shown -A list counts as a question when the line directly above it ends with a `?`: +**Only what the agent already knows.** Each consultation carries the eval's +opening `prompt`, every reply the responder has already given, and the agent's +last message. It does not carry `expected_output` and it does not carry the +assertions: those are the grading criteria, and a responder that had read them +could hand the agent the rubric. -``` -Which cache should I use? - -- An in-process LRU (Recommended) -- Redis -``` +It is told to answer as the person who asked for the work — take whatever the +agent marked as recommended, else the simplest option and the least work; add no +requirements; invent no facts; write no code; keep it short. -The `?` has to be the last thing said before the options appear. That is what -separates a real question from a closing summary, which is also mostly a -bulleted list and would otherwise be "answered" as though the finished task were -still open. +It answers with one of three verdicts: -Given a list, the choice is mechanical: +| Verdict | What it means | +| --- | --- | +| `answer` | What the user says next. Delivered as the following turn. | +| `done` | The agent is reporting the task finished and waiting on nothing. | +| `cannot_answer` | It could not answer without inventing something. | -| The list | Recommendation marked | The answer | -| --- | --- | --- | -| plain (`-`, `*`, `1.`) | yes | the first recommended option | -| plain | no | the first option | -| checkboxes (`- [ ]`) | yes | every recommended option | -| checkboxes | no | nothing — `None of these.` | +Because the responder decides `done`, completion is a judgement rather than the +absence of a question mark — and the judgement is recorded with its reason, so +a run that stopped early is legible rather than mysterious. -Plain lists ask for exactly one choice; checkboxes ask for zero or more. That -syntax is the only signal the heuristic uses to tell them apart. +## What is never delivered -An option counts as recommended when it carries a standalone `recommended` in -parentheses, brackets, or bold — `(Recommended)`, `[recommended]`, -`**Recommended**` — or when it is a pre-checked box, `- [x]`. +A reply that fails any of these checks is not sent to the agent. The run stops +instead, because an undelivered reply is a loud, greppable stop, while a bad one +enters the transcript as an ordinary user turn and is graded as though the +exchange really happened. -A message that asks more than one question is answered in one turn, numbered in -the order the questions appeared. +| Rejected when the reply | Recorded cause | +| --- | --- | +| is blank | `empty_reply` | +| runs past 2000 bytes | `reply_too_long` | +| contains a fenced code block | `reply_contains_code` | +| repeats the previous reply verbatim | `reply_repeated` | + +The length and code rules are the same rule twice: a simulated user answers in +sentences, so anything longer means the responder started doing the agent's work, +and crediting the agent under test with work it did not do would corrupt the +result. A repeat means the exchange is circling, and spending the remaining +turns on it would only reach the same place more expensively. + +A consultation that never produces a reply stops the run the same way, with its +own cause: `declined` when the responder honestly refused, and +`dispatch_failed`, `dispatch_timed_out`, `missing_verdict`, or +`malformed_verdict` when something broke. One outcome, because the run ended +mid-task either way; separate causes, because an honest refusal and a broken +dispatch call for different fixes. ## How a conversation ends | Recorded as | When | | --- | --- | -| `completed` | The agent's last message asked nothing. It considers the task done, so the run stops rather than burning its remaining turns. | -| `stopped`, `responder_cannot_answer` | The agent asked something with no option list. | +| `completed` | The responder judged the agent finished. The run stops rather than burning its remaining turns. | +| `stopped`, `responder_cannot_answer` | The responder produced no usable reply. `responder_outcome.cause` says why. | | `stopped`, `max_turns_reached` | The agent was still asking at the bound. | | `timed_out` | The task outran `dispatch --timeout`. | A `stopped` conversation is recorded, not failed: `dispatch` exits zero and `ingest` still records the run. But both responder stops end the conversation -with the task unfinished, so `dispatch` warns about each one by name. Read the -last assistant message before treating such a run as a data point beside a -completed one. - -Two properties are worth knowing before you read results: - -- The heuristic never guesses. A question it does not recognize stops the run - instead of inventing an answer, because a fabricated answer would silently - change what the agent was asked to do. -- It errs toward stopping. A question mark anywhere in an otherwise-finished - message stops the run rather than calling it complete. That costs a dispatch; - the alternative — recording a run as complete while the agent was still - waiting — would cost the result's credibility. - -Answering free-form questions needs a model, not rules. That is a separate -responder, and until it ships, `responder_cannot_answer` is where those runs -stop. +with the task unfinished, so `dispatch` warns about each one by name and +`aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. That count is the one to read first: one arm truncated more +often than the other is a threat to the comparison, not just to the run. ## Cross-harness behaviour -The heuristic reads plain Markdown out of the agent's message, so it needs no -per-harness support: any harness that can resume a session can run a responder -eval. Nothing is read from a harness-native question tool, and nothing needs to -be, because a dispatch runs headless with no channel to answer such a tool on. - -The shapes above are a contract, not a description of one agent. An agent that -offers options this way is answered; one that phrases them some other way stops -the run. If you are bringing your own harness and its agent asks in a shape the -table does not cover, that is a gap in the table, not in your descriptor. +The responder needs no per-harness support and no descriptor field. It reads +`TranscriptSummary::final_text`, which every harness's parser already +normalizes, replies through the existing `{prompt_arg}` slot, and runs its own +consultations through the same `[dispatch].exec_template` a judge uses. Any +harness that can resume a session can run a responder eval. + +Nothing is read from a harness-native question tool, and nothing needs to be: a +dispatch runs headless with stdin detached, so a tool that asks the user has no +channel to be answered on. Free text is the only mechanism that fits, and it +happens to be the portable one. + +Consultations run in the run's own `responder/` directory, which sits above the +task environment. That is deliberate — a consultation must not be able to write +into the codebase under measurement, and must not pick up that codebase's +`CLAUDE.md` or `AGENTS.md` as instructions to itself. ## Scripted turns diff --git a/docs/progressive-enhancements.md b/docs/progressive-enhancements.md index 084bab9..19f9de3 100644 --- a/docs/progressive-enhancements.md +++ b/docs/progressive-enhancements.md @@ -216,22 +216,23 @@ or normal guardrail-stopped scenario. `ingest` skips an interrupted task with no artifact. A scripted turn is gated by `agent_asks` (`?`) plus the optional response regex. A responder instead -*derives* each turn from the round's last assistant message and records the rule that produced it on +*derives* each turn by consulting a small model, once after every round, and records that origin on the turn itself. **The responder needs no descriptor field and no named capability of its own:** it -reads that message as plain Markdown — a question line followed by a list of options — so every -harness that resolves a resume template gets it for free, and none can be "missing" it. +reads the round's last assistant message out of `final_text`, which every transcript parser already +normalizes, and it dispatches its own consultations through the same `[dispatch].exec_template` a +judge uses. Every harness that resolves a resume template gets it for free, and none can be +"missing" it. That portability is not a happy accident, it is forced. A dispatch runs headless with stdin detached, so a harness-native question tool has no channel to be answered on; the runner can only send free text as the next user turn. Text is therefore the only mechanism that fits, and it is the one every transcript parser already normalizes into `final_text`. -What *is* borrowed from one harness is the convention — `(Recommended)` and checkbox lists are how -Claude Code's own question UI renders choices. The recognized shapes are documented as a -harness-neutral contract in `eval-magic docs conversations`, not as "what Claude does": an agent that -offers options that way is answered identically whatever harness runs it, and one that phrases them -differently stops the run with `responder_cannot_answer` — a documented gap in the shape table, not a -missing descriptor field. Widening the table is a runner change that benefits every harness at once. +A consultation binds the exec template's placeholders the way a judge dispatch does — guard +arguments off, its own capture directory, its own prompt — with one addition: `` is the +run's `responder/turn-N/` directory rather than the task env. A consultation must not be able to +write into the codebase under measurement, nor inherit that codebase's `CLAUDE.md` as instructions +to itself. *Fallback:* none. `run` rejects selected multi-turn evals — scripted or responder-driven — when the harness omits this capability; silently starting a fresh session would make the answer meaningless. diff --git a/harnesses/template.toml b/harnesses/template.toml index c200da7..b117556 100644 --- a/harnesses/template.toml +++ b/harnesses/template.toml @@ -143,7 +143,8 @@ label = "{label}" ## ------------------------------------------------------------------------------------------- ## [conversation] — native same-session continuation for multi-turn evals, both scripted `turns` ## and a `responder` that derives them. The responder needs nothing further from a descriptor: it -## reads the agent's own message as Markdown, so declaring this table is all it takes. +## reads the agent's own message out of the transcript and consults its model through the +## [dispatch].exec_template below, so declaring this table is all it takes. ## This capability has no generic fallback: run rejects multi-turn evals for a harness that omits ## it. It requires ## [dispatch].exec_template plus transcript parsing that exposes both ordered assistant messages diff --git a/profiles/shared/runbook.md b/profiles/shared/runbook.md index ab07ab5..157cde0 100644 --- a/profiles/shared/runbook.md +++ b/profiles/shared/runbook.md @@ -21,9 +21,10 @@ each task's `conversation.json`. A task that already has one is skipped, so reru command retries only what did not finish. A task that exceeds `--timeout` is recorded as timed out rather than left to stall the campaign, and a task that fails is recorded and named while the rest of the batch continues. A conversation that stops at a scripted gate is valid eval data, not a -failure. A conversation the responder stopped — because it could not answer the agent's question, -or because it hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` -warns about each one by name, and those runs are weaker evidence than a completed one. +failure. A conversation the responder stopped — because it produced no usable reply, or because it +hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` warns about +each one by name and cause, and `aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. Those runs are weaker evidence than a completed one. ``` {{INGEST_CMD}} diff --git a/schema/conversation.schema.json b/schema/conversation.schema.json index 584fc4d..9f82a1e 100644 --- a/schema/conversation.schema.json +++ b/schema/conversation.schema.json @@ -38,7 +38,8 @@ { "$ref": "#/definitions/conversationTool" } ] } - } + }, + "responder_outcome": { "$ref": "#/definitions/responderOutcome" } }, "allOf": [ { @@ -90,6 +91,28 @@ } ], "definitions": { + "responderOutcome": { + "type": "object", + "required": ["ending"], + "additionalProperties": false, + "description": "How the responder ended the conversation, when it was the responder that ended it. Absent for a scripted or one-shot task, for a timeout, and for max_turns_reached, which is the runner's bound rather than a verdict.", + "properties": { + "ending": { + "type": "string", + "enum": ["done", "cannot_answer"], + "description": "Whether the responder judged the agent finished, or produced no usable reply." + }, + "cause": { + "type": "string", + "enum": ["declined", "dispatch_failed", "dispatch_timed_out", "missing_verdict", "malformed_verdict", "empty_reply", "reply_too_long", "reply_contains_code", "reply_repeated"], + "description": "Why no usable reply was produced, so an honest refusal is distinguishable from a broken dispatch. Absent when ending is 'done'." + }, + "rationale": { + "type": "string", + "description": "The responder's own one-line account. Absent when the dispatch never answered." + } + } + }, "userMessage": { "type": "object", "required": ["type", "ordinal", "round", "text"], @@ -101,45 +124,18 @@ "text": { "type": "string" }, "origin": { "type": "object", - "required": ["responder", "answers"], + "required": ["responder"], "additionalProperties": false, "description": "How a responder derived this turn. Absent on the eval's opening prompt and on scripted turns, which are authored rather than derived.", "properties": { "responder": { "type": "string", - "enum": ["heuristic"], + "enum": ["llm"], "description": "Which responder produced the turn." }, - "answers": { - "type": "array", - "minItems": 1, - "description": "One entry per question the turn answered, in the order they were asked.", - "items": { - "type": "object", - "required": ["options", "rule", "chosen"], - "additionalProperties": false, - "properties": { - "question": { - "type": "string", - "description": "The question line the options hung from." - }, - "options": { - "type": "array", - "items": { "type": "string" }, - "description": "The options as the agent wrote them, before markers were stripped." - }, - "rule": { - "type": "string", - "enum": ["recommended_option", "first_option", "no_selection"], - "description": "The mechanical rule that picked this answer, so a reader can audit the selection without rerunning it." - }, - "chosen": { - "type": "array", - "items": { "type": "string" }, - "description": "The options selected, cleaned of their markers. Empty when the rule selected nothing." - } - } - } + "rationale": { + "type": "string", + "description": "The responder's own one-line account of why it answered this way. Absent when it offered none." } } } diff --git a/schema/evals.schema.json b/schema/evals.schema.json index e55a76b..88f1f48 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -136,8 +136,8 @@ "properties": { "type": { "type": "string", - "enum": ["heuristic"], - "description": "Which responder answers the agent. 'heuristic' is deterministic and free: it answers a question that offers a list of options, and stops the run on anything else. Required rather than defaulted, because the responder decides what the agent hears." + "enum": ["llm"], + "description": "Which responder answers the agent. 'llm' consults a small model through the same harness as the agent under test, once after every round: it answers, judges the task finished, or stops the run rather than guessing. Required rather than defaulted, because the responder decides what the agent hears. Choose the model with 'run --responder-model'." }, "max_turns": { "type": "integer", diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index ac49565..6fee4d9 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -95,6 +95,28 @@ } }, "definitions": { + "responderOutcome": { + "type": "object", + "required": ["ending"], + "additionalProperties": false, + "description": "How the responder ended the conversation, when it was the responder that ended it. Absent for a scripted or one-shot task, for a timeout, and for max_turns_reached, which is the runner's bound rather than a verdict.", + "properties": { + "ending": { + "type": "string", + "enum": ["done", "cannot_answer"], + "description": "Whether the responder judged the agent finished, or produced no usable reply." + }, + "cause": { + "type": "string", + "enum": ["declined", "dispatch_failed", "dispatch_timed_out", "missing_verdict", "malformed_verdict", "empty_reply", "reply_too_long", "reply_contains_code", "reply_repeated"], + "description": "Why no usable reply was produced, so an honest refusal is distinguishable from a broken dispatch. Absent when ending is 'done'." + }, + "rationale": { + "type": "string", + "description": "The responder's own one-line account. Absent when the dispatch never answered." + } + } + }, "conversation": { "type": "object", "required": ["status", "delivered_followups", "events"], @@ -131,7 +153,8 @@ { "$ref": "#/definitions/conversationTool" } ] } - } + }, + "responder_outcome": { "$ref": "#/definitions/responderOutcome" } }, "allOf": [ { @@ -194,45 +217,18 @@ "text": { "type": "string" }, "origin": { "type": "object", - "required": ["responder", "answers"], + "required": ["responder"], "additionalProperties": false, "description": "How a responder derived this turn. Absent on the eval's opening prompt and on scripted turns, which are authored rather than derived.", "properties": { "responder": { "type": "string", - "enum": ["heuristic"], + "enum": ["llm"], "description": "Which responder produced the turn." }, - "answers": { - "type": "array", - "minItems": 1, - "description": "One entry per question the turn answered, in the order they were asked.", - "items": { - "type": "object", - "required": ["options", "rule", "chosen"], - "additionalProperties": false, - "properties": { - "question": { - "type": "string", - "description": "The question line the options hung from." - }, - "options": { - "type": "array", - "items": { "type": "string" }, - "description": "The options as the agent wrote them, before markers were stripped." - }, - "rule": { - "type": "string", - "enum": ["recommended_option", "first_option", "no_selection"], - "description": "The mechanical rule that picked this answer, so a reader can audit the selection without rerunning it." - }, - "chosen": { - "type": "array", - "items": { "type": "string" }, - "description": "The options selected, cleaned of their markers. Empty when the rule selected nothing." - } - } - } + "rationale": { + "type": "string", + "description": "The responder's own one-line account of why it answered this way. Absent when it offered none." } } } diff --git a/src/cli/args.rs b/src/cli/args.rs index 82baca9..0930c32 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -368,6 +368,13 @@ pub struct PromoteBaselineArgs { /// `unspecified`. #[arg(long)] pub judge_model: Option, + /// Operator-declared responder model, recorded in `BASELINE.md`. + /// + /// Overrides a `responder_model` recorded in the iteration's + /// `conditions.json` (set via `run --responder-model`); when both are + /// absent, `BASELINE.md` shows `unspecified`. + #[arg(long)] + pub responder_model: Option, } /// `run` adds the build-time flags (mode/baseline selection, staging toggles, @@ -520,9 +527,8 @@ pub struct RunArgs { /// entries override them by key, with the last occurrence winning. Values /// may be empty and may contain `=`. The resolved map is recorded in /// `conditions.json` and `dispatch.json`, so do not use this flag for - /// secrets. This does not affect judge agents or runner-owned - /// `command_check` assertions. Unset keys keep inheriting the operator's - /// environment. + /// secrets. Runner-owned `command_check` assertions are unaffected. Unset + /// keys keep inheriting the operator's environment. #[arg(long, value_name = "KEY=VALUE")] pub agent_env: Vec, /// Default judge model for emitted judge tasks. @@ -533,6 +539,17 @@ pub struct RunArgs { /// `conditions.json` for `promote-baseline`. #[arg(long)] pub judge_model: Option, + /// Model that answers the agent for evals declaring a `responder`. + /// + /// `dispatch` consults it once after every round, through the same harness + /// as the agent under test, using the harness-native model flag. It is + /// run-level on purpose: answering one eval with a different model than its + /// neighbours puts a second uncontrolled variable inside the comparison. + /// Omit it to answer on the harness's default model. Also persists to + /// `conditions.json` for `promote-baseline`. See + /// `eval-magic docs conversations`. + #[arg(long)] + pub responder_model: Option, /// Provenance label for this run, persisted into `conditions.json`. /// /// Surfaced in `BASELINE.md` by `promote-baseline` (its own `--label` flag @@ -579,9 +596,11 @@ pub(crate) enum Commands { /// /// A case with effective run count `R` creates `2R` native agent sessions: one /// per condition and repetition. Scripted follow-ups add up to `2R × F` model - /// turns for `F` declared follow-ups, and each `llm_judge` assertion creates a - /// judge task per condition and repetition. Review the printed run summary and - /// obtain confirmation before spending model usage. + /// turns for `F` declared follow-ups. A `responder` case instead adds one + /// agent turn and one small responder dispatch per round, up to its + /// `max_turns` bound. Each `llm_judge` assertion creates a judge task per + /// condition and repetition. Review the printed run summary and obtain + /// confirmation before spending model usage. /// /// Git is required. Every task environment is initialized as an independent, /// clean repository on branch `work` with a deterministic baseline commit and @@ -614,10 +633,14 @@ pub(crate) enum Commands { /// delivers, and each round must report the same native session ID or that /// task fails. A completed or normally stopped conversation records /// `delivered_followups`; an interrupted task commits no artifact, so a - /// rerun picks it up. A responder that could not answer, or that hit its - /// `max_turns` bound, is recorded and warned about: the run ended mid-task, - /// so read its last assistant message before trusting it. See - /// `eval-magic docs conversations`. + /// rerun picks it up. + /// + /// A responder task adds one small consultation after every round, run + /// through the same harness on `run --responder-model` and captured under + /// the run's `responder/` directory. A responder that produced no usable + /// reply, or that hit its `max_turns` bound, is recorded and warned about by + /// cause: the run ended mid-task, so read its last assistant message before + /// trusting it. See `eval-magic docs conversations`. Dispatch(DispatchArgs), /// Snapshot a workspace baseline. /// diff --git a/src/cli/commands/run.rs b/src/cli/commands/run.rs index 36ee434..850fe25 100644 --- a/src/cli/commands/run.rs +++ b/src/cli/commands/run.rs @@ -52,6 +52,7 @@ pub(crate) fn run_run(args: RunArgs) -> anyhow::Result<()> { agent_model: args.agent_model.as_deref(), agent_env, judge_model: args.judge_model.as_deref(), + responder_model: args.responder_model.as_deref(), label: args.label.as_deref(), }, )?; diff --git a/src/cli/commands/workspace.rs b/src/cli/commands/workspace.rs index b5c63e8..0abda92 100644 --- a/src/cli/commands/workspace.rs +++ b/src/cli/commands/workspace.rs @@ -52,6 +52,7 @@ pub(crate) fn run_promote_baseline(args: PromoteBaselineArgs) -> anyhow::Result< label: args.label.as_deref(), agent_model: args.agent_model.as_deref(), judge_model: args.judge_model.as_deref(), + responder_model: args.responder_model.as_deref(), })?; let n = result.gradings_copied; diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 1735a51..1113890 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -98,6 +98,7 @@ fn dispatch(command: Option, harness_file: Option<&str>) -> anyhow::Re agent_model: None, agent_env: Vec::new(), judge_model: None, + responder_model: None, label: None, })); diff --git a/src/cli/run/conversation.rs b/src/cli/run/conversation.rs index 1d10be5..d9a694e 100644 --- a/src/cli/run/conversation.rs +++ b/src/cli/run/conversation.rs @@ -21,11 +21,12 @@ use crate::adapters::harness::HarnessAdapter; use crate::adapters::transcript::{TranscriptEvent, TranscriptSummary}; use crate::core::{ ConversationEvent, ConversationRecord, ConversationStatus, ConversationStopReason, - ShellOutcome, run_in_posix_shell, + ResponderOutcome, ResponderStopCause, ShellOutcome, run_in_posix_shell, }; use crate::validation::{SchemaName, validate_against_schema}; use super::dispatch::DispatchTask; +use responder::{Consultation, ResponderRuntime}; use turn_plan::{NextTurn, TurnPlan}; mod responder; @@ -45,6 +46,10 @@ pub enum TaskOutcome { /// stopped conversation — but carried as written rather than filled in /// with a guess, so an outcome can never name the wrong reason. reason: Option, + /// Why the responder produced no usable reply, when it was the + /// responder that stopped the run. An honest refusal and a broken + /// dispatch end the run identically, so the warning has to say which. + cause: Option, }, TimedOut { round: u32, @@ -85,13 +90,16 @@ impl TaskOutcome { Self::Stopped { before_followup, reason: Some(ConversationStopReason::ResponderCannotAnswer), + cause, } => format!( - "stopped before turn {before_followup} — the responder could not answer the \ - agent's question" + "stopped before turn {before_followup} — the responder produced no usable reply \ + ({})", + cause_label(*cause) ), Self::Stopped { before_followup, reason: Some(ConversationStopReason::MaxTurnsReached), + .. } => format!( "stopped at the responder's max_turns bound after {} turn(s)", before_followup.saturating_sub(1) @@ -105,17 +113,40 @@ impl TaskOutcome { } } +/// How a stop cause reads in a warning. Absent only for a stop the responder +/// did not produce, which no caller here words this way. +pub fn cause_label(cause: Option) -> &'static str { + cause.map_or("cause unrecorded", ResponderStopCause::wire_name) +} + +/// The dispatch-wide settings every task shares, frozen in `dispatch.json` and +/// read back once by the batch driver. Grouped rather than passed one by one +/// because they travel together and always come from the same envelope. +#[derive(Debug, Clone, Copy)] +pub struct DispatchSettings<'a> { + pub guard: bool, + pub agent_model: Option<&'a str>, + /// The model consulted after each round of a responder task. `None` runs the + /// consultation on the harness's default model. + pub responder_model: Option<&'a str>, + pub agent_env: &'a BTreeMap, +} + /// Execute one task: start a native session, deliver every scripted follow-up /// whose gate is met, and write the `conversation.json` completion artifact. pub fn run_task( adapter: &DescriptorAdapter, task: &DispatchTask, - guard: bool, - agent_model: Option<&str>, - agent_env: &BTreeMap, + settings: &DispatchSettings<'_>, overwrite: bool, timeout: Option, ) -> anyhow::Result { + let DispatchSettings { + guard, + agent_model, + responder_model, + agent_env, + } = *settings; // One budget for the whole task, not per round: a scripted conversation is // a single dispatch from the operator's point of view. let deadline = timeout.map(|timeout| Instant::now() + timeout); @@ -164,6 +195,23 @@ pub fn run_task( })?; } + // Consultations run outside every task env, so nothing the responder does + // reaches the codebase under measurement or picks up its `CLAUDE.md`. + let responder_runtime = match &plan { + TurnPlan::Responder(_) => Some(ResponderRuntime { + adapter, + model: responder_model, + agent_env, + responder_dir: PathBuf::from( + task.responder_dir + .as_deref() + .ok_or_else(|| anyhow!("responder task is missing responder_dir"))?, + ), + deadline, + }), + TurnPlan::OneShot | TurnPlan::Scripted(_) => None, + }; + let base_outputs = Path::new(&task.outputs_dir); let mut events = vec![ConversationEvent::UserMessage { ordinal: 0, @@ -204,6 +252,7 @@ pub fn run_task( stopped_before_followup: None, timed_out_in_round: Some(1), events, + responder_outcome: None, }, None, plan.source(), @@ -226,19 +275,41 @@ pub fn run_task( let mut stop_reason = None; let mut stopped_before_followup = None; let mut timed_out_in_round = None; + let mut responder_outcome: Option = None; + // Every reply the responder has produced, in order: the prompt for the next + // consultation, and the repeat guard's memory. + let mut responder_replies: Vec = Vec::new(); loop { let followup = delivered_followups.saturating_add(1); - let next = plan.next_turn(delivered_followups, &preceding_assistant, &final_message)?; + let consultation = Consultation { + task_prompt: &task.user_prompt, + prior_replies: &responder_replies, + final_message: &final_message, + }; + let next = plan.next_turn( + delivered_followups, + &preceding_assistant, + &consultation, + responder_runtime.as_ref(), + responder_replies.last().map(String::as_str), + )?; let (prompt, origin) = match next { - NextTurn::Done => break, - NextTurn::Stop(reason) => { + NextTurn::Done { responder } => { + responder_outcome = responder; + break; + } + NextTurn::Stop { reason, responder } => { stop_reason = Some(reason); stopped_before_followup = Some(followup); + responder_outcome = responder; break; } NextTurn::Deliver { text, origin } => (text, origin), }; + if origin.is_some() { + responder_replies.push(prompt.clone()); + } let round = followup.saturating_add(1); events.push(ConversationEvent::UserMessage { @@ -306,6 +377,9 @@ pub fn run_task( stopped_before_followup: timed_out_in_round.map_or(stopped_before_followup, |_| None), timed_out_in_round, events, + // A timeout outranks the responder's verdict for the same reason it + // outranks a gate stop: the round it judged never finished. + responder_outcome: timed_out_in_round.map_or(responder_outcome, |_| None), }, Some(final_message), plan.source(), @@ -344,6 +418,10 @@ fn write_conversation( ConversationStatus::Stopped => TaskOutcome::Stopped { before_followup: conversation.stopped_before_followup.unwrap_or_default(), reason: conversation.stop_reason, + cause: conversation + .responder_outcome + .as_ref() + .and_then(|outcome| outcome.cause), }, ConversationStatus::TimedOut => TaskOutcome::TimedOut { round: conversation.timed_out_in_round.unwrap_or(1), diff --git a/src/cli/run/conversation/responder.rs b/src/cli/run/conversation/responder.rs index ca716da..34e57b4 100644 --- a/src/cli/run/conversation/responder.rs +++ b/src/cli/run/conversation/responder.rs @@ -1,422 +1,463 @@ -//! The heuristic responder: what a user would say next, decided mechanically. +//! The responder: what the person who asked for the work says next. //! -//! It reads one round's final assistant message as plain Markdown, which is why -//! it needs no harness-specific code — every harness's transcript parser -//! normalizes that text into `TranscriptSummary::final_text` already. - -use std::sync::LazyLock; - -use regex::Regex; - -use crate::core::{ResponderAnswer, ResponderKind, ResponderRule, TurnOrigin}; +//! One small model, dispatched through the same harness as the agent under +//! test, consulted once after every round. It answers the agent's question, +//! judges the task finished, or says it cannot answer — and the runner never +//! delivers a reply that fails validation, because a reply nobody vouched for +//! silently changes what the agent was asked to do. + +use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use crate::adapters::descriptor_adapter::DescriptorAdapter; +use crate::adapters::harness::HarnessAdapter; +use crate::core::{ResponderStopCause, ShellOutcome, run_in_posix_shell}; + +use super::render_dispatch_command; + +/// How long one consultation may run. It is capped separately from the task's +/// own budget so a hung responder stops the run as a responder failure rather +/// than eating the agent's remaining time and being recorded as an agent +/// timeout. +const CONSULT_TIMEOUT: Duration = Duration::from_secs(300); + +/// The byte ceiling on a reply. A simulated user answers in sentences; well +/// past that means the responder started doing the agent's work, and putting +/// that in the transcript would credit the agent under test with work it did +/// not do. +const MAX_REPLY_BYTES: usize = 2_000; + +/// The line the responder is told to write its verdict by. Named here because +/// the prompt states it and a test reads it back. +const VERDICT_PATH_LINE: &str = "Write your verdict as a JSON file to:"; + +/// What the responder is shown. Deliberately only what the agent already knows: +/// the request it was given, what the user has said since, and what it just +/// said. The eval's `expected_output` and assertions are the grading criteria, +/// and a responder that had read them could hand the agent the rubric. +pub(super) struct Consultation<'a> { + pub(super) task_prompt: &'a str, + pub(super) prior_replies: &'a [String], + pub(super) final_message: &'a str, +} -/// What the heuristic made of one assistant turn. -pub(super) enum Reading { - /// No question was asked — the agent considers the task done. - NoQuestion, - /// A question with no option list the heuristic can answer. - Unanswerable, - /// A mechanically answerable question, with the reply and its provenance. - Answer { text: String, origin: TurnOrigin }, +/// What the responder decided. `Answer` carries a reply that has already passed +/// validation by the time it leaves [`ResponderRuntime::consult`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum Verdict { + Answer { + reply: String, + rationale: Option, + }, + Done { + rationale: Option, + }, + CannotAnswer { + rationale: Option, + }, } -/// A list item: `- text`, `* text`, `+ text`, `1. text`, or `1) text`. Up to -/// three leading spaces, matching Markdown's own tolerance before a deeper -/// indent turns the line into a continuation of the item above it. -static OPTION_LINE: LazyLock = - LazyLock::new(|| Regex::new(r"^ {0,3}(?:[-*+]|\d{1,3}[.)])\s+(.*)$").unwrap()); - -/// A standalone `recommended`, in parentheses, brackets, or bold. Deliberately -/// narrow: an option that merely discusses what it recommends is not a marker. -/// A task-list marker at the head of an option body: `[ ]`, `[x]`, or `[X]`. -/// Its presence anywhere in a group is what makes the group multi-select. -static CHECKBOX: LazyLock = LazyLock::new(|| Regex::new(r"^\[( |x|X)\]\s*").unwrap()); - -static RECOMMENDED: LazyLock = LazyLock::new(|| { - Regex::new(r"(?i)(\(\s*recommended\s*\)|\[\s*recommended\s*\]|\*\*\s*recommended\s*\*\*)") - .unwrap() -}); - -/// One list of options, with the lines that introduced it. -struct OptionGroup { - question: Option, - options: Vec, +/// The verdict file as written, before it is known to be usable. Every field +/// tolerates absence, following the judge's `JudgeResponse`: a sloppy responder +/// should fail one named validation gate, not blow up parsing. +#[derive(serde::Deserialize)] +struct RawVerdict { + #[serde(default)] + verdict: String, + #[serde(default)] + reply: Option, + #[serde(default)] + rationale: Option, } -pub(super) fn read(final_message: &str) -> Reading { - let lines: Vec<&str> = final_message.lines().collect(); - let answers: Vec = question_groups(&lines) - .iter() - .filter_map(answer_for) - .collect(); - if answers.is_empty() { - return if final_message.contains('?') { - Reading::Unanswerable - } else { - Reading::NoQuestion - }; - } - Reading::Answer { - text: render_reply(&answers), - origin: TurnOrigin { - responder: ResponderKind::Heuristic, - answers, - }, - } +/// Everything a consultation needs that does not change between rounds. +pub(super) struct ResponderRuntime<'a> { + pub(super) adapter: &'a DescriptorAdapter, + pub(super) model: Option<&'a str>, + pub(super) agent_env: &'a BTreeMap, + /// Where consultations run and are captured — outside every task env, so + /// nothing the responder does can reach the codebase under measurement or + /// pick up its `CLAUDE.md` as instructions. + pub(super) responder_dir: PathBuf, + pub(super) deadline: Option, } -/// Every option list in the message whose lead-in asks something. -fn question_groups(lines: &[&str]) -> Vec { - let mut groups = Vec::new(); - let mut index = 0; - while index < lines.len() { - let Some(option) = option_body(lines[index]) else { - index += 1; - continue; - }; - let start = index; - let mut options = vec![option]; - index += 1; - while index < lines.len() { - if let Some(option) = option_body(lines[index]) { - options.push(option); - index += 1; - } else if lines[index].trim().is_empty() - && lines - .get(index + 1) - .copied() - .and_then(option_body) - .is_some() - { - // A loose Markdown list puts a blank line between its items. - index += 1; - } else { - break; - } +impl ResponderRuntime<'_> { + /// Ask the responder what the user says after `round`. Every failure is a + /// named cause rather than an error: the run stops mid-task, which is a + /// recorded result, not a broken campaign. + pub(super) fn consult( + &self, + round: u32, + consultation: &Consultation<'_>, + previous_reply: Option<&str>, + ) -> Result { + let dir = self.responder_dir.join(format!("turn-{round}")); + fs::create_dir_all(&dir).map_err(|_| ResponderStopCause::DispatchFailed)?; + let prompt_path = dir.join("prompt.txt"); + let verdict_path = dir.join("verdict.json"); + + // Clear any verdict left by an earlier dispatch of this task, or by a + // consultation that was killed part way through writing one. The file's + // presence is the only evidence that this consultation answered, so a + // stale one would be read as a reply written about a different + // conversation. + match fs::remove_file(&verdict_path) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return Err(ResponderStopCause::DispatchFailed), } - if options.len() < 2 { - continue; + + fs::write(&prompt_path, build_prompt(consultation, &verdict_path)) + .map_err(|_| ResponderStopCause::DispatchFailed)?; + + // Guard arguments are deliberately off, for the reason a judge's are: + // this dispatch runs outside every guarded task env. + let template = self + .adapter + .cli_exec_command(false, self.model, self.agent_env) + .ok_or(ResponderStopCause::DispatchFailed)?; + let command = render_dispatch_command( + &template, + &dir.to_string_lossy(), + &prompt_path.to_string_lossy(), + &dir, + ); + + let budget = match self.deadline { + Some(deadline) => { + CONSULT_TIMEOUT.min(deadline.saturating_duration_since(Instant::now())) + } + None => CONSULT_TIMEOUT, + }; + match run_in_posix_shell(&command, &dir, self.agent_env, Some(budget)) { + Ok(ShellOutcome::Exited(status)) if status.success() => {} + Ok(ShellOutcome::Exited(_)) | Err(_) => return Err(ResponderStopCause::DispatchFailed), + Ok(ShellOutcome::TimedOut) => return Err(ResponderStopCause::DispatchTimedOut), } - if let Some(question) = lead_in_question(lines, start) { - groups.push(OptionGroup { - question: Some(question), - options, - }); + + let raw = fs::read_to_string(&verdict_path) + .ok() + .filter(|body| !body.trim().is_empty()) + .ok_or(ResponderStopCause::MissingVerdict)?; + let verdict = parse_verdict(&raw)?; + if let Verdict::Answer { reply, .. } = &verdict { + validate_reply(reply, previous_reply)?; } + Ok(verdict) } - groups -} - -/// The text of a list item, or `None` when the line is not one. -fn option_body(line: &str) -> Option { - let body = OPTION_LINE.captures(line)?.get(1)?.as_str().trim(); - (!body.is_empty()).then(|| body.to_string()) } -/// The question a group hangs from: the last line of the contiguous non-blank -/// block directly above it, and only when that line *ends* with `?`. +/// Build one consultation's prompt. Pure, so what the responder is and is not +/// shown is testable without dispatching anything. /// -/// Ending, not merely containing: a closing summary's list is introduced by a -/// line like `Here is what changed:`, and that line may well also carry a -/// rhetorical question earlier in the sentence. Answering such a list would -/// derail a finished task, so the `?` has to be the last thing said before the -/// options appear. -fn lead_in_question(lines: &[&str], start: usize) -> Option { - let mut end = start; - while end > 0 && lines[end - 1].trim().is_empty() { - end -= 1; - } - if end == 0 || option_body(lines[end - 1]).is_some() { - return None; - } - let question = strip_emphasis(lines[end - 1].trim()); - question.ends_with('?').then(|| question.to_string()) -} - -fn strip_emphasis(text: &str) -> &str { - text.trim_matches(|c| c == '*' || c == '_' || c == '`' || c == '#') - .trim() -} - -/// Apply the selection rules to one group. -fn answer_for(group: &OptionGroup) -> Option { - // Checkbox syntax is the only mechanical signal that a question takes zero - // or more answers rather than exactly one. - let multi_select = group.options.iter().any(|option| CHECKBOX.is_match(option)); - let recommended: Vec = group - .options - .iter() - .filter(|option| is_recommended(option)) - .map(|option| clean(option)) - .collect(); - let (rule, chosen) = match (recommended.is_empty(), multi_select) { - (false, true) => (ResponderRule::RecommendedOption, recommended), - (false, false) => ( - ResponderRule::RecommendedOption, - vec![recommended[0].clone()], - ), - (true, true) => (ResponderRule::NoSelection, Vec::new()), - (true, false) => (ResponderRule::FirstOption, vec![clean(&group.options[0])]), +/// The agent's own message goes in verbatim, and an agent could in principle +/// write instructions to the responder into it. That is contained by the +/// runner reading the verdict from the path it chose rather than one parsed +/// out of anything: a redirected write is a missing verdict, which stops the +/// run. +fn build_prompt(consultation: &Consultation<'_>, verdict_path: &Path) -> String { + let said_since = if consultation.prior_replies.is_empty() { + String::new() + } else { + let replies: Vec = consultation + .prior_replies + .iter() + .enumerate() + .map(|(index, reply)| format!("{}. {reply}", index + 1)) + .collect(); + format!("# What you have said since\n\n{}\n\n", replies.join("\n")) }; - Some(ResponderAnswer { - question: group.question.clone(), - options: group.options.clone(), - rule, - chosen, - }) + + [ + "You are the person who asked for this work. An AI coding agent is doing the task and has", + "stopped to say something. Decide what you say next.", + "", + "# What you originally asked for", + "", + consultation.task_prompt, + "", + // Folded into the heading rather than standing alone, so an absent + // section leaves no hole in a file a person reads while auditing. + &format!("{said_since}# What the agent just said"), + "", + consultation.final_message, + "", + "# How to decide", + "", + "- If the agent asked you something you can answer, answer it. Prefer whatever it marked", + " as recommended; failing that, the simplest option and the least work.", + "- Never add requirements, never introduce facts you have not already stated, and never do", + " the agent's work for it. No code, no file contents.", + "- Keep it to a couple of sentences, as a person typing a reply would.", + "- If the agent is reporting the task finished and is not waiting on you, the conversation", + " is over: answer `done`.", + "- If you genuinely cannot answer without inventing something, answer `cannot_answer`", + " rather than guessing.", + "", + "# Task", + "", + &format!("{VERDICT_PATH_LINE} {}", verdict_path.display()), + "", + "The JSON must match this schema (exactly these keys, no extra prose in the file):", + "", + "```json", + "{ \"verdict\": \"answer\"|\"done\"|\"cannot_answer\", \"reply\": \"what you say next\", \"rationale\": \"one line\" }", + "```", + "", + "`reply` is required for `answer` and ignored otherwise.", + "", + ] + .join("\n") } -/// A marked recommendation, or a pre-checked box — the plainest statement of a -/// suggested default a Markdown list can carry. -fn is_recommended(option: &str) -> bool { - RECOMMENDED.is_match(option) - || CHECKBOX - .captures(option) - .and_then(|caps| caps.get(1)) - .is_some_and(|marker| marker.as_str() != " ") +/// Read one verdict file. A fence is stripped first because models add one out +/// of habit, and stopping a run over punctuation would be a worse failure than +/// the three lines it costs to tolerate. +fn parse_verdict(raw: &str) -> Result { + let raw: RawVerdict = + serde_json::from_str(unfence(raw)).map_err(|_| ResponderStopCause::MalformedVerdict)?; + let rationale = raw + .rationale + .map(|line| line.trim().to_string()) + .filter(|line| !line.is_empty()); + match raw.verdict.as_str() { + "answer" => Ok(Verdict::Answer { + reply: raw.reply.unwrap_or_default(), + rationale, + }), + "done" => Ok(Verdict::Done { rationale }), + "cannot_answer" => Ok(Verdict::CannotAnswer { rationale }), + _ => Err(ResponderStopCause::MalformedVerdict), + } } -/// An option as a user would say it back: markers stripped, spacing tidied. -fn clean(option: &str) -> String { - RECOMMENDED - .replace_all(&CHECKBOX.replace(option, ""), "") - .split_whitespace() - .collect::>() - .join(" ") - .trim_end_matches(['-', '\u{2014}', ':', ',']) +/// Strip one wrapping code fence, if the whole body is inside it. +fn unfence(raw: &str) -> &str { + let trimmed = raw.trim(); + let Some(rest) = trimmed.strip_prefix("```") else { + return trimmed; + }; + let Some(body) = rest.split_once('\n').map(|(_language, body)| body) else { + return trimmed; + }; + body.trim_end() + .strip_suffix("```") + .unwrap_or(trimmed) .trim() - .to_string() } -/// What the responder says when a checkbox question recommends nothing. A user -/// still has to answer, and "nothing" is the answer the rules produced. -const NO_SELECTION_REPLY: &str = "None of these."; - -fn render_reply(answers: &[ResponderAnswer]) -> String { - answers - .iter() - .enumerate() - .map(|(index, answer)| { - let chosen = match answer.chosen.is_empty() { - true => NO_SELECTION_REPLY.to_string(), - false => answer.chosen.join(", "), - }; - // A single answer needs no ordinal; several do, so the agent can - // tell which reply belongs to which question it asked. - match answers.len() { - 1 => chosen, - _ => format!("{}. {chosen}", index + 1), - } - }) - .collect::>() - .join("\n") +/// Decide whether a reply may be delivered. Every rejection stops the run, +/// which is the safe direction: an undelivered reply is a loud, greppable stop, +/// while a bad one enters the transcript as an ordinary user turn and is graded +/// as though the exchange really happened. +fn validate_reply(reply: &str, previous: Option<&str>) -> Result<(), ResponderStopCause> { + let trimmed = reply.trim(); + if trimmed.is_empty() { + return Err(ResponderStopCause::EmptyReply); + } + if reply.len() > MAX_REPLY_BYTES { + return Err(ResponderStopCause::ReplyTooLong); + } + if reply.contains("```") { + return Err(ResponderStopCause::ReplyContainsCode); + } + if previous.is_some_and(|previous| previous.trim() == trimmed) { + return Err(ResponderStopCause::ReplyRepeated); + } + Ok(()) } #[cfg(test)] mod tests { - use super::{Reading, read}; - use crate::core::ResponderRule; + use super::*; + use std::path::Path; + + fn consultation() -> Consultation<'static> { + Consultation { + task_prompt: "Requests to the pricing API are slow. Add caching.", + prior_replies: &[], + final_message: "Which cache should I use?", + } + } - /// The happy path #244 names: an option marked as recommended is the answer. #[test] - fn a_recommended_option_is_chosen() { - let message = "\ -I can add caching two ways. Which do you want? - -- Use an in-process LRU cache (Recommended) -- Add Redis -"; - - let Reading::Answer { text, origin } = read(message) else { - panic!("a recommended option must be answerable"); - }; + fn the_prompt_carries_the_exchange_and_names_where_to_write() { + let prompt = build_prompt( + &consultation(), + Path::new("/w/responder/turn-1/verdict.json"), + ); - assert_eq!(text, "Use an in-process LRU cache"); - assert_eq!(origin.answers.len(), 1); - assert_eq!(origin.answers[0].rule, ResponderRule::RecommendedOption); - assert_eq!(origin.answers[0].chosen, ["Use an in-process LRU cache"]); + assert!(prompt.contains("Requests to the pricing API are slow. Add caching.")); + assert!(prompt.contains("Which cache should I use?")); + assert!( + prompt.contains(VERDICT_PATH_LINE), + "the responder is told where to write: {prompt}" + ); + assert!(prompt.contains("/w/responder/turn-1/verdict.json")); } - /// Exactly one choice is required and none is recommended, so the list's - /// first option wins. Mechanical, not a judgement about which is better. + /// A simulated user remembers what they already said, so a later round does + /// not contradict an earlier answer. #[test] - fn a_plain_list_with_no_recommendation_takes_the_first_option() { - let message = "\ -Which database should I target? - -1. PostgreSQL -2. MySQL -3. SQLite -"; - - let Reading::Answer { text, origin } = read(message) else { - panic!("a plain option list is answerable"); + fn prior_replies_are_carried_into_the_prompt() { + let replies = ["An in-process LRU is fine.".to_string()]; + let consultation = Consultation { + prior_replies: &replies, + ..consultation() }; - assert_eq!(text, "PostgreSQL"); - assert_eq!(origin.answers[0].rule, ResponderRule::FirstOption); - assert_eq!(origin.answers[0].chosen, ["PostgreSQL"]); + let prompt = build_prompt(&consultation, Path::new("/w/v.json")); + assert!(prompt.contains("1. An in-process LRU is fine.")); + assert!(!prompt.contains("\n\n\n"), "{prompt}"); } - /// A checkbox list asks for zero or more. With nothing recommended, zero is - /// the mechanical answer — the responder does not invent a preference. #[test] - fn a_checkbox_list_with_no_recommendation_selects_nothing() { - let message = "\ -Which extras should I include? - -- [ ] Unit tests -- [ ] Integration tests -- [ ] Benchmarks -"; + fn an_answer_verdict_parses_with_its_reply_and_rationale() { + let raw = + r#"{"verdict":"answer","reply":"Use the in-process LRU.","rationale":"simplest"}"#; - let Reading::Answer { text, origin } = read(message) else { - panic!("a checkbox list is answerable"); + let Verdict::Answer { reply, rationale } = parse_verdict(raw).unwrap() else { + panic!("expected an answer"); }; - - assert_eq!(text, "None of these."); - assert_eq!(origin.answers[0].rule, ResponderRule::NoSelection); - assert!(origin.answers[0].chosen.is_empty()); + assert_eq!(reply, "Use the in-process LRU."); + assert_eq!(rationale.as_deref(), Some("simplest")); } - /// Zero or more means every recommendation can be taken, unlike a - /// single-choice list where only the first can. #[test] - fn a_checkbox_list_selects_every_recommended_option() { - let message = "\ -Which extras should I include? + fn a_done_verdict_parses_and_carries_no_reply() { + let raw = r#"{"verdict":"done","rationale":"the agent reported the cache in place"}"#; -- [ ] Unit tests (Recommended) -- [ ] Integration tests -- [ ] Docs (Recommended) -"; - - let Reading::Answer { text, origin } = read(message) else { - panic!("a checkbox list is answerable"); + let Verdict::Done { rationale } = parse_verdict(raw).unwrap() else { + panic!("expected done"); }; - - assert_eq!(text, "Unit tests, Docs"); - assert_eq!(origin.answers[0].rule, ResponderRule::RecommendedOption); - assert_eq!(origin.answers[0].chosen, ["Unit tests", "Docs"]); + assert_eq!( + rationale.as_deref(), + Some("the agent reported the cache in place") + ); } - /// A pre-checked box is the plainest possible statement of a suggested - /// default, so it reads as a recommendation. #[test] - fn a_pre_checked_box_counts_as_a_recommendation() { - let message = "\ -Which extras should I include? + fn a_cannot_answer_verdict_parses() { + let raw = r#"{"verdict":"cannot_answer","rationale":"it asked for a credential"}"#; -- [ ] Unit tests -- [x] Docs -"; + assert!(matches!( + parse_verdict(raw).unwrap(), + Verdict::CannotAnswer { .. } + )); + } - let Reading::Answer { text, origin } = read(message) else { - panic!("a checkbox list is answerable"); + /// A rationale is a courtesy, not a contract: a verdict without one is + /// still usable, and refusing it would stop a run over prose. + #[test] + fn a_verdict_without_a_rationale_is_still_usable() { + let Verdict::Answer { rationale, .. } = + parse_verdict(r#"{"verdict":"answer","reply":"Yes, go ahead."}"#).unwrap() + else { + panic!("expected an answer"); }; - - assert_eq!(text, "Docs"); - assert_eq!(origin.answers[0].rule, ResponderRule::RecommendedOption); + assert_eq!(rationale, None); } - /// A message may ask more than one thing. Each list is answered under its - /// own rule, and the reply is numbered so the agent can tell them apart. + /// Models fence JSON out of habit. Stripping the fence costs three lines + /// and saves a run that would otherwise stop over punctuation. #[test] - fn two_option_groups_are_answered_in_order() { - let message = "\ -A couple of decisions before I start. + fn a_fenced_verdict_is_unwrapped_before_parsing() { + let raw = "```json\n{\"verdict\":\"done\"}\n```\n"; -Which database? - -- PostgreSQL (Recommended) -- MySQL + assert!(matches!(parse_verdict(raw).unwrap(), Verdict::Done { .. })); + } -Which extras do you want? + #[test] + fn an_unknown_verdict_is_malformed() { + assert_eq!( + parse_verdict(r#"{"verdict":"maybe","reply":"hmm"}"#).unwrap_err(), + ResponderStopCause::MalformedVerdict + ); + } -- [ ] Benchmarks -- [ ] Fuzzing -"; + #[test] + fn a_verdict_that_is_not_json_is_malformed() { + assert_eq!( + parse_verdict("I think you should use Redis.").unwrap_err(), + ResponderStopCause::MalformedVerdict + ); + } - let Reading::Answer { text, origin } = read(message) else { - panic!("two option lists are answerable"); + /// An `answer` with no reply is not an answer. Parsing yields an empty one + /// so a single validation gate rejects it, rather than two paths deciding + /// separately what "blank" means. + #[test] + fn an_answer_with_no_reply_parses_blank_and_fails_validation() { + let Verdict::Answer { reply, .. } = parse_verdict(r#"{"verdict":"answer"}"#).unwrap() + else { + panic!("expected an answer"); }; - - assert_eq!(text, "1. PostgreSQL\n2. None of these."); - assert_eq!(origin.answers.len(), 2); - assert_eq!(origin.answers[0].rule, ResponderRule::RecommendedOption); - assert_eq!(origin.answers[1].rule, ResponderRule::NoSelection); assert_eq!( - origin.answers[1].question.as_deref(), - Some("Which extras do you want?") + validate_reply(&reply, None).unwrap_err(), + ResponderStopCause::EmptyReply ); } - /// The load-bearing negative: a closing summary is mostly a bulleted list, - /// and answering one as if it were a question would derail a finished task. - /// Requiring the lead-in to ask something is what separates the two. #[test] - fn a_closing_summary_with_a_bulleted_list_is_not_a_question() { - let message = "\ -Done. I made these changes: - -- Fixed the date parser -- Added a regression test -- Updated the changelog -"; - - assert!(matches!(read(message), Reading::NoQuestion)); + fn a_whitespace_only_reply_is_empty() { + assert_eq!( + validate_reply(" \n\t ", None).unwrap_err(), + ResponderStopCause::EmptyReply + ); } - /// A question with no options is the branch the LLM responder takes over. - /// Until then it stops the run rather than inventing an answer. #[test] - fn a_free_form_question_is_unanswerable() { - let message = "Before I start — what should happen to rows with a null created_at?"; - - assert!(matches!(read(message), Reading::Unanswerable)); + fn an_ordinary_reply_validates() { + assert_eq!(validate_reply("Use the in-process LRU.", None), Ok(())); } - /// Completion detection: no question at all means the agent is done, so the - /// conversation ends instead of burning its remaining turns. + /// A simulated user answers in sentences. A reply this long means the + /// responder started doing the agent's work, and delivering it would put + /// work into the transcript that the agent under test did not do. #[test] - fn a_message_with_no_question_reads_as_done() { - let message = "Caching is in place and the pricing endpoint is under 40ms."; + fn a_reply_over_the_byte_cap_is_rejected() { + let long = "a".repeat(MAX_REPLY_BYTES + 1); - assert!(matches!(read(message), Reading::NoQuestion)); + assert_eq!( + validate_reply(&long, None).unwrap_err(), + ResponderStopCause::ReplyTooLong + ); + assert_eq!(validate_reply(&"a".repeat(MAX_REPLY_BYTES), None), Ok(())); } - /// Removing a trailing marker can leave the separator that introduced it - /// dangling, and echoing "Use an in-process LRU —" back at the agent reads - /// as a truncated thought. #[test] - fn a_separator_left_by_a_trailing_marker_is_cleaned_up() { - let message = "\ -Which cache should I use? - -- An in-process LRU — (Recommended) -- Redis -"; + fn a_reply_carrying_a_fenced_code_block_is_rejected() { + let reply = "Sure, use this:\n\n```rust\nlet cache = Lru::new(128);\n```\n"; - let Reading::Answer { text, .. } = read(message) else { - panic!("a recommended option must be answerable"); - }; - - assert_eq!(text, "An in-process LRU"); + assert_eq!( + validate_reply(reply, None).unwrap_err(), + ResponderStopCause::ReplyContainsCode + ); } - /// A deliberate, documented conservatism: a stray question mark anywhere in - /// an otherwise-finished message stops the run instead of calling it done. - /// Stopping wastes a dispatch; guessing "complete" while the agent waits - /// would record a half-finished run as data. + /// The same answer twice means the exchange is circling. Spending the + /// remaining turns on it would only cost more dispatches to reach the same + /// place, so stop where it is legible. #[test] - fn a_stray_question_mark_stops_rather_than_claiming_completion() { - let message = "\ -Why a decorator? It keeps the call sites untouched. Here is what changed: + fn a_reply_identical_to_the_previous_one_is_rejected() { + let reply = "Use the in-process LRU."; -- Wrapped the client -- Added the cache -"; + assert_eq!( + validate_reply(reply, Some(reply)).unwrap_err(), + ResponderStopCause::ReplyRepeated + ); + assert_eq!(validate_reply(reply, Some("Use Redis.")), Ok(())); + } - assert!(matches!(read(message), Reading::Unanswerable)); + /// Trailing whitespace is not a new answer. + #[test] + fn the_repeat_check_ignores_surrounding_whitespace() { + assert_eq!( + validate_reply(" Use the LRU.\n", Some("Use the LRU.")).unwrap_err(), + ResponderStopCause::ReplyRepeated + ); } } diff --git a/src/cli/run/conversation/turn_plan.rs b/src/cli/run/conversation/turn_plan.rs index 189a379..ad1775e 100644 --- a/src/cli/run/conversation/turn_plan.rs +++ b/src/cli/run/conversation/turn_plan.rs @@ -8,9 +8,13 @@ use anyhow::Context; use regex::Regex; use crate::cli::run::dispatch::DispatchTask; -use crate::core::{ConversationStopReason, DeliverWhen, ResponderPolicy, ScriptedTurn, TurnOrigin}; +use crate::core::{ + ConversationStopReason, DeliverWhen, ResponderEnding, ResponderKind, ResponderOutcome, + ResponderPolicy, ResponderStopCause, ScriptedTurn, TurnOrigin, +}; use super::{TurnSource, responder}; +use responder::{Consultation, ResponderRuntime, Verdict}; /// Where a task's follow-up turns come from. Resolving this once, up front, /// keeps the driver's loop to a single delivery path whatever shape of task it @@ -26,12 +30,16 @@ pub(super) enum TurnPlan<'a> { /// What the plan wants to happen after a round. pub(super) enum NextTurn { - /// The conversation is finished — a script ran out, or the agent stopped - /// asking. - Done, + /// The conversation is finished — a script ran out, or the responder + /// judged the agent done. `responder` records that judgement when it was + /// the responder that made it. + Done { responder: Option }, /// Halt and record why. A normal outcome, not a failure. - Stop(ConversationStopReason), - /// Send this as the next user turn. `origin` names the responder rule that + Stop { + reason: ConversationStopReason, + responder: Option, + }, + /// Send this as the next user turn. `origin` names the responder that /// produced it, and is absent for an authored scripted turn. Deliver { text: String, @@ -76,50 +84,96 @@ impl<'a> TurnPlan<'a> { &self, delivered: u32, preceding_assistant: &str, - final_message: &str, + consultation: &Consultation<'_>, + runtime: Option<&ResponderRuntime<'_>>, + previous_reply: Option<&str>, ) -> anyhow::Result { match self { - Self::OneShot => Ok(NextTurn::Done), + Self::OneShot => Ok(NextTurn::Done { responder: None }), Self::Scripted(turns) => { let Some(turn) = turns.get(delivered as usize) else { - return Ok(NextTurn::Done); + return Ok(NextTurn::Done { responder: None }); }; if let Some(reason) = unmet_gate(turn, preceding_assistant)? { - return Ok(NextTurn::Stop(reason)); + return Ok(NextTurn::Stop { + reason, + responder: None, + }); } Ok(NextTurn::Deliver { text: turn.prompt.clone(), origin: None, }) } - Self::Responder(policy) => Ok(responder_turn(policy, delivered, final_message)), + Self::Responder(policy) => { + let runtime = runtime + .expect("a responder plan resolved its runtime alongside the plan itself"); + let verdict = + runtime.consult(delivered.saturating_add(1), consultation, previous_reply); + Ok(next_from_verdict(policy, delivered, verdict)) + } } } } -/// Classify the agent's last message, then bound the result. +/// Turn one consultation into the next step, then bound the result. /// /// Classification comes first deliberately: an agent that has stopped asking /// has finished the task, and finishing on the last permitted turn is a /// completion, not a run that ran out of budget. -fn responder_turn(policy: &ResponderPolicy, delivered: u32, final_message: &str) -> NextTurn { - match responder::read(final_message) { - responder::Reading::NoQuestion => NextTurn::Done, - responder::Reading::Unanswerable => { - NextTurn::Stop(ConversationStopReason::ResponderCannotAnswer) +fn next_from_verdict( + policy: &ResponderPolicy, + delivered: u32, + verdict: Result, +) -> NextTurn { + let verdict = match verdict { + Ok(verdict) => verdict, + Err(cause) => return cannot_answer(cause, None), + }; + match verdict { + Verdict::Done { rationale } => NextTurn::Done { + responder: Some(ResponderOutcome { + ending: ResponderEnding::Done, + cause: None, + rationale, + }), + }, + Verdict::CannotAnswer { rationale } => { + cannot_answer(ResponderStopCause::Declined, rationale) } - responder::Reading::Answer { text, origin } => { + Verdict::Answer { reply, rationale } => { if delivered >= policy.max_turns() { - return NextTurn::Stop(ConversationStopReason::MaxTurnsReached); + // The bound is the runner's decision, not a verdict, so no + // responder outcome is recorded against it. + return NextTurn::Stop { + reason: ConversationStopReason::MaxTurnsReached, + responder: None, + }; } NextTurn::Deliver { - text, - origin: Some(origin), + text: reply, + origin: Some(TurnOrigin { + responder: ResponderKind::Llm, + rationale, + }), } } } } +/// Every way the responder fails to produce a usable reply ends the run the +/// same way; the cause is what tells an honest refusal from a broken dispatch. +fn cannot_answer(cause: ResponderStopCause, rationale: Option) -> NextTurn { + NextTurn::Stop { + reason: ConversationStopReason::ResponderCannotAnswer, + responder: Some(ResponderOutcome { + ending: ResponderEnding::CannotAnswer, + cause: Some(cause), + rationale, + }), + } +} + fn unmet_gate( turn: &ScriptedTurn, preceding_assistant: &str, @@ -142,8 +196,108 @@ fn unmet_gate( #[cfg(test)] mod tests { - use super::unmet_gate; - use crate::core::{ConversationStopReason, DeliverWhen, ScriptedTurn}; + use super::{NextTurn, next_from_verdict, unmet_gate}; + use crate::cli::run::conversation::responder::Verdict; + use crate::core::{ + ConversationStopReason, DeliverWhen, ResponderEnding, ResponderKind, ResponderPolicy, + ResponderStopCause, ScriptedTurn, + }; + + fn policy(max_turns: u32) -> ResponderPolicy { + ResponderPolicy { + kind: ResponderKind::Llm, + max_turns: Some(max_turns), + } + } + + fn answer(reply: &str) -> Verdict { + Verdict::Answer { + reply: reply.to_string(), + rationale: Some("the simplest option".to_string()), + } + } + + #[test] + fn an_answer_below_the_bound_is_delivered_with_its_origin() { + let NextTurn::Deliver { text, origin } = + next_from_verdict(&policy(8), 0, Ok(answer("Use the LRU."))) + else { + panic!("an answer under the bound is delivered"); + }; + assert_eq!(text, "Use the LRU."); + let origin = origin.expect("a derived turn names its origin"); + assert_eq!(origin.responder, ResponderKind::Llm); + assert_eq!(origin.rationale.as_deref(), Some("the simplest option")); + } + + /// Classification comes before the bound: an agent that finishes on its + /// last permitted turn completed, and only one still asking has run out. + #[test] + fn an_answer_at_the_bound_stops_without_delivering() { + let NextTurn::Stop { reason, responder } = + next_from_verdict(&policy(2), 2, Ok(answer("Use the LRU."))) + else { + panic!("the bound stops the conversation"); + }; + assert_eq!(reason, ConversationStopReason::MaxTurnsReached); + assert!( + responder.is_none(), + "the bound is the runner's decision, not the responder's verdict" + ); + } + + #[test] + fn a_done_verdict_ends_the_conversation_and_records_why() { + let NextTurn::Done { responder } = next_from_verdict( + &policy(8), + 1, + Ok(Verdict::Done { + rationale: Some("the agent reported the cache in place".to_string()), + }), + ) else { + panic!("done ends the conversation"); + }; + let outcome = responder.expect("a responder-ended conversation records how"); + assert_eq!(outcome.ending, ResponderEnding::Done); + assert_eq!(outcome.cause, None); + assert_eq!( + outcome.rationale.as_deref(), + Some("the agent reported the cache in place") + ); + } + + #[test] + fn a_declined_verdict_stops_with_the_declined_cause() { + let NextTurn::Stop { reason, responder } = next_from_verdict( + &policy(8), + 0, + Ok(Verdict::CannotAnswer { + rationale: Some("it asked for a credential I was never given".to_string()), + }), + ) else { + panic!("cannot_answer stops the conversation"); + }; + assert_eq!(reason, ConversationStopReason::ResponderCannotAnswer); + let outcome = responder.expect("the stop records why"); + assert_eq!(outcome.ending, ResponderEnding::CannotAnswer); + assert_eq!(outcome.cause, Some(ResponderStopCause::Declined)); + } + + /// A broken dispatch and an honest refusal end the run the same way — the + /// task is unfinished either way — but the cause tells them apart. + #[test] + fn a_failed_consultation_stops_with_its_own_cause() { + let NextTurn::Stop { reason, responder } = + next_from_verdict(&policy(8), 0, Err(ResponderStopCause::DispatchTimedOut)) + else { + panic!("a failed consultation stops the conversation"); + }; + assert_eq!(reason, ConversationStopReason::ResponderCannotAnswer); + let outcome = responder.expect("the stop records why"); + assert_eq!(outcome.ending, ResponderEnding::CannotAnswer); + assert_eq!(outcome.cause, Some(ResponderStopCause::DispatchTimedOut)); + assert_eq!(outcome.rationale, None); + } fn conditional(pattern: Option<&str>) -> ScriptedTurn { ScriptedTurn { diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index 870090b..086861c 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -68,6 +68,13 @@ pub struct DispatchTask { /// how the conversation was driven, not just what it produced. #[serde(default, skip_serializing_if = "Option::is_none")] pub responder: Option, + /// Where this task's responder consultations run and are captured. It sits + /// in the cell directory, above the env: a consultation must not be able to + /// reach the codebase under measurement, nor pick up its `CLAUDE.md` as + /// instructions. Absent unless the eval declares a responder, so a task + /// without one serializes exactly as it did before the field existed. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub responder_dir: Option, #[serde(default, skip_serializing)] pub dispatch_prompt: String, } @@ -299,6 +306,9 @@ pub fn build_dispatch_task(opts: &DispatchTaskOpts) -> Result, #[serde(default)] + pub responder_model: Option, + #[serde(default)] pub agent_env: BTreeMap, pub harness_descriptor: serde_json::Value, pub tasks: Vec, @@ -132,12 +134,14 @@ impl DispatchSummary { )), Ok(TaskOutcome::Stopped { reason: Some(ConversationStopReason::ResponderCannotAnswer), + cause, .. }) => Some(format!( - "{} stopped: the responder could not answer the agent's question, so the run \ - ended mid-task. Read the last assistant message under its outputs before \ + "{} stopped: the responder could not answer the agent's question ({}), so the \ + run ended mid-task. Read the last assistant message under its outputs before \ trusting this data point.", - report.description + report.description, + cause_label(*cause) )), Ok(TaskOutcome::Stopped { reason: Some(ConversationStopReason::MaxTurnsReached), @@ -195,9 +199,12 @@ pub fn command_dispatch( let result = run_task( &adapter, task, - envelope.guard, - envelope.agent_model.as_deref(), - &envelope.agent_env, + &DispatchSettings { + guard: envelope.guard, + agent_model: envelope.agent_model.as_deref(), + responder_model: envelope.responder_model.as_deref(), + agent_env: &envelope.agent_env, + }, overwrite, timeout, ) diff --git a/src/cli/run/orchestrate/build.rs b/src/cli/run/orchestrate/build.rs index 0051d86..ee499e3 100644 --- a/src/cli/run/orchestrate/build.rs +++ b/src/cli/run/orchestrate/build.rs @@ -63,6 +63,7 @@ pub(super) fn write_dispatch( agent_model: opts.agent_model.map(str::to_owned), agent_env: opts.agent_env.clone(), judge_model: opts.judge_model.map(str::to_owned), + responder_model: opts.responder_model.map(str::to_owned), label: opts.label.map(str::to_owned), codebases: r.codebases.iter().map(super::RunCodebase::usage).collect(), skill_source: Some(r.skill.record()), @@ -278,6 +279,7 @@ pub(super) fn write_dispatch( "runs": opts.runs, "agent_model": conditions.agent_model, "judge_model": conditions.judge_model, + "responder_model": conditions.responder_model, "label": conditions.label, "conditions": conditions.conditions, "harness": ctx.harness, diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index 437c498..d5c0918 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -59,6 +59,7 @@ pub struct RunOptions<'a> { /// Resolved descriptor defaults plus run-level agent environment overrides. pub agent_env: BTreeMap, pub judge_model: Option<&'a str>, + pub responder_model: Option<&'a str>, pub label: Option<&'a str>, } diff --git a/src/cli/run/util.rs b/src/cli/run/util.rs index 18cb216..7b3e57d 100644 --- a/src/cli/run/util.rs +++ b/src/cli/run/util.rs @@ -204,7 +204,7 @@ pub(crate) fn harness_run_preflight<'a>( trusting a run whose evals depend on the agent actually executing something." )); } - if (opts.agent_model.is_some() || opts.judge_model.is_some()) + if (opts.agent_model.is_some() || opts.judge_model.is_some() || opts.responder_model.is_some()) && adapter.cli_model_flag().is_none() { warnings.push(format!( diff --git a/src/core/types.rs b/src/core/types.rs index 90657f8..7990b46 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -345,6 +345,12 @@ pub struct ConditionsRecord { /// Operator-declared judge model (provenance, like `agent_model`). #[serde(skip_serializing_if = "Option::is_none")] pub judge_model: Option, + /// Operator-declared responder model (provenance, like `agent_model`). A + /// responder eval puts a third model in the attribution picture, so a + /// report that names the agent and the judge has to name this one too. + /// Appended last so a record written before it existed still round-trips. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub responder_model: Option, /// Operator-declared provenance label, surfaced in `BASELINE.md` on promote. #[serde(skip_serializing_if = "Option::is_none")] pub label: Option, @@ -427,6 +433,12 @@ pub struct ConversationRecord { #[serde(skip_serializing_if = "Option::is_none")] pub timed_out_in_round: Option, pub events: Vec, + /// How the responder ended the conversation, when it was the responder that + /// ended it. Absent for a scripted or one-shot task, for a timeout, and for + /// `max_turns_reached` — the bound is the runner's decision, not a verdict. + /// Appended last so a record written before it existed still round-trips. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub responder_outcome: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -445,9 +457,11 @@ pub enum ConversationStatus { pub enum ConversationStopReason { AgentDidNotAsk, AgentResponseMismatch, - /// The agent asked something the responder could not answer mechanically. - /// This is the branch an LLM responder takes over; until then the run stops - /// here rather than inventing a reply. + /// The responder did not produce a usable reply — it declined, its dispatch + /// failed, or what it wrote failed validation. The specific cause is on the + /// record's [`ResponderOutcome`]. One reason covers all of them because the + /// outcome is the same: the run ended mid-task rather than being handed a + /// reply nobody vouched for. ResponderCannotAnswer, /// The agent was still asking when the responder's `max_turns` bound was /// reached. A bounded conversation, not a failed one. @@ -491,45 +505,91 @@ pub enum ConversationEvent { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TurnOrigin { pub responder: ResponderKind, - /// One entry per question the turn answered, in the order they were asked. - pub answers: Vec, + /// One line from the responder on why it answered this way. Absent when it + /// offered none; the tag above is what marks the turn derived. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rationale: Option, } -/// Which responder produced a turn. `heuristic` is the only one that exists -/// today; the LLM answering agent adds its own so the two stay distinguishable -/// in a record. +/// Which responder produced a turn. Named in the record even though there is +/// one of them, because a record outlives the version that wrote it. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ResponderKind { - Heuristic, + Llm, } -/// How the responder answered one question, with the evidence it read. +/// How the responder brought a conversation to an end, recorded once on the +/// conversation rather than on a turn — no turn was delivered. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct ResponderAnswer { - /// The question line the options hung from, when the message carried one. +pub struct ResponderOutcome { + pub ending: ResponderEnding, + /// Why no usable reply was produced. Absent for [`ResponderEnding::Done`], + /// where nothing went wrong. #[serde(default, skip_serializing_if = "Option::is_none")] - pub question: Option, - /// The options as written, before markers were stripped. - pub options: Vec, - pub rule: ResponderRule, - /// Empty when the rule selected nothing. - pub chosen: Vec, + pub cause: Option, + /// The responder's own one-line account, when it produced one. A dispatch + /// that never answered has none. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rationale: Option, +} + +/// The two ways a responder ends a conversation. Deliberately not the parsed +/// verdict, which also carries an answer: an answer is recorded on the turn it +/// became, so it cannot reach here. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ResponderEnding { + /// The responder judged the agent finished and waiting on nothing. + Done, + /// No usable reply, for the reason in [`ResponderOutcome::cause`]. + CannotAnswer, } -/// The mechanical rule that picked one answer. Naming it on the turn is what -/// makes a synthesized conversation auditable rather than mysterious. +/// Why the responder produced no usable reply. Every variant stops the run with +/// [`ConversationStopReason::ResponderCannotAnswer`]; naming the cause is what +/// lets an operator tell an honest refusal from a broken dispatch. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum ResponderRule { - /// Exactly one option was marked as recommended, or every recommended - /// option of a checkbox list was taken. - RecommendedOption, - /// Exactly one choice was required and none was recommended, so the first - /// option won. - FirstOption, - /// A checkbox list required zero or more choices and recommended none. - NoSelection, +pub enum ResponderStopCause { + /// The responder said it could not answer without inventing something. + Declined, + /// The harness command exited nonzero or could not be spawned. + DispatchFailed, + /// The harness command outran the consultation budget. + DispatchTimedOut, + /// The dispatch succeeded but wrote no verdict file, or an empty one. + MissingVerdict, + /// The verdict file did not parse, or named a verdict that does not exist. + MalformedVerdict, + /// An `answer` verdict whose reply was blank. + EmptyReply, + /// The reply exceeded the byte cap — a simulated user answers in sentences, + /// so a long one means the responder started doing the agent's work. + ReplyTooLong, + /// The reply carried a fenced code block, for the same reason. + ReplyContainsCode, + /// The reply repeated the previous one verbatim: the exchange is circling, + /// and spending the remaining turns on it would only cost more. + ReplyRepeated, +} + +impl ResponderStopCause { + /// The cause's serialized name. Warnings print this rather than prose so + /// what an operator reads is what they would grep the artifacts for. + pub fn wire_name(self) -> &'static str { + match self { + Self::Declined => "declined", + Self::DispatchFailed => "dispatch_failed", + Self::DispatchTimedOut => "dispatch_timed_out", + Self::MissingVerdict => "missing_verdict", + Self::MalformedVerdict => "malformed_verdict", + Self::EmptyReply => "empty_reply", + Self::ReplyTooLong => "reply_too_long", + Self::ReplyContainsCode => "reply_contains_code", + Self::ReplyRepeated => "reply_repeated", + } + } } /// The result of grading one assertion. @@ -780,6 +840,7 @@ mod tests { agent_model: None, agent_env: BTreeMap::new(), judge_model: None, + responder_model: None, label: None, codebases: Vec::new(), skill_source: None, diff --git a/src/core/types/artifact_tests.rs b/src/core/types/artifact_tests.rs index d245dae..040f2fc 100644 --- a/src/core/types/artifact_tests.rs +++ b/src/core/types/artifact_tests.rs @@ -203,25 +203,25 @@ fn a_responder_record_satisfies_both_schemas_and_roundtrips() { "delivered_followups": 1, "stop_reason": "responder_cannot_answer", "stopped_before_followup": 2, + "responder_outcome": { + "ending": "cannot_answer", + "cause": "declined", + "rationale": "the agent asked for a credential I was never given" + }, "events": [ { "type": "user_message", "ordinal": 0, "round": 1, "text": "Add caching." }, - { "type": "assistant_message", "ordinal": 1, "round": 1, "text": "Which cache?\n\n- LRU (Recommended)\n- Redis\n" }, + { "type": "assistant_message", "ordinal": 1, "round": 1, "text": "Which cache should I use?" }, { "type": "user_message", "ordinal": 2, "round": 2, - "text": "LRU", + "text": "An in-process LRU is fine.", "origin": { - "responder": "heuristic", - "answers": [{ - "question": "Which cache?", - "options": ["LRU (Recommended)", "Redis"], - "rule": "recommended_option", - "chosen": ["LRU"] - }] + "responder": "llm", + "rationale": "the simplest option that needs no new service" } }, - { "type": "assistant_message", "ordinal": 3, "round": 2, "text": "What TTL suits you?" } + { "type": "assistant_message", "ordinal": 3, "round": 2, "text": "Which API key should it use?" } ] }); @@ -232,14 +232,24 @@ fn a_responder_record_satisfies_both_schemas_and_roundtrips() { parsed.stop_reason, Some(ConversationStopReason::ResponderCannotAnswer) ); + let outcome = parsed + .responder_outcome + .as_ref() + .expect("a responder-ended conversation records how it ended"); + assert_eq!(outcome.ending, ResponderEnding::CannotAnswer); + assert_eq!(outcome.cause, Some(ResponderStopCause::Declined)); + let ConversationEvent::UserMessage { origin, .. } = &parsed.events[2] else { panic!("event 2 is the synthesized turn"); }; let origin = origin .as_ref() .expect("a synthesized turn names its origin"); - assert_eq!(origin.responder, ResponderKind::Heuristic); - assert_eq!(origin.answers[0].rule, ResponderRule::RecommendedOption); + assert_eq!(origin.responder, ResponderKind::Llm); + assert_eq!( + origin.rationale.as_deref(), + Some("the simplest option that needs no new service") + ); // The seeded prompt is authored, not derived, so it carries no origin at // all — the field's absence is what distinguishes the two. @@ -254,7 +264,7 @@ fn a_responder_record_satisfies_both_schemas_and_roundtrips() { "skill_path": null, "prompt": "Add caching.", "files": [], - "final_message": "What TTL suits you?", + "final_message": "Which API key should it use?", "tool_invocations": [], "total_tokens": null, "duration_ms": null, @@ -269,6 +279,75 @@ fn a_responder_record_satisfies_both_schemas_and_roundtrips() { ); } +/// A conversation the responder judged finished records why, because +/// completion is now a model judgement rather than the absence of a question +/// mark. `cause` is absent: nothing went wrong. +#[test] +fn a_responder_completion_records_its_rationale_and_no_cause() { + use crate::validation::{SchemaName, validate_against_schema}; + + let conversation = json!({ + "status": "completed", + "delivered_followups": 1, + "responder_outcome": { + "ending": "done", + "rationale": "the agent reported the cache in place and asked nothing" + }, + "events": [ + { "type": "user_message", "ordinal": 0, "round": 1, "text": "Add caching." }, + { "type": "assistant_message", "ordinal": 1, "round": 1, "text": "Which cache?" }, + { + "type": "user_message", + "ordinal": 2, + "round": 2, + "text": "An in-process LRU is fine.", + "origin": { "responder": "llm" } + }, + { "type": "assistant_message", "ordinal": 3, "round": 2, "text": "Done — the LRU is wired in." } + ] + }); + + let parsed: ConversationRecord = + validate_against_schema(SchemaName::Conversation, &conversation, "conversation.json") + .unwrap(); + let outcome = parsed + .responder_outcome + .expect("a responder ended this one"); + assert_eq!(outcome.ending, ResponderEnding::Done); + assert_eq!(outcome.cause, None); + + // A turn whose responder offered no rationale still records its origin — + // the tag is what marks the turn derived, not the prose. + let ConversationEvent::UserMessage { origin, .. } = &parsed.events[2] else { + panic!("event 2 is the synthesized turn"); + }; + assert_eq!(origin.as_ref().unwrap().rationale, None); +} + +/// An operator reads a stop cause in a `dispatch` warning and greps for it in +/// `conversation.json`. Those are two spellings of one name, so they are pinned +/// to each other rather than kept in step by hand. +#[test] +fn every_stop_cause_prints_the_name_it_serializes_as() { + for cause in [ + ResponderStopCause::Declined, + ResponderStopCause::DispatchFailed, + ResponderStopCause::DispatchTimedOut, + ResponderStopCause::MissingVerdict, + ResponderStopCause::MalformedVerdict, + ResponderStopCause::EmptyReply, + ResponderStopCause::ReplyTooLong, + ResponderStopCause::ReplyContainsCode, + ResponderStopCause::ReplyRepeated, + ] { + assert_eq!( + serde_json::to_value(cause).unwrap(), + Value::String(cause.wire_name().to_string()), + "{cause:?}" + ); + } +} + /// A conversation that outran its deadline is written by the driver and read /// back by ingest, so the run-record schema has to accept the same shape /// `conversation.schema.json` does — including a round-1 timeout, whose only diff --git a/src/pipeline/aggregate.rs b/src/pipeline/aggregate.rs index dd9dfac..00730bf 100644 --- a/src/pipeline/aggregate.rs +++ b/src/pipeline/aggregate.rs @@ -21,7 +21,8 @@ use self::assertions::AssertionRollup; use crate::adapters::skill_shadow::PluginShadowArtifact; use crate::core::fs::write_json; use crate::core::{ - CodebaseUse, ConditionsRecord, GradingResult, Mode, SkillSource, TimingRecord, TimingSource, + CodebaseUse, ConditionsRecord, ConversationRecord, GradingResult, Mode, ResponderEnding, + ResponderStopCause, SkillSource, TimingRecord, TimingSource, }; use crate::pipeline::DiffScopeMetrics; use crate::pipeline::error::PipelineError; @@ -220,6 +221,10 @@ pub fn aggregate( .map(|condition| (condition.clone(), Vec::new())) .collect(); let mut missing_diff_scopes = Vec::new(); + // Per condition, the causes that ended a run before its task was finished. + // Tallied per condition on purpose: one arm being truncated more than the + // other is the threat to the comparison, not the raw total. + let mut responder_stops: HashMap> = HashMap::new(); for eval_dir in &eval_dirs { for cond in &condition_names { @@ -253,6 +258,10 @@ pub fn aggregate( missing_diff_scopes.push(format!("{eval_dir}/{cond}{run}")); } + if let Some(cause) = responder_stop_cause(&slot.dir) { + responder_stops.entry(cond.clone()).or_default().push(cause); + } + if !grading_path.exists() { let run = slot .run_index @@ -384,6 +393,22 @@ pub fn aggregate( } } + for cond in &condition_names { + let Some(causes) = responder_stops.get(cond).filter(|c| !c.is_empty()) else { + continue; + }; + let mut named: Vec<&str> = causes.to_vec(); + named.sort_unstable(); + named.dedup(); + validity_warnings.push(format!( + "condition '{cond}' had {} run(s) end before the task was finished because the \ + responder produced no usable reply ({}) — those runs measure an interrupted task, \ + so their gradings are not comparable with a completed run's.", + causes.len(), + named.join(", ") + )); + } + git_isolation::collect_warnings(iteration_dir, &mut validity_warnings); collect_stray_warnings(iteration_dir, &mut validity_warnings); collect_guard_denial_warnings(iteration_dir, &mut validity_warnings); @@ -481,6 +506,24 @@ fn timing_source_label(source: Option) -> String { .to_string() } +/// Why one run's responder ended it early, if it did. A conversation the +/// responder carried to completion, a scripted one, and a timeout all return +/// `None`: only an unfinished task threatens the comparison. Read leniently — +/// an unreadable artifact is the ingest stage's problem, not this one's. +fn responder_stop_cause(run_dir: &Path) -> Option<&'static str> { + let raw = fs::read_to_string(run_dir.join("conversation.json")).ok()?; + let record: ConversationRecord = serde_json::from_str(&raw).ok()?; + let outcome = record.responder_outcome?; + match outcome.ending { + ResponderEnding::Done => None, + ResponderEnding::CannotAnswer => Some( + outcome + .cause + .map_or("unrecorded", ResponderStopCause::wire_name), + ), + } +} + /// Add a warning per stray-write violation / live-source read. A malformed /// report is ignored rather than failing aggregation — the warnings are /// advisory, not a gate. diff --git a/src/pipeline/grade/transcript_check.rs b/src/pipeline/grade/transcript_check.rs index c4a8fe9..dde7fb5 100644 --- a/src/pipeline/grade/transcript_check.rs +++ b/src/pipeline/grade/transcript_check.rs @@ -345,6 +345,7 @@ mod tests { text: "Done.".into(), }, ], + responder_outcome: None, } } diff --git a/src/pipeline/record_runs/tests/conversation.rs b/src/pipeline/record_runs/tests/conversation.rs index 2a19c75..3cfab6d 100644 --- a/src/pipeline/record_runs/tests/conversation.rs +++ b/src/pipeline/record_runs/tests/conversation.rs @@ -293,7 +293,7 @@ fn a_responder_task_without_its_completion_artifact_is_skipped_as_incomplete() { let dispatch_path = iter.join("dispatch.json"); let mut dispatch: Value = serde_json::from_str(&fs::read_to_string(&dispatch_path).unwrap()).unwrap(); - dispatch["tasks"][0]["responder"] = json!({ "type": "heuristic" }); + dispatch["tasks"][0]["responder"] = json!({ "type": "llm" }); dispatch["tasks"][0]["conversation_path"] = json!( iter.join("eval-clarify") .join("with_skill") @@ -355,7 +355,7 @@ fn records_a_run_whose_conversation_timed_out_in_a_later_round() { let dispatch_path = iter.join("dispatch.json"); let mut dispatch: Value = serde_json::from_str(&fs::read_to_string(&dispatch_path).unwrap()).unwrap(); - dispatch["tasks"][0]["responder"] = json!({ "type": "heuristic" }); + dispatch["tasks"][0]["responder"] = json!({ "type": "llm" }); dispatch["tasks"][0]["conversation_path"] = json!(conversation_path.to_string_lossy().to_string()); fs::write( diff --git a/src/validation/evals.rs b/src/validation/evals.rs index ca22837..ad5abc9 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -476,11 +476,11 @@ mod tests { #[test] fn accepts_an_eval_declaring_only_a_responder() { let mut config = base(); - config["evals"][0]["responder"] = json!({ "type": "heuristic", "max_turns": 3 }); + config["evals"][0]["responder"] = json!({ "type": "llm", "max_turns": 3 }); let parsed = validate_evals_config(&config, "evals.json").unwrap(); let responder = parsed.evals[0].responder.as_ref().unwrap(); - assert_eq!(responder.kind, crate::core::ResponderKind::Heuristic); + assert_eq!(responder.kind, crate::core::ResponderKind::Llm); assert_eq!(responder.max_turns, Some(3)); } @@ -489,7 +489,7 @@ mod tests { #[test] fn rejects_responder_and_turns_together() { let mut config = base(); - config["evals"][0]["responder"] = json!({ "type": "heuristic" }); + config["evals"][0]["responder"] = json!({ "type": "llm" }); config["evals"][0]["turns"] = json!([{ "prompt": "go on", "deliver_when": "always" }]); let error = validate_evals_config(&config, "evals.json") @@ -505,7 +505,7 @@ mod tests { #[test] fn rejects_a_zero_max_turns() { let mut config = base(); - config["evals"][0]["responder"] = json!({ "type": "heuristic", "max_turns": 0 }); + config["evals"][0]["responder"] = json!({ "type": "llm", "max_turns": 0 }); let error = validate_evals_config(&config, "evals.json") .unwrap_err() @@ -519,7 +519,7 @@ mod tests { #[test] fn assistant_message_matches_accepts_a_responder_eval() { let mut config = base(); - config["evals"][0]["responder"] = json!({ "type": "heuristic" }); + config["evals"][0]["responder"] = json!({ "type": "llm" }); config["evals"][0]["assertions"] = json!([{ "id": "asked", "type": "transcript_check", diff --git a/src/workspace/promote.rs b/src/workspace/promote.rs index 34e71fe..c65b5f5 100644 --- a/src/workspace/promote.rs +++ b/src/workspace/promote.rs @@ -31,6 +31,7 @@ pub struct PromoteOptions<'a> { /// agent/judge itself, so it cannot observe these — record what was used. pub agent_model: Option<&'a str>, pub judge_model: Option<&'a str>, + pub responder_model: Option<&'a str>, } /// What [`promote_baseline`] wrote. @@ -362,6 +363,10 @@ fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head .judge_model .or_else(|| conditions.and_then(|c| c.judge_model.as_deref())) .unwrap_or("unspecified"); + let responder_model = opts + .responder_model + .or_else(|| conditions.and_then(|c| c.responder_model.as_deref())) + .unwrap_or("unspecified"); let run_label = opts .label .or_else(|| conditions.and_then(|c| c.label.as_deref())) @@ -389,6 +394,7 @@ fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head format!("| Harness | {harness} |"), format!("| Agent model | {agent_model} |"), format!("| Judge model | {judge_model} |"), + format!("| Responder model | {responder_model} |"), format!("| Conditions | {conditions_cell} |"), format!("| Run timestamp | {timestamp} |"), format!("| Label | {run_label} |"), diff --git a/src/workspace/promote/tests.rs b/src/workspace/promote/tests.rs index 77d5b79..53d2ae5 100644 --- a/src/workspace/promote/tests.rs +++ b/src/workspace/promote/tests.rs @@ -46,6 +46,7 @@ fn opts<'a>(f: &'a Fixture, iteration: u32) -> PromoteOptions<'a> { label: None, agent_model: None, judge_model: None, + responder_model: None, } } @@ -93,6 +94,7 @@ fn copies_benchmark_and_per_run_gradings_into_baseline() { assert!(provenance.contains("2026-05-27T00:00:00.000Z")); assert!(provenance.contains("Agent model | unspecified")); assert!(provenance.contains("Judge model | unspecified")); + assert!(provenance.contains("Responder model | unspecified")); assert!(provenance.contains("per-assertion pass counts")); } @@ -202,11 +204,13 @@ fn records_agent_and_judge_models_when_provided() { let mut o = opts(&f, 1); o.agent_model = Some("claude-haiku-4-5-20251001"); o.judge_model = Some("claude-opus-4-7"); + o.responder_model = Some("claude-haiku-4-5-20251001"); promote_baseline(&o).unwrap(); let provenance = fs::read_to_string(f.skill_subdir.join("evals/baseline/BASELINE.md")).unwrap(); assert!(provenance.contains("Agent model | claude-haiku-4-5-20251001")); assert!(provenance.contains("Judge model | claude-opus-4-7")); + assert!(provenance.contains("Responder model | claude-haiku-4-5-20251001")); } const CONDITIONS_WITH_PROVENANCE: &str = r#"{ @@ -219,6 +223,7 @@ const CONDITIONS_WITH_PROVENANCE: &str = r#"{ "harness": "claude-code", "agent_model": "claude-haiku-4-5-20251001", "judge_model": "claude-opus-4-8", + "responder_model": "claude-haiku-4-5-20251001", "label": "canonical-run" }"#; @@ -239,6 +244,7 @@ fn provenance_falls_back_to_manifest_models_and_label() { let provenance = fs::read_to_string(f.skill_subdir.join("evals/baseline/BASELINE.md")).unwrap(); assert!(provenance.contains("Agent model | claude-haiku-4-5-20251001")); assert!(provenance.contains("Judge model | claude-opus-4-8")); + assert!(provenance.contains("Responder model | claude-haiku-4-5-20251001")); assert!(provenance.contains("Label | canonical-run")); } diff --git a/tests/cli/aggregate.rs b/tests/cli/aggregate.rs index 13af70d..47fa906 100644 --- a/tests/cli/aggregate.rs +++ b/tests/cli/aggregate.rs @@ -56,6 +56,22 @@ fn write_grading_in(run_dir: &std::path::Path, pass_rate: f64) { .unwrap(); } +/// Write `eval-e1//conversation.json` (the cond dir must already exist). +fn write_conversation( + iteration_dir: &std::path::Path, + cond: &str, + conversation: serde_json::Value, +) { + fs::write( + iteration_dir + .join("eval-e1") + .join(cond) + .join("conversation.json"), + serde_json::to_string(&conversation).unwrap(), + ) + .unwrap(); +} + /// Write `eval-e1//timing.json` (the cond dir must already exist). fn write_timing(iteration_dir: &std::path::Path, cond: &str, timing: serde_json::Value) { write_timing_in(&iteration_dir.join("eval-e1").join(cond), timing); @@ -451,6 +467,96 @@ fn aggregate_warns_on_mixed_timing_sources() { })); } +/// A run the responder could not carry to completion measured an interrupted +/// task, so counting it beside a completed one biases the delta. The count is +/// per condition on purpose: one arm being truncated more than the other is +/// exactly the threat the reader needs to see. +#[test] +fn aggregate_warns_when_the_responder_ended_runs_early() { + use serde_json::json; + let (_tmp, root) = canonical_root(); + let (skill_dir, skill_md, iteration_dir, cwd) = setup_agg(&root); + new_skill_conditions(&iteration_dir, &skill_md); + for cond in ["with_skill", "without_skill"] { + write_grading(&iteration_dir, cond, 1.0); + } + write_conversation( + &iteration_dir, + "with_skill", + json!({ + "status": "stopped", + "delivered_followups": 1, + "stop_reason": "responder_cannot_answer", + "stopped_before_followup": 2, + "responder_outcome": { "ending": "cannot_answer", "cause": "declined" }, + "events": [ + { "type": "user_message", "ordinal": 0, "round": 1, "text": "Add caching." }, + { "type": "assistant_message", "ordinal": 1, "round": 1, "text": "Which credential?" } + ] + }), + ); + + agg_cmd(&cwd, &skill_dir).assert().success(); + + let b = read_benchmark(&iteration_dir); + let warns = b["validity_warnings"].as_array().unwrap(); + let warning = warns + .iter() + .find_map(|w| { + let s = w.as_str().unwrap(); + s.contains("responder").then_some(s) + }) + .unwrap_or_else(|| panic!("expected a responder warning in {warns:?}")); + assert!(warning.contains("with_skill"), "{warning}"); + assert!(warning.contains("declined"), "{warning}"); + assert!( + !warns + .iter() + .any(|w| w.as_str().unwrap().contains("without_skill") + && w.as_str().unwrap().contains("responder")), + "the untruncated arm is not warned about: {warns:?}" + ); +} + +/// A conversation the responder carried to completion is not a threat to the +/// comparison, so it must not add noise to every responder-driven campaign. +#[test] +fn aggregate_is_silent_when_the_responder_completed_every_run() { + use serde_json::json; + let (_tmp, root) = canonical_root(); + let (skill_dir, skill_md, iteration_dir, cwd) = setup_agg(&root); + new_skill_conditions(&iteration_dir, &skill_md); + for cond in ["with_skill", "without_skill"] { + write_grading(&iteration_dir, cond, 1.0); + write_conversation( + &iteration_dir, + cond, + json!({ + "status": "completed", + "delivered_followups": 1, + "responder_outcome": { "ending": "done" }, + "events": [ + { "type": "user_message", "ordinal": 0, "round": 1, "text": "Add caching." }, + { "type": "assistant_message", "ordinal": 1, "round": 1, "text": "Done." } + ] + }), + ); + } + + agg_cmd(&cwd, &skill_dir).assert().success(); + + let b = read_benchmark(&iteration_dir); + assert!( + !b["validity_warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w.as_str().unwrap().contains("responder")), + "{}", + b["validity_warnings"] + ); +} + /// `aggregate`: no timing-source warning when all runs share one source. #[test] fn aggregate_no_warning_when_timing_sources_match() { diff --git a/tests/golden/claude-code/runbook.golden.md b/tests/golden/claude-code/runbook.golden.md index da4b5e3..84256b6 100644 --- a/tests/golden/claude-code/runbook.golden.md +++ b/tests/golden/claude-code/runbook.golden.md @@ -21,9 +21,10 @@ each task's `conversation.json`. A task that already has one is skipped, so reru command retries only what did not finish. A task that exceeds `--timeout` is recorded as timed out rather than left to stall the campaign, and a task that fails is recorded and named while the rest of the batch continues. A conversation that stops at a scripted gate is valid eval data, not a -failure. A conversation the responder stopped — because it could not answer the agent's question, -or because it hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` -warns about each one by name, and those runs are weaker evidence than a completed one. +failure. A conversation the responder stopped — because it produced no usable reply, or because it +hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` warns about +each one by name and cause, and `aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. Those runs are weaker evidence than a completed one. ``` eval-magic ingest --skill-dir /tmp/skills --skill widget-skill --iteration 2 --harness claude-code diff --git a/tests/golden/cline/runbook.golden.md b/tests/golden/cline/runbook.golden.md index fb6e3fe..e5eec40 100644 --- a/tests/golden/cline/runbook.golden.md +++ b/tests/golden/cline/runbook.golden.md @@ -21,9 +21,10 @@ each task's `conversation.json`. A task that already has one is skipped, so reru command retries only what did not finish. A task that exceeds `--timeout` is recorded as timed out rather than left to stall the campaign, and a task that fails is recorded and named while the rest of the batch continues. A conversation that stops at a scripted gate is valid eval data, not a -failure. A conversation the responder stopped — because it could not answer the agent's question, -or because it hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` -warns about each one by name, and those runs are weaker evidence than a completed one. +failure. A conversation the responder stopped — because it produced no usable reply, or because it +hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` warns about +each one by name and cause, and `aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. Those runs are weaker evidence than a completed one. ``` eval-magic ingest --skill-dir /tmp/skills --skill widget-skill --iteration 2 --harness cline diff --git a/tests/golden/codex/runbook.golden.md b/tests/golden/codex/runbook.golden.md index 0908bc5..8aab67a 100644 --- a/tests/golden/codex/runbook.golden.md +++ b/tests/golden/codex/runbook.golden.md @@ -21,9 +21,10 @@ each task's `conversation.json`. A task that already has one is skipped, so reru command retries only what did not finish. A task that exceeds `--timeout` is recorded as timed out rather than left to stall the campaign, and a task that fails is recorded and named while the rest of the batch continues. A conversation that stops at a scripted gate is valid eval data, not a -failure. A conversation the responder stopped — because it could not answer the agent's question, -or because it hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` -warns about each one by name, and those runs are weaker evidence than a completed one. +failure. A conversation the responder stopped — because it produced no usable reply, or because it +hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` warns about +each one by name and cause, and `aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. Those runs are weaker evidence than a completed one. ``` eval-magic ingest --skill-dir /tmp/skills --skill widget-skill --iteration 2 --harness codex diff --git a/tests/golden/opencode/runbook.golden.md b/tests/golden/opencode/runbook.golden.md index 4683741..34d83ee 100644 --- a/tests/golden/opencode/runbook.golden.md +++ b/tests/golden/opencode/runbook.golden.md @@ -21,9 +21,10 @@ each task's `conversation.json`. A task that already has one is skipped, so reru command retries only what did not finish. A task that exceeds `--timeout` is recorded as timed out rather than left to stall the campaign, and a task that fails is recorded and named while the rest of the batch continues. A conversation that stops at a scripted gate is valid eval data, not a -failure. A conversation the responder stopped — because it could not answer the agent's question, -or because it hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` -warns about each one by name, and those runs are weaker evidence than a completed one. +failure. A conversation the responder stopped — because it produced no usable reply, or because it +hit `max_turns` — is recorded too, but it ended with the task unfinished; `dispatch` warns about +each one by name and cause, and `aggregate` counts them per condition in `benchmark.json`'s +`validity_warnings`. Those runs are weaker evidence than a completed one. ``` eval-magic ingest --skill-dir /tmp/skills --skill widget-skill --iteration 2 --harness opencode diff --git a/tests/run/conversation.rs b/tests/run/conversation.rs index 0d3dd2d..fdf87fe 100644 --- a/tests/run/conversation.rs +++ b/tests/run/conversation.rs @@ -9,6 +9,7 @@ use std::path::Path; mod dispatch; mod responder; +mod responder_guards; #[test] fn multi_turn_eval_dispatch_records_followups_and_conversation_artifact_path() { diff --git a/tests/run/conversation/responder.rs b/tests/run/conversation/responder.rs index 2d9f3ac..a802c82 100644 --- a/tests/run/conversation/responder.rs +++ b/tests/run/conversation/responder.rs @@ -1,7 +1,13 @@ -//! Conversations driven by the heuristic responder rather than a script. +//! Conversations the LLM responder drives to an end. //! -//! Each test swaps the frozen descriptor's dispatch templates for a POSIX stub -//! that answers differently per round, the way every driver test here does. +//! The guardrails around what it is shown and what it is allowed to say live +//! next door, in `responder_guards`; the stub and the scaffolding both files +//! share are here. +//! +//! Each test swaps the frozen descriptor's dispatch templates for a POSIX stub. +//! The same `exec_template` runs both the agent's first round and every +//! responder consultation, so the stub tells them apart the only way the runner +//! does: by the prompt file it is pointed at. use super::{dispatch_one, stub_exec_template}; use crate::helpers::*; @@ -11,7 +17,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// An evals config whose single eval is driven by the responder. -fn responder_evals(max_turns: Option) -> String { +pub(super) fn responder_evals(max_turns: Option) -> String { let bound = match max_turns { Some(turns) => format!(", \"max_turns\": {turns}"), None => String::new(), @@ -22,15 +28,15 @@ fn responder_evals(max_turns: Option) -> String { "evals": [{{ "id": "caching", "prompt": "Requests to the pricing API are slow. Add caching.", - "expected_output": "caching is in place", - "responder": {{ "type": "heuristic"{bound} }} + "expected_output": "a working cache keyed on the pricing endpoint", + "responder": {{ "type": "llm"{bound} }} }}] }}"# ) } /// Prepare a responder-driven iteration against the codex harness. -fn prepare(skill_dir: &Path, cwd: &Path) { +pub(super) fn prepare(skill_dir: &Path, cwd: &Path) { skill_eval() .current_dir(cwd) .args(["run", "--skill-dir"]) @@ -43,21 +49,41 @@ fn prepare(skill_dir: &Path, cwd: &Path) { "--harness", "codex", "--no-guard", + "--responder-model", + "test-responder-model", ]) .assert() .success(); } -/// A stub emitting `$2` as its agent message for every round, plus the session -/// id and usage events a transcript needs to parse. Written as a POSIX script -/// and invoked through `sh`, because that is the shape of a real exec template. +/// A stub standing in for both agents. Pointed at a responder prompt it copies +/// the canned verdict for that round into the consultation's output directory; +/// pointed at a task prompt it emits `$3` as the agent's message, plus the +/// session id and usage events a transcript needs to parse. Two canned bodies +/// are sentinels rather than verdicts: `EXIT-NONZERO` fails the dispatch, and +/// `NO-WRITE` succeeds while writing nothing. fn stub(dir: &Path, name: &str) -> PathBuf { let script = dir.join(name); fs::write( &script, r#"#!/bin/sh outputs=$1 -message=$2 +prompt_path=$2 +message=$3 +verdicts=$4 +case "$prompt_path" in + */responder/*) + round=$(basename "$outputs" | sed 's/^turn-//') + file="$verdicts/$round.json" + [ -f "$file" ] || file="$verdicts/default.json" + case "$(cat "$file")" in + EXIT-NONZERO) exit 3 ;; + NO-WRITE) exit 0 ;; + esac + cat "$file" > "$outputs/verdict.json" + exit 0 + ;; +esac printf '%s\n' '{"type":"thread.started","thread_id":"session-1"}' > "$outputs/codex-events.jsonl" printf '%s' '{"type":"item.completed","item":{"id":"m1","type":"agent_message","text":"' >> "$outputs/codex-events.jsonl" printf '%s' "$message" >> "$outputs/codex-events.jsonl" @@ -69,52 +95,84 @@ printf '%s\n' '{"type":"turn.completed","usage":{"input_tokens":2,"output_tokens script } -/// Wire an initial message and a resume message into the frozen descriptor. -fn stub_rounds(tmp: &Path, cwd: &Path, initial: &str, resumed: &str) { +/// Wire the agent's two messages and a directory of canned verdicts into the +/// frozen descriptor. `verdicts` maps a consultation round to the verdict file +/// the responder "writes"; `default` covers every round without one. +pub(super) fn stub_rounds( + tmp: &Path, + cwd: &Path, + initial: &str, + resumed: &str, + verdicts: &[(&str, &str)], +) -> PathBuf { let script = stub(tmp, "fake-codex.sh"); let quoted = script.to_string_lossy().to_string(); + + let verdict_dir = tmp.join("verdicts"); + fs::create_dir_all(&verdict_dir).unwrap(); + for (round, body) in verdicts { + fs::write(verdict_dir.join(format!("{round}.json")), body).unwrap(); + } + let verdict_dir_quoted = verdict_dir.to_string_lossy().to_string(); + stub_exec_template( cwd, - &format!("sh \"{quoted}\" \"{initial}\" "), + &format!( + "sh \"{quoted}\" \"{initial}\" \"{verdict_dir_quoted}\" " + ), ); let dispatch_path = iteration_dir(cwd).join("dispatch.json"); let mut dispatch = read_json(&dispatch_path); - dispatch["harness_descriptor"]["conversation"]["resume_exec_template"] = - serde_json::json!(format!( - "sh \"{quoted}\" \"{resumed}\" {{session_arg}} {{prompt_arg}}" - )); + dispatch["harness_descriptor"]["conversation"]["resume_exec_template"] = serde_json::json!( + format!( + "sh \"{quoted}\" \"{resumed}\" \"{verdict_dir_quoted}\" {{session_arg}} {{prompt_arg}}" + ) + ); fs::write( &dispatch_path, format!("{}\n", serde_json::to_string_pretty(&dispatch).unwrap()), ) .unwrap(); + verdict_dir +} + +pub(super) const ANSWER: &str = r#"{"verdict":"answer","reply":"An in-process LRU is fine.","rationale":"the simplest option that needs no new service"}"#; +pub(super) const DONE: &str = + r#"{"verdict":"done","rationale":"the agent reported the cache in place and asked nothing"}"#; + +pub(super) fn conversation_of(cwd: &Path, task: usize) -> serde_json::Value { + let dispatch = read_json(&iteration_dir(cwd).join("dispatch.json")); + let path = dispatch["tasks"][task]["conversation_path"] + .as_str() + .unwrap() + .to_string(); + read_json(Path::new(&path)) } -/// The acceptance criterion from the ticket: a responder eval with no scripted -/// turns runs to completion, and the recommended option is both selected and -/// recorded as the reason it was selected. +/// The acceptance criterion from the ticket: a free-form question the old +/// heuristic could not classify is answered, and the run continues to +/// completion. #[test] -fn a_responder_eval_answers_a_recommended_option_and_completes() { +fn a_free_form_question_is_answered_and_the_run_completes() { let tmp = tempfile::TempDir::new().unwrap(); let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); prepare(&skill_dir, &cwd); stub_rounds( tmp.path(), &cwd, - "Which cache should I use?\\n\\n- In-process LRU (Recommended)\\n- Redis\\n", + "What should happen to rows with a null created_at?", "Caching is in place and the endpoint is under 40ms.", + &[("1", ANSWER), ("2", DONE)], ); dispatch_one(&skill_dir, &cwd, "codex", 0, false) .assert() .success(); - let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); - let task = &dispatch["tasks"][0]; - let conversation = read_json(Path::new(task["conversation_path"].as_str().unwrap())); - + let conversation = conversation_of(&cwd, 0); assert_eq!(conversation["status"], "completed", "{conversation}"); assert_eq!(conversation["delivered_followups"], 1); + let synthesized = conversation["events"] .as_array() .unwrap() @@ -122,17 +180,14 @@ fn a_responder_eval_answers_a_recommended_option_and_completes() { .filter(|event| event["type"] == "user_message") .nth(1) .expect("the responder delivered a second user turn"); - assert_eq!(synthesized["text"], "In-process LRU"); + assert_eq!(synthesized["text"], "An in-process LRU is fine."); assert_eq!(synthesized["round"], 2); - assert_eq!(synthesized["origin"]["responder"], "heuristic"); - assert_eq!( - synthesized["origin"]["answers"][0]["rule"], - "recommended_option" - ); + assert_eq!(synthesized["origin"]["responder"], "llm"); assert_eq!( - synthesized["origin"]["answers"][0]["question"], - "Which cache should I use?" + synthesized["origin"]["rationale"], + "the simplest option that needs no new service" ); + assert_eq!(conversation["responder_outcome"]["ending"], "done"); } /// The opening prompt is authored, not derived, so it carries no origin. The @@ -142,17 +197,20 @@ fn the_opening_prompt_carries_no_responder_origin() { let tmp = tempfile::TempDir::new().unwrap(); let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); prepare(&skill_dir, &cwd); - stub_rounds(tmp.path(), &cwd, "Done, caching is in place.", "unused"); + stub_rounds( + tmp.path(), + &cwd, + "Done, caching is in place.", + "unused", + &[("default", DONE)], + ); dispatch_one(&skill_dir, &cwd, "codex", 0, false) .assert() .success(); - let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); - let conversation = read_json(Path::new( - dispatch["tasks"][0]["conversation_path"].as_str().unwrap(), - )); - assert_eq!(conversation["status"], "completed"); + let conversation = conversation_of(&cwd, 0); + assert_eq!(conversation["status"], "completed", "{conversation}"); assert_eq!(conversation["delivered_followups"], 0); assert!( conversation["events"][0]["origin"].is_null(), @@ -167,8 +225,24 @@ fn a_responder_run_that_reaches_max_turns_is_recorded_not_failed() { let tmp = tempfile::TempDir::new().unwrap(); let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(Some(2))); prepare(&skill_dir, &cwd); - let asking = "Which cache should I use?\\n\\n- In-process LRU (Recommended)\\n- Redis\\n"; - stub_rounds(tmp.path(), &cwd, asking, asking); + let asking = "Which cache should I use?"; + stub_rounds( + tmp.path(), + &cwd, + asking, + asking, + &[ + ("1", ANSWER), + ( + "2", + r#"{"verdict":"answer","reply":"Redis is fine too.","rationale":"still asking"}"#, + ), + ( + "3", + r#"{"verdict":"answer","reply":"Whatever you prefer.","rationale":"still asking"}"#, + ), + ], + ); dispatch_one(&skill_dir, &cwd, "codex", 0, false) .assert() @@ -183,46 +257,14 @@ fn a_responder_run_that_reaches_max_turns_is_recorded_not_failed() { assert_eq!(conversation["delivered_followups"], 2); assert_eq!(conversation["stopped_before_followup"], 3); assert!( - !Path::new(task["outputs_dir"].as_str().unwrap()) - .join("turn-4") - .exists(), - "the bound is the last round dispatched" - ); -} - -/// The greppable branch the LLM responder will take over. It stops the run -/// rather than inventing an answer, and says so loudly — a conversation that -/// ended mid-task must not be mistaken for a clean data point. -#[test] -fn a_question_the_responder_cannot_classify_stops_the_run() { - let tmp = tempfile::TempDir::new().unwrap(); - let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); - prepare(&skill_dir, &cwd); - stub_rounds( - tmp.path(), - &cwd, - "What should happen to rows with a null created_at?", - "unused", + conversation["responder_outcome"].is_null(), + "the bound is the runner's decision, not a verdict: {conversation}" ); - - dispatch_one(&skill_dir, &cwd, "codex", 0, false) - .assert() - .success() - .stderr(contains("could not answer")); - - let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); - let task = &dispatch["tasks"][0]; - let conversation = read_json(Path::new(task["conversation_path"].as_str().unwrap())); - - assert_eq!(conversation["status"], "stopped", "{conversation}"); - assert_eq!(conversation["stop_reason"], "responder_cannot_answer"); - assert_eq!(conversation["delivered_followups"], 0); - assert_eq!(conversation["stopped_before_followup"], 1); assert!( !Path::new(task["outputs_dir"].as_str().unwrap()) - .join("turn-2") + .join("turn-4") .exists(), - "an unanswerable question delivers no turn" + "the bound is the last round dispatched" ); } @@ -266,6 +308,21 @@ exec_template = "cool-cli run --cd {model_arg} ); } +/// The responder is a second model in the attribution picture, so the run has +/// to say which one answered. +#[test] +fn the_responder_model_is_recorded_as_run_provenance() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + + let conditions = read_json(&iteration_dir(&cwd).join("conditions.json")); + assert_eq!(conditions["responder_model"], "test-responder-model"); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + assert_eq!(dispatch["responder_model"], "test-responder-model"); +} + /// Mode B parity: a revision run drives a responder conversation the same way a /// new-skill run does, against the snapshot/promote path. #[test] @@ -298,8 +355,9 @@ fn revision_mode_runs_a_responder_eval() { stub_rounds( tmp.path(), &cwd, - "Which cache should I use?\\n\\n- In-process LRU (Recommended)\\n- Redis\\n", + "Which cache should I use?", "Caching is in place.", + &[("1", ANSWER), ("2", DONE)], ); skill_eval() @@ -325,7 +383,7 @@ fn revision_mode_runs_a_responder_eval() { assert_eq!(conversation["delivered_followups"], 1); } assert_eq!( - dispatch["tasks"][0]["responder"]["type"], "heuristic", + dispatch["tasks"][0]["responder"]["type"], "llm", "the plan records how the conversation was driven" ); } diff --git a/tests/run/conversation/responder_guards.rs b/tests/run/conversation/responder_guards.rs new file mode 100644 index 0000000..93cdc53 --- /dev/null +++ b/tests/run/conversation/responder_guards.rs @@ -0,0 +1,216 @@ +//! What the responder is shown, and what it is never allowed to deliver. +//! +//! The scaffolding is `responder`'s: these tests drive the same stub, and vary +//! only the verdict it writes. + +use super::dispatch_one; +use super::responder::{ANSWER, DONE, conversation_of, prepare, responder_evals, stub_rounds}; +use crate::helpers::*; +use predicates::prelude::PredicateBooleanExt; +use predicates::str::contains; +use std::fs; +use std::path::Path; + +/// The responder may only tell the agent what the agent already knows. The +/// eval's `expected_output` is the grading criterion, and a responder that had +/// read it could hand the agent the rubric. +#[test] +fn the_consultation_prompt_withholds_the_grading_criteria() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + stub_rounds( + tmp.path(), + &cwd, + "Which cache should I use?", + "Caching is in place.", + &[("1", ANSWER), ("2", DONE)], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success(); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let responder_dir = dispatch["tasks"][0]["responder_dir"] + .as_str() + .expect("a responder task records where its consultations live") + .to_string(); + let prompt = fs::read_to_string(Path::new(&responder_dir).join("turn-1").join("prompt.txt")) + .expect("the first consultation wrote its prompt"); + + assert!(prompt.contains("Requests to the pricing API are slow.")); + assert!(prompt.contains("Which cache should I use?")); + assert!( + !prompt.contains("a working cache keyed on the pricing endpoint"), + "the grading criterion must not reach the responder: {prompt}" + ); +} + +/// A responder that honestly cannot answer stops the run rather than inventing +/// a reply, and the cause distinguishes the refusal from a broken dispatch. +#[test] +fn a_declined_question_stops_the_run() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + stub_rounds( + tmp.path(), + &cwd, + "Which production credential should I use?", + "unused", + &[( + "default", + r#"{"verdict":"cannot_answer","rationale":"it asked for a credential I was never given"}"#, + )], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success() + .stderr(contains("could not answer").and(contains("declined"))); + + let conversation = conversation_of(&cwd, 0); + assert_eq!(conversation["status"], "stopped", "{conversation}"); + assert_eq!(conversation["stop_reason"], "responder_cannot_answer"); + assert_eq!(conversation["responder_outcome"]["cause"], "declined"); + assert_eq!(conversation["delivered_followups"], 0); + assert_eq!(conversation["stopped_before_followup"], 1); +} + +/// A consultation that fails is a stop, not a failed run: `dispatch` still +/// exits zero and the artifact is still written, so the campaign keeps going +/// and the cause says what broke. +#[test] +fn a_failed_consultation_stops_the_run_without_failing_the_dispatch() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + stub_rounds( + tmp.path(), + &cwd, + "Which cache should I use?", + "unused", + &[("default", "EXIT-NONZERO")], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success() + .stdout(contains("1 stopped")) + .stderr(contains("dispatch_failed")); + + let conversation = conversation_of(&cwd, 0); + assert_eq!(conversation["status"], "stopped", "{conversation}"); + assert_eq!(conversation["stop_reason"], "responder_cannot_answer"); + assert_eq!( + conversation["responder_outcome"]["cause"], + "dispatch_failed" + ); +} + +/// A dispatch that succeeds but writes nothing leaves no reply to deliver. It +/// stops for the same reason a refusal does, with its own cause. +#[test] +fn a_consultation_that_writes_no_verdict_stops_the_run() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + stub_rounds( + tmp.path(), + &cwd, + "Which cache should I use?", + "unused", + &[("default", "")], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success(); + + let conversation = conversation_of(&cwd, 0); + assert_eq!(conversation["stop_reason"], "responder_cannot_answer"); + assert_eq!( + conversation["responder_outcome"]["cause"], + "missing_verdict" + ); +} + +/// A reply that fails validation is never delivered. The run stops loudly +/// rather than putting the responder's own work into the transcript. +#[test] +fn a_reply_carrying_code_is_never_delivered() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + stub_rounds( + tmp.path(), + &cwd, + "Which cache should I use?", + "unused", + &[( + "default", + r#"{"verdict":"answer","reply":"Use this:\n\n```rust\nlet c = Lru::new(128);\n```\n"}"#, + )], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success(); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let task = &dispatch["tasks"][0]; + let conversation = read_json(Path::new(task["conversation_path"].as_str().unwrap())); + + assert_eq!(conversation["stop_reason"], "responder_cannot_answer"); + assert_eq!( + conversation["responder_outcome"]["cause"], + "reply_contains_code" + ); + assert_eq!(conversation["delivered_followups"], 0); + assert!( + !Path::new(task["outputs_dir"].as_str().unwrap()) + .join("turn-2") + .exists(), + "a rejected reply delivers no turn" + ); +} + +/// A rerun must consult afresh. Reusing the verdict a previous dispatch left +/// on disk would answer this run's agent with a reply written about a different +/// conversation — the silent contamination the responder exists to avoid. +#[test] +fn a_rerun_does_not_reuse_the_previous_dispatch_verdict() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), &responder_evals(None)); + prepare(&skill_dir, &cwd); + let verdicts = stub_rounds( + tmp.path(), + &cwd, + "Which cache should I use?", + "Caching is in place.", + &[("1", ANSWER), ("2", DONE)], + ); + + dispatch_one(&skill_dir, &cwd, "codex", 0, false) + .assert() + .success(); + assert_eq!(conversation_of(&cwd, 0)["status"], "completed"); + + // The responder now writes nothing at all. Its previous verdict is still on + // disk, and must not be read as this run's answer. + fs::write(verdicts.join("1.json"), "NO-WRITE").unwrap(); + fs::write(verdicts.join("2.json"), "NO-WRITE").unwrap(); + + dispatch_one(&skill_dir, &cwd, "codex", 0, true) + .assert() + .success(); + + let conversation = conversation_of(&cwd, 0); + assert_eq!(conversation["status"], "stopped", "{conversation}"); + assert_eq!( + conversation["responder_outcome"]["cause"], "missing_verdict", + "{conversation}" + ); + assert_eq!(conversation["delivered_followups"], 0); +}