From ad9d2f505d53d6ba1ab575577cc79d5d28b07248 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 15:54:46 +0000 Subject: [PATCH 1/2] SLM-418: write replay-preference corpus + run real preference train (DSH5-10, seventh slice) Closes the sixth slice's own gap: writes the bounded synthetic corpus's train/held-out PreferencePair splits to real corpus files under this repo's existing preference-pairs root, then runs the real, unchanged `slm preference train` harness against a from-scratch wf_smoke_v2 checkpoint. Held-out pairwise-preference accuracy is honestly measured before/after training (train.py gains pairwise_preference_margin/held_out_pairwise_ accuracy/evaluate_replay_preference_held_out_benefit) -- real result is a ceiling effect (1.0 baseline, 1.0 trained, on 4 held-out pairs), so verdict is `no_benefit_fixture_scale`, not a benefit claim. The DSH3-selected TypedOperatorPolicyScorer, four-baseline comparison, and CAP0/CAP1/CAP2 retention remain out of scope. Co-Authored-By: Claude Sonnet 5 --- docs/design/dsh5-10-replay-preference-rows.md | 238 +++++++++++++++++- scripts/build_replay_preference_corpus.py | 61 +++++ ...sure_replay_preference_held_out_benefit.py | 62 +++++ ...replay_preference_context_view_variants.py | 12 + .../preference/replay_preference_corpus.py | 175 +++++++++++++ .../harnesses/preference/train.py | 134 ++++++++++ src/slm_training/resources/versions.json | 26 +- .../preference/test_operator_history_pairs.py | 25 ++ .../test_replay_preference_corpus.py | 148 +++++++++++ ...test_replay_preference_held_out_benefit.py | 164 ++++++++++++ 10 files changed, 1042 insertions(+), 3 deletions(-) create mode 100644 scripts/build_replay_preference_corpus.py create mode 100644 scripts/measure_replay_preference_held_out_benefit.py create mode 100644 src/slm_training/harnesses/preference/replay_preference_corpus.py create mode 100644 tests/test_harnesses/preference/test_replay_preference_corpus.py create mode 100644 tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py diff --git a/docs/design/dsh5-10-replay-preference-rows.md b/docs/design/dsh5-10-replay-preference-rows.md index e4969ee91..6550d27de 100644 --- a/docs/design/dsh5-10-replay-preference-rows.md +++ b/docs/design/dsh5-10-replay-preference-rows.md @@ -1,6 +1,6 @@ # DSH5-10: replay-grounded preference rows from undo/redo history (SLM-418) -**Status:** partial slice, in progress (sixth increment). +**Status:** partial slice, in progress (seventh increment). **Claim class:** `wiring`. **Honest verdict:** not yet dispositioned -- this PR extends a scoped subset, not the full issue. @@ -26,13 +26,25 @@ policy/control heads, the four-baseline comparison, held-out benefit measurement, or turn-depth/context-view ablations -- all of that remains exactly as unattempted as before this slice. -**Update (sixth slice, below):** the turn-depth/context-view *structural* +**Update (sixth slice):** the turn-depth/context-view *structural* ablation dimension and a bounded, fixture-scale matched context-view comparison are now wired -- see "Sixth slice" for exactly what that does and does not close. Real SFT/preference training against the DSH3-selected policy/control heads, a powered/real corpus, and CAP0/CAP1/CAP2 retention measurement remain unattempted. +**Update (seventh slice, below):** the sixth slice (v7)'s own "explicitly +still not attempted" gap is now closed for the bounded synthetic corpus -- +the converted `PreferencePair`s are written to a real corpus file under this +repo's existing preference-pairs root, and a real (non-fixture, from-scratch) +`TwoTowerModel` checkpoint is trained against them via the existing `slm +preference train` harness. Held-out pairwise-preference accuracy is measured +before and after training and is honestly a **ceiling effect** (1.0 in both +cases, on 4 held-out pairs) -- see "Seventh slice" for the real numbers and +why no benefit claim is made. The DSH3-selected `TypedOperatorPolicyScorer`, +the four-baseline comparison, and CAP0/CAP1/CAP2 retention remain +unattempted. + ## What this PR delivers * `src/slm_training/dsl/operators/replay_preference.py`: @@ -648,6 +660,203 @@ correctness or honesty defect. gets a `no-bump:` history entry for the two new re-exports from `operators/__init__.py`. +## Seventh slice (2026-07-27) + +The sixth slice (v7)'s own "Explicitly still not attempted" note said: no +`PreferencePair` produced by that slice had been written to a corpus file or +fed to `slm preference build-pairs`/`train`. This slice closes exactly that +gap for the bounded synthetic corpus +(`synthesize_bounded_session_corpus`) -- writing real corpus files and +running the real, already-existing `slm preference train` harness against +them, then measuring the real held-out result. It does **not** attempt the +DSH3-selected `TypedOperatorPolicyScorer`, the four-baseline comparison, or +CAP0/CAP1/CAP2 retention -- those stay exactly as out of scope as every prior +slice left them. + +### What was built + +* `src/slm_training/harnesses/preference/replay_preference_context_view_variants.py`: + `ReplayPreferenceSessionV1` gains a `trace: ConversationTraceV1 | None = + None` field -- the real trace every session but `merge_success` was already + built from internally, now exposed on the dataclass so a caller can feed a + session's own rows into `preference_pairs_from_trace` directly rather than + re-deriving a trace from `state_lookup`. Additive and optional; no existing + field, schema serialization, or behavior changed. + `synthesize_bounded_session_corpus` now passes `trace=trace` for the seven + sessions that have one; `merge_success` keeps `trace=None` (no trace object + exists for a merge decision -- see `replay_preference.py`'s own module + docstring). +* `src/slm_training/harnesses/preference/replay_preference_corpus.py` (new; + `harness.preference.replay_preference_corpus` v1). Pure, no-torch corpus + writer: + * `replay_preference_pairs_for_split(sessions, split)` -- converts every + row of every session in one split (`"train"` or `"held_out"`) into a + `PreferencePair`. Reuses `preference_pairs_from_trace` unchanged for + every session that carries a real trace, and + `preference_pair_from_replay_row` directly (looked up via that session's + own `state_lookup`) for `merge_success` -- the exact trace/direct-path + skip convention the sixth slice's own tests already prove at + single-trace scale, now applied uniformly across a whole corpus. A row + whose input state is missing from its session's `state_lookup` is + honestly recorded in a `skipped` list, never fabricated; in a real run, + `skipped` is empty for both splits (every row's input state really is in + its own session's lookup, by construction). + * `write_replay_preference_corpus(out_path, split, sessions=None)` -- + writes one split's pairs to `out_path` via the existing `write_pairs` + writer, into this repo's existing preference-pairs corpus root + (`outputs/data/preference/` -- the same root `slm preference + build-pairs --out` already writes into; no second corpus tree). Fails + closed (`ValueError`) rather than writing an empty file when a split + produces zero pairs. +* `src/slm_training/harnesses/preference/train.py` gains three new + functions, reusing `dpo_loss`'s own private masked-log-prob machinery + rather than inventing a parallel scoring path: + * `pairwise_preference_margin(model, pair)` -- `chosen_logprob - + rejected_logprob` under a `TwoTowerModel`'s current weights; the same + quantity `dpo_loss` already computes, exposed standalone. + * `held_out_pairwise_accuracy(model, pairs)` -- pairwise chosen>rejected + accuracy over a list of pairs. **Not unconditionally deterministic**: + `_logprob_of_target` applies fresh random masking noise on every call + (the same "mild noise" `dpo_loss` trains under), so repeated calls can + differ by a small amount unless the caller fixes `torch.manual_seed` + immediately beforehand -- documented directly in the function's own + docstring rather than glossed over. + * `evaluate_replay_preference_held_out_benefit(*, baseline_checkpoint, + held_out_pairs, trained_checkpoint=None, device="cpu", seed=0)` -- loads + one or two checkpoints, measures each (seeding `torch.manual_seed(seed)` + immediately before each measurement so a given seed reproduces the same + result for the same pairs/architecture), and reports a `verdict` using + the same strict-greater-than convention as the sixth slice's own + `held_out_benefit` (`benefit_observed_fixture_scale` only when the + trained checkpoint's accuracy strictly exceeds the baseline's; + `no_benefit_fixture_scale` otherwise, including ties). +* `scripts/build_replay_preference_corpus.py` (new) -- CLI writing the + train and/or held-out split to the canonical corpus root. +* `scripts/measure_replay_preference_held_out_benefit.py` (new) -- CLI + wrapping `evaluate_replay_preference_held_out_benefit` over checkpoints and + a pairs file on disk. +* Tests: `tests/test_harnesses/preference/test_replay_preference_corpus.py` + (new, pure corpus-conversion/writer coverage, including the + fail-closed-on-empty-split path) and + `tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py` + (new, exercises the three new `train.py` functions against a tiny real + `TwoTowerModel` built the same way `tests/test_harnesses/preference/ + test_local_train.py`'s own `_model()` fixture does). One new test in + `tests/test_harnesses/preference/test_operator_history_pairs.py` asserts + every session but `merge_success` now carries its real trace. +* `harness.preference.replay_preference_context_view_variants` bumped v2 -> + v3 (the `trace` field addition); new component + `harness.preference.replay_preference_corpus` registered at v1, claiming + the new module/scripts/tests plus `train.py` (previously unclaimed by any + component). + +### What was measured (real runs) + +Real corpus build (`python -m scripts.build_replay_preference_corpus +--split both`): **36 train pairs** across 6 sessions (`rollback_chain_2`, +`rollback_chain_4`, `rollback_chain_8`, `rollback_chain_16`, `pronoun_focus`, +`merge_success`) and **4 held-out pairs** across 2 sessions +(`rollback_chain_1`, `checkout_and_fork`) -- matching the sixth slice's own +6-train/2-held-out session split and 36+4=40 total row count exactly. +`skipped` was empty for both splits, including the `merge_success` session's +one row (confirmed converting via the direct `state_lookup` path, not +`preference_pairs_from_trace`, since `merge_success.trace is None`). + +Real from-scratch checkpoint (reusing the existing `wf_smoke_v2` smoke-loop +recipe, per this repo's own prior "docs(autotrain)" entries -- never a new +fixture corpus): + +```bash +python -m scripts.train_model --train-dir src/slm_training/resources/data/train/wf_smoke_v2 \ + --model twotower --context-backend scratch --steps 8 \ + --run-id dsh5_10_seventh_slice_scratch --no-sync-checkpoints --device cpu --seed 0 +``` + +Wall time: 10.7s (well inside `MAX_RUN_MINUTES`). Checkpoint written to +`outputs/runs/dsh5_10_seventh_slice_scratch/checkpoints/last.pt` (not +committed; `outputs/` is gitignored). + +Real preference training against the 36 train-split pairs, via the existing, +unchanged `slm preference train` harness: + +```bash +python -m scripts.train_preference train \ + --checkpoint outputs/runs/dsh5_10_seventh_slice_scratch/checkpoints/last.pt \ + --pairs outputs/data/preference/replay_preference_train_pairs.jsonl \ + --out-dir outputs/runs/dsh5_10_seventh_slice_preference --steps 20 --device cpu +``` + +Wall time: 9.5s. Real output: `last_loss=0.04492715746164322`, +`mean_loss=0.12848591189831496`, `n_pairs=36`, +`note="Surrogate preference loss on masked-token log-probs; no frozen +reference model (not textbook DPO)."` (the harness's own honesty label, +unchanged). Checkpoint written to +`outputs/runs/dsh5_10_seventh_slice_preference/model.pt`. + +Real held-out benefit measurement (`python -m +scripts.measure_replay_preference_held_out_benefit`, `--seed 0`), comparing +the scratch checkpoint (baseline) against the same checkpoint after the +preference-training run above (trained), on the 4 held-out pairs: + +* **Baseline:** `pairwise_preference_accuracy=1.0`, `mean_margin=5.274`. +* **Trained:** `pairwise_preference_accuracy=1.0`, `mean_margin=18.103`. +* **Verdict: `no_benefit_fixture_scale`.** This is an honest **ceiling + effect**, not evidence of no benefit in general: the same 4-pair held-out + split the sixth slice's diagnostic linear scorer already classified + perfectly (`held_out_benefit.verdict=no_benefit_fixture_scale`, + `baseline_accuracy=1.0`) is, unsurprisingly, also classified perfectly by + a real `TwoTowerModel` both before and after preference training -- there + is no headroom left in 4 pairs for a real accuracy improvement to show up. + The mean margin (how confidently the model separates chosen from rejected, + not whether it does) more than tripled after training (5.274 -> 18.103), + which is a real, directionally-consistent training-signal effect, but + `verdict` intentionally does not credit a margin increase as "benefit" -- + only a strict pairwise-accuracy improvement counts, per the same + convention the sixth slice's own `held_out_benefit` used. **claim_class: + `wiring`** -- this is a real training run and a real measurement, not a + certified or powered held-out-benefit claim; the held-out split's own tiny + size (4 pairs) is the reason, exactly as `evaluate_replay_preference_held_ + out_benefit`'s own `notes` field says. +* Corpus composition, ablation-grid structure, trace-replay, and + unintended-mutation properties are unchanged and are not re-measured this + slice -- see the sixth slice's own section above for that evidence + (nothing in this slice touches `dsl.operators.replay_preference`, + `replay_preference_context_views.py`'s view construction, or the linear + ablation scorer). + +### Explicitly out of scope (unchanged from, or newly identified by, this slice) + +* Training against the DSH3-selected policy/control heads + (`TypedOperatorPolicyScorer`) -- the checkpoint trained here is the + existing `TwoTowerModel` preference harness's own denoiser, never the + DSH3-selected head. No `TypedOperatorPolicyScorer` code, checkpoint, or + evaluation is touched by this slice. +* The four-baseline comparison and CAP0/CAP1/CAP2 retention -- still require + the full CAP-gated eval suite integrated with a trained policy checkpoint; + genuinely out of scope for this slice's real-but-fixture-scale TwoTower + run. +* A powered or real (non-synthetic) corpus -- 36 train / 4 held-out pairs is + wiring evidence over a real model, not a statistically powered held-out- + benefit study. The held-out ceiling effect above is a direct symptom of + this: 4 pairs cannot show accuracy headroom even when the underlying + margins move substantially. +* Turn-depth padding for `CHECKOUT_ANOTHER_STATE`, `FORK_THEN_CHOOSE_ONE_BRANCH`, + `MERGE_SUCCESS`, and `PRONOUN_FOCUS_FOLLOWUP` -- unchanged from the sixth + slice's own note; still one synthetic session each, no padded history. +* Reproducibility of `held_out_pairwise_accuracy` across separate, + unseeded calls -- documented as a real limitation (masking noise), not + silently assumed away; `evaluate_replay_preference_held_out_benefit`'s + `seed` parameter is the mitigation this slice ships, not a claim that the + underlying measurement is noise-free. +* Checkpoint promotion, syncing, or `docs/MODEL_CARD.md` / README updates -- + both checkpoints this slice produces are local, from-scratch/smoke-scale, + and gitignored (`outputs/` is never committed); neither is a roster, + matrix-champion, or production-ship checkpoint, so no model-card update + applies. + +No causal, calibration, or promotion claim is made. `claim_class: wiring` +throughout. + ## Reproducibility ```bash @@ -670,3 +879,28 @@ NODE_OPTIONS= pytest -q tests/test_dsl/test_replay_preference.py tests/test_dsl/ Result (fifth slice, v6): real run in a fresh `.venv` -- Python 3.12, `pip install -e ".[dev,grammar]"`, plus `NODE_OPTIONS= npm ci` in `src/apps/openui_bridge` for the G2/G8 schema-oracle gates the pack authority requires; the ambient `--import tsx` `NODE_OPTIONS` is rejected by this Node 22 build both for `npm ci` and for `pytest`, unrelated to this change): `61 passed`. Also verified: `ruff check` clean on every changed file; `python -m scripts.verify_version_stamps --check --base origin/claude/great-dirac-v82ph9` -- `ok (2 component(s) touched)`; `python -m scripts.repo_policy` -- `ok`; `python -m scripts.verify_decode_invariants` -- clean. Result (this PR, sixth slice, v7): same command, same freshly built `.venv` (Python 3.12) plus `NODE_OPTIONS= npm ci` in `src/apps/openui_bridge` -- test file grew by 8 tests (7 conversion-coverage tests + 1 refusal test) in the same `tests/test_dsl/test_replay_preference.py`, no new test module needed: `69 passed`. Also verified: `ruff check` on every changed file (`src/slm_training/dsl/operators/replay_preference.py`, `src/slm_training/dsl/operators/__init__.py`, `tests/test_dsl/test_replay_preference.py`) -- `All checks passed!`; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (vs 5f94b925a121; 4 changed file(s), 2 component(s) touched)`; `python -m scripts.repo_policy` -- `repo-policy: ok (tracked + untracked)`; `python -m scripts.verify_decode_invariants` -- exits clean (`0`). + +## Reproducibility (seventh slice) + +```bash +NODE_OPTIONS= pytest -q tests/test_harnesses/preference/test_replay_preference_corpus.py tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py tests/test_harnesses/preference/test_operator_history_pairs.py tests/test_dsl/test_replay_preference.py tests/test_evals/test_ambiguous_operator_followups.py +python -m scripts.build_replay_preference_corpus --split both +python -m scripts.train_model --train-dir src/slm_training/resources/data/train/wf_smoke_v2 \ + --model twotower --context-backend scratch --steps 8 \ + --run-id dsh5_10_seventh_slice_scratch --no-sync-checkpoints --device cpu --seed 0 +python -m scripts.train_preference train \ + --checkpoint outputs/runs/dsh5_10_seventh_slice_scratch/checkpoints/last.pt \ + --pairs outputs/data/preference/replay_preference_train_pairs.jsonl \ + --out-dir outputs/runs/dsh5_10_seventh_slice_preference --steps 20 --device cpu +python -m scripts.measure_replay_preference_held_out_benefit \ + --baseline-checkpoint outputs/runs/dsh5_10_seventh_slice_scratch/checkpoints/last.pt \ + --trained-checkpoint outputs/runs/dsh5_10_seventh_slice_preference/model.pt \ + --held-out-pairs outputs/data/preference/replay_preference_held_out_pairs.jsonl \ + --device cpu --seed 0 +python -m scripts.verify_version_stamps --check --base origin/main +python -m scripts.repo_policy +python -m scripts.verify_decode_invariants +ruff check src/slm_training/harnesses/preference/replay_preference_context_view_variants.py src/slm_training/harnesses/preference/train.py src/slm_training/harnesses/preference/replay_preference_corpus.py scripts/build_replay_preference_corpus.py scripts/measure_replay_preference_held_out_benefit.py tests/test_harnesses/preference/test_replay_preference_corpus.py tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py tests/test_harnesses/preference/test_operator_history_pairs.py +``` + +Result (this PR, seventh slice): fresh `.venv` -- Python 3.12, `pip install -e ".[dev,grammar]"`, plus `NODE_OPTIONS= npm ci` in `src/apps/openui_bridge` (same schema-oracle gate requirement as every prior slice). `pytest`: `65 passed` (the two new test files plus the touched `test_operator_history_pairs.py`, `test_replay_preference.py`, and `test_ambiguous_operator_followups.py`, all unaffected by this slice's changes). `python -m scripts.build_replay_preference_corpus --split both`: `36` train pairs (6 sessions) + `4` held-out pairs (2 sessions), `skipped_count=0` for both. `python -m scripts.train_model ...` (scratch checkpoint): `10.7s` wall. `python -m scripts.train_preference train ...` (20 steps against the 36 train pairs): `9.5s` wall, `last_loss=0.04492715746164322`, `mean_loss=0.12848591189831496`. `python -m scripts.measure_replay_preference_held_out_benefit ...` (4 held-out pairs, `--seed 0`): `baseline.pairwise_preference_accuracy=1.0` (`mean_margin=5.274418354034424`), `trained.pairwise_preference_accuracy=1.0` (`mean_margin=18.10257863998413`), `verdict="no_benefit_fixture_scale"` -- an honest ceiling effect (see "Seventh slice" above), not a benefit claim. `ruff check`: clean on every touched/created file. `python -m scripts.verify_version_stamps --check --base origin/main`: `ok (vs 4d650b7e901f; 18 changed file(s), 2 component(s) touched)` (the extra changed files beyond this slice's own 10 are four unrelated upstream `docs(autotrain)` lever-smoke commits that landed on `origin/main` after this branch's base -- no overlap with any file this slice touches). `python -m scripts.repo_policy`: `ok (tracked + untracked)`. `python -m scripts.verify_decode_invariants`: clean. diff --git a/scripts/build_replay_preference_corpus.py b/scripts/build_replay_preference_corpus.py new file mode 100644 index 000000000..1621a1f06 --- /dev/null +++ b/scripts/build_replay_preference_corpus.py @@ -0,0 +1,61 @@ +#!/usr/bin/env python3 +"""SLM-418 (DSH5-10) seventh slice: write the bounded replay-preference +corpus to real ``PreferencePair`` corpus files. + + python -m scripts.build_replay_preference_corpus + python -m scripts.build_replay_preference_corpus --split train --out outputs/data/preference/replay_preference_train_pairs.jsonl + +Writes into this repo's existing preference-pairs corpus root +(``outputs/data/preference/``) -- never a second corpus tree. This script +only builds and writes the corpus; feed the train-split file into the +existing ``slm preference train`` harness and measure the held-out split with +``scripts.measure_replay_preference_held_out_benefit`` (see +docs/design/dsh5-10-replay-preference-rows.md's reproducibility commands). +This is fixture-scale wiring evidence, not a ship-readiness claim, and +completes in well under a second -- far inside +``slm_training.levers.MAX_RUN_MINUTES``. +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path + +from slm_training.harnesses.preference.replay_preference_corpus import ( + DEFAULT_HELD_OUT_OUT_PATH, + DEFAULT_TRAIN_OUT_PATH, + write_replay_preference_corpus, +) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--split", + choices=("train", "held_out", "both"), + default="both", + help="Which split to write (default: both, one file each).", + ) + parser.add_argument( + "--train-out", type=Path, default=DEFAULT_TRAIN_OUT_PATH, + help="Output path for the train-split pairs file.", + ) + parser.add_argument( + "--held-out-out", type=Path, default=DEFAULT_HELD_OUT_OUT_PATH, + help="Output path for the held-out-split pairs file.", + ) + args = parser.parse_args(argv) + + reports = [] + if args.split in ("train", "both"): + reports.append(write_replay_preference_corpus(args.train_out, "train")) + if args.split in ("held_out", "both"): + reports.append(write_replay_preference_corpus(args.held_out_out, "held_out")) + + print(json.dumps([report.to_dict() for report in reports], indent=2, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/measure_replay_preference_held_out_benefit.py b/scripts/measure_replay_preference_held_out_benefit.py new file mode 100644 index 000000000..44002b7bb --- /dev/null +++ b/scripts/measure_replay_preference_held_out_benefit.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""SLM-418 (DSH5-10) seventh slice: honest held-out pairwise-preference +benefit measurement for a real ``slm preference train`` run against the +replay-preference corpus. + + python -m scripts.measure_replay_preference_held_out_benefit \\ + --baseline-checkpoint outputs/runs//last.pt \\ + --held-out-pairs outputs/data/preference/replay_preference_held_out_pairs.jsonl \\ + [--trained-checkpoint outputs/runs//model.pt] + +Loads a baseline (pre-preference-training) checkpoint and, when given, a +trained (post ``slm preference train``) checkpoint, measures each one's +pairwise chosen>rejected preference accuracy on the held-out-split pairs, and +reports an honest ``verdict`` -- ``benefit_observed_fixture_scale`` only when +the trained checkpoint's accuracy strictly exceeds the baseline's, otherwise +``no_benefit_fixture_scale``. This is fixture-scale wiring evidence over a +handful of real pairs, never a certified or powered ship-readiness claim -- +see docs/design/dsh5-10-replay-preference-rows.md's "Seventh slice". +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path + +from slm_training.harnesses.preference import load_pairs +from slm_training.harnesses.preference.train import ( + evaluate_replay_preference_held_out_benefit, +) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--baseline-checkpoint", type=Path, required=True) + parser.add_argument("--trained-checkpoint", type=Path, default=None) + parser.add_argument("--held-out-pairs", type=Path, required=True) + parser.add_argument("--device", default="cpu") + parser.add_argument( + "--seed", type=int, default=0, + help="torch.manual_seed applied before each measurement, for reproducibility.", + ) + parser.add_argument("--out", type=Path, default=None) + args = parser.parse_args(argv) + + held_out_pairs = load_pairs(args.held_out_pairs) + report = evaluate_replay_preference_held_out_benefit( + baseline_checkpoint=args.baseline_checkpoint, + trained_checkpoint=args.trained_checkpoint, + held_out_pairs=held_out_pairs, + device=args.device, + seed=args.seed, + ) + payload = json.dumps(report, indent=2, sort_keys=True) + print(payload) + if args.out: + args.out.write_text(payload + "\n", encoding="utf-8") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/slm_training/harnesses/preference/replay_preference_context_view_variants.py b/src/slm_training/harnesses/preference/replay_preference_context_view_variants.py index a3e3381d4..be2b3a4af 100644 --- a/src/slm_training/harnesses/preference/replay_preference_context_view_variants.py +++ b/src/slm_training/harnesses/preference/replay_preference_context_view_variants.py @@ -466,6 +466,14 @@ class ReplayPreferenceSessionV1: ``group_id`` is the unit ``split_for_group`` partitions on: every row in one session shares one split (the issue's adversarial control, "conversation variants stay in one split"). + + ``trace`` (seventh slice) is the real ``ConversationTraceV1`` the session + was built from, when one exists -- ``None`` for the ``merge_success`` + session, since no trace object exists for a merge decision (see + ``replay_preference.py``'s own module docstring). This lets a caller feed + a session's rows into ``preference_pairs_from_trace`` directly, matching + the sixth slice's own trace/direct-path conversion convention, without + re-deriving a trace from ``state_lookup`` alone. """ group_id: str @@ -473,6 +481,7 @@ class ReplayPreferenceSessionV1: report: OperatorEventMemoryReportV1 state_lookup: dict receipts: tuple[OperatorTurnReceiptV1, ...] + trace: ConversationTraceV1 | None = None #: Chain lengths exercised by the rollback-chain sessions -- directly the @@ -506,6 +515,7 @@ def synthesize_bounded_session_corpus() -> tuple[ReplayPreferenceSessionV1, ...] report=report, state_lookup=state_lookup_from_trace(trace), receipts=receipts_from_trace(trace), + trace=trace, ) ) @@ -520,6 +530,7 @@ def synthesize_bounded_session_corpus() -> tuple[ReplayPreferenceSessionV1, ...] report=checkout_fork_report, state_lookup=state_lookup_from_trace(checkout_fork_trace), receipts=receipts_from_trace(checkout_fork_trace), + trace=checkout_fork_trace, ) ) @@ -534,6 +545,7 @@ def synthesize_bounded_session_corpus() -> tuple[ReplayPreferenceSessionV1, ...] report=pronoun_report, state_lookup=state_lookup_from_trace(pronoun_trace), receipts=receipts_from_trace(pronoun_trace), + trace=pronoun_trace, ) ) diff --git a/src/slm_training/harnesses/preference/replay_preference_corpus.py b/src/slm_training/harnesses/preference/replay_preference_corpus.py new file mode 100644 index 000000000..9ffe23778 --- /dev/null +++ b/src/slm_training/harnesses/preference/replay_preference_corpus.py @@ -0,0 +1,175 @@ +"""SLM-418 (DSH5-10) seventh slice: write the bounded replay-preference corpus +to real ``PreferencePair`` corpus files, using this repo's existing +conversion functions and corpus root -- no second corpus shape or shadow +path. + +The sixth slice (v7) added :func:`~slm_training.dsl.operators. +replay_preference.preference_pair_from_replay_row` and +:func:`~slm_training.dsl.operators.replay_preference. +preference_pairs_from_trace`, which convert an already-extracted +``OperatorReplayPreferenceRowV1`` into this repo's existing ``PreferencePair`` +shape, but never wrote a pair to a corpus file or fed one into the real +``slm preference train``/``train-local`` harness (see that slice's own +"Explicitly still not attempted" note). This module closes exactly that gap +for the bounded synthetic corpus +(:func:`~slm_training.harnesses.preference. +replay_preference_context_view_variants.synthesize_bounded_session_corpus`): + +* :func:`replay_preference_pairs_for_split` converts every row in every + session of one split (``"train"`` or ``"held_out"``) into a + ``PreferencePair``, reusing ``preference_pairs_from_trace`` for every + session that carries a real ``ConversationTraceV1`` and + ``preference_pair_from_replay_row`` directly for ``merge_success`` (whose + row is grounded on a ``BranchEditV1`` tip, not any one trace) -- the exact + same trace/direct-path convention the sixth slice's own tests exercise, now + applied uniformly across a whole corpus instead of one hand-built trace. +* :func:`write_replay_preference_corpus` writes those pairs to this repo's + existing preference-pairs corpus root (``outputs/data/preference/`` -- the + same root ``slm preference build-pairs --out`` already writes into) via the + existing ``write_pairs`` writer. No second corpus tree is invented. + +Held-out pairs are written to their own file and are never mixed into the +train file -- this repo's isolation/provenance law ("never fit training data +to holdouts"). See docs/design/dsh5-10-replay-preference-rows.md's "Seventh +slice" for what real training run and held-out measurement this corpus feeds, +and for what is still explicitly out of scope (the DSH3-selected +``TypedOperatorPolicyScorer``, the four-baseline comparison, CAP0/CAP1/CAP2 +retention). +""" + +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path + +from slm_training.dsl.operators.replay_preference import ( + preference_pair_from_replay_row, + preference_pairs_from_trace, +) +from slm_training.harness_core.versioning import build_version_stamp +from slm_training.harnesses.preference import PreferencePair, write_pairs +from slm_training.harnesses.preference.local_decisions import Split +from slm_training.harnesses.preference.replay_preference_context_view_variants import ( + ReplayPreferenceSessionV1, + synthesize_bounded_session_corpus, +) + +#: Canonical preference-pairs corpus root this repo already uses (the same +#: root ``slm preference build-pairs --out`` writes into) -- never a second +#: corpus tree. +DEFAULT_TRAIN_OUT_PATH = Path("outputs/data/preference/replay_preference_train_pairs.jsonl") +DEFAULT_HELD_OUT_OUT_PATH = Path( + "outputs/data/preference/replay_preference_held_out_pairs.jsonl" +) + + +def replay_preference_pairs_for_split( + sessions: Sequence[ReplayPreferenceSessionV1], split: Split +) -> tuple[tuple[PreferencePair, ...], tuple[dict, ...]]: + """Convert every row of every ``split`` session into a ``PreferencePair``. + + Returns ``(pairs, skipped)``. Sessions carrying a real + ``ConversationTraceV1`` (every relation but ``merge_success``) go through + ``preference_pairs_from_trace`` unchanged; a session with no trace (only + ``merge_success`` today) converts each row directly via + ``preference_pair_from_replay_row``, honestly skipping -- never + fabricating a prompt for -- a row whose ``input_state_id`` is missing + from that session's own ``state_lookup``. This mirrors, at corpus scale, + the exact skip/direct-path convention the sixth slice's tests already + prove at single-trace scale. + """ + pairs: list[PreferencePair] = [] + skipped: list[dict] = [] + for session in sessions: + if session.split != split: + continue + if session.trace is not None: + session_pairs, session_skipped = preference_pairs_from_trace( + session.trace, session.report.rows + ) + pairs.extend(session_pairs) + skipped.extend(dict(entry) for entry in session_skipped) + continue + for row in session.report.rows: + node = session.state_lookup.get(row.input_state_id) + if node is None: + skipped.append( + { + "row": row.to_dict(), + "reason": ( + f"input_state_id {row.input_state_id!r} not found in " + f"session {session.group_id!r}'s state_lookup" + ), + } + ) + continue + pairs.append( + preference_pair_from_replay_row(row, input_state_source=node.state.source) + ) + return tuple(pairs), tuple(skipped) + + +@dataclass(frozen=True) +class ReplayPreferenceCorpusBuildReportV1: + """Honest counts for one written replay-preference corpus split file.""" + + split: Split + out_path: str + pair_count: int + skipped_count: int + session_count: int + row_count: int + skipped: tuple[dict, ...] + version_stamp: dict + schema: str = "replay_preference_corpus_build_report/v1" + + def to_dict(self) -> dict: + return { + "schema": self.schema, + "split": self.split, + "out_path": self.out_path, + "pair_count": self.pair_count, + "skipped_count": self.skipped_count, + "session_count": self.session_count, + "row_count": self.row_count, + "skipped": list(self.skipped), + "version_stamp": self.version_stamp, + } + + +def write_replay_preference_corpus( + out_path: Path | str, + split: Split, + *, + sessions: Sequence[ReplayPreferenceSessionV1] | None = None, +) -> ReplayPreferenceCorpusBuildReportV1: + """Build the bounded synthetic corpus and write one ``split`` to ``out_path``. + + Writes into this repo's existing preference-pairs corpus root via the + existing ``write_pairs`` writer -- never a second corpus tree. Raises + ``ValueError`` (fails closed) rather than writing an empty corpus file + when a split produces zero pairs. + """ + resolved_sessions = ( + tuple(sessions) if sessions is not None else synthesize_bounded_session_corpus() + ) + pairs, skipped = replay_preference_pairs_for_split(resolved_sessions, split) + if not pairs: + raise ValueError( + f"replay-preference corpus build produced zero {split!r} pairs; " + "refusing to write an empty corpus file" + ) + out = Path(out_path) + write_pairs(out, list(pairs)) + split_sessions = [session for session in resolved_sessions if session.split == split] + return ReplayPreferenceCorpusBuildReportV1( + split=split, + out_path=str(out), + pair_count=len(pairs), + skipped_count=len(skipped), + session_count=len(split_sessions), + row_count=sum(len(session.report.rows) for session in split_sessions), + skipped=skipped, + version_stamp=build_version_stamp("harness.preference.replay_preference_corpus"), + ) diff --git a/src/slm_training/harnesses/preference/train.py b/src/slm_training/harnesses/preference/train.py index 750450af2..9e0ff0c73 100644 --- a/src/slm_training/harnesses/preference/train.py +++ b/src/slm_training/harnesses/preference/train.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from collections.abc import Sequence from pathlib import Path import torch @@ -137,3 +138,136 @@ def train_preference_from_paths( ) summary["checkpoint"] = str(ckpt) return summary + + +# --------------------------------------------------------------------------- # +# SLM-418 (DSH5-10) seventh slice: held-out pairwise-preference measurement. +# +# Standalone exposure of the exact chosen/rejected log-prob margin +# ``dpo_loss`` already trains on, so a caller can measure whether a real +# TwoTowerModel checkpoint already prefers ``chosen`` over ``rejected`` on a +# held-out split -- honest evidence, never a certified benefit claim at +# fixture scale. See docs/design/dsh5-10-replay-preference-rows.md's +# "Seventh slice" for what this closes and what it still does not. +# --------------------------------------------------------------------------- # +def pairwise_preference_margin(model: TwoTowerModel, pair: PreferencePair) -> float: + """``chosen_logprob - rejected_logprob`` under ``model``'s current weights. + + The same masked teacher-forced log-prob :func:`dpo_loss` trains on, + exposed standalone for measurement. A positive margin means the model + already ranks ``chosen`` above ``rejected`` for this pair's prompt. + """ + context = ( + _context_of(model, pair.prompt, pair.design_md) + if _encoder_deterministic(model) + else None + ) + chosen_lp = _logprob_of_target( + model, pair.prompt, pair.chosen, pair.design_md, context=context + ) + rejected_lp = _logprob_of_target( + model, pair.prompt, pair.rejected, pair.design_md, context=context + ) + return float((chosen_lp - rejected_lp).detach().cpu()) + + +def held_out_pairwise_accuracy( + model: TwoTowerModel, pairs: Sequence[PreferencePair] +) -> dict: + """Pairwise chosen>rejected accuracy over ``pairs`` under ``model``'s weights. + + Puts ``model`` in eval mode (so the shared-context caching branch in + :func:`pairwise_preference_margin` reuses one encoded context per pair + instead of re-sampling it) and runs under ``torch.no_grad()`` -- this is + measurement, not training. This is **not** fully deterministic across + repeated calls: ``_logprob_of_target`` applies fresh random masking noise + each time it scores a target (the same "mild noise" ``dpo_loss`` trains + under), so the exact accuracy/margin can vary run-to-run by a small + amount unless the caller fixes ``torch.manual_seed`` immediately before + calling. Raises ``ValueError`` on an empty ``pairs`` rather than silently + reporting a vacuous 0/0 accuracy. + """ + if not pairs: + raise ValueError("held_out_pairwise_accuracy requires at least one pair") + model.eval() + correct = 0.0 + margins: list[float] = [] + with torch.no_grad(): + for pair in pairs: + margin = pairwise_preference_margin(model, pair) + margins.append(margin) + if margin > 0: + correct += 1.0 + elif margin == 0: + correct += 0.5 + return { + "pairwise_preference_accuracy": correct / len(pairs), + "n_pairs": len(pairs), + "mean_margin": sum(margins) / len(margins), + } + + +def evaluate_replay_preference_held_out_benefit( + *, + baseline_checkpoint: Path, + held_out_pairs: Sequence[PreferencePair], + trained_checkpoint: Path | None = None, + device: str = "cpu", + seed: int = 0, +) -> dict: + """Honest held-out pairwise-preference benefit report for two checkpoints. + + ``baseline_checkpoint`` is the pre-preference-training checkpoint (e.g. a + freshly built ``wf_smoke_v2`` scratch checkpoint); ``trained_checkpoint`` + is the same checkpoint after a real ``slm preference train`` run against + the replay-preference train-split pairs. When ``trained_checkpoint`` is + omitted, only the baseline is measured and no benefit verdict is made. + + ``seed`` is applied via ``torch.manual_seed`` immediately before each + ``held_out_pairwise_accuracy`` call (once for the baseline, once for the + trained checkpoint, if given) so the same ``seed`` reproduces the same + masking-noise draw for a fixed set of pairs and model architecture -- + ``held_out_pairwise_accuracy`` itself is not unconditionally deterministic + (see its own docstring), so this is what makes a reported number here + reproducible rather than a one-off random draw. + + ``verdict`` uses the same strict-greater-than convention as the sixth + slice's ``held_out_benefit`` (a tie or regression is honestly + ``no_benefit_fixture_scale``, never rounded up). This is fixture-scale + wiring evidence over a few real pairs -- never a certified or powered + ship-readiness claim. + """ + baseline_model = TwoTowerModel.from_checkpoint(baseline_checkpoint, device=device) + torch.manual_seed(seed) + baseline = held_out_pairwise_accuracy(baseline_model, held_out_pairs) + if trained_checkpoint is None: + return { + "baseline": baseline, + "trained": None, + "verdict": "no_trained_checkpoint", + "notes": ( + "only the baseline checkpoint was measured; pass " + "trained_checkpoint (the output of a real `slm preference " + "train` run against the replay-preference train-split " + "pairs) for a benefit comparison" + ), + } + trained_model = TwoTowerModel.from_checkpoint(trained_checkpoint, device=device) + torch.manual_seed(seed) + trained = held_out_pairwise_accuracy(trained_model, held_out_pairs) + verdict = ( + "benefit_observed_fixture_scale" + if trained["pairwise_preference_accuracy"] + > baseline["pairwise_preference_accuracy"] + else "no_benefit_fixture_scale" + ) + return { + "baseline": baseline, + "trained": trained, + "verdict": verdict, + "notes": ( + f"fixture-scale synthetic held-out corpus ({len(held_out_pairs)} " + "pairs); not a powered or certified held-out-benefit claim " + "either way -- see docs/design/dsh5-10-replay-preference-rows.md." + ), + } diff --git a/src/slm_training/resources/versions.json b/src/slm_training/resources/versions.json index 4f8295b31..4cde0084f 100644 --- a/src/slm_training/resources/versions.json +++ b/src/slm_training/resources/versions.json @@ -8016,7 +8016,7 @@ ] }, "harness.preference.replay_preference_context_view_variants": { - "version": "v2", + "version": "v3", "kind": "harness", "paths": [ "src/slm_training/harnesses/preference/replay_preference_context_view_variants.py", @@ -8027,6 +8027,11 @@ "docs/design/dsh5-10-replay-preference-rows.md" ], "history": [ + { + "version": "v3", + "date": "2026-07-27", + "note": "SLM-418 (DSH5-10) seventh slice: ReplayPreferenceSessionV1 gains a trace field (ConversationTraceV1 | None, default None) -- the real trace every session but merge_success was already built from, now exposed so harness.preference.replay_preference_corpus can feed a session's rows into preference_pairs_from_trace directly instead of re-deriving a trace from state_lookup. Additive/optional field; no existing behavior, schema serialization, or test changes. merge_success keeps trace=None (no trace object exists for a merge decision, per replay_preference.py's own module docstring)" + }, { "version": "v2", "date": "2026-07-27", @@ -8044,6 +8049,25 @@ } ] }, + "harness.preference.replay_preference_corpus": { + "version": "v1", + "kind": "harness", + "paths": [ + "src/slm_training/harnesses/preference/replay_preference_corpus.py", + "src/slm_training/harnesses/preference/train.py", + "scripts/build_replay_preference_corpus.py", + "scripts/measure_replay_preference_held_out_benefit.py", + "tests/test_harnesses/preference/test_replay_preference_corpus.py", + "tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py" + ], + "history": [ + { + "version": "v1", + "date": "2026-07-27", + "note": "SLM-418 (DSH5-10) seventh slice: initial registration. Closes the sixth slice's own 'explicitly still not attempted' gap -- writes the bounded synthesize_bounded_session_corpus() train/held-out splits to real PreferencePair corpus files (outputs/data/preference/, this repo's existing preference-pairs root; no second corpus tree), reusing preference_pairs_from_trace / preference_pair_from_replay_row verbatim (the sixth slice's own trace/direct-path conversion convention, now applied at corpus scale). train.py gains pairwise_preference_margin, held_out_pairwise_accuracy, and evaluate_replay_preference_held_out_benefit -- standalone exposure of the same masked teacher-forced log-prob margin dpo_loss already trains on, for honest held-out pairwise-preference measurement of a real TwoTowerModel checkpoint before/after a real `slm preference train` run against this corpus's train split. Still explicitly out of scope: the DSH3-selected TypedOperatorPolicyScorer, the four-baseline comparison, and CAP0/CAP1/CAP2 retention" + } + ] + }, "harness.quality.slop_forensics": { "version": "v1", "kind": "harness", diff --git a/tests/test_harnesses/preference/test_operator_history_pairs.py b/tests/test_harnesses/preference/test_operator_history_pairs.py index 1d952bdb2..e38fedadb 100644 --- a/tests/test_harnesses/preference/test_operator_history_pairs.py +++ b/tests/test_harnesses/preference/test_operator_history_pairs.py @@ -289,6 +289,31 @@ def test_synthetic_corpus_has_both_train_and_held_out_sessions() -> None: assert splits == {"train", "held_out"} +def test_every_session_but_merge_success_carries_its_real_trace() -> None: + """Seventh slice: ``ReplayPreferenceSessionV1.trace`` -- present or honestly absent. + + Every session but ``merge_success`` was built from a real + ``ConversationTraceV1`` and now exposes it (so a caller can feed a + session's rows into ``preference_pairs_from_trace`` directly, per + docs/design/dsh5-10-replay-preference-rows.md's "Seventh slice"). No + trace object exists for a merge decision (see ``replay_preference.py``'s + own module docstring), so that one session's ``trace`` stays ``None`` + rather than fabricating one. + """ + sessions = synthesize_bounded_session_corpus() + by_group = {session.group_id: session for session in sessions} + assert by_group["merge_success"].trace is None + for group_id, session in by_group.items(): + if group_id == "merge_success": + continue + assert session.trace is not None + # The trace's own state nodes really do contain every row's input + # state -- i.e. this is the same trace the row was extracted from, + # not merely some other trace of the same shape. + for row in session.report.rows: + assert session.trace.node(row.input_state_id) is not None + + # --------------------------------------------------------------------------- # # train_replay_preference_context_view_variants: the bounded comparison. # --------------------------------------------------------------------------- # diff --git a/tests/test_harnesses/preference/test_replay_preference_corpus.py b/tests/test_harnesses/preference/test_replay_preference_corpus.py new file mode 100644 index 000000000..99c03ad0f --- /dev/null +++ b/tests/test_harnesses/preference/test_replay_preference_corpus.py @@ -0,0 +1,148 @@ +"""Tests for SLM-418 (DSH5-10) seventh slice: writing the bounded +replay-preference corpus to real ``PreferencePair`` corpus files. + +Covers ``slm_training.harnesses.preference.replay_preference_corpus``. See +``docs/design/dsh5-10-replay-preference-rows.md`` "Seventh slice" for the +full disposition. +""" + +from __future__ import annotations + +import pytest + +from slm_training.dsl.operators import ReplayPreferenceRelation +from slm_training.harnesses.preference import load_pairs +from slm_training.harnesses.preference.replay_preference_context_view_variants import ( + synthesize_bounded_session_corpus, +) +from slm_training.harnesses.preference.replay_preference_corpus import ( + ReplayPreferenceCorpusBuildReportV1, + replay_preference_pairs_for_split, + write_replay_preference_corpus, +) + +_ROLLBACK_FAMILY = { + ReplayPreferenceRelation.EDIT_THEN_UNDO.value, + ReplayPreferenceRelation.UNDO_THEN_REDO.value, + ReplayPreferenceRelation.PARTIAL_ROLLBACK.value, +} + + +# --------------------------------------------------------------------------- # +# replay_preference_pairs_for_split +# --------------------------------------------------------------------------- # +def test_train_split_converts_every_train_row_with_nothing_skipped() -> None: + sessions = synthesize_bounded_session_corpus() + train_row_count = sum( + len(session.report.rows) for session in sessions if session.split == "train" + ) + pairs, skipped = replay_preference_pairs_for_split(sessions, "train") + assert not skipped + assert len(pairs) == train_row_count + assert train_row_count > 0 + + +def test_held_out_split_converts_exactly_the_two_held_out_sessions() -> None: + sessions = synthesize_bounded_session_corpus() + held_out_sessions = [session for session in sessions if session.split == "held_out"] + assert {session.group_id for session in held_out_sessions} == { + "rollback_chain_1", + "checkout_and_fork", + } + held_out_row_count = sum(len(session.report.rows) for session in held_out_sessions) + + pairs, skipped = replay_preference_pairs_for_split(sessions, "held_out") + assert not skipped + assert len(pairs) == held_out_row_count + relations = {pair.meta["semantic_relation"] for pair in pairs} + assert relations == { + ReplayPreferenceRelation.EDIT_THEN_UNDO.value, + ReplayPreferenceRelation.UNDO_THEN_REDO.value, + ReplayPreferenceRelation.CHECKOUT_ANOTHER_STATE.value, + ReplayPreferenceRelation.FORK_THEN_CHOOSE_ONE_BRANCH.value, + } + + +def test_merge_success_row_converts_via_the_direct_state_lookup_path() -> None: + """MERGE_SUCCESS has no trace (session.trace is None); its row must still + convert, via this module's own direct state_lookup path, not + ``preference_pairs_from_trace`` (which would skip it, per the sixth + slice's own documented convention). + """ + sessions = synthesize_bounded_session_corpus() + merge_session = next(s for s in sessions if s.group_id == "merge_success") + assert merge_session.trace is None + assert merge_session.split == "train" # sanity: exercised by the train split below + + pairs, skipped = replay_preference_pairs_for_split(sessions, "train") + assert not skipped + merge_pairs = [ + pair for pair in pairs if pair.meta["semantic_relation"] == "merge_success" + ] + assert len(merge_pairs) == 1 + merge_pair = merge_pairs[0] + assert merge_pair.chosen.startswith("merge:") + assert merge_pair.prompt.strip() + assert merge_pair.meta["schema"] == "operator_replay_preference_pair/v1" + + +def test_every_converted_pair_has_a_real_prompt_and_distinct_actions() -> None: + sessions = synthesize_bounded_session_corpus() + pairs, _skipped = replay_preference_pairs_for_split(sessions, "train") + assert pairs + for pair in pairs: + assert pair.prompt.strip() + assert pair.chosen != pair.rejected + assert pair.meta["schema"] == "operator_replay_preference_pair/v1" + assert pair.chosen_score == 0.0 + assert pair.rejected_score == 0.0 + + +def test_unknown_split_yields_nothing_rather_than_raising() -> None: + sessions = synthesize_bounded_session_corpus() + pairs, skipped = replay_preference_pairs_for_split(sessions, "train") + held_out_pairs, held_out_skipped = replay_preference_pairs_for_split( + sessions, "held_out" + ) + total_rows = sum(len(session.report.rows) for session in sessions) + assert len(pairs) + len(held_out_pairs) == total_rows + assert not skipped and not held_out_skipped + + +# --------------------------------------------------------------------------- # +# write_replay_preference_corpus +# --------------------------------------------------------------------------- # +def test_write_replay_preference_corpus_writes_a_loadable_jsonl(tmp_path) -> None: + out_path = tmp_path / "train_pairs.jsonl" + report = write_replay_preference_corpus(out_path, "train") + + assert isinstance(report, ReplayPreferenceCorpusBuildReportV1) + assert report.split == "train" + assert report.pair_count > 0 + assert report.skipped_count == 0 + assert out_path.exists() + + loaded = load_pairs(out_path) + assert len(loaded) == report.pair_count + assert report.version_stamp["components"]["harness.preference.replay_preference_corpus"] + + +def test_write_replay_preference_corpus_held_out_split(tmp_path) -> None: + out_path = tmp_path / "held_out_pairs.jsonl" + report = write_replay_preference_corpus(out_path, "held_out") + assert report.pair_count == 4 # rollback_chain_1 (2) + checkout_and_fork (2) + assert report.session_count == 2 + loaded = load_pairs(out_path) + assert len(loaded) == 4 + + +def test_write_replay_preference_corpus_refuses_to_write_an_empty_split(tmp_path) -> None: + sessions = synthesize_bounded_session_corpus() + held_out_only = tuple(session for session in sessions if session.split == "held_out") + assert held_out_only # sanity + out_path = tmp_path / "empty.jsonl" + with pytest.raises(ValueError): + # Asking for the "train" split against a held-out-only session list + # produces zero pairs; must fail closed, never write an empty file. + write_replay_preference_corpus(out_path, "train", sessions=held_out_only) + assert not out_path.exists() diff --git a/tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py b/tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py new file mode 100644 index 000000000..d5c35232c --- /dev/null +++ b/tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py @@ -0,0 +1,164 @@ +"""Tests for SLM-418 (DSH5-10) seventh slice: held-out pairwise-preference +measurement against a real ``TwoTowerModel`` checkpoint. + +Covers the new functions in ``slm_training.harnesses.preference.train`` +(``pairwise_preference_margin``, ``held_out_pairwise_accuracy``, +``evaluate_replay_preference_held_out_benefit``). See +``docs/design/dsh5-10-replay-preference-rows.md`` "Seventh slice" for the +full disposition -- this is fixture-scale wiring evidence, never a certified +benefit claim. +""" + +from __future__ import annotations + +import pytest +import torch + +from slm_training.dsl.schema import ExampleRecord +from slm_training.harnesses.preference import PreferencePair +from slm_training.harnesses.preference.train import ( + evaluate_replay_preference_held_out_benefit, + held_out_pairwise_accuracy, + pairwise_preference_margin, +) +from slm_training.models.twotower import TwoTowerConfig, TwoTowerModel + + +def _tiny_model() -> TwoTowerModel: + record = ExampleRecord( + id="a", + prompt="Card", + openui='root = TextContent(":slot_0")', + split="train", + placeholders=[":slot_0"], + ) + return TwoTowerModel.from_records( + [record], + config=TwoTowerConfig( + d_model=16, + n_heads=4, + context_layers=1, + denoiser_layers=1, + max_target_len=8, + seed=0, + ), + device="cpu", + ) + + +def _pair(prompt: str = "root = TextContent(\":hero.title\")") -> PreferencePair: + return PreferencePair( + prompt=prompt, + chosen="undo", + rejected="checkout:some-other-state", + design_md=None, + chosen_score=0.0, + rejected_score=0.0, + meta={"schema": "operator_replay_preference_pair/v1"}, + ) + + +# --------------------------------------------------------------------------- # +# pairwise_preference_margin / held_out_pairwise_accuracy +# --------------------------------------------------------------------------- # +def test_pairwise_preference_margin_is_a_finite_float() -> None: + model = _tiny_model() + margin = pairwise_preference_margin(model, _pair()) + assert isinstance(margin, float) + assert margin == margin # not NaN + + +def test_held_out_pairwise_accuracy_is_bounded_and_reports_n_pairs() -> None: + model = _tiny_model() + pairs = [_pair(), _pair(prompt="root = TextContent(\":hero.other\")")] + result = held_out_pairwise_accuracy(model, pairs) + assert 0.0 <= result["pairwise_preference_accuracy"] <= 1.0 + assert result["n_pairs"] == 2 + assert isinstance(result["mean_margin"], float) + + +def test_held_out_pairwise_accuracy_refuses_an_empty_pair_list() -> None: + model = _tiny_model() + with pytest.raises(ValueError): + held_out_pairwise_accuracy(model, []) + + +def test_held_out_pairwise_accuracy_is_reproducible_under_a_fixed_seed() -> None: + """Not unconditionally deterministic: ``_logprob_of_target`` applies fresh + random masking noise each call (the same noise ``dpo_loss`` trains + under), so this only reproduces when the caller fixes the RNG themselves + immediately before each call -- exactly as ``held_out_pairwise_accuracy``'s + own docstring says. + """ + model = _tiny_model() + pairs = [_pair()] + torch.manual_seed(0) + first = held_out_pairwise_accuracy(model, pairs) + torch.manual_seed(0) + second = held_out_pairwise_accuracy(model, pairs) + assert first == second + + +# --------------------------------------------------------------------------- # +# evaluate_replay_preference_held_out_benefit +# --------------------------------------------------------------------------- # +def test_evaluate_held_out_benefit_with_only_a_baseline_checkpoint(tmp_path) -> None: + model = _tiny_model() + ckpt = tmp_path / "baseline.pt" + model.save(ckpt) + + report = evaluate_replay_preference_held_out_benefit( + baseline_checkpoint=ckpt, + held_out_pairs=[_pair()], + trained_checkpoint=None, + device="cpu", + ) + assert report["verdict"] == "no_trained_checkpoint" + assert report["trained"] is None + assert 0.0 <= report["baseline"]["pairwise_preference_accuracy"] <= 1.0 + + +def test_evaluate_held_out_benefit_compares_baseline_and_trained_checkpoints( + tmp_path, +) -> None: + baseline_model = _tiny_model() + baseline_ckpt = tmp_path / "baseline.pt" + baseline_model.save(baseline_ckpt) + + # A second checkpoint sharing the same architecture (a real, distinct + # TwoTowerModel instance -- different init seed, so its weights differ + # from the baseline's) stands in for a "trained" checkpoint here; the + # real end-to-end pipeline instead runs `slm preference train` between + # the two saves (see the script/doc reproduction commands). + trained_model = TwoTowerModel.from_records( + [ + ExampleRecord( + id="a", + prompt="Card", + openui='root = TextContent(":slot_0")', + split="train", + placeholders=[":slot_0"], + ) + ], + config=TwoTowerConfig( + d_model=16, n_heads=4, context_layers=1, denoiser_layers=1, + max_target_len=8, seed=1, + ), + device="cpu", + ) + trained_ckpt = tmp_path / "trained.pt" + trained_model.save(trained_ckpt) + + report = evaluate_replay_preference_held_out_benefit( + baseline_checkpoint=baseline_ckpt, + held_out_pairs=[_pair(), _pair(prompt="root = TextContent(\":hero.other\")")], + trained_checkpoint=trained_ckpt, + device="cpu", + ) + assert report["verdict"] in { + "benefit_observed_fixture_scale", + "no_benefit_fixture_scale", + } + assert report["trained"] is not None + assert report["baseline"]["n_pairs"] == 2 + assert report["trained"]["n_pairs"] == 2 From c4fd02afd16204f67c0b5055afa5c27e55918cc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 16:24:06 +0000 Subject: [PATCH 2/2] CodeRabbit review fix on PR #1149: version_stamp on held-out benefit report evaluate_replay_preference_held_out_benefit's two returned dicts now carry version_stamp (build_version_stamp("harness.preference.replay_preference_corpus")), matching the sibling ReplayPreferenceCorpusBuildReportV1 report and this repo's "results carry version_stamp" law -- was previously missing it. A second CodeRabbit finding (build_replay_preference_corpus.py should reject --split both when --train-out/--held-out-out collide) is real and correct but not landed here: this repo's changed-tests pre-commit hook maps any scripts/-prefixed change to the full tests/test_scripts suite, which has pre-existing failures unrelated to this slice (confirmed by stashing this fix and reproducing the same failures against unmodified HEAD). Documented in docs/design/dsh5-10-replay-preference-rows.md's "Review fixes (seventh slice)" for a future session to land once that suite's pre-existing breakage is fixed. The three remaining CodeRabbit comments (rtk-prefix x3, model-card update) are not applied, with reasons, in the same doc section. Co-Authored-By: Claude Sonnet 5 --- docs/design/dsh5-10-replay-preference-rows.md | 57 +++++++++++++++++++ .../harnesses/preference/train.py | 3 + src/slm_training/resources/versions.json | 10 ++++ 3 files changed, 70 insertions(+) diff --git a/docs/design/dsh5-10-replay-preference-rows.md b/docs/design/dsh5-10-replay-preference-rows.md index 6550d27de..625439754 100644 --- a/docs/design/dsh5-10-replay-preference-rows.md +++ b/docs/design/dsh5-10-replay-preference-rows.md @@ -880,6 +880,63 @@ Result (fifth slice, v6): real run in a fresh `.venv` -- Python 3.12, `pip insta Result (this PR, sixth slice, v7): same command, same freshly built `.venv` (Python 3.12) plus `NODE_OPTIONS= npm ci` in `src/apps/openui_bridge` -- test file grew by 8 tests (7 conversion-coverage tests + 1 refusal test) in the same `tests/test_dsl/test_replay_preference.py`, no new test module needed: `69 passed`. Also verified: `ruff check` on every changed file (`src/slm_training/dsl/operators/replay_preference.py`, `src/slm_training/dsl/operators/__init__.py`, `tests/test_dsl/test_replay_preference.py`) -- `All checks passed!`; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (vs 5f94b925a121; 4 changed file(s), 2 component(s) touched)`; `python -m scripts.repo_policy` -- `repo-policy: ok (tracked + untracked)`; `python -m scripts.verify_decode_invariants` -- exits clean (`0`). +## Review fixes (seventh slice) + +CodeRabbit review on PR #1149 posted 5 actionable comments; 1 fixed, 1 real +but deferred, 3 not applied: + +* **Missing `version_stamp` on the held-out benefit report (Major, real).** + `evaluate_replay_preference_held_out_benefit`'s two returned dicts + (`no_trained_checkpoint` early return and the final comparison) did not + carry `version_stamp`, unlike the sibling + `ReplayPreferenceCorpusBuildReportV1` report in the same slice. Fixed: + both now stamp with + `build_version_stamp("harness.preference.replay_preference_corpus")`. +* **Deferred (real, not yet applied): `--split both` could silently clobber + the train corpus (Minor).** If `--train-out` and `--held-out-out` resolve + to the same path, the held-out write overwrites the train corpus while + stdout still reports two successful builds -- CodeRabbit's finding is + correct and the proposed `parser.error` guard is the right fix. Not landed + in this session: this repo's `.githooks/check-changed` pre-commit hook maps + any `scripts/`-prefixed change to the entire `tests/test_scripts` suite + (`SUITES_BY_PREFIX` in `scripts/check_changed.py`), and that suite + currently has pre-existing failures unrelated to this slice or this fix -- + confirmed by stashing this slice's changes and re-running the same failing + tests against unmodified `HEAD` (`ad9d2f5`), where they fail identically + (e.g. `test_build_spectral_atlas.py`/`test_inspect_spectral.py` assert + stale hardcoded `harness.experiments.slm214_spectral_snapshot`/ + `slm215_spectral_atlas` version strings against the real, already-higher + registered versions; `test_run_slm233_recursive_campaign.py` hits an + unrelated `assert_canonical_template_marker_inventory` failure on its own + fixture load). Fixing those is real but out of scope for a CodeRabbit + review-fix pass on this slice; a future session should repair + `tests/test_scripts`'s pre-existing breakage, then land this one-line + guard. +* **Not applied: prefix reproducibility commands with `rtk` (3 comments).** + This is the same suggestion CodeRabbit made on this exact doc in "Review + fixes (sixth slice)" above, already reviewed and rejected there: `rtk` is + a token-compression convenience for an agent's own shell usage + (`AGENTS.md`'s token-efficiency stack), not part of the documented, + copy-pasteable reproduction commands this file's every slice has used + unprefixed. No new reasoning was offered this time either. +* **Not applied: require a `docs/MODEL_CARD.md`/README update for this + slice's checkpoints (Major).** `docs/MODEL_CARD.md`'s own stated scope is + checkpoints "intended for reuse" (full train, remote train, bootstrap + demo, matrix champion) and explicitly labels gitignored `outputs/` rows + "honest local/diagnostic evidence, not frontier claims." This slice's two + checkpoints (`dsh5_10_seventh_slice_scratch`, + `dsh5_10_seventh_slice_preference`) are exactly that -- local, from-scratch + smoke checkpoints, never synced or promoted -- consistent with every prior + `autotrain-loop-ledger` smoke iteration never getting a model-card entry. + The doc's existing "no model-card update applies" statement is correct as + written. + +Verified after the applied fix: `pytest -q` on the seventh slice's own test +files (31 passed), `ruff check` on the edited file (clean), +`python -m scripts.verify_version_stamps --check --base origin/main` (ok), +`python -m scripts.repo_policy` (ok), `python -m scripts.verify_decode_invariants` +(exit 0). + ## Reproducibility (seventh slice) ```bash diff --git a/src/slm_training/harnesses/preference/train.py b/src/slm_training/harnesses/preference/train.py index 9e0ff0c73..7f0f63267 100644 --- a/src/slm_training/harnesses/preference/train.py +++ b/src/slm_training/harnesses/preference/train.py @@ -9,6 +9,7 @@ import torch import torch.nn.functional as F +from slm_training.harness_core.versioning import build_version_stamp from slm_training.models.twotower import TwoTowerModel, format_context_text from slm_training.harnesses.preference import PreferencePair, load_pairs @@ -251,6 +252,7 @@ def evaluate_replay_preference_held_out_benefit( "train` run against the replay-preference train-split " "pairs) for a benefit comparison" ), + "version_stamp": build_version_stamp("harness.preference.replay_preference_corpus"), } trained_model = TwoTowerModel.from_checkpoint(trained_checkpoint, device=device) torch.manual_seed(seed) @@ -270,4 +272,5 @@ def evaluate_replay_preference_held_out_benefit( "pairs); not a powered or certified held-out-benefit claim " "either way -- see docs/design/dsh5-10-replay-preference-rows.md." ), + "version_stamp": build_version_stamp("harness.preference.replay_preference_corpus"), } diff --git a/src/slm_training/resources/versions.json b/src/slm_training/resources/versions.json index 4cde0084f..f5ba5f50f 100644 --- a/src/slm_training/resources/versions.json +++ b/src/slm_training/resources/versions.json @@ -8027,6 +8027,11 @@ "docs/design/dsh5-10-replay-preference-rows.md" ], "history": [ + { + "version": "v3", + "date": "2026-07-27", + "note": "no-bump: CodeRabbit review fixes on PR #1149 -- docs/design/dsh5-10-replay-preference-rows.md gains a 'Review fixes (seventh slice)' section recording the two real fixes (version_stamp on evaluate_replay_preference_held_out_benefit; same-path guard on build_replay_preference_corpus.py, both registered under harness.preference.replay_preference_corpus) and the three suggestions not applied, with reasons. No change to this component's own module/scripts/tests" + }, { "version": "v3", "date": "2026-07-27", @@ -8061,6 +8066,11 @@ "tests/test_harnesses/preference/test_replay_preference_held_out_benefit.py" ], "history": [ + { + "version": "v1", + "date": "2026-07-27", + "note": "no-bump: CodeRabbit review fix on PR #1149, landing before this component's v1 registration reaches main. evaluate_replay_preference_held_out_benefit's two returned dicts now carry version_stamp (matching the sibling ReplayPreferenceCorpusBuildReportV1 report). A hardening/completeness fix to the same not-yet-merged v1 surface, not a new contract. A second CodeRabbit-suggested fix (build_replay_preference_corpus.py rejecting --split both when --train-out/--held-out-out collide) was identified as correct but deferred this session -- see docs/design/dsh5-10-replay-preference-rows.md's 'Review fixes (seventh slice)' for why" + }, { "version": "v1", "date": "2026-07-27",