Skip to content

fix(sleep): close five ways invalid evidence reaches the gate - #310

Open
Bogdan (Dan) Baciu (bogdanbaciu21) wants to merge 5 commits into
microsoft:mainfrom
bogdanbaciu21:exc-005-gate-evidence-integrity
Open

Bogdan (Dan) Baciu (bogdanbaciu21) wants to merge 5 commits into
microsoft:mainfrom
bogdanbaciu21:exc-005-gate-evidence-integrity

Conversation

@bogdanbaciu21

Copy link
Copy Markdown
Contributor

What Problem This Solves

The nightly gate is only as trustworthy as the evidence it scores. This PR closes five independent paths by which invalid evidence reaches it on current main. Each was found by executing a documented claim and watching it fail, and each is reproduced by a regression test that fails on main and passes here.

# Documented claim What actually happened on main Effect on the gate
1 Recall "never" lets held-out tasks back into training (dream.py) Recall blocks held-out tasks by id only. Ids hash project + intent, and the archive is shared across projects, so another project's copy of tonight's val task is recalled into training. _split's leak check is id-based too. A two-project cycle flips from reject to accept_new_best with holdout_leaked=False
2 Tool replay detects calls "from the shim's log, not from a self-reported marker" (backend.py, replay.py) The backend correctly returns tools_called=[], then the rule judge falls back to a TOOL_CALL: regex over the response and passes anyway A Claude replay whose shim log is empty scores 1.0 on tool_called
3 The judge returns a "0..1" score (prompts.py) CliBackend.judge accepts any float, including NaN/Infinity and replies on another scale On the default mixed metric, a candidate with flat hard accuracy and a 1.0 → 0.0 regression reads 0.625 → 0.750 and is accepted
4 An add that already exists "is skipped" and adopted rules are kept (memory.py) Items are written verbatim but read back one - line per item: a multi-line item loses its continuation lines on the next edit, - X passes the duplicate check for X, and lstrip('- ') eats --force A helpful edit is rejected (0.5 vs 0.5 instead of 0.5 → 1.0) because applying it silently deletes an adopted rule
5 Engine replay sessions are filtered from harvest (harvest.py) None of the static markers match the current attempt, judge, reflect, or miner prompts, and the duration fallback only covers prompts under 200 characters With projects: "all", every engine session is harvested and can be mined back as a user task

Why This Change Was Made

These are correctness fixes to evidence the project already relies on, not new behavior. Each commit is self-contained and can be reviewed or reverted on its own:

  1. Held-out twins. task_content_key() (normalized intent plus context excerpt) gives recall a content check alongside the id check. dream_consolidate passes tonight's val/test tasks as exclude_tasks, and _split treats a val task with a content twin in train as leaked, so the existing reject_unverified abstention applies on any route. This follows up the content-hash hardening that fix(sleep): honor val_fraction and test_fraction in the nightly cycle #235 listed as a non-goal.
  2. Measured tool calls. score_rule_judge(_with_feedback) takes verified_tools. replay_one sets it for tool tasks, which always go through attempt_with_tools, and only measured calls then satisfy tool_called. The inherited attempt_with_tools already converts markers into calls for backends without a tool loop, so single-shot backends are unchanged. Unmeasured judging keeps the documented marker approximation.
  3. Judge score contract. Scores must be finite and in [0, 1]; anything else fails closed as judge-score-out-of-range with the raw value in the rationale. Clamping was rejected because it would turn an 8/10 reply into a perfect 1.0.
  4. Learned items. One item is one line (whitespace collapsed, exactly one leading bullet removed) for add, replace, and set_learned. current_learned_lines joins continuation lines of blocks written by earlier versions onto their item instead of dropping them, so already-adopted text survives.
  5. Replay markers. Markers include the opening line of every registry template, default and active override, plus a phrase shared by the Claude, Codex, Copilot, and OpenCode tool-attempt prompts. Rewording a prompt can no longer silently reopen the gap. The existing static markers stay for older transcripts.

