From 620348343fd2355396183aeaf8e55ea294d297ba Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Wed, 19 Aug 2026 15:25:54 +0530 Subject: [PATCH 1/2] feat(adaptive): triage an errand instead of authoring a one-off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some goals are not a procedure. "How much disk is this directory using" has nothing in it worth writing down, and the loop had no way to say so: selection declined, authoring paid a large planning call to produce a one-step graph, and `keep` then filed that graph where it dilutes every later selection with a row that matches once and never again. `select` gains a third answer. It costs nothing where it fires — the call was already being made — and `Approach::Errand` is deliberately the narrowest of the three: judged like any other attempt, but kept by nothing, repaired by nothing, and signed with a constant so a second errand in one episode is visibly a repeat. Three things were harder than they looked. The prompt has to separate "short" from "no procedure in it". A one-step workflow can be the most reused thing on the shelf, so brevity is not the test and the guidance says so with examples in both directions — without it the flag quietly eats the shelf. `select` short-circuited on an empty shelf, on the reasoning that with nothing to choose from the answer could only be "none". That stopped being true the moment there was a third answer, and a cold store is exactly where a trivial goal is most likely — so the short-circuit would have made the errand path unreachable where it pays most while every test of the *answer* still passed. It now returns early only when there is also no errand to offer. The cost is real and is stated in the test that used to assert the opposite: a cold-shelf episode that is not an errand pays one small triage call, against saving a full authoring call and its run whenever it is. The one-turn graph goes through `recipe::lower` rather than being built by hand. Hand-building two nodes looks simpler and would be a second, unexercised definition of what an `ask` compiles to — which is how the `item.json.text` envelope bug got in. Also: one errand per episode, enforced from the exclusion list rather than from the prompt, because "you already tried that" is the instruction a model talks itself out of on attempt three; and a plain satisfied errand skips consolidation, since paying a consolidator to be told a trivial goal taught nothing gives most of the saving back. Both new guards are falsified — restoring the old short-circuit fails the cold-shelf test, and dropping the spent-errand check fails the escalation test. Co-Authored-By: Claude Fable 5 --- crates/adaptive/src/closing/consolidate.rs | 53 +++++ crates/adaptive/src/closing/mod.rs | 9 +- crates/adaptive/src/contracts.rs | 57 +++++ crates/adaptive/src/driver.rs | 7 +- crates/adaptive/src/intake/mod.rs | 59 ++++- crates/adaptive/src/intake/recipe.rs | 27 +++ crates/adaptive/src/intake/recipe_tests.rs | 59 +++++ crates/adaptive/src/intake/select.rs | 249 +++++++++++++++++++-- crates/adaptive/tests/driver.rs | 149 ++++++++++++ crates/adaptive/tests/intake.rs | 140 +++++++++--- 10 files changed, 754 insertions(+), 55 deletions(-) diff --git a/crates/adaptive/src/closing/consolidate.rs b/crates/adaptive/src/closing/consolidate.rs index 2cb4a60..d840be1 100644 --- a/crates/adaptive/src/closing/consolidate.rs +++ b/crates/adaptive/src/closing/consolidate.rs @@ -77,6 +77,9 @@ pub async fn consolidate( if rows.is_empty() { return Vec::new(); } + if was_a_plain_errand(satisfied, &rows) { + return Vec::new(); + } // Everything already stored, not a retrieval view. Retrieval answers "what // applies to this task" and cuts by help rate, so a lesson written moments // ago — nothing has had the chance to apply it — sorts last and is dropped. @@ -121,6 +124,24 @@ pub async fn consolidate( kept } +/// An episode that was one errand, and worked. +/// +/// The only shape worth skipping, and the test is deliberately narrow. An +/// errand is a goal with no procedure in it, answered in a turn — there is +/// nothing there for a different task to act on, and asking costs a call to be +/// told so. That is the whole economic case for the errand path, and paying a +/// consolidation call on every trivial goal would give most of it straight back. +/// +/// Every other shape still consolidates, including the two that look similar: +/// +/// * **an errand that failed** — the most informative errand there is. Something +/// read as one turn of work and was not, and *that* generalises. +/// * **an errand followed by a plan** — more than one row, so the trail is a +/// real one and worth reading whole. +fn was_a_plain_errand(satisfied: bool, rows: &[LedgerRow]) -> bool { + satisfied && rows.len() == 1 && rows[0].approach_sig == "errand" +} + /// One line per attempt, numbered, because the model cites rows by number. fn render(goal: &Goal, satisfied: bool, rows: &[LedgerRow], existing: &[Lesson]) -> String { let attempts = rows @@ -309,4 +330,36 @@ mod tests { assert!(rendered.contains("- L7:"), "{rendered}"); assert!(rendered.contains("corroborate by id"), "{rendered}"); } + + #[test] + fn a_satisfied_one_turn_errand_is_not_worth_a_consolidation_call() { + // The economics the errand path exists for. Three calls become four if + // every trivial goal still pays a consolidator to be told there was + // nothing in it. + assert!(was_a_plain_errand(true, &[row("r1", "errand")])); + } + + #[test] + fn an_errand_that_failed_is_the_most_informative_kind_there_is() { + // Something read as one turn of work and was not. That generalises, + // and it is exactly what the triage needs told back to it. + assert!(!was_a_plain_errand(false, &[row("r1", "errand")])); + } + + #[test] + fn an_errand_followed_by_a_real_plan_still_consolidates() { + // More than one row means a real trail, whatever the first row was. + assert!(!was_a_plain_errand( + true, + &[row("r1", "errand"), row("r2", "authored:abc")] + )); + } + + #[test] + fn an_ordinary_satisfied_episode_is_untouched_by_the_gate() { + // The gate must be narrow: one wrong `true` here silently stops the + // whole loop learning, and nothing downstream would report it. + assert!(!was_a_plain_errand(true, &[row("r1", "selected:weekly")])); + assert!(!was_a_plain_errand(true, &[row("r1", "authored:abc")])); + } } diff --git a/crates/adaptive/src/closing/mod.rs b/crates/adaptive/src/closing/mod.rs index 8535a58..8f789ae 100644 --- a/crates/adaptive/src/closing/mod.rs +++ b/crates/adaptive/src/closing/mod.rs @@ -158,7 +158,10 @@ pub async fn close( // scoring its parent instead would leave the two indistinguishable and // the promotion gate with nothing to compare. Approach::Selected { workflow_id, .. } => Some(workflow_id.clone()), - Approach::Authored { .. } => None, + // Neither has a stored procedure behind it, so there is no counter to + // move. An errand additionally has nothing to *become* one: the row it + // leaves is the whole record of it. + Approach::Authored { .. } | Approach::Errand { .. } => None, }; let row_id = ledger .append(&LedgerRow { @@ -265,7 +268,9 @@ fn decide_next(verdict: &Verdict, attempt: u32, stalled: u32, budget: &Budget) - fn why(approach: &Approach) -> String { match approach { - Approach::Selected { why, .. } | Approach::Authored { why, .. } => why.clone(), + Approach::Selected { why, .. } + | Approach::Authored { why, .. } + | Approach::Errand { why } => why.clone(), } } diff --git a/crates/adaptive/src/contracts.rs b/crates/adaptive/src/contracts.rs index 934102a..d29a532 100644 --- a/crates/adaptive/src/contracts.rs +++ b/crates/adaptive/src/contracts.rs @@ -264,6 +264,25 @@ pub enum Approach { /// and makes an identical re-author visible as the repeat it is. fingerprint: String, }, + /// One turn of work with no procedure in it. + /// + /// The third answer to "does anything stored do this", and the one that + /// says the question was wrong: some goals are not a procedure at all. + /// "What is the disk usage of this directory" has nothing in it worth + /// writing down, and putting it through authoring pays a large planning + /// call to produce a one-step graph, then files that graph where it dilutes + /// every later selection. + /// + /// It is deliberately the *narrowest* of the three. An errand is judged + /// like any other attempt and can fail; what it cannot do is leave anything + /// behind. Nothing is kept ([`crate::closing::keep`] takes only authored + /// graphs), nothing is repaired (there is no procedure to vary), and the + /// signature is a constant so a second errand inside one episode is visibly + /// a repeat — if one turn did not do it, the goal was not an errand. + Errand { + /// Why no stored workflow was needed and none is worth writing. + why: String, + }, } impl Approach { @@ -277,6 +296,13 @@ impl Approach { match self { Self::Selected { workflow_id, .. } => format!("selected:{workflow_id}"), Self::Authored { fingerprint, .. } => format!("authored:{fingerprint}"), + // No discriminator, on purpose. Two authored attempts are told + // apart by their fingerprints because the second may be a genuinely + // different graph; two errands cannot be, because an errand carries + // no plan to differ in. Signing them the same is what makes the + // second one read as the repeat it is — and what lets `decide` stop + // offering the option once it has been spent. + Self::Errand { .. } => "errand".to_string(), } } } @@ -285,6 +311,37 @@ impl Approach { mod tests { use super::*; + #[test] + fn two_errands_in_one_episode_are_visibly_the_same_attempt() { + // Unlike two authored graphs, which may genuinely differ and are told + // apart by their fingerprints. An errand carries no plan to differ in, + // so the constant signature is what puts it in the exclusion list and + // stops an episode spending its budget on identical single turns. + let first = Approach::Errand { + why: "one turn of work".into(), + }; + let second = Approach::Errand { + why: "still one turn, honestly".into(), + }; + assert_eq!(first.signature(), "errand"); + assert_eq!(first.signature(), second.signature()); + } + + #[test] + fn an_errand_cannot_collide_with_a_stored_workflow_called_errand() { + // The namespacing that makes the constant safe: `selected:` prefixes + // every workflow id, so no shelf entry can occupy the errand slot. + let selected = Approach::Selected { + workflow_id: "errand".into(), + why: String::new(), + }; + assert_eq!(selected.signature(), "selected:errand"); + assert_ne!( + selected.signature(), + Approach::Errand { why: String::new() }.signature() + ); + } + #[test] fn an_unrecognised_blocker_is_continuable_rather_than_terminal() { // `goal_not_meet` — one letter — used to end a run at attempt 3 of 12. diff --git a/crates/adaptive/src/driver.rs b/crates/adaptive/src/driver.rs index 320ac0a..057d588 100644 --- a/crates/adaptive/src/driver.rs +++ b/crates/adaptive/src/driver.rs @@ -341,7 +341,12 @@ impl Loop<'_> { // and the next attempt writes another, seeing why this one fell // short. A variant of a one-off is a stored procedure nobody asked // for. - Approach::Authored { .. } => return None, + // + // An errand is the same argument at its limit — there is no + // procedure at all, only a turn that did not land. What it needs is + // a plan, and `decide` will write one: the errand is spent, so the + // next attempt cannot choose it again. + Approach::Authored { .. } | Approach::Errand { .. } => return None, }; let evidence = ran.evidence(); if !graph_is_suspect(&closed.verdict, &evidence) { diff --git a/crates/adaptive/src/intake/mod.rs b/crates/adaptive/src/intake/mod.rs index b4e2b20..0c53835 100644 --- a/crates/adaptive/src/intake/mod.rs +++ b/crates/adaptive/src/intake/mod.rs @@ -26,6 +26,7 @@ use serde_json::{Map, Value}; use tinyflows::caps::Capabilities; use tinyflows::model::WorkflowGraph; use tinyflows::store::{WorkflowStore, WorkflowSummary}; +use tinyflows::validate::validate_all; use crate::contracts::{Approach, Goal}; use crate::host::HostFacts; @@ -156,7 +157,18 @@ pub async fn decide( let shown: Vec = lessons.iter().map(|l| l.id.clone()).collect(); - if let Some(chosen) = select(goal, &candidates, &past, caps, conn).await? { + // One errand per episode. A goal answered in a turn is an errand; a goal + // that took a turn and is still not done was never one, and offering the + // option again would let an episode spend its whole budget on single turns + // that each fall the same way short. + let errand_allowed = !tried.iter().any(|sig| sig == "errand"); + + if let Some(chosen) = select(goal, &candidates, &past, errand_allowed, caps, conn).await? { + // An errand names no stored workflow, so there is nothing to bind and + // nothing to load: its graph is lowered from the goal itself. + if matches!(chosen.approach, Approach::Errand { .. }) { + return errand_attempt(goal, chosen, facts, shown); + } // `select` answers with an id; the graph and the input check come from // the store. Returning the choice unbound would hand the engine an // empty graph, which compiles to nothing and reads as the work failing. @@ -178,7 +190,11 @@ pub async fn decide( Supply a value for every required input this time, or \ decline so a graph is written instead." ); - if let Some(retry) = select(goal, &candidates, ¬ed, caps, conn).await? { + // No errand on the retry: the question was asked and answered + // one call ago, and this round exists to fix a binding slip. + // Re-offering it would let a model that could not fill an input + // reach for the one answer that needs none. + if let Some(retry) = select(goal, &candidates, ¬ed, false, caps, conn).await? { match bind(retry, store) { Ok(attempt) => { return Ok(Attempt { @@ -215,6 +231,45 @@ pub async fn decide( }) } +/// Finish an errand: lower the one-step graph and hold it to the same gates. +/// +/// Both gates matter here even though no model wrote the graph. `validate_all` +/// is free and catches a lowering that stopped producing a runnable shape. +/// `HostFacts::check` is the one that earns its place: an errand is an agent +/// turn, and a host with no agent capability must refuse it as +/// [`Unsupported`](IntakeError::Unsupported) — the same answer authoring would +/// give — rather than emit a graph the runner will fail on and the judge will +/// then blame on the work. +fn errand_attempt( + goal: &Goal, + chosen: Attempt, + facts: &HostFacts, + shown: Vec, +) -> Result { + let graph = recipe::errand(&goal.text)?; + + let problems = validate_all(&graph); + if !problems.is_empty() { + return Err(IntakeError::Invalid( + problems + .iter() + .map(ToString::to_string) + .collect::>() + .join("; "), + )); + } + let refused = facts.check(&graph); + if !refused.is_empty() { + return Err(IntakeError::Unsupported(refused.join("; "))); + } + + Ok(Attempt { + graph, + lessons_shown: shown, + ..chosen + }) +} + /// The stored workflows worth offering, with what is known about each. /// /// Three filters, each removing something a planner must not be shown: diff --git a/crates/adaptive/src/intake/recipe.rs b/crates/adaptive/src/intake/recipe.rs index bb4971e..6a7654e 100644 --- a/crates/adaptive/src/intake/recipe.rs +++ b/crates/adaptive/src/intake/recipe.rs @@ -300,6 +300,33 @@ pub fn lower( Ok((graph, inputs, why)) } +/// The one-step graph an [`Errand`](crate::contracts::Approach::Errand) runs. +/// +/// Built by handing [`lower`] a recipe nobody wrote, rather than assembling +/// nodes directly. The graph is trivial enough that hand-building it would look +/// like the simpler option, and that is the trap: it would be a second, +/// unexercised definition of what an `ask` step compiles to, and the first +/// change to the envelope path or the trigger node would leave errands quietly +/// producing a shape the rest of the loop no longer reads. Going through the +/// same door as an authored plan costs one `json!` and cannot drift. +/// +/// Deterministic in the goal alone — no model call. The whole economic argument +/// for the errand path is that recognising one is free once `select` has +/// answered, so lowering it must not spend anything either. +/// +/// # Errors +/// Only if `lower` refuses this recipe, which would mean the shared lowering +/// path had stopped accepting a bare `ask` step. +pub(crate) fn errand(goal: &str) -> Result { + let recipe = json!({ + "why": "one turn of work, no procedure in it", + "declared": [], + "inputs": {}, + "steps": [{ "id": "errand", "ask": goal.trim() }], + }); + lower(&recipe, &[]).map(|(graph, _, _)| graph) +} + /// Ask steps that restate a declared value instead of relying on the /// attachment. Only distinctive values count — refusing a plan because an /// ask contains the word "on" would block perfectly reusable recipes — and diff --git a/crates/adaptive/src/intake/recipe_tests.rs b/crates/adaptive/src/intake/recipe_tests.rs index e31ac96..0a6ce79 100644 --- a/crates/adaptive/src/intake/recipe_tests.rs +++ b/crates/adaptive/src/intake/recipe_tests.rs @@ -691,3 +691,62 @@ fn a_scripts_stdout_is_read_from_inside_its_json_because_that_is_where_it_is() { "declared inputs too: {rendered}" ); } + +#[test] +fn an_errand_lowers_to_one_agent_turn_that_validates() { + let graph = super::errand("how much disk is this directory using").expect("lowers"); + + assert!( + validate_all(&graph).is_empty(), + "the engine must accept it: {:?}", + validate_all(&graph) + ); + // A trigger and exactly one agent node. Anything more means the errand + // grew a procedure, which is the one thing it is defined as not having. + assert_eq!(graph.nodes.len(), 2, "{:?}", graph.nodes); + assert_eq!(graph.nodes[0].kind, NodeKind::Trigger); + assert_eq!(graph.nodes[1].kind, NodeKind::Agent); + assert!( + graph.inputs.is_empty(), + "an errand declares nothing: it is answered from the goal alone" + ); +} + +#[test] +fn an_errands_prompt_actually_carries_the_goal() { + // Evaluated, not string-matched. The `item.json.text` defect shipped past a + // reviewer *and* a test because both read the expression instead of running + // it — a prompt that resolves to nothing looks fine as source. + let graph = super::errand(" how much disk is this directory using ").expect("lowers"); + let prompt = graph.nodes[1].config["prompt"] + .as_str() + .expect("the agent node carries a prompt"); + let scope = json!({ + "run": { "inputs": {} }, "inputs": {}, + "item": null, "items": [], "nodes": {} + }); + let rendered = tinyflows::expr::resolve(&json!(prompt), &scope); + let rendered = rendered.as_str().expect("resolves to a string"); + assert_eq!( + rendered, "how much disk is this directory using", + "the goal, trimmed, and nothing else bolted on" + ); +} + +#[test] +fn an_errand_is_the_same_lowering_an_authored_ask_gets() { + // Why `errand` goes through `lower` rather than building two nodes by hand: + // a second definition of what an `ask` compiles to would drift silently the + // first time the envelope path changed. + let errand = super::errand("say something").expect("lowers"); + let (authored, _, _) = lower( + &json!({ + "why": "one turn of work, no procedure in it", + "declared": [], "inputs": {}, + "steps": [{ "id": "errand", "ask": "say something" }] + }), + &[], + ) + .expect("lowers"); + assert_eq!(errand.nodes[1].config, authored.nodes[1].config); +} diff --git a/crates/adaptive/src/intake/select.rs b/crates/adaptive/src/intake/select.rs index 2a5304b..43ba95a 100644 --- a/crates/adaptive/src/intake/select.rs +++ b/crates/adaptive/src/intake/select.rs @@ -76,9 +76,11 @@ impl Candidate { const SYSTEM: &str = "\ You choose whether a saved workflow already does what a goal asks for. -Return JSON: {\"workflow_id\": str | null, \"why\": str, \"inputs\": {name: value}} +Return JSON: {\"workflow_id\": str | null, \"errand\": bool, \"why\": str, + \"inputs\": {name: value}} - workflow_id: the id of the workflow that does this, or null. +- errand: true only when the goal is one turn of work with no procedure in it. - why: one line. When you decline, say what is missing — it is read by whoever writes the replacement. - inputs: values for that workflow's declared inputs, taken from the goal. Only @@ -95,12 +97,37 @@ when it plainly matches; it just carries no evidence. When this episode has already tried something, decline rather than choose a workflow that would fall short the same way. Being told a second time that the -report has no numbers in it costs a full run and establishes nothing."; +report has no numbers in it costs a full run and establishes nothing. + +Set errand only when there is no procedure in the goal — one turn of work, +answered and finished, with nothing a later goal would want to reuse. Ask +whether you would want this written down and offered as a choice next month. + + errand \"how much disk is this directory using\" + errand \"what did the last commit change\" + NOT \"summarise a paper into three bullets\" — one step, and exactly the + kind of thing worth having on the shelf + NOT \"check the PR and fix whatever CI says\" — one sentence, many turns + +Short is not the test, and a single step is not the test: a one-step procedure +can be the most reused thing here. The test is whether a *procedure* exists. + +An errand is not an escape from a hard goal. Anything that needs several turns, +or that could fail in a way worth retrying differently, is not one — say so and +decline instead, so a graph gets written."; /// Ask whether any candidate does the job, and bind its inputs if one does. /// /// `Ok(None)` means nothing fitted — the ordinary case on a cold store, and the -/// caller's cue to author. +/// caller's cue to author. `Ok(Some)` is either a +/// [`Selected`](Approach::Selected) whose graph the caller loads from the store, +/// or an [`Errand`](Approach::Errand) whose graph the caller lowers; both come +/// back with an empty graph, because what fills it is not this function's job. +/// +/// `errand_allowed` is false once this episode has already spent its errand — +/// see [`Approach::signature`]. Withholding the option is structural rather +/// than left to the prompt, because "you already tried that" is exactly the +/// instruction a model talks itself out of on attempt three. /// /// # Errors /// When inference fails, or the chosen workflow cannot be loaded or bound. @@ -108,30 +135,70 @@ pub async fn select( goal: &Goal, candidates: &[Candidate], past: &str, + errand_allowed: bool, caps: &Capabilities, conn: Option<&str>, ) -> Result> { - // Not a shortcut — a correctness point. With nothing to choose from the - // answer can only be "none", and asking costs a call to be told so. - if candidates.is_empty() { + // With nothing to choose from and no errand to offer, the answer can only + // be "none" and asking costs a call to be told so. + // + // This used to be unconditional, on that reasoning — and the reasoning + // stopped holding the moment there was a third answer. A cold store is + // precisely where a trivial goal is most likely, so short-circuiting here + // would have made the errand path unreachable exactly where it pays most, + // while looking correct. The cost is honest and worth stating: a cold-store + // episode that is *not* an errand now pays one small extra call, against + // saving a full authoring call and its run whenever it is. + if candidates.is_empty() && !errand_allowed { return Ok(None); } - let listing = candidates - .iter() - .map(Candidate::render) - .collect::>() - .join("\n"); - let user = format!( - "# Goal\n{}\n\n# Saved workflows\n{listing}{past}", - goal.text.trim() - ); + let shelf = if candidates.is_empty() { + "# Saved workflows\n(none yet — nothing to choose from, so the only \ + question is whether this is an errand)" + .to_string() + } else { + format!( + "# Saved workflows\n{}", + candidates + .iter() + .map(Candidate::render) + .collect::>() + .join("\n") + ) + }; + let spent = if errand_allowed { + String::new() + } else { + "\n\n# This episode has already spent its errand\nOne turn was tried \ + and did not finish the goal, so it is not an errand. Choose a workflow \ + or decline; `errand` will be ignored." + .to_string() + }; + let user = format!("# Goal\n{}\n\n{shelf}{past}{spent}", goal.text.trim()); let answer = ask(caps, conn, Tier::Select, SYSTEM, &user).await?; let Some(id) = answer["workflow_id"] .as_str() .filter(|s| !s.trim().is_empty()) else { + // Declining and calling it an errand are different answers, and only + // one of them skips authoring. Read second so a model that names a + // workflow *and* sets the flag is taken at its first word — the + // workflow is the more specific claim, and the more easily checked. + if errand_allowed && answer["errand"].as_bool().unwrap_or(false) { + return Ok(Some(Attempt { + approach: Approach::Errand { + why: answer["why"].as_str().unwrap_or_default().to_string(), + }, + // Lowered by the caller, which is what holds the host facts the + // one-step graph has to be checked against. + graph: WorkflowGraph::default(), + inputs: Map::new(), + resume: None, + lessons_shown: Vec::new(), + })); + } return Ok(None); }; // A model naming something that is not on the list has hallucinated an id; @@ -216,6 +283,7 @@ fn inputs_of(answer: &Value) -> Map { #[cfg(test)] mod tests { use super::*; + use serde_json::json; fn candidate(id: &str, applied: u32, helped: u32) -> Candidate { Candidate { @@ -256,6 +324,157 @@ mod tests { assert!(c.render().contains("name: only-an-id")); } + /// A model that answers with `reply` and records what it was shown. + /// + /// The call count is the point of several tests below: whether `select` + /// asks at all is a cost decision, and asserting on the *answer* would not + /// notice a version that skipped the call and returned the same thing. + struct Scripted { + reply: Value, + asked: std::sync::Mutex>, + } + + impl Scripted { + fn new(reply: Value) -> std::sync::Arc { + std::sync::Arc::new(Self { + reply, + asked: std::sync::Mutex::new(Vec::new()), + }) + } + fn calls(&self) -> usize { + self.asked.lock().expect("log").len() + } + fn last(&self) -> String { + self.asked + .lock() + .expect("log") + .last() + .cloned() + .unwrap_or_default() + } + } + + #[async_trait::async_trait] + impl tinyflows::caps::LlmProvider for Scripted { + async fn complete( + &self, + request: Value, + _conn: Option<&str>, + ) -> tinyflows::error::Result { + self.asked.lock().expect("log").push( + request["messages"][1]["content"] + .as_str() + .unwrap_or_default() + .to_string(), + ); + Ok(self.reply.clone()) + } + } + + async fn choose( + provider: &std::sync::Arc, + candidates: &[Candidate], + errand_allowed: bool, + ) -> Option { + let caps = Capabilities { + llm: provider.clone(), + ..tinyflows::caps::mock::mock_capabilities() + }; + select( + &Goal::new("how much disk is this directory using"), + candidates, + "", + errand_allowed, + &caps, + None, + ) + .await + .expect("selection answers") + } + + #[tokio::test] + async fn an_errand_answer_becomes_an_errand_approach() { + let provider = Scripted::new(json!({ + "workflow_id": null, + "errand": true, + "why": "one turn of work, no procedure in it", + })); + let chosen = choose(&provider, &[candidate("pr-review", 4, 4)], true) + .await + .expect("an errand is an answer, not a decline"); + match chosen.approach { + Approach::Errand { why } => assert!(why.contains("one turn")), + other => panic!("expected an errand, got {other:?}"), + } + } + + #[tokio::test] + async fn an_empty_shelf_is_still_asked_when_an_errand_is_possible() { + // The bug this exists to prevent, and it would have been invisible: a + // cold store is exactly where a trivial goal is most likely, so the old + // unconditional short-circuit made the errand path unreachable at the + // one moment it pays for itself — while every test of the *answer* + // still passed. + let provider = + Scripted::new(json!({ "workflow_id": null, "errand": true, "why": "trivial" })); + let chosen = choose(&provider, &[], true).await; + assert_eq!(provider.calls(), 1, "an empty shelf must still be asked"); + assert!(matches!( + chosen.map(|a| a.approach), + Some(Approach::Errand { .. }) + )); + } + + #[tokio::test] + async fn an_empty_shelf_with_no_errand_left_is_not_asked_at_all() { + // The other half: with nothing to choose from and no errand to offer, + // the answer can only be "none", and the call is pure cost. + let provider = Scripted::new(json!({ "workflow_id": null, "errand": true, "why": "x" })); + assert!(choose(&provider, &[], false).await.is_none()); + assert_eq!(provider.calls(), 0, "nothing to ask about"); + } + + #[tokio::test] + async fn a_spent_errand_is_refused_even_when_the_model_asks_for_one() { + // Prompt-only enforcement is not enforcement: attempt three is exactly + // where a model talks itself back into the answer that needs no inputs. + let provider = + Scripted::new(json!({ "workflow_id": null, "errand": true, "why": "again" })); + let chosen = choose(&provider, &[candidate("pr-review", 4, 4)], false).await; + assert!(chosen.is_none(), "a spent errand reads as a decline"); + assert!( + provider.last().contains("already spent its errand"), + "and the prompt says why: {}", + provider.last() + ); + } + + #[tokio::test] + async fn naming_a_workflow_wins_over_also_setting_the_errand_flag() { + // A contradictory answer taken at its more specific — and more easily + // checked — word, rather than at whichever field is read first. + let provider = Scripted::new(json!({ + "workflow_id": "pr-review", + "errand": true, + "why": "both", + "inputs": { "repo": "openhuman" }, + })); + let chosen = choose(&provider, &[candidate("pr-review", 4, 4)], true) + .await + .expect("an answer"); + assert!(matches!(chosen.approach, Approach::Selected { .. })); + } + + #[test] + fn the_prompt_refuses_to_treat_a_short_goal_as_an_errand() { + // The distinction the whole triage turns on. A one-step procedure can + // be the most reused thing in the store, so brevity must not be the + // test — if this guidance goes, the flag starts eating the shelf. + assert!(SYSTEM.contains("no procedure in the goal")); + assert!(SYSTEM.contains("Short is not the test")); + assert!(SYSTEM.contains("not an escape from a hard goal")); + } + #[test] fn the_prompt_tells_the_model_that_declining_is_allowed() { // The single most important line in it: a model pushed to always pick diff --git a/crates/adaptive/tests/driver.rs b/crates/adaptive/tests/driver.rs index a544f2e..0f60b3b 100644 --- a/crates/adaptive/tests/driver.rs +++ b/crates/adaptive/tests/driver.rs @@ -854,3 +854,152 @@ async fn a_failed_goal_run_leaves_no_residue_anywhere_durable() { "the attempts are on the record even though no graph was kept" ); } + +#[tokio::test] +async fn an_errand_answers_the_goal_without_leaving_a_procedure_behind() { + // The whole claim of the errand path, end to end: a goal with no procedure + // in it is answered in one turn, and the shelf is exactly as it was. + struct Triage; + #[async_trait] + impl LlmProvider for Triage { + async fn complete(&self, request: Value, _conn: Option<&str>) -> EngineResult { + Ok(match request["tier"].as_str().unwrap_or_default() { + "select" => json!({ + "workflow_id": null, "errand": true, + "why": "one turn of work, no procedure in it" + }), + "judge" => json!({ + "satisfied": true, "blocker": "", "gap": "", "advanced": true + }), + // Reached only if the loop wrongly authored or consolidated — + // both are asserted absent below. + _ => json!({ "why": "should not be asked", "inputs": {}, "steps": [] }), + }) + } + } + + let caps = Capabilities { + llm: Arc::new(Triage), + ..mock_capabilities() + }; + let ledger = MemoryLedger::new(); + let store = store("errand"); + // A workflow on the shelf, so `select` is genuinely asked rather than + // short-circuited — this test is about the answer, not about the cold-store + // path, which `select`'s own tests cover. + let seeded = store.list().expect("list").len(); + + let runner = Local { + caps: &caps, + workspace: &Unobserved, + }; + let engine = Loop { + ledger: &ledger, + store: &store, + caps: &caps, + facts: &HostFacts::unknown(), + runner: &runner, + clock: &Frozen, + budget: Default::default(), + conn: None, + }; + + let finished = engine + .run( + "ep-errand", + &Goal::new("how much disk is this directory using"), + ) + .await + .expect("the episode runs"); + + assert_eq!(finished.status, EpisodeStatus::Satisfied); + assert_eq!(finished.attempts, 1, "one turn, not a retry loop"); + + let rows = ledger.rows("ep-errand").await.expect("rows"); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].approach_sig, "errand"); + assert!( + rows[0].workflow_id.is_none(), + "an errand scores no procedure, because there is none" + ); + + // The point of the whole path: nothing filed. A one-off on the shelf is + // worse than useless — it dilutes every later selection with a row that + // matches once and can never match again. + assert_eq!( + store.list().expect("list").len(), + seeded, + "an errand must not be kept" + ); + assert!( + finished.lessons.is_empty(), + "and a plain errand skips consolidation entirely" + ); +} + +#[tokio::test] +async fn a_failed_errand_escalates_to_authoring_instead_of_repeating_itself() { + // The guard that stops the cheap path becoming a trap. If one turn did not + // do it, the goal was never an errand — and a model that keeps saying it is + // must not be able to spend the whole budget on identical single turns. + struct Insistent { + seen: Mutex>, + } + #[async_trait] + impl LlmProvider for Insistent { + async fn complete(&self, request: Value, _conn: Option<&str>) -> EngineResult { + let tier = request["tier"].as_str().unwrap_or_default().to_string(); + self.seen.lock().expect("lock").push(tier.clone()); + Ok(match tier.as_str() { + // Always insists, every attempt. + "select" => json!({ "workflow_id": null, "errand": true, "why": "trivial" }), + "judge" => json!({ + "satisfied": false, "blocker": "goal_not_met", + "gap": "it did not finish", "advanced": false + }), + "consolidate" => json!({ "lessons": [], "corroborate": [] }), + _ => json!({ + "why": "a real plan", + "inputs": {}, + "steps": [{ "id": "attempt", "run": "echo attempt-done" }], + }), + }) + } + } + let provider = Arc::new(Insistent { + seen: Mutex::new(Vec::new()), + }); + let caps = Capabilities { + llm: provider.clone(), + ..mock_capabilities() + }; + let ledger = MemoryLedger::new(); + let store = store("errand-escalate"); + let runner = Local { + caps: &caps, + workspace: &Unobserved, + }; + let engine = Loop { + ledger: &ledger, + store: &store, + caps: &caps, + facts: &HostFacts::unknown(), + runner: &runner, + clock: &Frozen, + budget: Default::default(), + conn: None, + }; + + let goal = Goal::new("do something that only looks trivial"); + engine.attempt("ep-esc", &goal).await.expect("attempt one"); + engine.attempt("ep-esc", &goal).await.expect("attempt two"); + + let rows = ledger.rows("ep-esc").await.expect("rows"); + assert_eq!(rows.len(), 2); + assert_eq!(rows[0].approach_sig, "errand"); + assert!( + rows[1].approach_sig.starts_with("authored:"), + "the second attempt must be a real plan, got {}", + rows[1].approach_sig + ); +} diff --git a/crates/adaptive/tests/intake.rs b/crates/adaptive/tests/intake.rs index 9424e00..e09d324 100644 --- a/crates/adaptive/tests/intake.rs +++ b/crates/adaptive/tests/intake.rs @@ -146,11 +146,36 @@ fn stored(id: &str, description: &str, required_input: Option<&str>) -> Workflow } } +/// A selection call that declines, scripted ahead of an authoring reply. +/// +/// Needed since the errand triage landed: `select` now has a third answer, so +/// it is asked even when the shelf is empty — the old short-circuit assumed the +/// answer could only be "none", and that stopped being true. Written into each +/// script rather than defaulted inside the harness, so a test still shows every +/// call its path makes instead of hiding one behind a lenient double. +fn select_declines() -> Value { + json!({ "workflow_id": null, "errand": false, "why": "nothing stored fits" }) +} + +/// The authoring prompt, found by what it says rather than by its position. +/// +/// Positional indexing broke the moment a call was added in front of it, and +/// would break again; the authoring system prompt is self-identifying. +fn authoring_prompt(llm: &std::sync::Arc) -> String { + llm.prompts() + .into_iter() + .find(|p| p.contains("You plan how to achieve a goal")) + .expect("the author was asked") +} + #[tokio::test] async fn an_empty_store_authors_without_asking_whether_to_select() { // With nothing to choose from the answer can only be "none". Spending a // call to be told so is the cost of every cold start. - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("fresh", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("fresh", None), + ])); let caps = caps_with(llm.clone()); let (store, _root) = empty_store("1"); let ledger = MemoryLedger::new(); @@ -168,13 +193,19 @@ async fn an_empty_store_authors_without_asking_whether_to_select() { .expect("decide"); assert!(matches!(attempt.approach, Approach::Authored { .. })); - assert_eq!( - llm.prompts().len(), - 1, - "exactly one call: the authoring one" + // Two calls, and the first one is new: a triage that costs a small call to + // ask whether this is an errand at all. It used to be one, on the reasoning + // that with nothing to choose from the answer could only be "none" — true + // until `select` gained a third answer. The trade is deliberate: a cold + // shelf is exactly where a trivial goal would otherwise pay the full + // authoring call and get a one-step graph filed for it. + assert_eq!(llm.prompts().len(), 2, "a triage call, then authoring"); + assert!( + llm.prompts()[0].contains("whether a saved workflow already does"), + "the first call is the triage" ); assert!( - llm.prompts()[0].contains("You plan how to achieve a goal"), + authoring_prompt(&llm).contains("You plan how to achieve a goal"), "authoring must speak the recipe surface, not graph syntax" ); } @@ -257,7 +288,10 @@ async fn a_workflow_already_tried_this_episode_is_not_offered_again() { // The property the whole retry edge rests on. Without it attempt two // re-selects what attempt one already failed on, and the episode pays // twice for one dead end. - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("written", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("written", None), + ])); let caps = caps_with(llm.clone()); let (store, _root) = empty_store("4"); store @@ -285,10 +319,22 @@ async fn a_workflow_already_tried_this_episode_is_not_offered_again() { matches!(attempt.approach, Approach::Authored { .. }), "the only stored workflow was excluded, so authoring is the only path left" ); - assert_eq!( - llm.prompts().len(), - 1, - "with every candidate excluded the list is empty and selection is skipped entirely" + // The triage still runs — a goal can be an errand whatever the shelf holds + // — but the excluded workflow must not appear in front of it. Asserting on + // what the chooser was *shown* is the claim this test is named for; + // asserting the call never happened only ever stood in for it. + // Precisely: absent from the *shelf*. It still appears further down, in the + // rendered history — that is the exclusion list doing its job, and asserting + // the id is absent altogether would forbid the very thing that tells the + // planner not to repeat it. + let shown = &llm.prompts()[0]; + assert!( + shown.contains("(none yet"), + "with every candidate excluded the shelf is empty: {shown}" + ); + assert!( + shown.contains("[selected:pr-review]"), + "and the history still says what was tried: {shown}" ); } @@ -447,7 +493,12 @@ async fn an_authored_graph_that_does_not_validate_is_an_error_not_a_return_value "why": "forgot the steps", "inputs": {}, }); - let llm = std::sync::Arc::new(Scripted::new(vec![broken.clone(), broken.clone(), broken])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + broken.clone(), + broken.clone(), + broken, + ])); let caps = caps_with(llm); let (store, _root) = empty_store("7"); let ledger = MemoryLedger::new(); @@ -468,7 +519,10 @@ async fn an_authored_graph_that_does_not_validate_is_an_error_not_a_return_value #[tokio::test] async fn a_disabled_workflow_is_never_offered() { - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("written", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("written", None), + ])); let caps = caps_with(llm.clone()); let (store, _root) = empty_store("8"); let mut off = stored("switched-off", "would have matched", None); @@ -488,10 +542,10 @@ async fn a_disabled_workflow_is_never_offered() { .await .expect("decide"); - assert_eq!( - llm.prompts().len(), - 1, - "offering a disabled workflow invites a choice that cannot be honoured" + assert!( + !llm.prompts()[0].contains("switched-off"), + "offering a disabled workflow invites a choice that cannot be honoured: {}", + llm.prompts()[0] ); } @@ -509,6 +563,7 @@ async fn a_graph_naming_a_worker_this_host_lacks_is_refused_before_it_runs() { "steps": [{ "id": "work", "ask": "do the thing", "worker": "desktop" }], }); let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), insistent.clone(), insistent.clone(), insistent, @@ -549,11 +604,14 @@ async fn a_graph_naming_a_worker_this_host_lacks_is_refused_before_it_runs() { async fn the_authoring_prompt_carries_what_the_host_permits() { // The facts below say agent work must name a worker, so the reply's ask // step names one — the same gate this test exists to see rendered. - let llm = std::sync::Arc::new(Scripted::new(vec![json!({ - "why": "fine", - "inputs": {}, - "steps": [{ "id": "work", "ask": "Do it directly.", "worker": "laptop" }], - })])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + json!({ + "why": "fine", + "inputs": {}, + "steps": [{ "id": "work", "ask": "Do it directly.", "worker": "laptop" }], + }), + ])); let caps = caps_with(llm.clone()); let (store, _root) = empty_store("facts-rendered"); let ledger = MemoryLedger::new(); @@ -578,7 +636,7 @@ async fn the_authoring_prompt_carries_what_the_host_permits() { .await .expect("decide"); - let prompt = &llm.prompts()[0]; + let prompt = &authoring_prompt(&llm); assert!(prompt.contains("What this host permits"), "{prompt}"); assert!(prompt.contains("every agent node must name config.agent_ref")); assert!(prompt.contains("Only manual triggers fire here.")); @@ -752,7 +810,10 @@ async fn the_author_is_shown_what_this_episode_already_tried() { // because nothing told it otherwise. The exclusion list only guards // *selection*; authoring has no structural guard at all. let (store, ledger, _root) = with_history("retry-1").await; - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("third-idea", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("third-idea", None), + ])); let caps = caps_with(llm.clone()); decide( @@ -767,7 +828,7 @@ async fn the_author_is_shown_what_this_episode_already_tried() { .await .expect("decide"); - let prompt = &llm.prompts()[0]; + let prompt = &authoring_prompt(&llm); assert!(prompt.contains("Already tried this episode"), "{prompt}"); assert!( prompt.contains("asked an agent to write it from memory"), @@ -832,7 +893,10 @@ async fn lessons_from_other_episodes_reach_the_planner() { .await .expect("promote"); - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("informed", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("informed", None), + ])); let caps = caps_with(llm.clone()); decide( @@ -847,7 +911,7 @@ async fn lessons_from_other_episodes_reach_the_planner() { .await .expect("decide"); - let prompt = &llm.prompts()[0]; + let prompt = &authoring_prompt(&llm); assert!(prompt.contains("Learned from earlier episodes"), "{prompt}"); assert!(prompt.contains("read them from the source"), "{prompt}"); } @@ -858,7 +922,10 @@ async fn a_first_attempt_is_told_nothing_it_would_have_to_ignore() { // empty "already tried" heading reads as a claim that something was. let (store, _root) = empty_store("retry-4"); let ledger = MemoryLedger::new(); - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply("first", None)])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply("first", None), + ])); let caps = caps_with(llm.clone()); decide( @@ -873,9 +940,12 @@ async fn a_first_attempt_is_told_nothing_it_would_have_to_ignore() { .await .expect("decide"); - let prompt = &llm.prompts()[0]; - assert!(!prompt.contains("Already tried"), "{prompt}"); - assert!(!prompt.contains("Learned from earlier"), "{prompt}"); + // Every prompt, not just one: an empty heading is noise whichever planner + // reads it, and the triage call sees the same rendered past the author does. + for prompt in llm.prompts() { + assert!(!prompt.contains("Already tried"), "{prompt}"); + assert!(!prompt.contains("Learned from earlier"), "{prompt}"); + } } #[tokio::test] @@ -887,10 +957,10 @@ async fn two_authored_attempts_leave_two_distinct_signatures() { let mut signatures = Vec::new(); for (n, name) in [(0, "shape-one"), (1, "shape-two")] { - let llm = std::sync::Arc::new(Scripted::new(vec![authored_reply( - name, - if n == 1 { Some("repo") } else { None }, - )])); + let llm = std::sync::Arc::new(Scripted::new(vec![ + select_declines(), + authored_reply(name, if n == 1 { Some("repo") } else { None }), + ])); let attempt = decide( &Goal::new("write the weekly report"), "ep-sigs", From 3c4e4a49d166390d4610b4566daca59a3f2ff783 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Wed, 19 Aug 2026 15:36:07 +0530 Subject: [PATCH 2/2] test(adaptive): assert the errand skipped consolidation, not that it kept nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback, and correct: `finished.lessons.is_empty()` is also what a consolidator that ran and found nothing returns, so the assertion passed whether or not the gate existed. It now reads the tiers the model was actually asked for. Removing the gate makes it fail with `["select", "", "judge", "consolidate"]`, which the old one did not. Also pins the control-character behaviour the same review raised as a suspected break. It is not one — every char in U+0000..U+001F survives `jq_quote` and resolves, because jaq's literal parser is laxer than strict JSON — but the errand path feeds raw goal text into that quoting, which is a wider door than an authored `ask`, so the property is worth holding a jaq bump to rather than rediscovering in a prompt. Co-Authored-By: Claude Fable 5 --- crates/adaptive/src/intake/recipe_tests.rs | 24 ++++++++++++++++++++ crates/adaptive/tests/driver.rs | 26 +++++++++++++++++----- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/adaptive/src/intake/recipe_tests.rs b/crates/adaptive/src/intake/recipe_tests.rs index 0a6ce79..aba79e3 100644 --- a/crates/adaptive/src/intake/recipe_tests.rs +++ b/crates/adaptive/src/intake/recipe_tests.rs @@ -750,3 +750,27 @@ fn an_errand_is_the_same_lowering_an_authored_ask_gets() { .expect("lowers"); assert_eq!(errand.nodes[1].config, authored.nodes[1].config); } + +#[test] +fn every_control_character_in_a_goal_survives_as_a_valid_jq_literal() { + let scope = json!({ "run": { "inputs": {} }, "inputs": {}, "item": null, + "items": [], "nodes": {} }); + let mut broke = Vec::new(); + for code in 0u32..0x20 { + let ch = char::from_u32(code).expect("control char"); + let goal = format!("disk{ch}usage"); + let Ok(graph) = super::errand(&goal) else { + broke.push(format!("U+{code:04X}: lowering refused it")); + continue; + }; + let prompt = graph.nodes[1].config["prompt"].as_str().expect("prompt"); + let rendered = tinyflows::expr::resolve(&json!(prompt), &scope); + if rendered.as_str().is_none() { + broke.push(format!("U+{code:04X}: {rendered:?}")); + } + } + assert!( + broke.is_empty(), + "control characters that broke the prompt: {broke:?}" + ); +} diff --git a/crates/adaptive/tests/driver.rs b/crates/adaptive/tests/driver.rs index 0f60b3b..c01df08 100644 --- a/crates/adaptive/tests/driver.rs +++ b/crates/adaptive/tests/driver.rs @@ -859,11 +859,15 @@ async fn a_failed_goal_run_leaves_no_residue_anywhere_durable() { async fn an_errand_answers_the_goal_without_leaving_a_procedure_behind() { // The whole claim of the errand path, end to end: a goal with no procedure // in it is answered in one turn, and the shelf is exactly as it was. - struct Triage; + struct Triage { + tiers: Mutex>, + } #[async_trait] impl LlmProvider for Triage { async fn complete(&self, request: Value, _conn: Option<&str>) -> EngineResult { - Ok(match request["tier"].as_str().unwrap_or_default() { + let tier = request["tier"].as_str().unwrap_or_default().to_string(); + self.tiers.lock().expect("lock").push(tier.clone()); + Ok(match tier.as_str() { "select" => json!({ "workflow_id": null, "errand": true, "why": "one turn of work, no procedure in it" @@ -878,8 +882,11 @@ async fn an_errand_answers_the_goal_without_leaving_a_procedure_behind() { } } + let provider = Arc::new(Triage { + tiers: Mutex::new(Vec::new()), + }); let caps = Capabilities { - llm: Arc::new(Triage), + llm: provider.clone(), ..mock_capabilities() }; let ledger = MemoryLedger::new(); @@ -931,10 +938,19 @@ async fn an_errand_answers_the_goal_without_leaving_a_procedure_behind() { seeded, "an errand must not be kept" ); + // Asserted on the *calls*, not on the result. An empty lesson list is also + // what a consolidator that ran and found nothing returns, so the weaker + // assertion would pass with the gate removed entirely. + let tiers = provider.tiers.lock().expect("lock").clone(); + assert!( + !tiers.iter().any(|t| t == "consolidate"), + "a plain errand must not pay a consolidation call: {tiers:?}" + ); assert!( - finished.lessons.is_empty(), - "and a plain errand skips consolidation entirely" + !tiers.iter().any(|t| t == "author"), + "nor an authoring one: {tiers:?}" ); + assert!(finished.lessons.is_empty()); } #[tokio::test]