feat(skill-use): progressive disclosure, as an arm and not a switch
Trigger — did the agent reach for the skill when it applied? — cannot be measured while every body is inlined into the prompt. Nothing was reached for. `skill_use` has been reporting `NotObservable` for that reason, and it was right to. The skills door made retrieval possible; this makes it a delivery arm. `index` sends each pinned skill's name, description, `when_to_use` and the uri that returns its body, and the agent fetches what it judges relevant. `inline` is unchanged and stays the default. An A/B rather than a switch, because `index` can only cost Compliance: under `inline` the procedure sits in front of the model whether or not it noticed it applied. Trading a measured axis for an unmeasured regression in another is not an improvement, so both arms stay runnable and the arm is recorded on the mission row. Three things the mechanism refuses to do: - `index` without a door falls back to `inline`. An index names bodies and says how to fetch them; with no `clawmates_skills` server reachable that is a list of dead ends, and it fails as an agent ignoring its skills rather than as a missing config. `install_skills_door` now returns whether it installed, because the caller needs the answer and not just the log line. - The scorer reads the arm off the recorded PROMPT, not off the mission row. The row says what the mission is configured to do now; the score is being computed against a turn that ran then. - Under `index`, a skill that was offered and never read is a Fail, not the inline arm's `NotObservable` — but only where the skill had a checkable consequence in that phase. Reusing the inline text would have said "this skill was inlined into the prompt" about a skill whose body was never sent, and scoring a real miss as a structural blind spot is the failure this measurement already made once. The arm is per mission (`config.skill_delivery`), not only per deployment. Both arms run against one server process; restarting between them would put a confound in the comparison that the numbers would not show. 829 tests, 108 binaries, green. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b58f0347e6
commit
f52cff3e04
+175
-14
@@ -189,6 +189,55 @@ pub fn retrieved_skills(ev: &Evidence<'_>) -> Vec<String> {
|
||||
out
|
||||
}
|
||||
|
||||
/// Did the agent reach for this skill?
|
||||
///
|
||||
/// The answer depends on whether it was ever given the chance, which is what
|
||||
/// the delivery arm decides — so this takes the arm rather than assuming one.
|
||||
/// Getting that wrong is not a rounding error: under `Index` the old text
|
||||
/// would have said "this skill was inlined into the prompt" about a skill that
|
||||
/// was not, and scored a real miss as a structural blind spot.
|
||||
fn trigger_verdict(
|
||||
mode: crate::skill_delivery::Mode,
|
||||
retrieved: bool,
|
||||
compliance: &Verdict,
|
||||
boundary: &Verdict,
|
||||
) -> Verdict {
|
||||
if retrieved {
|
||||
// The agent reached for it. That is the paper's Trigger, and it is a
|
||||
// recorded tool call like any other.
|
||||
return Verdict::Pass;
|
||||
}
|
||||
match mode {
|
||||
// Handed over, so there was no reaching-for to observe. Not a failure
|
||||
// and not a pass — the axis simply does not exist in this arm.
|
||||
crate::skill_delivery::Mode::Inline => Verdict::NotObservable(
|
||||
"this skill was inlined into the prompt, not retrieved — the agent \
|
||||
was handed it, so there is no reaching-for to observe. Serve it \
|
||||
through the door instead and this becomes a tool call"
|
||||
.into(),
|
||||
),
|
||||
crate::skill_delivery::Mode::Index => {
|
||||
// Offered by name and `when_to_use`, and never opened. Whether that
|
||||
// is a miss depends on whether the skill had anything to say about
|
||||
// this phase at all: a skill with no machine-checkable consequence
|
||||
// here is one an agent is right to pass over, and scoring that as a
|
||||
// failure would punish correct triage.
|
||||
if matches!(compliance, Verdict::NotApplicable)
|
||||
&& matches!(boundary, Verdict::NotApplicable)
|
||||
{
|
||||
Verdict::NotApplicable
|
||||
} else {
|
||||
Verdict::Fail(
|
||||
"offered in the index with its `when_to_use`, and never read — \
|
||||
the agent had the entry in front of it and did not fetch the \
|
||||
procedure"
|
||||
.into(),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Score every skill a phase's prompt delivered, against what the agent produced.
|
||||
pub fn score(
|
||||
prompt: &str,
|
||||
@@ -196,6 +245,10 @@ pub fn score(
|
||||
source_kinds: &dyn Fn(&str) -> String,
|
||||
) -> Vec<SkillUse> {
|
||||
let retrieved = retrieved_skills(evidence);
|
||||
// Read off the prompt that was actually sent, not off the mission row: the
|
||||
// row says what the mission is configured to do now, and this is scoring a
|
||||
// turn that ran then.
|
||||
let mode = crate::skill_delivery::mode_in_prompt(prompt);
|
||||
// Delivered by either route. A skill that was retrieved and never inlined
|
||||
// is invisible to `skills_in_prompt`, and under progressive disclosure that
|
||||
// is EVERY skill — so scoring only the prompt would report zero for the
|
||||
@@ -212,19 +265,12 @@ pub fn score(
|
||||
let (compliance, boundary) = check(&skill, evidence);
|
||||
SkillUse {
|
||||
source_kind: source_kinds(&skill),
|
||||
trigger: if retrieved.contains(&skill) {
|
||||
// The agent reached for it. That is the paper's Trigger,
|
||||
// and it is now a recorded tool call like any other.
|
||||
Verdict::Pass
|
||||
} else {
|
||||
Verdict::NotObservable(
|
||||
"this skill was inlined into the prompt, not retrieved — \
|
||||
the agent was handed it, so there is no reaching-for to \
|
||||
observe. Serve it through the door instead and this \
|
||||
becomes a tool call"
|
||||
.into(),
|
||||
)
|
||||
},
|
||||
trigger: trigger_verdict(
|
||||
mode,
|
||||
retrieved.contains(&skill),
|
||||
&compliance,
|
||||
&boundary,
|
||||
),
|
||||
compliance,
|
||||
boundary,
|
||||
skill,
|
||||
@@ -855,7 +901,122 @@ mod tests {
|
||||
.iter()
|
||||
.map(|(n, b)| crate::topology_exec::render_pinned_skill(n, b))
|
||||
.collect();
|
||||
crate::topology_exec::compose_turn_prompt("Task: do the thing", Some(&body))
|
||||
crate::topology_exec::compose_turn_prompt(
|
||||
"Task: do the thing",
|
||||
Some(&body),
|
||||
crate::skill_delivery::Mode::Inline,
|
||||
)
|
||||
}
|
||||
|
||||
/// A prompt as the delivery layer renders it under the INDEX arm.
|
||||
fn rendered_index(skills: &[(&str, &str)]) -> String {
|
||||
let body: String = skills
|
||||
.iter()
|
||||
.map(|(name, when)| {
|
||||
crate::topology_exec::render_pinned_skill(
|
||||
name,
|
||||
&crate::skill_delivery::index_entry(
|
||||
"a procedure",
|
||||
Some(when),
|
||||
&format!("skill:global/{name}"),
|
||||
),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
crate::topology_exec::compose_turn_prompt(
|
||||
"Task: do the thing",
|
||||
Some(&body),
|
||||
crate::skill_delivery::Mode::Index,
|
||||
)
|
||||
}
|
||||
|
||||
fn read_skill(uri: &str) -> ToolEvidence {
|
||||
ToolEvidence {
|
||||
tool: "ReadMcpResourceTool".into(),
|
||||
path: None,
|
||||
input: json!({ "server": "clawmates_skills", "uri": uri }),
|
||||
response: serde_json::Value::Null,
|
||||
}
|
||||
}
|
||||
|
||||
/// The whole loop, end to end: the index writes a uri, the agent reads that
|
||||
/// exact uri back, and the scorer recovers the skill's name from it.
|
||||
///
|
||||
/// Three components have to agree on one string — `mcp_skills::skill_uri`
|
||||
/// writes it, `skill_delivery::index_entry` puts it in the prompt, and
|
||||
/// `parse_uri` reads it. Asserting them separately would let any pair drift
|
||||
/// while each one's own test stayed green.
|
||||
#[test]
|
||||
fn the_uri_the_index_advertises_is_the_one_the_scorer_recovers() {
|
||||
let prompt = rendered_index(&[("workspace-repo-commit-protocol", "before committing")]);
|
||||
assert!(
|
||||
prompt.contains("uri=\"skill:global/workspace-repo-commit-protocol\""),
|
||||
"the entry must name the uri to fetch:\n{prompt}"
|
||||
);
|
||||
let tools = vec![read_skill("skill:global/workspace-repo-commit-protocol")];
|
||||
let ev = Evidence::new("", &tools);
|
||||
assert_eq!(
|
||||
retrieved_skills(&ev),
|
||||
vec!["workspace-repo-commit-protocol"],
|
||||
"the uri the prompt advertised must parse back to the skill's name"
|
||||
);
|
||||
let scored = score(&prompt, &ev, &builtin);
|
||||
assert_eq!(scored.len(), 1);
|
||||
assert!(
|
||||
matches!(scored[0].trigger, Verdict::Pass),
|
||||
"reaching for an indexed skill IS the Trigger axis: {:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// The index arm must not report a miss as a blind spot.
|
||||
///
|
||||
/// Under `Inline` "never retrieved" is `NotObservable`, and that is honest
|
||||
/// there. Reusing it here would say "this skill was inlined into the
|
||||
/// prompt" about a skill whose body was never sent — the exact shape of a
|
||||
/// check reporting a system defect where an agent behaviour belongs.
|
||||
#[test]
|
||||
fn an_indexed_skill_that_was_never_read_is_a_miss_not_a_blind_spot() {
|
||||
let prompt = rendered_index(&[("workspace-repo-commit-protocol", "before committing")]);
|
||||
// It wrote outside the mission checkout, so the boundary check applies
|
||||
// — this skill had something to say about this phase and went unread.
|
||||
let tools = acted(&[("Write", Some("/tmp/scratch.rs"), json!({}))]);
|
||||
let scored = score(&prompt, &Evidence::new("", &tools), &builtin);
|
||||
assert!(
|
||||
matches!(scored[0].trigger, Verdict::Fail(_)),
|
||||
"offered by name and when_to_use, never fetched: {:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// ...but only when the skill had a consequence to check.
|
||||
///
|
||||
/// A skill with nothing machine-checkable in this phase is one an agent is
|
||||
/// right to pass over, and scoring that as a Trigger failure would punish
|
||||
/// correct triage — which is the behaviour progressive disclosure is
|
||||
/// supposed to reward.
|
||||
#[test]
|
||||
fn passing_over_a_skill_with_nothing_to_check_is_not_a_trigger_failure() {
|
||||
let prompt = rendered_index(&[("some-unchecked-skill", "when writing prose")]);
|
||||
let scored = score(&prompt, &narrative("wrote the report"), &builtin);
|
||||
assert!(
|
||||
matches!(scored[0].trigger, Verdict::NotApplicable),
|
||||
"{:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// The control arm has to be unchanged, or the A/B measures this edit too.
|
||||
#[test]
|
||||
fn the_inline_arm_still_reports_trigger_as_unobservable() {
|
||||
let prompt = rendered(&[("workspace-repo-commit-protocol", "body")]);
|
||||
let tools = acted(&[("Write", Some("/tmp/scratch.rs"), json!({}))]);
|
||||
let scored = score(&prompt, &Evidence::new("", &tools), &builtin);
|
||||
assert!(
|
||||
matches!(scored[0].trigger, Verdict::NotObservable(_)),
|
||||
"{:?}",
|
||||
scored[0].trigger
|
||||
);
|
||||
}
|
||||
|
||||
/// The prompt is the record of what was delivered, so parsing it must match
|
||||
|
||||
Reference in New Issue
Block a user