From 4d9740214bb34eaf25d658a6266df642a1f5c8af Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 08:09:16 +0000 Subject: [PATCH] SLM-418: add merge_success to the demo pairs corpus + rerun (DSH5-10) Eighth slice, stacked on #1125 (the seventh slice's demo trace + first real training run). Adds build_demo_merge_scenario to scripts/build_replay_preference_pairs.py: a second scratch fixture (two branches forked from a shared base, editing disjoint node refs), mirroring the exact disjoint-target shape tests/test_dsl/test_operator_merge.py already verifies merges cleanly, replayably, and order-invariantly. Reaches merge_success, the one named pattern the seventh slice's single-trace corpus structurally cannot (extract_merge_preference_row never operates on a shared ConversationTraceV1). main() now combines both sources into one report and one pairs.jsonl: 4 rows (3 trace-scan + 1 merge_success), 3 render -- undo_then_redo is still the only drop, for the same structural reason the seventh slice documented. Reran the full training chain against the richer corpus: same deterministic SFT checkpoint (last_loss=32.610084533691406, matching every prior wf_smoke_v2/seed-0/8-step row), preference training now over 3 pairs instead of 2 (9 steps, last_loss=0.531). Still not a training or held-out-benefit claim -- a larger, more structurally diverse scratch corpus, not evidence the signal helps the model. harness.preference.replay_pairs bumped v2 -> v3. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EDctCUvBtGrdMvgNd34oHR --- docs/design/dsh5-10-replay-preference-rows.md | 75 +++++- scripts/build_replay_preference_pairs.py | 242 ++++++++++++++++-- src/slm_training/resources/versions.json | 7 +- .../test_build_replay_preference_pairs.py | 26 +- 4 files changed, 319 insertions(+), 31 deletions(-) diff --git a/docs/design/dsh5-10-replay-preference-rows.md b/docs/design/dsh5-10-replay-preference-rows.md index f1936a4cd..005797dfa 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 (seventh increment). +**Status:** partial slice, in progress (eighth increment). **Claim class:** `wiring`. **Honest verdict:** not yet dispositioned -- this PR extends a scoped subset, not the full issue. @@ -17,16 +17,19 @@ The fifth slice added the seventh and final named pattern, bringing extraction coverage to 7 of 7 (see "Fifth slice (v6)" below). The sixth slice added the first (and, until then, entirely missing) converter from an extracted row to the `PreferencePair` shape `scripts/train_preference.py` -actually consumes, but still did not run any training. This slice runs the -first real (`fixture_or_scratch`) end-to-end pass: a scratch SFT checkpoint, -a demo replay-preference pairs corpus, and one bounded -`scripts/train_preference.py train` call against it -- see "Seventh slice" -below. This is still not the issue's actual training/measurement claim: -what remains is a *real* pairs corpus (no captured conversation-trace data -exists anywhere in this repo -- see below), training against the -DSH3-selected policy/control heads specifically (not the generic TwoTower -pair format used here), the four-baseline comparison, held-out benefit -measurement, and turn-depth/context-view ablations. +actually consumes. The seventh slice ran the first real +(`fixture_or_scratch`) end-to-end pass: a scratch SFT checkpoint, a demo +replay-preference pairs corpus covering 3 of the 7 named patterns, and one +bounded `scripts/train_preference.py train` call against it. This slice +extends the demo corpus to a fourth pattern, `merge_success` -- the one +pattern the seventh slice's trace-scan corpus could not reach -- and reruns +the same training chain against the now-richer 3-pair corpus; see "Eighth +slice" below. This is still not the issue's actual training/measurement +claim: what remains is a *real* pairs corpus (no captured +conversation-trace data exists anywhere in this repo -- see below), +training against the DSH3-selected policy/control heads specifically (not +the generic TwoTower pair format used here), the four-baseline comparison, +held-out benefit measurement, and turn-depth/context-view ablations. ## What this PR delivers @@ -421,6 +424,52 @@ training/evaluation work enumerated above. `src/slm_training/resources/versions.json` (adds the new script + test to its watched paths). +## Eighth slice + +* Adds `build_demo_merge_scenario` to `scripts/build_replay_preference_pairs.py`: + a second scratch fixture -- two branches forked from a shared base editing + disjoint node refs (title vs body) -- mirroring the exact disjoint-target + shape `tests/test_dsl/test_operator_merge.py` already verifies merges + cleanly, replayably, and order-invariantly. This reaches `merge_success`, + the one named pattern the seventh slice's single-trace corpus structurally + cannot: `extract_merge_preference_row` never operates on a shared + `ConversationTraceV1` (see the sixth slice's `merge_node_resolver`), so it + needs its own two-branch construction rather than another turn in the + same trace. +* `main()` now combines both sources into one report and one `pairs.jsonl`: + 4 rows total (3 from the trace-scan corpus + 1 `merge_success`), 3 render. + `undo_then_redo` is still the only drop, for the same structural reason + the seventh slice documented (never a bug to fix on this fixture family). +* Reran the full training chain against the now-richer 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 replay_pref_sft_ckpt2 --no-sync-checkpoints --device cpu --seed 0 + python -m scripts.build_replay_preference_pairs \ + --out outputs/data/preference/replay_demo_pairs_v2.jsonl + python -m scripts.train_preference train \ + --checkpoint outputs/runs/replay_pref_sft_ckpt2/checkpoints/last.pt \ + --pairs outputs/data/preference/replay_demo_pairs_v2.jsonl \ + --out-dir outputs/runs/replay_pref_dpo2 --steps 9 --device cpu + ``` + SFT step: `last_loss=32.610084533691406` again -- the same deterministic + artifact every prior `wf_smoke_v2`/seed-0/8-step row in the smoke-loop + ledger reproduces. Preference step, now over 3 pairs instead of 2: + `{"steps": 9, "last_loss": 0.5314897894859314, "mean_loss": + 0.7201318964362144, "n_pairs": 3, "reference_free": true}` + (`outputs/runs/replay_pref_dpo2/preference_summary.json`, not committed). + Both commands again well under `MAX_RUN_MINUTES=3`. +* **Still not a training or held-out-benefit claim** -- `n_pairs=3` on a + scratch fixture is a larger, more structurally diverse smoke corpus (now + covering 4 of the 7 named patterns instead of 3), not evidence the signal + helps the model or generalizes. The three remaining un-exercised-in-a-script + patterns (`partial_rollback`, `fork_then_choose_one_branch`, and + `pronoun_focus_followup`) are left for a future slice rather than piling + more scratch fixtures onto this one; see the doc's still-open + training/measurement scope above. +* `harness.preference.replay_pairs` bumped `v2` -> `v3` in + `src/slm_training/resources/versions.json`. + ## Reproducibility ```bash @@ -431,4 +480,6 @@ Result (fifth-slice PR, real run in a fresh `.venv` -- Python 3.12, `pip install Result (sixth-slice PR #1124, real run in a fresh `.venv-dsh510` -- Python 3.12, `pip install -e ".[dev,grammar]"`, plus `env -u NODE_OPTIONS npm ci` in `src/apps/openui_bridge` -- the ambient `NODE_OPTIONS="--import tsx" --max-old-space-size=8192` is rejected outright by Node for both `npm ci` and `pytest` in this environment, so it has to be unset, not just locally overridden, unlike the fifth slice's note above): `69 passed` against `main` HEAD `5f94b92` (includes the fifth slice, already merged). Also verified: `ruff check` clean on both new files; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (1 component(s) touched)`; `python -m scripts.repo_policy` -- `ok`; `python -m scripts.verify_decode_invariants` -- clean. No training run in this slice; `outputs/` untouched. -Result (this PR, seventh slice, real run in a fresh `.venv-dsh510`, same environment recipe as the sixth slice above, stacked on top of PR #1124 which was still unmerged when this slice started): `71 passed` (69 from the sixth slice + 2 new). Also verified: `ruff check` clean on both new files; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (1 component(s) touched)`; `python -m scripts.repo_policy` -- `ok`. Plus the real training run described above (SFT checkpoint + demo pairs + preference-training pass, both commands well under `MAX_RUN_MINUTES=3`); its `outputs/runs/replay_pref_sft_ckpt/` and `outputs/runs/replay_pref_dpo/` are not committed (`outputs/` is gitignored) per this repo's checked-not-committed convention for scratch run artifacts. +Result (seventh-slice PR #1125, real run in a fresh `.venv-dsh510`, same environment recipe as the sixth slice above, stacked on top of PR #1124 which was still unmerged when this slice started): `71 passed` (69 from the sixth slice + 2 new). Also verified: `ruff check` clean on both new files; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (1 component(s) touched)`; `python -m scripts.repo_policy` -- `ok`. Plus the real training run described above (SFT checkpoint + demo pairs + preference-training pass, both commands well under `MAX_RUN_MINUTES=3`); its `outputs/runs/replay_pref_sft_ckpt/` and `outputs/runs/replay_pref_dpo/` are not committed (`outputs/` is gitignored) per this repo's checked-not-committed convention for scratch run artifacts. + +Result (this PR, eighth slice, real run in a fresh `.venv-dsh510`, same environment recipe as above, stacked on top of PR #1125 which was still unmerged when this slice started): `72 passed` (71 from the seventh slice + 1 new). Also verified: `ruff check` clean; `python -m scripts.verify_version_stamps --check --base origin/main` -- `ok (1 component(s) touched)`; `python -m scripts.repo_policy` -- `ok`; `python -m scripts.verify_decode_invariants` -- clean. Plus the reran training chain described above; `outputs/runs/replay_pref_sft_ckpt2/` and `outputs/runs/replay_pref_dpo2/` are not committed (`outputs/` is gitignored). diff --git a/scripts/build_replay_preference_pairs.py b/scripts/build_replay_preference_pairs.py index 3bf4fe55c..557c8d1c8 100644 --- a/scripts/build_replay_preference_pairs.py +++ b/scripts/build_replay_preference_pairs.py @@ -17,17 +17,26 @@ ``docs/design/autotrain-loop-ledger-20260725.md`` -- not a claim about real usage patterns, model quality, or held-out benefit. -One of the three rows this trace produces (``undo_then_redo``) never -renders: for any deterministic, zero-argument operator, ``redo`` and -"reapply the same operator at the same input state" are, by definition, -the identical text -- no cycle-length trick avoids this; only a genuinely -non-deterministic or argument-bearing operator would. The renderer's own -dedup guard (``render_replay_preference_pair`` in +One of the four rows this produces (``undo_then_redo``) never renders: for +any deterministic, zero-argument operator, ``redo`` and "reapply the same +operator at the same input state" are, by definition, the identical text -- +no cycle-length trick avoids this; only a genuinely non-deterministic or +argument-bearing operator would. The renderer's own dedup guard +(``render_replay_preference_pair`` in ``slm_training.harnesses.preference.replay_pairs``) correctly declines rather than emitting a self-contradictory pair -- see this script's own ``pairs_dropped`` in its printed report, and ``test_undo_then_redo_row_declines_a_degenerate_collapse`` in ``tests/test_harnesses/preference/test_replay_pairs.py``. + +A second, separate scenario (``build_demo_merge_scenario``) covers +``merge_success``, the one named pattern ``build_demo_trace`` cannot reach: +``extract_merge_preference_row`` never operates on a shared +``ConversationTraceV1`` the way the other six patterns do (see +``merge_node_resolver`` in ``slm_training.harnesses.preference.replay_pairs``), +so it needs its own two-branch fixture with disjoint-target edits, mirroring +the shape ``tests/test_dsl/test_operator_merge.py`` already verifies is +valid, replayable, and order-invariant. """ from __future__ import annotations @@ -42,9 +51,14 @@ ActionEffectV1, ApplicationProvenanceV1, AstOperatorV1, + BranchEditV1, CompilerCoverage, + ConversationStateNodeV1, + EffectDeltaKind, + EffectDeltaV1, OperatorLibraryV1, OperatorMutationV1, + OperatorReplayPreferenceRowV1, OperatorStateV1, RefKind, ReferenceDescriptorV1, @@ -53,15 +67,24 @@ branch_fingerprint, build_reference_table, checkout_conversation_state, + clone_reference_table_for_branch, create_conversation_trace, + extract_merge_preference_row, extract_replay_preference_rows, + merge_conversation_branches, redo_conversation, undo_conversation, ) from slm_training.dsl.operators.conversation import ConversationTraceV1 from slm_training.dsl.pack import DslPack, get_pack -from slm_training.harnesses.preference import write_pairs -from slm_training.harnesses.preference.replay_pairs import render_replay_preference_pairs +from slm_training.harnesses.preference import PreferencePair, write_pairs +from slm_training.harnesses.preference.replay_pairs import ( + merge_node_resolver, + render_replay_preference_pair, + render_replay_preference_pairs, +) + +_MERGE_SOURCE = 'root = Card([TextContent(":hero.title"), TextContent(":hero.body")], "clear")' _SOURCE = 'root = TextContent(":hero.title")' _OPERATOR_ID = "openui.demo_cycle_text" @@ -181,6 +204,185 @@ def apply_once(current: ConversationTraceV1, *, seed: int) -> ConversationTraceV return pack, library, checked_out +def _merge_branch_edit( + *, + base_pack: DslPack, + base_state: OperatorStateV1, + base_node: ConversationStateNodeV1, + root_table, + name: str, + target_name: str, + replacement: str, + seed: int, + authorities: dict, +) -> BranchEditV1: + branch = branch_fingerprint(base_state.state_digest, _sha(f"demo-merge-{name}")) + table = clone_reference_table_for_branch(root_table, branch_digest=branch, seed=seed) + input_node = ConversationStateNodeV1( + parent_state_id=base_node.state_id, + branch_digest=branch, + state=base_state, + reference_table=table, + ) + target = next( + entry.ref for entry in table.entries if entry.descriptor.value_type == f"openui.{target_name}" + ) + operator_id = f"openui.demo_merge_{name}" + declaration = AstOperatorV1( + operator_id=operator_id, + version="v1", + domain="openui.ast", + codomain="openui.ast", + argument_slots=(), + preconditions=(), + effect_signature=(EffectDeltaKind("property"),), + locality="node", + cost=1.0, + ) + before = f":hero.{target_name}" + + def execute(state: OperatorStateV1, _arguments) -> OperatorMutationV1: + if before not in state.source: + raise ValueError("demo.no_transition") + return OperatorMutationV1( + source=state.source.replace(before, replacement), + effect=ActionEffectV1( + property_deltas=( + EffectDeltaV1( + kind=EffectDeltaKind("property"), target=target, before="before", after="after" + ), + ), + compiler_coverage=CompilerCoverage.EXACT, + ), + ) + + branch_library = OperatorLibraryV1((RegisteredOperatorV1(declaration, execute),)) + branch_pack = replace(base_pack, operator_library=branch_library) + applied = branch_library.apply( + branch_pack, input_node.state, operator_id, (), _provenance(input_node.state) + ) + assert applied.succeeded and applied.state is not None + output_table = build_reference_table( + request_id=table.request_id, + state_digest=applied.state.state_digest, + branch_digest=branch, + descriptors=tuple(entry.descriptor for entry in table.entries), + seed=seed + 10, + ) + output_node = ConversationStateNodeV1( + parent_state_id=input_node.state_id, + branch_digest=branch, + state=applied.state, + reference_table=output_table, + ) + authorities[input_node.state_id] = (branch_pack, branch_library) + return BranchEditV1(input_node, output_node, applied.application) + + +def _rebuild_merged_table(_pack, merged_state: OperatorStateV1, branch_digest: str, seed: int): + """Fixture-only compiler facts, derived anew from the canonical merged source.""" + descriptors = tuple( + ReferenceDescriptorV1( + ref_kind=RefKind.NODE, + semantic_fingerprint=_sha(f"canonical:{marker}"), + value_type=f"openui.{marker.removeprefix(':hero.')}", + ) + for marker in (":hero.heading", ":hero.copy") + if marker in merged_state.source + ) + return build_reference_table( + request_id=_REQUEST_ID, + state_digest=merged_state.state_digest, + branch_digest=branch_digest, + descriptors=descriptors, + seed=seed, + ) + + +def build_demo_merge_scenario() -> tuple[list[OperatorReplayPreferenceRowV1], list[PreferencePair]]: + """A second scratch scenario covering ``merge_success``, the one pattern + ``build_demo_trace`` cannot reach (see module docstring). + + Two branches fork from a shared base and edit disjoint node refs + (title vs body) -- the same disjoint-target shape + ``tests/test_dsl/test_operator_merge.py`` verifies merges cleanly, + replayably, and order-invariantly. + """ + base_pack = get_pack("openui") + base_state = OperatorStateV1.from_source(base_pack, _MERGE_SOURCE) + root_branch = branch_fingerprint(base_state.state_digest, _sha("demo-merge-root")) + descriptors = tuple( + ReferenceDescriptorV1( + ref_kind=RefKind.NODE, semantic_fingerprint=_sha(name), value_type=f"openui.{name}" + ) + for name in ("title", "body") + ) + root_table = build_reference_table( + request_id=_REQUEST_ID, + state_digest=base_state.state_digest, + branch_digest=root_branch, + descriptors=descriptors, + seed=21, + ) + base_node = ConversationStateNodeV1( + parent_state_id=None, branch_digest=root_branch, state=base_state, reference_table=root_table + ) + + authorities: dict = {} + left = _merge_branch_edit( + base_pack=base_pack, + base_state=base_state, + base_node=base_node, + root_table=root_table, + name="left", + target_name="title", + replacement=":hero.heading", + seed=22, + authorities=authorities, + ) + right = _merge_branch_edit( + base_pack=base_pack, + base_state=base_state, + base_node=base_node, + root_table=root_table, + name="right", + target_name="body", + replacement=":hero.copy", + seed=23, + authorities=authorities, + ) + + def resolve(node: ConversationStateNodeV1): + return authorities[node.state_id] + + decision = merge_conversation_branches( + pack=base_pack, + base=base_node, + left=left, + right=right, + authority_resolver=resolve, + reference_table_builder=_rebuild_merged_table, + ) + if not decision.succeeded or decision.continuation is None: + raise RuntimeError("demo merge scenario did not produce a mergeable pair") + + row = extract_merge_preference_row( + left=left, right=right, decision=decision, authority_resolver=resolve, provenance_for=_provenance + ) + if row is None: + return [], [] + + left_pack, left_library = resolve(left.input_node) + pair = render_replay_preference_pair( + row, + resolve_node=merge_node_resolver(left, right, decision), + pack=left_pack, + library=left_library, + provenance_for=_provenance, + ) + return [row], ([pair] if pair is not None else []) + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument( @@ -189,24 +391,34 @@ def main(argv: list[str] | None = None) -> int: args = parser.parse_args(argv) pack, library, trace = build_demo_trace() - report = extract_replay_preference_rows( + trace_report = extract_replay_preference_rows( trace, pack=pack, library=library, provenance_for=_provenance ) - pairs = render_replay_preference_pairs( - report.rows, + trace_pairs = render_replay_preference_pairs( + trace_report.rows, resolve_node=trace.node, pack=pack, library=library, provenance_for=_provenance, ) - n = write_pairs(args.out, pairs) + + merge_rows, merge_pairs = build_demo_merge_scenario() + + all_rows = list(trace_report.rows) + merge_rows + all_pairs = trace_pairs + merge_pairs + counts_by_relation = dict(trace_report.counts_by_relation) + for row in merge_rows: + key = row.semantic_relation.value + counts_by_relation[key] = counts_by_relation.get(key, 0) + 1 + + n = write_pairs(args.out, all_pairs) print( json.dumps( { - "rows": len(report.rows), - "counts_by_relation": report.counts_by_relation, + "rows": len(all_rows), + "counts_by_relation": counts_by_relation, "pairs_rendered": n, - "pairs_dropped": len(report.rows) - n, + "pairs_dropped": len(all_rows) - n, "out": str(args.out), "corpus_kind": "fixture_or_scratch", }, diff --git a/src/slm_training/resources/versions.json b/src/slm_training/resources/versions.json index bb5bc1910..b6ffb2ea2 100644 --- a/src/slm_training/resources/versions.json +++ b/src/slm_training/resources/versions.json @@ -7942,7 +7942,7 @@ ] }, "harness.preference.replay_pairs": { - "version": "v2", + "version": "v3", "kind": "harness", "paths": [ "src/slm_training/harnesses/preference/replay_pairs.py", @@ -7951,6 +7951,11 @@ "tests/test_scripts/test_build_replay_preference_pairs.py" ], "history": [ + { + "version": "v3", + "date": "2026-07-27", + "note": "SLM-418 (DSH5-10) eighth slice: adds build_demo_merge_scenario, a second scratch fixture (two branches with disjoint-target edits, mirroring tests/test_dsl/test_operator_merge.py's own verified-mergeable shape) covering merge_success -- the one named pattern the original build_demo_trace could not reach, since extract_merge_preference_row never operates on a shared ConversationTraceV1. main() now combines both sources: 4 rows (3 from the trace + 1 merge), 3 render (still only undo_then_redo drops, for the structural reason documented in the seventh slice), 1 combined pairs.jsonl. No change to the sixth slice's renderer itself" + }, { "version": "v2", "date": "2026-07-27", diff --git a/tests/test_scripts/test_build_replay_preference_pairs.py b/tests/test_scripts/test_build_replay_preference_pairs.py index 0432f3954..e139b9b65 100644 --- a/tests/test_scripts/test_build_replay_preference_pairs.py +++ b/tests/test_scripts/test_build_replay_preference_pairs.py @@ -5,7 +5,12 @@ import json from pathlib import Path -from scripts.build_replay_preference_pairs import _provenance, build_demo_trace, main +from scripts.build_replay_preference_pairs import ( + _provenance, + build_demo_merge_scenario, + build_demo_trace, + main, +) from slm_training.dsl.operators import ReplayPreferenceRelation, extract_replay_preference_rows from slm_training.harnesses.preference import load_pairs @@ -23,6 +28,18 @@ def test_build_demo_trace_exercises_three_named_patterns() -> None: } +def test_build_demo_merge_scenario_yields_one_real_merge_success_pair() -> None: + rows, pairs = build_demo_merge_scenario() + + assert len(rows) == 1 + assert rows[0].semantic_relation is ReplayPreferenceRelation.MERGE_SUCCESS + assert len(pairs) == 1 + pair = pairs[0] + assert pair.meta["pair_corpus"] == "replay_preference" + assert pair.meta["semantic_relation"] == "merge_success" + assert pair.chosen != pair.rejected + + def test_main_writes_a_real_pairs_file_and_reports_honest_counts( tmp_path: Path, capsys ) -> None: @@ -33,20 +50,23 @@ def test_main_writes_a_real_pairs_file_and_reports_honest_counts( assert exit_code == 0 report = json.loads(capsys.readouterr().out) assert report["corpus_kind"] == "fixture_or_scratch" - assert report["rows"] == 3 + assert report["rows"] == 4 assert report["counts_by_relation"] == { "edit_then_undo": 1, "undo_then_redo": 1, "checkout_another_state": 1, + "merge_success": 1, } # undo_then_redo never renders here: redo and "reapply the same # deterministic zero-arg operator" are the identical text by # construction, so the renderer's dedup guard correctly declines it. - assert report["pairs_rendered"] == 2 + assert report["pairs_rendered"] == 3 assert report["pairs_dropped"] == 1 pairs = load_pairs(out_path) assert len(pairs) == report["pairs_rendered"] + relations = {pair.meta["semantic_relation"] for pair in pairs} + assert relations == {"edit_then_undo", "checkout_another_state", "merge_success"} for pair in pairs: assert pair.meta["pair_corpus"] == "replay_preference" assert pair.chosen != pair.rejected