Project Fit

  • Keeps the gate's verdicts tied to held-out evidence, measured tool use, in-contract judge scores, the documents actually adopted, and genuine user tasks.
  • No new configuration, no new dependency, and no change for runs that never hit these paths.

User Impact

Operators get fewer certified-but-wrong verdicts and clearer diagnostics: holdout_leaked now catches content twins, a self-reported tool call reads as failed: tool_called=search, and an off-scale judge reply reads as judge-score-out-of-range: 8. Adopted multi-line rules stop disappearing between nights.

Proof

All tests are deterministic and offline. Provider boundaries are faked (subprocess.run or a scripted _call), or MockBackend is used. 44 new tests run against both trees:

Defect New tests On main (343db22) On this branch
1 Held-out twins 5 4 fail (recall twin, _split, gate abstention, two-project cycle); 1 passes 5 pass
2 Measured tool calls 6 4 fail (Claude shim route, measured empty list, judge API, optimizer feedback); 2 controls pass 6 pass
3 Judge score contract 14 7 fail (2.0, 8, -0.1, Infinity, NaN, boolean, the gate scenario); 7 in-range controls pass 14 pass
4 Learned items 5 5 fail 5 pass
5 Replay markers 14 9 fail (4 registry prompts, override, 3 tool-attempt prompts, harvest(scope="all")); 5 controls pass (4 real user prompts kept) 14 pass

The controls that pass on both trees are deliberate. Measured calls still pass. The default marker route still converts markers for single-shot backends. In-range judge scores are unchanged. Real user prompts that resemble engine wording are still harvested.

Testing

Current head 964d9c050935d57b73999901961d6657f25a5788

Fork-runner validation asserts actual_sha == expected_sha == 964d9c050935d57b73999901961d6657f25a5788 before testing. These are contributor-run checks, not official upstream CI.

Platform Python Complete suite Focused (tests/test_gate.py) Strict docs
Ubuntu 3.10 1,758 passed; 12 skipped; 359 subtests 22 passed n/a
Ubuntu 3.11 1,758 passed; 12 skipped; 359 subtests 22 passed n/a
Ubuntu 3.12 1,758 passed; 12 skipped; 359 subtests 22 passed passed
macOS arm64 3.12 1,758 passed; 12 skipped; 359 subtests 22 passed passed
Windows 3.12 1,707 passed; 62 skipped; 354 subtests; 6 failed, all pre-existing 22 passed passed

The six Windows failures are test_home_is_refused_even_when_it_is_a_git_root and five subcases of test_cycle_stages_only_documents_that_changed. The same Windows job on unmodified main (343db22) fails the identical six (6 failed, 1,663 passed). The 44-test difference is exactly the new tests, and none of the six touch code changed here. They are reported, not fixed, to keep this PR scoped.

Local macOS x86_64 / Python 3.12: the complete suite reported 1,759 passed, 11 skipped, 359 subtests; the five new files reported 44 passed; strict docs and git diff --check passed; ruff findings on the touched modules are identical to main.

This branch and #263 also merge cleanly with each other in both orders, and the combined tree reports 1,821 passed, 11 skipped, 359 subtests.

No paid provider was called. These are contributor-run checks, not official upstream CI.

Limitations & Negative Results

  • Content twins are normalized-exact (case and whitespace). Near-duplicate paraphrases are not detected; that remains the open follow-up from fix(sleep): honor val_fraction and test_fraction in the nightly cycle #235.
  • Recall still draws across projects. Only exact held-out twins are blocked; whether recall should be scoped per project is a design decision this PR does not make.
  • An out-of-range judge reply now scores 0.0 rather than being rescaled, so a judge prompt override that asks for another scale will visibly fail closed until it is corrected.
  • The replay-marker fix covers the Claude-format harvester and the harvesters that reuse _is_headless_replay. The Codex harvester is skillopt-sleep Codex harvest ingests its own headless replay sessions #286's scope and is untouched. --no-session-persistence was not added to claude -p, because older CLI versions would reject the flag.
  • Learned items are now stored as single lines, so a multi-line item proposed by reflection is joined with spaces rather than kept as a block.

