Skip to content

fix(stella-cli): pick the skill holdout from the skills this turn matched - #6496

Merged
macanderson merged 1 commit into
mainfrom
fix/6464-holdout-picks-from-matched-skills
Sep 11, 2026
Merged

fix(stella-cli): pick the skill holdout from the skills this turn matched#6496
macanderson merged 1 commit into
mainfrom
fix/6464-holdout-picks-from-matched-skills

Conversation

@macanderson

@macanderson macanderson commented Sep 11, 2026

Copy link
Copy Markdown
Owner

What & why

The skill holdout picked from the whole loaded catalog before knowing what this turn's prompt matched. When the ordinal named a skill the turn never matched, nothing was withheld, the slot produced no control trial, and the schedule still counted it. A skill that is never measured is never promoted under require_measured_lift and never retired.

The pick now reads the matched population: the survivors plus the ones the top-k cut dropped, which cleared the same score floor. It is settled by the first selection pass to score and cached on the turn through the same cell the memory and rule arms already use (trials::held_for), so every later pass reads the same answer and a turn holds back one skill no matter how many sites render. held_for gained a borrow form for a caller whose names are already &str.

Closes #6464

The witness

  • This PR includes a witness test (fails on main, passes here): the_holdout_cannot_pick_a_skill_the_turn_never_matched in crates/stella-cli/src/memory/learning/skill_lifecycle.rs. The fixture adds an unmatched skill that sorts first, so holdout ordinal 0 names it when the pick reads the whole catalog. On main the turn injects the matched skill and records a second selected row; here it withholds the matched skill and records a control row.

The gate

  • cargo fmt --check
  • clippy over the workspace (CI)
  • the workspace test suite (CI; cargo test -p stella-cli holdout passes locally, 6 tests)
  • Docs updated where behavior/flags changed (doc comments on apply_holdout and the new helper)
  • CLA signed
  • Closes #6464 appears both above and as a commit trailer

Fix over file

  • Extra fixes in this PR: none
  • Nothing was deferred

Ground-rule check

  • No I/O added to stella-core; no new deps

Anything reviewers should know?

The first draft of the witness passed on main too: it assumed the second turn was holdout ordinal 1, but at rate 2 the first holdout turn is ordinal 0, and ordinal 0 picks the first sorted name, which was the matched skill. The fixture now makes the unmatched skill sort first. The fail-on-old run is quoted in the PR conversation.

Summary by Sourcery

Select skill holdouts from the turn’s matched skill population and consistently record the resulting control trial.

Bug Fixes:

  • Ensure skill holdouts are selected only from skills matched by the current turn, preventing unmatched skills from consuming holdout slots without producing trials.

Enhancements:

  • Cache the per-turn skill holdout through the shared trial state so repeated rendering passes consistently withhold the same skill.

Tests:

  • Add a regression test confirming unmatched skills are excluded from holdout selection and matched skills receive the control trial.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @macanderson, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 4 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Skill holdouts are now selected from the current turn’s matched skill population rather than the full catalog, with the choice cached per turn and covered by a regression test that confirms matched skills receive control measurements while unmatched skills are ignored.

Sequence diagram for matched skill holdout selection

sequenceDiagram
    participant SessionMemory
    participant Selection
    participant Trials
    participant Ledger

    SessionMemory->>Selection: note_turn_skills(prompt)
    Selection-->>SessionMemory: selected + over_top_k
    SessionMemory->>Trials: held_for_skill(matched_names)
    Trials->>Trials: holdout::pick(ordinal, matched_names)
    Trials-->>SessionMemory: held skill
    SessionMemory->>Selection: retain(skill != held)
    SessionMemory->>Ledger: record_episode()
    Ledger-->>SessionMemory: control trial for held skill

    Note over Trials: Cached in shared context_trials cell
    SessionMemory->>Trials: held_for_skill(matched_names)
    Trials-->>SessionMemory: same held skill
Loading

File-Level Changes

Change Details Files
Restrict skill holdout selection to the skills matched by the current turn, while preserving starvation-based selection within that population.
  • Build the candidate population from selected skills and top-k exclusions.
  • Apply control-arm starvation filtering only to matched candidates.
  • Use the shared per-turn holdout cache so repeated rendering sites reuse one skill selection.
  • Return the borrowed selected skill name without reloading the full catalog.
crates/stella-cli/src/memory.rs
crates/stella-cli/src/memory/trials.rs
Add a borrow-based holdout helper and a regression test proving unmatched skills are never withheld or measured.
  • Add held_for_refs and expose skill-specific cached holdout lookup.
  • Verify an unmatched, lexically first skill cannot be selected by the holdout ordinal.
  • Verify the matched skill produces a control trial and the unmatched skill receives no trial.
crates/stella-cli/src/memory/trials.rs
crates/stella-cli/src/memory/learning/skill_lifecycle.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#6464 Choose the skill holdout from the skills matched by the current turn, including skills dropped by the top-k selection cutoff, so a holdout cannot name an unrelated loaded skill and leave the slot without a trial.
#6464 Cache the skill holdout choice on the turn using the shared trial state so all scoring and rendering sites agree on one selected skill and consistently withhold it across later passes.
#6464 Add a regression witness and update relevant documentation/comments to verify that an unmatched skill is not selected and that the matched skill records a control trial.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@macanderson

Copy link
Copy Markdown
Owner Author

Witness evidence, run locally with cargo test -p stella-cli the_holdout_cannot_pick_a_skill_the_turn_never_matched.

Against main's memory.rs and trials.rs (the test file from this branch):

test memory::learning::skill_lifecycle::the_holdout_cannot_pick_a_skill_the_turn_never_matched ... FAILED
assertion `left == right` failed: the holdout must withhold the matched skill, not the unmatched one
  left: ["prefer-updating-witness-test-assertions-e2010443"]
 right: []

With this branch's code:

test memory::learning::skill_lifecycle::the_holdout_cannot_pick_a_skill_the_turn_never_matched ... ok

All six holdout tests in stella-cli pass on the branch.

@github-actions

Copy link
Copy Markdown
Contributor

SCR-003 — this PR cannot close its issue yet.

An issue closes only when every DoD item is satisfied and verified. Tick the
boxes once each item is genuinely done, or split the remainder into a new issue
(triage label only, SCR-004).

Ticking those boxes does not by itself re-run this check — it only reacts to
the pull request. Push any change to the PR afterward (even an empty commit,
git commit --allow-empty -m 'chore: re-run dod-check') to get a fresh run.

@macanderson
macanderson enabled auto-merge (squash) September 11, 2026 18:42
…ched

The pick read the whole loaded catalog, so an ordinal could name a skill
the prompt never matched. Nothing was then withheld, the slot produced no
control trial, and the schedule counted it anyway. The pick now reads the
matched population, and the first selection pass to score settles it on
the turn's shared holdout cell, so every later pass withholds the same
skill.

Closes #6464
@macanderson
macanderson force-pushed the fix/6464-holdout-picks-from-matched-skills branch from fe9cc47 to 859d8c1 Compare September 11, 2026 18:57
@macanderson
macanderson merged commit 09abe29 into main Sep 11, 2026
24 of 26 checks passed
@macanderson
macanderson deleted the fix/6464-holdout-picks-from-matched-skills branch September 11, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The skill holdout can pick a skill the turn never matched, and the slot produces nothing

1 participant