Wave 21: adversarial-review fixes — all 9 verified findings closed

3 reviewers (correctness/security/fidelity) + independent verifier over the
Stage-0 pipeline commit; every P0/P1 empirically confirmed before fixing.

P0: reverse_unified_diff silently corrupted file-add/delete patches (git
apply rc 128: naive +/- swap inverts the ---/+++ order and leaves a stale
`new file mode` header). Fix: route them to the UNREVERSED_MARKER
fallback; regression test with a /dev/null patch.
P1: DECONTAMINATION_LIST omitted SWE-Gym's 11 eval repos (the architecture
named both families) — added; V3 now fully closed; regression test.
P1: COMPOSER_RECIPE_MAPPING.md:20 still asserted KL(teacher||student) as
blog-stated — corrected to direction-unverified per F-12.
P1: build_corpus budget was a soft ceiling (check-then-spend overshot by one
rollout) — now a pre-charge hard ceiling; test asserts cost <= budget.
P2: dedup stats double-counted rows that were both within-run and cross-gen
dups (disjoint partition now); run_id path-traversal guard on RunLayout;
gold_apply_plan() helper dispatches -R for marked diffs; removed unused
collect_trajectory budget param; OpenRouterPolicy key name-mangled +
masked __repr__ + model_slug now recorded in trajectory provenance.