Reproduce It Yourself

Linux or macOS, shell in a fresh working directory:

git clone https://github.com/bogdanbaciu21/SkillOpt.git SkillOpt-review-gate-evidence
cd SkillOpt-review-gate-evidence
git checkout --detach 964d9c050935d57b73999901961d6657f25a5788
python3 -m venv .venv
.venv/bin/python -m pip install -e '.[dev,docs]'
.venv/bin/python -m pytest tests/test_recall_holdout_twins.py tests/test_tool_evidence_judging.py tests/test_judge_score_contract.py tests/test_learned_block_roundtrip.py tests/test_harvest_engine_prompt_markers.py -q
.venv/bin/python -m pytest -q
.venv/bin/python -m mkdocs build --strict

recall_similar blocked tonight's held-out tasks by id only. Ids hash the
project with the intent and the recall archive is shared across projects,
so another project's copy of tonight's val task was recalled into training
under a different id. The leak check in _split was id-based too, so the
gate certified the result: in a two-project cycle the verdict flipped from
reject to accept_new_best with holdout_leaked=False.

- task_content_key(): normalized intent plus context excerpt
- recall_similar(exclude_tasks=...) blocks archived tasks whose content key
  matches a held-out task; dream_consolidate passes tonight's val/test tasks
- _split treats a val task with a content twin in train as leaked, so the
  existing reject_unverified abstention applies on any route

Follows up the content-hash hardening listed as a non-goal in microsoft#235.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l route

Tool-loop backends detect real calls (for example from the search shim's
call log) and return an empty tools_called when the agent never ran the
tool. The rule judge then fell back to a TOOL_CALL marker regex over the
response text and passed the check anyway, undoing the verification one
step later: a Claude replay whose shim log was empty scored 1.0.

- _check / score_rule_judge(_with_feedback) take verified_tools; when set,
  only measured calls satisfy tool_called
- replay_one sets it for tool tasks, which always go through
  attempt_with_tools; the inherited marker fallback still converts markers
  into calls there, so single-shot backends are unchanged
- optimizer feedback for a self-reported call asks for a real call
- unmeasured single-shot judging keeps the documented marker approximation

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The judge prompt asks for a 0..1 score, but CliBackend.judge accepted any
float, including NaN and Infinity (json.loads parses both) and replies on
another scale. On the default mixed gate metric, one out-of-range soft score
let a candidate with flat hard accuracy and a 1.0 -> 0.0 regression read as
0.625 -> 0.750 and be accepted.

Scores must now be finite and within [0, 1]; anything else fails closed as
judge-score-out-of-range with the raw value in the rationale. Clamping was
rejected because it would turn an 8/10 reply into a perfect 1.0. Boolean
scores fall through to judge-parse-failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The learned block is read back one "- " bullet per item, but set_learned
wrote items verbatim. A multi-line item lost every continuation line the
next time any edit was applied; a "- X" add slipped past the duplicate check
for an existing "X"; and lstrip('- ') removed leading dashes such as
"--force". Losing adopted text changes what the gate scores: a helpful edit
was rejected (0.5 vs 0.5) because applying it silently deleted an adopted
rule held on a continuation line.

- _learned_item(): one item is one line (whitespace collapsed) with exactly
  one leading bullet marker removed; used for add, replace and set_learned
- current_learned_lines joins continuation lines of blocks written by
  earlier versions onto their item instead of dropping them

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_is_headless_replay recognised the engine's own sessions by static prompt
markers written for earlier prompt wording. None of them match the current
attempt, judge, reflect or miner templates, and the duration fallback only
covers prompts under 200 characters, so with projects="all" every engine
session was harvested and could be mined back as a user task.

- markers now include the opening line of every registry template, default
  and active override, so rewording a prompt cannot reopen the gap
- a shared phrase of the Claude, Codex, Copilot and OpenCode tool-attempt
  prompts is matched case-insensitively
- the existing static markers stay for transcripts from older versions;
  the Codex harvester (microsoft#286) is left to its own fix

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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