fix(sleep): honor val_fraction and test_fraction in the nightly cycle

[Bogdan (Dan) Baciu (bogdanbaciu21)]

What Problem This Solves

Fixes an issue where operators who set test_fraction (or val_fraction) in their Sleep config silently got neither: the nightly cycle only ever forwarded the legacy holdout_fraction alias, so test_fraction was dead config. No untouched test split could exist, which means every nightly lift number was a validation-split number, and validation is the same signal candidate skills are selected against.

The first review round also surfaced three hygiene bugs that made the wiring unsafe to merge: archived test tasks could leak back into dream recall as train, split assignment could flip when task order changed, and explicit val_fraction: 0.0 was overwritten by the alias default.

Why This Change Was Made

config.py documents val_fraction and test_fraction, and assign_splits() already implements both (with holdout_fraction as its alias), but mine() could not carry them and cycle.py passed only the alias. This PR wires what the config promises and closes the review blockers:

• mine() mirrors the assign_splits() knobs; holdout_fraction stays a legacy alias.
• load_config() records _user_config_keys so alias precedence follows config key provenance, not value guessing (holdout_fraction wins only when the user set it and did not set val_fraction; explicit val_fraction: 0.0 is preserved).
• Split assignment is id-stable: hash-bucket test/val/train only; the old order-dependent test carve is removed so append order cannot flip a task's split.
• recall_similar() accepts exclude_ids so archived val/test tasks and tonight's held-out ids never re-enter dream recall as train when recall_k > 0.
• Nights that produce test-split tasks score the night's final documents on the untouched test split and write a write-only test/held_out_score row to evidence.jsonl. The gate never reads it.
• docs/sleep/README.md documents the split knobs and alias rules.

Non-goals: no gate-semantics change, and no content-hash near-duplicate hardening (separate follow-up).

