fix(sleep): never certify a gate verdict measured on the training tasks

[lufen]

_split() fell back to �al = train when no validation task existed, so on a thin mine the gate scored the very tasks the edits were derived from. That comparison cannot detect overfitting, yet the night reports it as a validated held-out improvement -- a silent wrong answer rather than a visible failure.

Observed: a single mined task whose rule judge was section_present=Results. The optimizer added a '## Results' heading, the gate scored the same task it had just optimized, and the night reported held-out 0.250 -> 1.000 and accepted the edit. Any batch where every task hashes to one split reaches the same state, in both directions -- with only a val task, train borrows from it.

_split now reports whether val is disjoint from train (including overlap by id), and consolidate fails closed: non-disjoint slices yield the action
reject_unverified instead of a verdict the evidence cannot support. Edits are still staged and report.md states that they are unverified suggestions, so a thin night still surfaces material for a human to read -- it just stops claiming the material was validated.

tests/test_holdout_integrity.py covers both halves. The second matters as much as the first: a gate that rejects everything would be trivially safe and useless, so there is an explicit test that a candidate which genuinely scores better on tasks the optimizer never saw is still accepted.

[lufen]

@microsoft-github-policy-service agree company="Microsoft"

[Copilot]

Pull request overview

This PR hardens SkillOpt-Sleep’s consolidation gate so it never “validates” edits using the same tasks the optimizer trained on, preventing silent holdout leakage from being reported as a held-out improvement.

Changes:

• Extend consolidate._split() to return a holdout_leaked flag when train/val are not disjoint (including overlap by task id).
• Fail closed in consolidate() when holdout leakage is detected by emitting an “unverified” gate action and not accepting the candidate.
• Surface the condition in the night report via SleepReport.holdout_leaked and add regression tests covering both split integrity and end-to-end gating behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_holdout_integrity.py	Adds regression coverage for split leakage detection and gate abstention vs. genuine improvements.
tests/test_consolidate_split.py	Updates existing split tests for the new _split() return shape.
skillopt_sleep/types.py	Adds SleepReport.holdout_leaked to persist/report leakage state.
skillopt_sleep/cycle.py	Renders an explicit “Not validated” warning and records holdout_leaked in diagnostics.
skillopt_sleep/consolidate.py	Implements leakage detection in _split() and fail-closed gate behavior on leaked holdout slices.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

skillopt_sleep/consolidate.py:323

• PR description says the non-disjoint holdout case should yield action eject_unverified, but the implementation (and tests) use reject_unverified. Also, the gate action is only overridden when the candidate would have been accepted; if the intent is to always surface an explicit “unverified” action whenever holdout_leaked is true, this currently won’t happen.

        if accepted and holdout_leaked:
            action = "reject_unverified"
            accepted = False

skillopt_sleep/cycle.py:183

• The holdout-leak banner is keyed only off report.holdout_leaked, but in gate_mode=off the gate does no validation scoring at all. In that case the message “The gate scored the same tasks…” is inaccurate/misleading. Consider suppressing this banner when gate_mode is off (or adjust the wording for the greedy path).

    if report.holdout_leaked:
        lines[-1:] = [
            "> **Not validated.** The gate scored the same tasks the optimizer "
            "saw, so the comparison above cannot detect overfitting. Mine more "
            "tasks so a disjoint validation slice exists. Any edits below are "

tests/test_holdout_integrity.py:52

• Test name has an extra "test_" prefix ("test_test_split...") which makes the intent harder to scan in test output. Consider renaming to a single, descriptive prefix.

def test_test_split_is_never_used_as_train_or_val() -> None:

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

skillopt_sleep/consolidate.py:323

• reject_unverified sets accepted = False, which later causes applied edits to be moved into rejected_edits and the returned new_skill/new_memory to fall back to the originals. As a result, the staging step can’t write proposed_SKILL.md/proposed_CLAUDE.md, even though the PR description says unverified edits are still staged for human review. Consider returning the candidate skill/memory separately (or otherwise surfacing the patched docs) so the cycle can stage them while keeping accepted=False.

        if accepted and holdout_leaked:
            action = "reject_unverified"
            accepted = False

skillopt_sleep/cycle.py:184

• The holdout_leaked condition covers any overlap between train and val (including partial overlap by id), but this banner says the gate scored “the same tasks” the optimizer saw, which reads like full equality. Wording it in terms of overlap/non-disjoint slices would be more accurate and match the new _split() semantics.

            "> **Not validated.** The gate scored the same tasks the optimizer "
            "saw, so the comparison above cannot detect overfitting. Mine more "
            "tasks so a disjoint validation slice exists. Any edits below are "
            "unverified suggestions.",

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[lufen]

Follow-up in df08665 addresses the re-review's suppressed suggestions: the not-validated banner now says the validation slice was "non-disjoint (overlapping)" rather than "the same tasks", and on a leaked-holdout night the surfaced edits are labeled "Unverified suggestions (not validated)" instead of "negative feedback". rejected_edits are informational only (report/logging), not fed back as training signal, so no material is lost. All suites green.

[Yif-Yang]

Validated the current head and holdout-integrity regressions. Failing closed when train and validation are not disjoint prevents a misleading certified improvement while still preserving the candidate for human review. Thank you for fixing this correctness issue.