fix(missions): a caller's own task does not inherit the recipe's done_when
A recipe's completion condition is a condition on the recipe's own task. phases_for_create merged the recipe config under the caller's, so a phase that supplied a different task and no condition inherited a condition about work it was never given: research_and_code's coding phase carries "an implementation for each INT-XX item in IMPLEMENTATION_BRIEF", and a phase asked to write CHAIN.md failed on it, honestly, every time (01a0c20d, 01a0c493). Decided from the caller's config before the merge (afterwards a recipe task and a caller task look the same): caller task + no caller condition → the recipe's done_when/done_when_check are not inherited. A caller condition is kept; a phase with neither keeps the recipe's pair. Test fixture now carries a recipe task+condition. Harness: the triage agreement line dedupes per skill and excludes skills whose Trigger is not observable (always_inject is inlined). First live datapoint, 01a0c493: for 'create CHAIN.md and commit' Jev's top picks were workspace-repo-commit-protocol 0.63 / small-focused-commits 0.57; the agent read code-review-checklist (~0) and nothing else. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz
This commit is contained in:
co-authored by
Claude Opus 5
parent
37eb6bcad1
commit
0d46f892db
@@ -206,7 +206,21 @@ fn phases_for_create(
|
||||
})
|
||||
.map(|rp| rp.config.clone())
|
||||
.unwrap_or(Value::Null);
|
||||
// Decided from the caller's config BEFORE the merge: afterwards a
|
||||
// recipe task and a caller task are indistinguishable.
|
||||
let caller_task = p
|
||||
.config
|
||||
.get("task")
|
||||
.and_then(|t| t.as_str())
|
||||
.is_some_and(|s| !s.trim().is_empty());
|
||||
let caller_condition =
|
||||
p.config.get("done_when").is_some() || p.config.get("done_when_check").is_some();
|
||||
let config = merge_config(base, p.config);
|
||||
let config = if caller_task && !caller_condition {
|
||||
drop_orphaned_condition(config, &p.kind, p.order_idx)
|
||||
} else {
|
||||
config
|
||||
};
|
||||
// Say what this phase asked for that will not happen. A config key
|
||||
// nothing reads is silent by construction — `task` sat unread
|
||||
// through every mission until two phases with different tasks
|
||||
@@ -221,6 +235,36 @@ fn phases_for_create(
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// A recipe's completion condition is a condition on the recipe's own task.
|
||||
/// When the caller supplied a different `task` and no condition of its own,
|
||||
/// keeping the recipe's `done_when` judges the phase against work it was
|
||||
/// never asked to do — `research_and_code`'s coding phase inherits "an
|
||||
/// implementation for each INT-XX item in IMPLEMENTATION_BRIEF", and a
|
||||
/// phase asked to write CHAIN.md fails on it, honestly, every time (missions
|
||||
/// 01a0c20d, 01a0c493). The condition and the check travel with the task
|
||||
/// they were written for; a phase with its own task gets its own, or none.
|
||||
///
|
||||
/// Called only when the caller supplied a task and no condition; the
|
||||
/// decision is made before the merge, where the two are still telling apart.
|
||||
fn drop_orphaned_condition(config: Value, kind: &str, order_idx: i32) -> Value {
|
||||
let Value::Object(mut c) = config else { return config };
|
||||
let dropped: Vec<&str> = ["done_when", "done_when_check"]
|
||||
.into_iter()
|
||||
.filter(|k| c.contains_key(*k))
|
||||
.collect();
|
||||
if !dropped.is_empty() {
|
||||
eprintln!(
|
||||
"phase {kind}[{order_idx}]: caller supplied its own task and no completion \
|
||||
condition — the recipe's {} is NOT inherited (it describes the recipe's task)",
|
||||
dropped.join("/")
|
||||
);
|
||||
for k in dropped {
|
||||
c.remove(k);
|
||||
}
|
||||
}
|
||||
Value::Object(c)
|
||||
}
|
||||
|
||||
/// Shallow-merge `over` onto `base`, key by key.
|
||||
///
|
||||
/// Shallow is deliberate: phase config is a flat settings bag, and a caller
|
||||
@@ -1755,6 +1799,44 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A caller's own task does not inherit the recipe's condition; a
|
||||
/// caller's own condition is kept; a phase with neither keeps the
|
||||
/// recipe's pair as before.
|
||||
#[test]
|
||||
fn a_custom_task_does_not_inherit_the_recipes_done_when() {
|
||||
let recipe = test_recipe();
|
||||
let coding_has_condition = recipe
|
||||
.phases
|
||||
.iter()
|
||||
.any(|p| p.kind == "coding" && p.config.get("done_when").is_some());
|
||||
assert!(coding_has_condition, "the fixture must carry a recipe condition");
|
||||
let phases = phases_for_create(
|
||||
Some(&recipe),
|
||||
vec![
|
||||
PhaseSpec {
|
||||
kind: "coding".into(),
|
||||
order_idx: 1,
|
||||
config: serde_json::json!({"task": "write CHAIN.md"}),
|
||||
},
|
||||
PhaseSpec {
|
||||
kind: "coding".into(),
|
||||
order_idx: 2,
|
||||
config: serde_json::json!({"task": "write X", "done_when": "X exists"}),
|
||||
},
|
||||
PhaseSpec {
|
||||
kind: "coding".into(),
|
||||
order_idx: 3,
|
||||
config: Value::Null,
|
||||
},
|
||||
],
|
||||
);
|
||||
assert!(phases[0].config.get("done_when").is_none(), "{:?}", phases[0].config);
|
||||
assert_eq!(phases[1].config["done_when"], "X exists");
|
||||
assert!(phases[2].config.get("done_when").is_some(), "{:?}", phases[2].config);
|
||||
// The rest of the recipe's config still backfills the custom-task phase.
|
||||
assert!(phases[0].config.get("loop").is_some());
|
||||
}
|
||||
|
||||
/// Omitting phases entirely takes the recipe's list wholesale.
|
||||
#[test]
|
||||
fn phases_default_to_the_recipe() {
|
||||
@@ -1853,7 +1935,10 @@ mod tests {
|
||||
order_idx: 1,
|
||||
config: serde_json::json!({
|
||||
"loop": "until_no_more_int_items",
|
||||
"commit_policy": "on_green_tests"
|
||||
"commit_policy": "on_green_tests",
|
||||
// The recipe's own task and the condition written for it.
|
||||
"task": "implement every INT-XX item",
|
||||
"done_when": "an implementation exists for each INT-XX item"
|
||||
}),
|
||||
},
|
||||
],
|
||||
|
||||
@@ -1246,12 +1246,21 @@ assert_skill_triage() { # <token> <mission> <label>
|
||||
# not expect. Only meaningful when the arm is a retrieval arm.
|
||||
agree=$(api "$token" GET "/api/missions/$mission/skill-use" | python3 -c '
|
||||
import json,sys
|
||||
d=json.load(sys.stdin); sk=d.get("skills") or []
|
||||
if not sk or all(s.get("triage_p") is None for s in sk): print("n/a"); sys.exit()
|
||||
def read(s): return s["trigger"].get("verdict") in ("pass",) if isinstance(s.get("trigger"),dict) else str(s.get("trigger")).startswith("pass")
|
||||
applies=[s for s in sk if (s.get("triage_p") or 0)>=0.5]
|
||||
hit=sum(1 for s in applies if read(s)); read_unexpected=sum(1 for s in sk if read(s) and (s.get("triage_p") or 0)<0.5)
|
||||
print(f"oracle says {len(applies)} of {len(sk)} delivered skills apply; agent read {hit} of those and {read_unexpected} it did not expect")' 2>/dev/null)
|
||||
d=json.load(sys.stdin); rows=d.get("skills") or []
|
||||
if not rows or all(s.get("triage_p") is None for s in rows): print("n/a"); sys.exit()
|
||||
# One row per skill per phase: collapse to the skill, keeping any pass. Skills
|
||||
# whose Trigger is not observable (always_inject, inlined) cannot be "not read".
|
||||
by={}
|
||||
for s in rows:
|
||||
v=s["trigger"].get("verdict") if isinstance(s.get("trigger"),dict) else str(s.get("trigger"))
|
||||
e=by.setdefault(s["skill"],{"p":s.get("triage_p") or 0,"read":False,"obs":False})
|
||||
e["p"]=max(e["p"],s.get("triage_p") or 0)
|
||||
if v=="pass": e["read"]=True
|
||||
if v in ("pass","fail","not_applicable"): e["obs"]=True
|
||||
obs={k:v for k,v in by.items() if v["obs"]}
|
||||
applies=[k for k,v in obs.items() if v["p"]>=0.5]
|
||||
hit=[k for k in applies if obs[k]["read"]]; unexpected=[k for k,v in obs.items() if v["read"] and v["p"]<0.5]
|
||||
print(f"of {len(obs)} observable skills the oracle says {len(applies)} apply ({", ".join(applies) or "-"}); agent read {len(hit)} of those and {len(unexpected)} it did not expect ({", ".join(unexpected) or "-"})")' 2>/dev/null)
|
||||
case "$agree" in
|
||||
""|n/a) pass "$label-triage: agreement n/a on this arm (no retrieval to compare against)" ;;
|
||||
*) pass "$label-triage: $agree" ;;
|
||||
|
||||
Reference in New Issue
Block a user