fix(sleep): keep train and val splits non-empty - #272

@microsoft-github-policy-service agree

Reviewed 58522d7fcadf. This is a useful, bounded correctness fix: it handles both the all-test assignment and the adjacent single-val/rest-test case without consuming the only validation task to populate train.

Local Linux/Python 3.11 verification: 34 passed, comprising the split-hardening/wiring suites and two independent all-test / val-plus-test invariant checks. I have not reproduced an additional runtime defect in this focused review.

For a reproducible acceptance record, please attach the command or small script used for the reported 6,300-case invariant sweep, with its seed/count/fraction/order ranges. That is an evidence request, not a request to add a large framework or perform paid-model experiments. Preserve the documented minimum-reassignment rule, warnings identifying the affected task IDs, and the distinction between stable hash assignment and the small-batch fallback.

Please keep this as one coherent solution to #271 rather than stacking overlapping split fixes. Official CI for this exact head is still action_required and awaits upstream approval; that is a maintainer-side gate, not a failed author test. This comment is a focused review receipt, not merge approval.

Thanks for the focused review. I reproduced the 6,300-case invariant sweep on commit 58522d7 with this exact matrix:

• seed: range(50) (0..49)
• task count: range(2, 9) (2..8 real tasks)
• (val_fraction, test_fraction): (0.00, 0.00), (0.01, 0.00), (0.10, 0.00), (0.34, 0.00), (0.50, 0.00), (0.01, 0.80), (0.10, 0.80), (0.20, 0.50), (0.40, 0.50)
• input order: original and reversed

That is 50 x 7 x 9 x 2 = 6,300 combinations. Each case asserts that the result preserves task count, uses only train/val/test, and keeps both train and val non-empty. Result: 6,300 combinations passed. The high-test-fraction cases emitted the expected warning with the reassigned task IDs; no paid-model or external API calls were used.

The command was run with uv run python using the PR head checkout and the existing assign_splits implementation; no source changes were needed for this evidence request.

Following up on this PR at head 58522d7:

• The requested reproducible invariant sweep is documented above: 50 x 7 x 9 x 2 = 6,300 combinations passed.
• Focused review reported 34 local tests passing with no additional runtime defect found.
• The remaining blocker appears to be the maintainer-side CI approval gate; the current head has license/cla passing but no code-test result.

If this fix is still wanted as the coherent solution for #271, could a maintainer please approve the pending CI run and provide the final review? The PR is currently mergeable and targets main, but is one commit behind it.

Jamie Magee (@JamieMagee) Yifan Yang (@Yif-Yang) Following up on the maintainer-side gate for this PR.

The current head 58522d7 has the requested reproducible 6,300-case invariant sweep documented above, and the focused review reported 34 local tests passing. The only visible check is license/cla; the code CI run remains action_required.

Could you please route or provide the final maintainer review, and approve the pending CI run if appropriate? If #272 is no longer the preferred vehicle for #271, please let me know so I can avoid keeping a stale PR open.

Thank you for supplying the requested invariant sweep and following up on the maintainer-side CI gate.

The requested 6,300-case sweep receipt is now satisfied, and I independently confirmed the one-batch invariants, minimum reassignment, and per-ID warnings. I also ran the full suite at 58522d7 and in a clean merge onto current main: 1,497 tests pass in each.

A two-night integration check found a separate integrity gap. With seed 42 and val/test fractions 0.10/0.80, night 1 mines all-test-1 through all-test-5; the fallback sends all-test-2 to train and the optimizer uses it. On night 2, re-mining the same IDs plus one normal train-hash task and one val-hash task resets all-test-2 to test. The real cycle emits no warning, reports holdout_leaked=false, and includes that previously trained task in held_out_score. The archive contains its earlier train assignment, but the new split assignment does not consult persistent exposure history. All inputs, backend responses, and adopted files in this reproduction were synthetic.

Please retain the useful one-night fallback while ensuring that a fallback-exposed task cannot later be reported as pristine held out. Persist split/exposure provenance or exclude previously exposed IDs and their derivatives; a fail-closed path is also acceptable when the required pools cannot be formed safely. Add a multi-night growing/shrinking-pool regression through the real cycle/archive path. This is new evidence beyond the earlier focused review, not a repeat of the completed sweep request.