Skip to content

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

Open
Bogdan (Dan) Baciu (bogdanbaciu21) wants to merge 1 commit into
microsoft:mainfrom
bogdanbaciu21:exc-001-three-way-split
Open

fix(sleep): honor val_fraction and test_fraction in the nightly cycle#235
Bogdan (Dan) Baciu (bogdanbaciu21) wants to merge 1 commit into
microsoft:mainfrom
bogdanbaciu21:exc-001-three-way-split

Conversation

@bogdanbaciu21

@bogdanbaciu21 Bogdan (Dan) Baciu (bogdanbaciu21) commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

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.

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:

  • mine() now mirrors the assign_splits() knobs; holdout_fraction stays a legacy alias with unchanged override semantics for existing callers.
  • The cycle resolves the alias with documented value-based precedence (the merged config has no key provenance, so the alias wins only when it disagrees with a val_fraction still at its shipped default) and records both fractions in the evidence config row.
  • Nights that produce test-split tasks score the night's final documents on the untouched test split, using the same replay_batch plus aggregate_scores path the experiment harness uses, and write a write-only test/held_out_score row to evidence.jsonl. The gate never reads it.

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

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.
  • A claim that survives an untouched split is the minimum honest form of stable, reliable gains rather than single-slice lift.

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): the new wiring tests cannot pass. mine(..., val_fraction=..., test_fraction=...) is a TypeError because the signature cannot carry the knobs, a seeded night with a test-split task writes no held-out row, and no cycle-level alias resolution exists.

After (this branch):

$ python -m pytest tests/test_split_wiring.py -q
10 passed in 0.09s

$ python -m pytest -q
1106 passed, 11 skipped, 130 subtests passed in 37.51s

Baseline at the same commit before this change: 1096 passed, 11 skipped. The delta is exactly the 10 new tests, with zero regressions.

The behavior-preservation claims are pinned by tests rather than asserted: test_default_call_is_two_way_like_before (defaults reproduce the legacy split), test_legacy_holdout_alias_still_wins_when_passed (alias override identical to explicit val_fraction), and test_no_score_row_without_test_tasks (legacy nights write nothing new).

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, 10 tests pinning (a) mine() forwards both fractions into assign_splits, (b) cycle-level alias precedence (legacy holdout_fraction configs keep their behavior; a user-set val_fraction wins), and (c) the test/held_out_score evidence row appears exactly when test-split tasks exist. Full suite: python -m pytest -q.

Platform Python Result on this branch
Linux (local venv) 3.12.3 1106 passed, 11 skipped, 0 failed
Linux (CI ubuntu) 3.12 1105 passed, 1 failed (pre-existing)
macOS (CI) 3.12 1103 passed, 3 failed (pre-existing)
Windows (CI) 3.12 1064 passed, 37 failed (pre-existing), 16 skipped

Every failure above is present on stock main at the same base commit and is unrelated to this diff: one Copilot backend parse case (Linux and macOS), two macOS /var symlink path checks in the Superpowers adapter tests, and 37 POSIX-only adapter tests on Windows (baseline there is 1054 passed with the same 37 failures). Relative to that baseline, this branch adds exactly the 10 new tests as passes on every platform, with zero new failures anywhere.

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. It is kept out deliberately so the default-stability claim stays airtight, and it is a natural follow-up.
  • Alias precedence is value-based because no config provenance exists: a user who explicitly sets val_fraction to exactly its default while also setting holdout_fraction gets the alias. This is documented in the _resolve_split_fractions docstring.
  • 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 checkout <this branch>
python -m venv .venv && ./.venv/bin/python -m pip install -e ".[dev]"
./.venv/bin/python -m pytest tests/test_split_wiring.py -q   # 10 passed
./.venv/bin/python -m pytest -q                              # full suite

config.py documents val_fraction and test_fraction and assign_splits()
implements both, but the nightly path only ever forwarded the legacy
holdout_fraction alias: mine() could not carry the new knobs, so
test_fraction was dead config -- no untouched test split could exist and
no held-out test score was ever recorded.

- mine() now mirrors assign_splits(): val_fraction/test_fraction are the
  real controls, holdout_fraction stays a legacy alias with unchanged
  override semantics for existing callers.
- run_sleep_cycle() resolves the alias (documented value-based precedence,
  since the merged config has no key provenance) and passes both fractions
  through; both are now recorded in the evidence config row.
- Nights that produce test-split tasks score the night's FINAL documents
  on the untouched test split (same replay_batch + aggregate_scores path
  the experiment harness uses) and write a write-only
  test/held_out_score row to evidence.jsonl. The gate never reads it.
- Defaults are bit-for-bit unchanged: test_fraction=0.0 yields the legacy
  two-way split, no test tasks, no extra calls.

Tests: tests/test_split_wiring.py pins the wiring end to end (mine
forwarding, alias precedence, evidence row present/absent).
@bogdanbaciu21

Copy link
Copy Markdown
Contributor Author

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

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.

1 participant