Full suite: 516 passed / 66 skipped. Also includes the deepread vault sources
fetched by the reader agents (research/notes/*).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

composer_replication/datagen/repo_gate.py

@@ -251,13 +251,18 @@ def license_tier(info: LicenseInfo) -> Tier:
 # Benchmark decontamination (V3 / D-5)
 # ---------------------------------------------------------------------------
 
-#: The canonical 12 SWE-bench test repos (SWE-bench / -Lite / -Verified /
-#: -Multimodal all draw eval instances from these). Training on ANY of them
-#: contaminates every SWE-bench-family score we report (V3). Lowercase
-#: "org/repo" form. Extend via a JSON file (list of "org/repo" strings)
-#: passed to is_eval_contaminated(extra_list=...) — e.g. SWE-Gym eval splits.
+#: Eval repos we must never train on (V3). Two families:
+#:   * the canonical 12 SWE-bench test repos (SWE-bench / -Lite / -Verified /
+#:     -Multimodal all draw eval instances from these);
+#:   * SWE-Gym's 11 repos (deliberately disjoint from SWE-bench's 12, but they
+#:     are a benchmark/eval surface of their own — added per the Wave-21
+#:     adversarial review, which caught the architecture spec naming them
+#:     while the shipped list silently admitted all 11).
+#: Lowercase "org/repo" form. Extend via a JSON file (list of "org/repo"
+#: strings) passed to is_eval_contaminated(extra_list=...) for new releases.
 DECONTAMINATION_LIST: frozenset[str] = frozenset(
     {
+        # --- SWE-bench family (12) ---
         "astropy/astropy",
         "django/django",
         "matplotlib/matplotlib",
@@ -270,6 +275,18 @@ DECONTAMINATION_LIST: frozenset[str] = frozenset(
         "scikit-learn/scikit-learn",
         "sphinx-doc/sphinx",
         "sympy/sympy",
+        # --- SWE-Gym (11, arXiv:2412.21139 Table 1) ---
+        "pandas-dev/pandas",
+        "bokeh/bokeh",
+        "project-monai/monai",
+        "python/mypy",
+        "getmoto/moto",
+        "iterative/dvc",
+        "dask/dask",
+        "conan-io/conan",
+        "pydantic/pydantic",
+        "modin-project/modin",
+        "facebookresearch/hydra",
     }
 )
 

composer_replication/datagen/rollout_harness.py

@@ -74,9 +74,14 @@ class OpenRouterPolicy:
             ) from e
         from composer_replication.teacher_replay import _load_api_key
         self.model_slug = model_slug
-        self.api_key = api_key or _load_api_key()
+        # Name-mangled so vars()/asdict-style dumps and debug reprs don't
+        # expose the key (Wave-21 review P2).
+        self.__api_key = api_key or _load_api_key()
         self.max_tokens = max_tokens
 
+    def __repr__(self) -> str:  # mask the credential in any debug output
+        return f"OpenRouterPolicy(model_slug={self.model_slug!r}, api_key='***')"
+
     def act(self, observation: str, history: list[TrajectoryStep]) -> ToolCall | str:
         import httpx  # noqa: PLC0415
 
@@ -86,7 +91,7 @@ class OpenRouterPolicy:
             OPENROUTER_URL,
             json={"model": self.model_slug, "messages": messages,
                   "max_tokens": self.max_tokens, "temperature": 0.2},
-            headers={"Authorization": f"Bearer {self.api_key}"},
+            headers={"Authorization": f"Bearer {self.__api_key}"},
             timeout=120.0,
         )
         r.raise_for_status()
@@ -110,7 +115,6 @@ def collect_trajectory(
     policy: RolloutPolicy,
     *,
     max_turns: int = 40,
-    budget_usd: float | None = None,
     provenance: dict | None = None,
 ) -> CanonicalTrajectory:
     """Run one episode and return the graded CanonicalTrajectory.
@@ -149,6 +153,9 @@ def collect_trajectory(
         final = env.step({"type": "submit"})
 
     info = final.info or {}
+    # Model attribution (Wave-21 review P2): a corpus must record WHICH model
+    # produced each trajectory; policies expose it via a `model_slug` attr.
+    model = getattr(policy, "model_slug", None)
     return CanonicalTrajectory(
         task_id=task.task_id,
         steps=steps,
@@ -157,6 +164,7 @@ def collect_trajectory(
         hacked=bool(info.get("hacked", False)),
         provenance={"source": "rollout_harness",
                     "policy": type(policy).__name__,
+                    **({"model": model} if model else {}),
                     **(provenance or {})},
     )
 

composer_replication/datagen/swesmith_adapter.py

@@ -113,8 +113,14 @@ def reverse_unified_diff(patch: str) -> str | None:
     """
     if not patch or "@@" not in patch:
         return None
+    # "new file mode"/"deleted file mode" added per the Wave-21 adversarial
+    # review (P0): naively swapping +/- on a file-add patch emits
+    # `+++ /dev/null` before `--- a/<file>` with a stale `new file mode`
+    # header — git apply REJECTS it (rc 128, "inconsistent new filename"),
+    # so these must take the UNREVERSED_MARKER fallback, not silent corruption.
     unsupported = ("old mode ", "new mode ", "rename from ", "rename to ",
-                   "copy from ", "copy to ", "GIT binary patch")
+                   "copy from ", "copy to ", "GIT binary patch",
+                   "new file mode ", "deleted file mode ")
     if any(marker in patch for marker in unsupported):
         return None
 
@@ -225,6 +231,21 @@ class SwesmithAdapter:
         return task, SwesmithMeta(strategy=strategy, diff_reversed=diff_reversed)
 
 
+def gold_apply_plan(task_golden_diff: str) -> tuple[str, str]:
+    """How gate-4 validation must apply this task's gold diff.
+
+    Returns ``(diff_text, git_apply_flag)`` where flag is ``""`` (apply
+    forward) or ``"-R"`` (apply in reverse — the UNREVERSED_MARKER case where
+    `golden_diff` still holds the FORWARD bug patch). Exported per the Wave-21
+    adversarial review (P2): without this dispatch a naive `git apply` on a
+    marked diff conflicts and gate-4 silently discards a perfectly good task —
+    and the marker path is now the NORMAL route for file-add/delete patches.
+    """
+    if task_golden_diff.startswith(UNREVERSED_MARKER):
+        return task_golden_diff[len(UNREVERSED_MARKER):], "-R"
+    return task_golden_diff, ""
+
+
 def load_swesmith_instances(
     path_or_hf_id: str,
     *,
@@ -263,6 +284,7 @@ __all__ = [
     "SwesmithAdapter",
     "SwesmithMeta",
     "UNREVERSED_MARKER",
+    "gold_apply_plan",
     "load_swesmith_instances",
     "parse_strategy",
     "reverse_unified_diff",

composer_replication/datagen/tests/test_repo_gate.py

@@ -301,10 +301,14 @@ def test_license_tier_mapping(spdx: str, tier: Tier):
 # ---------------------------------------------------------------------
 
 
-def test_decontamination_list_has_the_canonical_12():
-    assert len(DECONTAMINATION_LIST) == 12
+def test_decontamination_list_covers_swebench_and_swegym():
+    # 12 SWE-bench eval repos + 11 SWE-Gym repos (Wave-21 review P1: the
+    # architecture names both families; the original list shipped only 12).
+    assert len(DECONTAMINATION_LIST) == 23
     assert "django/django" in DECONTAMINATION_LIST
     assert "sympy/sympy" in DECONTAMINATION_LIST
+    assert "pandas-dev/pandas" in DECONTAMINATION_LIST   # SWE-Gym
+    assert "getmoto/moto" in DECONTAMINATION_LIST        # SWE-Gym
 
 
 @pytest.mark.parametrize(
@@ -325,7 +329,6 @@ def test_is_eval_contaminated_hits(repo: str):
 @pytest.mark.parametrize(
     "repo",
     [
-        "pandas-dev/pandas",
         "https://github.com/torvalds/linux",
         "someuser/django",  # fork-org differs: NOT the eval repo
     ],
@@ -334,6 +337,13 @@ def test_is_eval_contaminated_misses(repo: str):
     assert is_eval_contaminated(repo) is False
 
 
+def test_swegym_repos_are_contaminated():
+    """Wave-21 review P1 regression: SWE-Gym's 11 repos must hit the gate."""
+    for repo in ("pandas-dev/pandas", "getmoto/moto", "python/mypy",
+                 "https://github.com/Project-MONAI/MONAI"):
+        assert is_eval_contaminated(repo) is True, repo
+
+
 def test_normalize_repo_forms():
     assert normalize_repo("git@github.com:PSF/Requests.git") == "psf/requests"
     assert normalize_repo("https://github.com/pydata/xarray/tree/main") == "pydata/xarray"

composer_replication/datagen/tests/test_swesmith_adapter.py

@@ -163,3 +163,36 @@ def test_load_local_jsonl(tmp_path):
     rows = load_swesmith_instances(str(p), limit=3)
     assert len(rows) == 3
     assert rows[0]["instance_id"] == "r__x.abc.pr_0"
+
+
+# ---------------------------------------------------------------------
+# Wave-21 adversarial-review regressions
+# ---------------------------------------------------------------------
+
+FILE_ADD_PATCH = """\
+diff --git a/newfile.py b/newfile.py
+new file mode 100644
+index 0000000..1111111
+--- /dev/null
++++ b/newfile.py
+@@ -0,0 +1,2 @@
++def fresh():
++    return 1
+"""
+
+
+def test_reverse_refuses_file_add_and_delete_patches():
+    """Review P0: naive reversal of a file-add patch emits git-apply-rejected
+    output (+++ before ---, stale `new file mode`). Must take the marker path."""
+    assert reverse_unified_diff(FILE_ADD_PATCH) is None
+    task, meta = SwesmithAdapter().to_task_with_meta(_instance(patch=FILE_ADD_PATCH))
+    assert meta.diff_reversed is False
+    assert task.golden_diff.startswith(UNREVERSED_MARKER)
+
+
+def test_gold_apply_plan_dispatch():
+    from composer_replication.datagen.swesmith_adapter import gold_apply_plan
+    diff, flag = gold_apply_plan("normal diff text")
+    assert (diff, flag) == ("normal diff text", "")
+    diff, flag = gold_apply_plan(UNREVERSED_MARKER + "bug patch body")
+    assert diff == "bug patch body" and flag == "-R"

composer_replication/pipeline/build_corpus.py

@@ -87,7 +87,12 @@ def build_corpus(
     traj_rows: list[dict] = []
     partial = False
     for task in train_tasks:
-        if manifest.over_budget:
+        # Hard ceiling (Wave-21 review P1): a rollout only starts if its cost
+        # still fits — pre-charging prevents the one-rollout overshoot the
+        # old check-then-spend ordering allowed.
+        if manifest.budget_usd is not None and (
+            manifest.cost_usd + cost_per_rollout_usd > manifest.budget_usd
+        ):
             partial = True
             break
         traj = collect_trajectory(env_factory(), task, policy_factory(),

composer_replication/pipeline/dedup.py

@@ -96,18 +96,18 @@ def dedup(
     """
     pairs = find_near_duplicates(rows, key_fn, threshold,
                                  prior_signatures=prior_signatures)
-    drop: set[int] = set()
-    for i, j in pairs:
-        if j < 0:
-            drop.add(i)          # duplicates a prior-run row
-        else:
-            drop.add(j)          # keep-first within this run
+    # Partition into disjoint drop-reason sets (Wave-21 review P2: a row that
+    # is both a within-run AND cross-generation duplicate must count once;
+    # cross-generation wins the attribution since the prior run owns the row).
+    drop_cross: set[int] = {i for i, j in pairs if j < 0}
+    drop_within: set[int] = {j for _, j in pairs if j >= 0} - drop_cross
+    drop = drop_cross | drop_within
     kept = [r for i, r in enumerate(rows) if i not in drop]
     return kept, {
         "rows_in": len(rows),
         "rows_kept": len(kept),
-        "dropped_within_run": len({j for _, j in pairs if j >= 0} & drop),
-        "dropped_cross_generation": len({i for i, j in pairs if j < 0} & drop),
+        "dropped_within_run": len(drop_within),
+        "dropped_cross_generation": len(drop_cross),
         "threshold": threshold,
     }
 

composer_replication/pipeline/s3_contract.py

@@ -77,6 +77,16 @@ class RunLayout:
     root: str
     run_id: str
 
+    def __post_init__(self) -> None:
+        # Defense-in-depth (Wave-21 review P2): run_id is operator-supplied,
+        # but a separator or `..` would silently escape the corpus root.
+        if not self.run_id or "/" in self.run_id or "\\" in self.run_id \
+                or ".." in self.run_id:
+            raise ValueError(
+                f"run_id {self.run_id!r} must be a single non-empty path "
+                "segment (no separators, no '..')."
+            )
+
     def _p(self, *parts: str) -> str:
         base = self.root.rstrip("/")
         return f"{base}/runs/{self.run_id}/" + "/".join(parts)

composer_replication/pipeline/tests/test_pipeline.py

@@ -221,3 +221,38 @@ def test_dataset_card_contents(tmp_path):
     assert "sft_rows: 3" in card
     assert "REDISTRIBUTABLE: 3" in card
     assert "Decontamination" in card
+
+
+# ---------------------------------------------------------------------
+# Wave-21 adversarial-review regressions
+# ---------------------------------------------------------------------
+
+
+def test_budget_is_a_hard_ceiling(tmp_path):
+    """Review P1: cost must never exceed budget (pre-charge check)."""
+    tasks = [_task(i) for i in range(6)]
+    lay = RunLayout(root=str(tmp_path), run_id="hardcap")
+    manifest = RunManifest(run_id="hardcap", created_at="2026-06-09T00:00:00Z",
+                           source="fixture", budget_usd=0.25)
+    out = build_corpus(tasks, _env, _passing_policy, lay, manifest,
+                       holdout_frac=0.2, holdout_seed=7,
+                       cost_per_rollout_usd=0.1)
+    assert out.cost_usd <= out.budget_usd
+    assert out.status == "partial"
+
+
+def test_run_id_path_traversal_rejected():
+    """Review P2: separators / .. in run_id must be rejected at construction."""
+    for bad in ("../../escape", "a/b", "a\\b", "", ".."):
+        with pytest.raises(ValueError, match="path"):
+            RunLayout(root="/data", run_id=bad)
+
+
+def test_dedup_stats_partition_disjoint():
+    """Review P2: a row that is both within-run and cross-gen dup counts once."""
+    prior_sigs = [minhash_signature(_TEXT_A)]
+    rows = [{"text": _TEXT_A}, {"text": _TEXT_A2}]
+    kept, stats = dedup(rows, lambda r: r["text"], threshold=0.5,
+                        prior_signatures=prior_sigs)
+    total_dropped = stats["dropped_within_run"] + stats["dropped_cross_generation"]
+    assert total_dropped == stats["rows_in"] - stats["rows_kept"]

docs/COMPOSER_RECIPE_MAPPING.md

@@ -17,7 +17,7 @@ The Cursor blog discusses **only three** training innovations explicitly. Everyt
 - **Same model** acts as both teacher and student. Not two separate models.
 - The teacher is "the policy at this turn, *with* a hint inserted into the context."
 - The student is "the policy at this turn, *without* the hint" (the original context).
-- Loss = on-policy KL divergence: `KL( teacher_logits_at_turn_t || student_logits_at_turn_t )`, applied **only at the problematic turn**, not over the full trajectory.
+- Loss = on-policy distillation KL at the turn. DIRECTION UNVERIFIED (deepread F-12): the blog says only "moves the student's token probabilities toward the teacher's" — directionless; published SDPO Eq. 1 is student-first, `KL( student || stopgrad(teacher) )`. Applied **only at the problematic turn**, not over the full trajectory.
 - Sits **on top of** an outer RLVR (verifiable-reward RL) objective; doesn't replace it.
 
 **Cited prior art** (Cursor's footnote 1):