Rebased onto current upstream main at 3c8873f (preserves OpenCode evidence config from #241).

Project Fit

• An untouched test measure is the floor under any future claim that a change genuinely improves the method. consolidate._split() already documents that test is held out entirely and scored by the caller; in the nightly, this PR is that caller.
• Split hygiene (no recall leak, stable assignments, honest alias handling) is prerequisite robustness for any evidence-backed Sleep feature Yifan listed as a priority.

User Impact

Operators setting test_fraction now get what the config promises: a real untouched test split and a per-night held-out score row in evidence.jsonl. Operators who leave defaults alone see zero change: with test_fraction: 0.0 the split is bit-for-bit the legacy two-way split, no test tasks exist, and the scoring block never runs (no extra backend calls, no extra tokens).

Proof

Before (stock main at 3c8873f): the wiring tests cannot pass; recall can treat archived test tasks as train; split labels can change when task order changes; val_fraction: 0.0 silently becomes 0.34.

After (this branch, head bbfd321):

$ python -m pytest tests/test_split_wiring.py -q
17 passed in 0.22s

$ python -m pytest -q
1364 passed, 11 skipped in 36.75s

Baseline at rebase parent 3c8873f before this diff: 1347 passed, 11 skipped. The delta is exactly the 17 new tests, with zero regressions.

Pinned by tests: default two-way behavior unchanged; alias provenance (not value collision); val_fraction=0.0 honored; split stability under append order; recall excludes val/test archives; no score row without test tasks.

Academic Support

The measurement-validity motivation, briefly:

1. Dwork, Feldman, Hardt, Pitassi, Reingold, Roth (2015). "The reusable holdout: Preserving validity in adaptive data analysis." Science 349(6248). Sleep is an adaptive optimizer: every night's change is chosen using prior evaluations, which is exactly the regime where a re-used holdout stops measuring generalization.
2. Cawley, Talbot (2010). "On Over-fitting in Model Selection and Subsequent Selection Bias in Performance Evaluation." JMLR 11. The nightly gate is a model-selection criterion; selecting against a finite validation set induces optimistic bias that only an untouched test measure bounds.
3. Recht, Roelofs, Schmidt, Shankar (2019). "Do ImageNet Classifiers Generalize to ImageNet?" ICML 2019, arXiv:1902.10811. Fresh held-out data reveals overfitting to the incumbent evaluation; this motivates a genuinely untouched split rather than more validation.

Testing

New: tests/test_split_wiring.py, 17 tests pinning mine/cycle wiring, alias provenance, zero val_fraction, split stability, recall exclusions, and held-out scoring. Full suite: python -m pytest -q.

Platform	Python	Result on this branch
Linux (local venv)	3.12	1364 passed, 11 skipped, 0 failed
Linux / macOS / Windows (CI)	3.12	Refresh via exc-skillopt-3os-receipt on fork branch after push (prior baseline in research/2026-08-17/receipts/2026-08-17-baseline-3os.md; expect +17 wiring passes, zero new failures)

Limitations & Negative Results

• Content-hash disjointness for near-duplicate tasks across splits is not in this PR; the hash-bucket split is id-stable but not content-aware. Kept out deliberately; natural follow-up.
• With small nightly task counts, a nonzero test_fraction reduces train and val mass; 0.0 stays the default for that reason.

Reproduce It Yourself

git fetch origin pull/235/head:exc-001-three-way-split   # or: fork branch exc-001-three-way-split @ bbfd321
python -m venv .venv && ./.venv/bin/python -m pip install -e ".[dev]"
./.venv/bin/python -m pytest tests/test_split_wiring.py -q   # 17 passed
./.venv/bin/python -m pytest -q                              # full suite

[Bogdan (Dan) Baciu (bogdanbaciu21)]

All good here? Tried a new (greatly expanded) approach anxiously waiting to see how it landed.

[Yifan Yang (Yif-Yang)]

Thanks — I retested the current head (427096a), and there are still three correctness issues that block merge:

• Test data can leak back into training across nights. run_sleep_cycle() archives every non-dream task, including test tasks (skillopt_sleep/cycle.py:584-585), but dream_consolidate() passes only current train tasks to recall_similar() (skillopt_sleep/dream.py:127-130). Since recall_similar() excludes only those train IDs and copies matches as split="train" (dream.py:71-94), I can reproduce an archived same-id test task returning as recall:same-id with its reference. Please restrict recall to eligible historical train tasks and exclude current val/test IDs and source IDs, with a multi-night regression test.
• The empty-test fallback is not stable across nights (skillopt_sleep/mine.py:323-328). With seed 42, val_fraction=.34, and test_fraction=.10, t4 is test for [t0,t3,t4], but after appending t1, t4 becomes train and t1 becomes test. That exposes a former test item to training. Please remove the order-dependent carve or persist/use an assignment that does not change existing IDs when tasks are reordered or appended.
• _resolve_split_fractions() (skillopt_sleep/cycle.py:48-50) converts an explicit val_fraction=0.0 back to the 0.34 default. Please preserve explicit zero and reject invalid fraction values/sums.

The branch also conflicts with current main in cycle.py; while rebasing, please preserve the OpenCode evidence configuration already on main.

[Yifan Yang (Yif-Yang)]

Thanks for this, and sorry for the slow first response.

The core claim checks out: I confirmed on upstream/main that cycle.py forwarded only holdout_fraction and mine() dropped test_fraction entirely, so it really was dead config. I also confirmed held_out_score is written in exactly one place and read by no production code — the write-only property holds as documented. Suite on your branch: 1107 passed, 10 skipped, 130 subtests.

Two things to fix.

Blocker — archived test tasks can re-enter training via recall

cycle.py:585 archives every non-dream task, including the ones just tagged split='test'. On a later night with recall_k > 0, recall_similar (dream.py:83-94) pulls them out of the archive and re-emits them with split="train":

tags=list(h.tags) + ["recall"], split="train", origin="real",

So a task held out as the untouched final measure becomes training material for a subsequent night, and — because splits are hash-stable over seed + t.id — it is still scored as "held-out" on that later night too. That silently converts the number this PR exists to produce back into a contaminated one.

It only triggers with recall_k > 0 and test_fraction > 0, so no current user is affected (recall_k defaults to 0). But that combination is precisely the setup this feature invites, so I'd like it closed before merge. recall_similar should skip tasks whose archived split is test, or the archive write should exclude them.

I'll note the surrounding design is careful about exactly this — consolidate.py:85-90 explicitly refuses to fall back to test as train or val, with a comment saying so. The recall path just wasn't covered by that reasoning.

Should fix — value-based alias resolution has an unreachable case

_resolve_split_fractions infers intent from values because the merged config has no key provenance, so the alias wins only when val == DEFAULTS["val_fraction"] (0.34). That can't distinguish "user left val_fraction alone" from "user explicitly wrote val_fraction: 0.34". Concretely:

_resolve_split_fractions(load_config(val_fraction=0.34, holdout_fraction=0.5))
# -> (0.5, 0.0)

The explicit val_fraction is discarded — the opposite of your own docstring ("A user-changed val_fraction always beats the alias"). Your test test_explicit_val_fraction_beats_explicit_alias uses 0.5/0.2, which dodges the collision.

The trigger is narrow and the blast radius is one ratio, so this is not a blocker — but there's a clean fix available: load_config already does data = dict(DEFAULTS); data.update(...), so real provenance can be recorded there instead of guessed. Also note holdout_fraction ships with a default of 0.34 (config.py:44), so it is never None and the is not None check does nothing.

Minor

• Conflicts with main in skillopt_sleep/cycle.py; needs a rebase. I merged it locally against current main and the result was clean — 1358 passed, 10 skipped, 269 subtests — so the conflict is mechanical.
• Neither val_fraction/test_fraction nor the alias precedence rule appears anywhere in docs/. Since the precedence is subtle, please document it.

On cost: I want to correct a concern I had initially. Enabling test_fraction does not add rollouts — mine() is capped by max_tasks_per_night and assign_splits partitions that same fixed pool, and test tasks are excluded from consolidation, so they stop costing the 2-4 rollouts per train/val task and cost exactly one each here. Net cost goes down. No gating needed.

Good, well-scoped fix. Close the recall path and I'll merge.

[Bogdan (Dan) Baciu (bogdanbaciu21)]

Coming with the required work.

[Bogdan (Dan) Baciu (bogdanbaciu21)]

Thanks for the thorough review. Rebased onto current main (3c8873f) and pushed bbfd321 to the same fork branch. Changes since 427096a:

1. Recall leak: recall_similar(..., exclude_ids=) skips archived val/test and tonight's held-out task ids before recall runs.
2. Unstable splits: removed the order-dependent test carve; splits are hash-bucket only so append order cannot flip labels.
3. val_fraction=0.0: _resolve_split_fractions() uses _user_config_keys provenance and no longer treats 0.0 as falsy.
4. Alias precedence: holdout_fraction wins only when the user set it and did not set val_fraction (documented in docs/sleep/README.md).
5. Tests: tests/test_split_wiring.py now 17 cases; full suite 1364 passed locally on Python 3.12.

Happy to adjust if you want content-hash disjointness in this PR rather than a follow-up.