Fix: port build_env to modular expert_harmonizer (restores main broken by #8)

src/folds/build_env.py

@@ -6,24 +6,23 @@
 materialize a per-config sandbox under ``.runs/<name>/`` containing only:
 
 * the **skills** (curator + harmonizer), copied verbatim;
-* the **ablated mapping JSON** — the gold mapping with the held-out cluster's
+* the **filtered mapping JSON** — the gold mapping with the held-out cluster's
   entries removed, written to the path the skills read for exemplars
   (``data/processed/ess-dive_wfsfa_soil_datasets/sm_data_harmonization_mapping.json``);
-* the **ablated monolith** — the expert harmonization script with the held-out
-  blocks spliced out (see :mod:`src.folds.ablate_monolith`), written to the path
-  the harmonizer reads as a code-pattern reference
-  (``notebooks/harmonize_ess-dive_soilmoisture_data.py``).
+* the **held-out-free expert code** — ``common.py`` plus the kept
+  ``dataset_NN.py`` modules of the modular expert harmonizer (see
+  :mod:`src.folds.expert_harmonizer`), copied to the path the harmonizer reads
+  as a code-pattern reference (``data/gold/expert_code/harmonize_sm/``). The
+  held-out cluster's modules are simply not copied.
 
-Isolation is by *absence*: the two answer-bearing artifacts in the env have the
-held-out cluster removed, and they sit at the exact relative paths the skills
-resolve, so the skill picks up the ablated copy rather than the original. Shared
-*inputs* are deliberately NOT treated as leakage and stay where they are:
+Isolation is by *absence*: the held-out cluster is missing from both the
+exemplar mapping and the reference modules, which sit at the exact relative
+paths the skills resolve. Shared *inputs* are deliberately NOT treated as
+leakage and stay where they are:
 
 * raw per-dataset CSVs at ``~/ess-dive_wfsfa_soil_datasets/<dsid>/`` — read by
   generated code via an absolute home path, so they need no env copy;
-* cached ESS-DIVE metadata — optionally symlinked in via ``metadata_dir`` (the
-  curator reads it to evaluate a dataset; seeing a dataset's own metadata is the
-  task input, not the answer).
+* cached ESS-DIVE metadata — optionally symlinked in via ``metadata_dir``.
 
 The agent can in principle escape the sandbox (``../`` or absolute paths); the
 backstop is instruction (``AGENT_INSTRUCTIONS.md``) plus a post-hoc trace audit
@@ -38,19 +37,20 @@
 
 import typer
 
-from src.folds.ablate_monolith import (
-    DEFAULT_INPUT as DEFAULT_MONOLITH,
+from src.folds.expert_harmonizer import (
     DEFAULT_MAPPING,
-    ablate,
+    PACKAGE_DIR as DEFAULT_PACKAGE,
     block_indices,
+    kept_indices,
+    kept_module_paths,
     resolve_holdout,
 )
 
 DEFAULT_SKILLS = Path("skills")
 DEFAULT_ENV_ROOT = Path(".runs")
 
 # Where each artifact lands inside the env, matching the skills' read paths.
-MONOLITH_REL = Path("notebooks/harmonize_ess-dive_soilmoisture_data.py")
+HARMONIZER_REL = Path("data/gold/expert_code/harmonize_sm")
 MAPPING_REL = Path("data/processed/ess-dive_wfsfa_soil_datasets/sm_data_harmonization_mapping.json")
 METADATA_REL = Path("data/external/ess-dive_meta")
 
@@ -80,7 +80,7 @@ def build_env(
     holdout: set[int],
     name: Optional[str] = None,
     env_root: Path = DEFAULT_ENV_ROOT,
-    monolith_path: Path = DEFAULT_MONOLITH,
+    package_dir: Path = DEFAULT_PACKAGE,
     mapping_path: Path = DEFAULT_MAPPING,
     skills_dir: Path = DEFAULT_SKILLS,
     metadata_dir: Optional[Path] = None,
@@ -89,21 +89,25 @@ def build_env(
 
     Returns the env directory. Overwrites an existing env of the same name.
     """
+    holdout = set(holdout)
     mapping = json.loads(Path(mapping_path).read_text())
-    source = Path(monolith_path).read_text()
 
-    # ablate() raises if a hold-out index has no block (rejects REF_IDX 0 and
-    # the excluded datasets) — fail loudly before writing anything.
-    ablated_py = ablate(source, holdout)
+    # kept_module_paths() raises if a hold-out index has no module (rejects
+    # REF_IDX 0 and the excluded datasets) — fail loudly before writing anything.
+    kept_paths = kept_module_paths(holdout, package_dir)
+    kept = kept_indices(holdout, package_dir)
     filtered = filter_mapping(mapping, holdout)
 
     env = Path(env_root) / (name or default_name(holdout))
     if env.exists():
         shutil.rmtree(env)
     env.mkdir(parents=True)
 
-    (env / MONOLITH_REL).parent.mkdir(parents=True, exist_ok=True)
-    (env / MONOLITH_REL).write_text(ablated_py)
+    # held-out-free expert code: copy common.py + the kept dataset modules
+    pkg_dest = env / HARMONIZER_REL
+    pkg_dest.mkdir(parents=True, exist_ok=True)
+    for path in kept_paths:
+        shutil.copy(path, pkg_dest / path.name)
 
     (env / MAPPING_REL).parent.mkdir(parents=True, exist_ok=True)
     (env / MAPPING_REL).write_text(json.dumps(filtered, indent=2))
@@ -120,10 +124,10 @@ def build_env(
         "name": env.name,
         "holdout_indices": sorted(holdout),
         "holdout_identifiers": held_ids,
-        "exemplar_indices": block_indices(ablated_py),
+        "exemplar_indices": kept,
         "n_exemplars": len(filtered),
         "sources": {
-            "monolith": str(monolith_path),
+            "package": str(package_dir),
             "mapping": str(mapping_path),
             "skills": str(skills_dir),
         },
@@ -140,7 +144,7 @@ def _instructions(held_ids: list) -> str:
         "Harmonize the held-out dataset(s) below using ONLY:\n"
         "- the skills in `skills/`,\n"
         "- the exemplars in `data/processed/.../sm_data_harmonization_mapping.json` "
-        "and the code patterns in `notebooks/harmonize_ess-dive_soilmoisture_data.py` "
+        "and the code patterns in `data/gold/expert_code/harmonize_sm/` "
         "(both have the held-out cluster removed),\n"
         "- the shared raw inputs under `~/ess-dive_wfsfa_soil_datasets/` and the "
         "cached metadata under `data/external/ess-dive_meta/`.\n\n"
@@ -158,19 +162,19 @@ def main(
     holdout: str = typer.Option(..., "--holdout", help="Comma-separated indices or dataset_identifiers to hold out."),
     name: Optional[str] = typer.Option(None, "--name", help="Env dir name (default: holdout-<ids>)."),
     env_root: Path = typer.Option(DEFAULT_ENV_ROOT, "--env-root", help="Parent dir for run environments."),
-    monolith: Path = typer.Option(DEFAULT_MONOLITH, "--monolith", help="Expert monolith to ablate."),
+    package: Path = typer.Option(DEFAULT_PACKAGE, "--package", help="Modular expert harmonizer dir to draw modules from."),
     mapping: Path = typer.Option(DEFAULT_MAPPING, "--mapping", help="Gold mapping JSON."),
     skills: Path = typer.Option(DEFAULT_SKILLS, "--skills", help="Skills dir to copy."),
     metadata_dir: Optional[Path] = typer.Option(None, "--metadata-dir", help="Cached ESS-DIVE metadata to symlink in."),
 ) -> None:
     """Assemble `.runs/<name>/` for one hold-out config."""
     holdout_idx = resolve_holdout(holdout.split(","), mapping)
     env = build_env(
-        holdout_idx, name=name, env_root=env_root, monolith_path=monolith,
+        holdout_idx, name=name, env_root=env_root, package_dir=package,
         mapping_path=mapping, skills_dir=skills, metadata_dir=metadata_dir,
     )
     typer.echo(f"built {env} (held out {sorted(holdout_idx)}; "
-               f"{len(block_indices((env / MONOLITH_REL).read_text()))} exemplar blocks remain)")
+               f"{len(block_indices(env / HARMONIZER_REL))} exemplar modules remain)")
 
 
 if __name__ == "__main__":

tests/test_build_env.py

@@ -1,47 +1,21 @@
 """Tests for the leave-one-cluster-out run-environment builder."""
 from __future__ import annotations
 
-import ast
 import json
 from pathlib import Path
 
 import pytest
 from typer.testing import CliRunner
 
 from src.folds.build_env import (
+    HARMONIZER_REL,
     MAPPING_REL,
-    MONOLITH_REL,
     app,
     build_env,
     default_name,
     filter_mapping,
 )
 
-# Synthetic monolith: header + three dataset blocks + footer (mirrors the real
-# `# %%` / `idx = N` structure) so the suite needs no corpus.
-MONOLITH = """\
-# %%
-acc = []
-
-# %%
-# Dataset 1
-idx = 1
-acc.append(("ds1", idx))
-
-# %%
-# Dataset 2
-idx = 2
-acc.append(("ds2", idx))
-
-# %%
-# Dataset 5
-idx = 5
-acc.append(("ds5", idx))
-
-# %%
-print(acc)  # footer
-"""
-
 MAPPING = [
     {"index": 1, "dataset_identifier": "ess-dive_a", "doi": "doi:1"},
     {"index": 2, "dataset_identifier": "ess-dive_b", "doi": "doi:2"},
@@ -51,35 +25,39 @@
 
 @pytest.fixture
 def sources(tmp_path):
-    """Write a synthetic monolith, mapping, and skills dir; return their paths."""
-    mono = tmp_path / "monolith.py"
-    mono.write_text(MONOLITH)
+    """A synthetic modular harmonizer package + mapping + skills dir."""
+    pkg = tmp_path / "harmonize_sm"
+    pkg.mkdir()
+    (pkg / "common.py").write_text("SHARED = 1\n")
+    for i in (1, 2, 5):  # indices must be in expert_harmonizer.DATASET_INDICES
+        (pkg / f"dataset_{i:02d}.py").write_text(f"# dataset {i}\nIDX = {i}\n")
     mapping = tmp_path / "mapping.json"
     mapping.write_text(json.dumps(MAPPING))
     skills = tmp_path / "skills"
     (skills / "essdive_sm_curator").mkdir(parents=True)
     (skills / "essdive_sm_curator" / "SKILL.md").write_text("curator")
-    return mono, mapping, skills
+    return pkg, mapping, skills
 
 
 def test_filter_mapping_drops_holdout_keeps_index():
-    out = filter_mapping(MAPPING, {2})
-    assert [e["index"] for e in out] == [1, 5]
+    assert [e["index"] for e in filter_mapping(MAPPING, {2})] == [1, 5]
 
 
 def test_default_name_is_sorted():
     assert default_name({5, 1, 2}) == "holdout-1-2-5"
 
 
 def test_build_env_layout_and_content(tmp_path, sources):
-    mono, mapping, skills = sources
-    env = build_env({2}, env_root=tmp_path / ".runs", monolith_path=mono,
-                     mapping_path=mapping, skills_dir=skills)
+    pkg, mapping, skills = sources
+    env = build_env({2}, env_root=tmp_path / ".runs", package_dir=pkg,
+                    mapping_path=mapping, skills_dir=skills)
 
-    # ablated monolith at the harmonizer's read path, holdout block gone, parseable
-    py = (env / MONOLITH_REL).read_text()
-    assert "ds2" not in py and "ds1" in py and "ds5" in py
-    ast.parse(py)
+    # held-out-free modular code at the harmonizer's read path
+    hdir = env / HARMONIZER_REL
+    assert (hdir / "common.py").exists()
+    assert (hdir / "dataset_01.py").exists()
+    assert (hdir / "dataset_05.py").exists()
+    assert not (hdir / "dataset_02.py").exists()  # held out -> not copied
 
     # filtered mapping at the exemplar read path, holdout entry gone
     mp = json.loads((env / MAPPING_REL).read_text())
@@ -91,31 +69,30 @@ def test_build_env_layout_and_content(tmp_path, sources):
     assert manifest["holdout_indices"] == [2]
     assert manifest["holdout_identifiers"] == ["ess-dive_b"]
     assert manifest["exemplar_indices"] == [1, 5]
-    instr = (env / "AGENT_INSTRUCTIONS.md").read_text()
-    assert "ess-dive_b" in instr  # held-out id named for the trace audit
+    assert "ess-dive_b" in (env / "AGENT_INSTRUCTIONS.md").read_text()
 
 
 def test_build_env_metadata_symlink(tmp_path, sources):
-    mono, mapping, skills = sources
+    pkg, mapping, skills = sources
     meta = tmp_path / "meta"
     meta.mkdir()
     (meta / "x.json").write_text("{}")
-    env = build_env({1}, env_root=tmp_path / ".runs", monolith_path=mono,
+    env = build_env({1}, env_root=tmp_path / ".runs", package_dir=pkg,
                     mapping_path=mapping, skills_dir=skills, metadata_dir=meta)
     link = env / "data" / "external" / "ess-dive_meta"
     assert link.is_symlink() and (link / "x.json").exists()
 
 
-def test_build_env_rejects_non_block_holdout(tmp_path, sources):
-    mono, mapping, skills = sources
+def test_build_env_rejects_holdout_without_module(tmp_path, sources):
+    pkg, mapping, skills = sources
     with pytest.raises(ValueError):
-        build_env({99}, env_root=tmp_path / ".runs", monolith_path=mono,
-                  mapping_path=mapping, skills_dir=skills)
+        build_env({3}, env_root=tmp_path / ".runs", package_dir=pkg,
+                  mapping_path=mapping, skills_dir=skills)  # no dataset_03.py
 
 
 def test_build_env_overwrites_existing(tmp_path, sources):
-    mono, mapping, skills = sources
-    kw = dict(env_root=tmp_path / ".runs", monolith_path=mono, mapping_path=mapping, skills_dir=skills)
+    pkg, mapping, skills = sources
+    kw = dict(env_root=tmp_path / ".runs", package_dir=pkg, mapping_path=mapping, skills_dir=skills)
     first = build_env({2}, name="cfg", **kw)
     (first / "stale.txt").write_text("old")
     second = build_env({1}, name="cfg", **kw)
@@ -124,10 +101,38 @@ def test_build_env_overwrites_existing(tmp_path, sources):
 
 
 def test_cli_builds_env(tmp_path, sources):
-    mono, mapping, skills = sources
+    pkg, mapping, skills = sources
     result = CliRunner().invoke(app, [
         "--holdout", "2", "--name", "cli", "--env-root", str(tmp_path / ".runs"),
-        "--monolith", str(mono), "--mapping", str(mapping), "--skills", str(skills),
+        "--package", str(pkg), "--mapping", str(mapping), "--skills", str(skills),
     ])
     assert result.exit_code == 0, result.output
-    assert (tmp_path / ".runs" / "cli" / MONOLITH_REL).exists()
+    assert (tmp_path / ".runs" / "cli" / HARMONIZER_REL / "dataset_01.py").exists()
+    assert not (tmp_path / ".runs" / "cli" / HARMONIZER_REL / "dataset_02.py").exists()
+
+
+# --- Against the real modular harmonizer, when present ---
+
+PKG = Path("data/gold/expert_code/harmonize_sm")
+needs_pkg = pytest.mark.skipif(not PKG.exists(), reason="modular expert harmonizer not present")
+
+
+@needs_pkg
+def test_real_build_env_holds_out_cluster(tmp_path):
+    holdout = {1, 2, 3, 6, 16, 27}  # cluster_1 "sg_ph_micromet"
+    env = build_env(holdout, env_root=tmp_path / ".runs")
+    hdir = env / HARMONIZER_REL
+    assert (hdir / "common.py").exists()
+    for i in holdout:
+        assert not (hdir / f"dataset_{i:02d}.py").exists()
+    for i in (4, 5, 7, 8, 9, 10, 15, 17, 18, 23, 24, 25, 26):
+        assert (hdir / f"dataset_{i:02d}.py").exists()
+    mp = json.loads((env / MAPPING_REL).read_text())
+    assert not any(e["index"] in holdout for e in mp)
+
+
+@needs_pkg
+def test_real_build_env_rejects_ref_idx_and_excluded(tmp_path):
+    for bad in ({0}, {11}, {19}):
+        with pytest.raises(ValueError):
+            build_env(bad, env_root=tmp_path / ".runs")