Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions crates/stella-cli/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,28 +874,37 @@ impl SessionMemory {
if trials::holdout_kind(ordinal) != Some(stella_learn::ledger::ArtifactKind::Skill) {
return None;
}
let loaded = self.load_skills();
// The population is the skills this turn's prompt matched — the
// survivors plus the ones the top-k cut threw away, which cleared the
// same score floor. A pick from the whole catalog can name a skill
// this turn never matched; nothing is then withheld, the slot
// produces no trial, and the schedule still counts it. The pick is
// settled here, at the first site to score, and read back by
// every later site (`trials::held_for`), so a turn holds one skill
// back no matter how many times it renders.
let matched: Vec<&str> = selection
.selected
.iter()
.chain(selection.over_top_k.iter())
.map(|s| s.skill.name.as_str())
.collect();
let counts = appraisals::control_arm_counts(
&self.workspace_root,
stella_learn::ledger::ArtifactKind::Skill,
);
let bar = stella_learn::skills::appraisal::AppraisalConfig::default()
.selection
.min_samples_per_arm;
let starved: Vec<&str> = loaded
let starved: Vec<&str> = matched
.iter()
.map(|s| s.name.as_str())
.copied()
.filter(|name| counts.get(*name).copied().unwrap_or(0) < bar)
.collect();
let names: Vec<&str> = if starved.is_empty() {
loaded.iter().map(|s| s.name.as_str()).collect()
} else {
starved
};
let held = stella_learn::holdout::pick(ordinal, &names)?;
let names: Vec<&str> = if starved.is_empty() { matched } else { starved };
let held = self.held_for_skill(&names)?;
let before = selection.selected.len();
selection.selected.retain(|s| s.skill.name != held);
(selection.selected.len() != before).then(|| held.to_string())
(selection.selected.len() != before).then_some(held)
}

/// Render selected skills as `(name, why)` — the matched domains and terms
Expand Down
76 changes: 76 additions & 0 deletions crates/stella-cli/src/memory/learning/skill_lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,82 @@ async fn the_holdout_picks_the_skill_whose_control_arm_is_short() {
);
}

/// **The holdout cannot name a skill the turn never matched.** The pick used
/// to read the whole loaded catalog, so a turn whose prompt matched only one
/// of two skills could have the other one's name drawn — nothing was then
/// withheld, the slot produced no trial, and the schedule still counted it.
/// The pick now comes from the trigger-matched population, so a holdout
/// turn always pays for exactly one control row.
///
/// The fixture arms the schedule so the ordinal points at the skill the
/// prompt does NOT match. Before the fix that pick withheld nothing and the
/// turn recorded no trial; after it, the pick lands on the matched skill and
/// the ledger gains a control row for it.
#[tokio::test]
async fn the_holdout_cannot_pick_a_skill_the_turn_never_matched() {
let dir = workspace_with_log();
set_gate(dir.path(), false);

let mut miner = session(dir.path());
miner.auto_create_skills(&log_path(dir.path()), true);
let written = skill_files(dir.path());
assert_eq!(written.len(), 1, "the lesson promoted into a skill");
let name = written[0].trim_end_matches(".md").to_string();

// A second skill the prompt does not match, so the catalog holds a name
// the turn never scored. It sorts before the mined skill, so the first
// holdout ordinal names it when the pick reads the whole catalog. The
// pick must not be able to reach it.
let other = dir.path().join(".stella/skills/aaa-unrelated.md");
std::fs::write(
&other,
"---\nname: aaa-unrelated\ndescription: quantum knitting patterns for sweaters\n---\n\nknit one purl one\n",
)
.expect("write unrelated skill");

let mut memory = session(dir.path());

// Turn 1 is not a holdout turn at rate 2; turn 2 is holdout ordinal 0,
// which the arm rotation points at skills and the pick resolves to the
// first name in sorted order. In the full catalog that is
// `aaa-unrelated`; in the matched population it is the mined skill.
assert!(
!memory.arm_controls_at(0, 2),
"the plane control is off here"
);
let _ = memory.note_turn_skills(MATCHING_PROMPT);
memory
.record_episode(MATCHING_PROMPT, EpisodeOutcome::Success, &[], 1_000, None)
.await;

// Turn 2: the holdout. The prompt still matches only the mined skill.
assert!(!memory.arm_controls_at(0, 2), "still no plane control");
let injected: Vec<String> = memory
.note_turn_skills(MATCHING_PROMPT)
.into_iter()
.map(|(n, _)| n)
.collect();
assert_eq!(
injected,
Vec::<String>::new(),
"the holdout must withhold the matched skill, not the unmatched one"
);
memory
.record_episode(MATCHING_PROMPT, EpisodeOutcome::Success, &[], 1_000, None)
.await;

assert_eq!(
trials(dir.path(), &name),
vec![true, false],
"the turn records a control trial for the skill it actually matched"
);
assert_eq!(
trials(dir.path(), "aaa-unrelated"),
Vec::<bool>::new(),
"the unmatched skill is not evidence about anything"
);
}

/// **The two schedules cannot land on the same turn.** A turn the plane
/// control already took claims no holdout number, so the holdout's counter
/// advances only over the turns it could act on.
Expand Down
20 changes: 18 additions & 2 deletions crates/stella-cli/src/memory/trials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,22 +150,38 @@ impl SessionMemory {
/// `None` on three counts: this turn is not a holdout turn, the schedule
/// is on another kind, or there was nothing to pick from.
fn held_for(&self, kind: ArtifactKind, population: &[String]) -> Option<String> {
let ids: Vec<&str> = population.iter().map(String::as_str).collect();
self.held_for_refs(kind, &ids)
}

/// The borrow form of [`Self::held_for`], for a caller whose population
/// is already `&str`.
fn held_for_refs(&self, kind: ArtifactKind, population: &[&str]) -> Option<String> {
let ordinal = self.holdout_ordinal?;
if holdout_kind(ordinal) != Some(kind) {
return None;
}
let mut guard = self.context_trials.lock().ok()?;
if guard.held.is_none() {
let ids: Vec<&str> = population.iter().map(String::as_str).collect();
guard.held =
stella_learn::holdout::pick(ordinal, &ids).map(|id| (kind, id.to_string()));
stella_learn::holdout::pick(ordinal, population).map(|id| (kind, id.to_string()));
}
match &guard.held {
Some((held, id)) if *held == kind => Some(id.clone()),
_ => None,
}
}

/// The skill this turn holds back, settled from the first selection pass
/// to ask and read back by every later one.
///
/// Skills keep their own join on the session, but the holdout pick is one
/// per turn across all three kinds, so it lives on the shared cell beside
/// the memory and rule picks rather than on `turn_skill_join`.
pub(super) fn held_for_skill(&self, population: &[&str]) -> Option<String> {
self.held_for_refs(ArtifactKind::Skill, population)
}

/// Fold one render pass into this turn's join for `kind`.
fn note_context_trial(&self, kind: ArtifactKind, offered: &[String], selected: &[String]) {
let Ok(mut guard) = self.context_trials.lock() else {
Expand Down
Loading