Remeasure retained 1SAR models with current evidence contracts

scripts/test_toolchain.py

@@ -269,4 +269,209 @@ def check(label: str, condition: bool) -> None:
       and toolchain.EXTERNAL_TOOL_SPECS["gemmi"]["expected_version"] == toolchain.GEMMI_VERSION)
 check("tool paths are defined only in toolchain.py", not duplicate_constants)
 
+with tempfile.TemporaryDirectory(prefix="refmac wrapper ; $() ") as tmp:
+    root = Path(tmp)
+    spaced_py_dir = root / "spaced python interpreter ; dir"
+    spaced_py_dir.mkdir()
+    spaced_python = spaced_py_dir / "python3"
+    spaced_python.symlink_to(Path(sys.executable))
+    fake_refmac = root / "refmac5 ; $(never-run)"
+    fake_refmac.write_text(
+        "#!/usr/bin/env python3\n"
+        "import json, os, sys\n"
+        "print(json.dumps({\n"
+        "    'argv': sys.argv,\n"
+        "    'stdin': sys.stdin.read(),\n"
+        "    'marker': os.environ.get('PROTSTRUCT_REFMAC_ENV_MARKER'),\n"
+        "    'executable': sys.executable,\n"
+        "    'path_head': os.environ.get('PATH', '').split(os.pathsep)[0],\n"
+        "}))\n"
+    )
+    fake_refmac.chmod(0o755)
+    xyzin = root / "in ; model.pdb"
+    hklin = root / "in $(obs).mtz"
+    xyzout = root / "out ; model.pdb"
+    hklout = root / "out $(obs).mtz"
+    xmlout = root / "out ; xml $(literal).xml"
+    log_path = root / "refmac ; run.log"
+    custom_env = dict(os.environ)
+    custom_env["PATH"] = f"{spaced_py_dir}{os.pathsep}{os.environ.get('PATH', '')}"
+    custom_env["PROTSTRUCT_REFMAC_ENV_MARKER"] = "explicit-ccp4-env ; $()"
+
+    ccp4_env_calls = 0
+    orig_ccp4_environment = toolchain.ccp4_environment
+    try:
+        def _stub_ccp4_environment(setup_path=toolchain.CCP4_SETUP):
+            global ccp4_env_calls
+            ccp4_env_calls += 1
+            env_copy = dict(os.environ)
+            env_copy["PATH"] = f"{spaced_py_dir}{os.pathsep}{os.environ.get('PATH', '')}"
+            env_copy["PROTSTRUCT_REFMAC_ENV_MARKER"] = "stubbed-ccp4-default-env"
+            return env_copy
+
+        toolchain.ccp4_environment = _stub_ccp4_environment
+
+        proc = toolchain.run_refmac(
+            xyzin,
+            hklin,
+            xyzout,
+            hklout,
+            log_path,
+            "NCYC 0\nEND\n",
+            cwd=root,
+            timeout=10,
+            xml_output=xmlout,
+            executable=fake_refmac,
+            env=custom_env,
+        )
+        payload = json.loads(log_path.read_text())
+        check("run_refmac with explicit executable and xml_output succeeds", proc.returncode == 0)
+        check(
+            "fake executable runs under whitespace-safe shebang via spaced interpreter PATH",
+             payload["path_head"] == str(spaced_py_dir)
+            and Path(payload["executable"]).resolve() == Path(sys.executable).resolve(),
+        )
+        check(
+            "run_refmac passes XMLOUT and metacharacter paths literally",
+            payload["argv"]
+            == [
+                str(fake_refmac),
+                "XYZIN",
+                str(xyzin),
+                "HKLIN",
+                str(hklin),
+                "XYZOUT",
+                str(xyzout),
+                "HKLOUT",
+                str(hklout),
+                "XMLOUT",
+                str(xmlout),
+            ],
+        )
+        check(
+            "run_refmac forwards stdin keywords and explicit env without calling ccp4_environment",
+            payload["stdin"] == "NCYC 0\nEND\n"
+            and payload["marker"] == "explicit-ccp4-env ; $()"
+            and ccp4_env_calls == 0,
+        )
+
+        # Explicit env with ccp4=False is also authoritative and never calls ccp4_environment
+        log_path_false = root / "refmac ; ccp4-false.log"
+        proc_false = toolchain.run_refmac(
+            xyzin,
+            hklin,
+            xyzout,
+            hklout,
+            log_path_false,
+            "NCYC 0\nEND\n",
+            cwd=root,
+            timeout=10,
+            executable=fake_refmac,
+            env=custom_env,
+            ccp4=False,
+        )
+        payload_false = json.loads(log_path_false.read_text())
+        check(
+            "run_refmac with explicit env and ccp4=False preserves supplied env",
+            proc_false.returncode == 0
+            and payload_false["marker"] == "explicit-ccp4-env ; $()"
+            and ccp4_env_calls == 0,
+        )
+
+        # Explicit env combined with ccp4=True must raise ValueError without calling ccp4_environment
+        rejected_combo = False
+        try:
+            toolchain.run_refmac(
+                xyzin,
+                hklin,
+                xyzout,
+                hklout,
+                root / "refmac ; should-not-run.log",
+                "NCYC 0\nEND\n",
+                cwd=root,
+                timeout=10,
+                executable=fake_refmac,
+                env=custom_env,
+                ccp4=True,
+            )
+        except ValueError as exc:
+            rejected_combo = "ccp4=True" in str(exc) and "env" in str(exc)
+        check(
+            "run_refmac rejects explicit env combined with ccp4=True",
+            rejected_combo
+            and ccp4_env_calls == 0
+            and not (root / "refmac ; should-not-run.log").exists(),
+        )
+
+        # Legacy neither option (env=None, ccp4=None) loads ccp4_environment()
+        log_path_legacy = root / "refmac ; legacy-ccp4.log"
+        proc_legacy = toolchain.run_refmac(
+            xyzin,
+            hklin,
+            xyzout,
+            hklout,
+            log_path_legacy,
+            "END\n",
+            cwd=root,
+            timeout=10,
+            executable=fake_refmac,
+        )
+        payload_legacy = json.loads(log_path_legacy.read_text())
+        check(
+            "legacy run_refmac without env loads ccp4_environment by default",
+            proc_legacy.returncode == 0
+            and ccp4_env_calls == 1
+            and payload_legacy["marker"] == "stubbed-ccp4-default-env",
+        )
+    finally:
+        toolchain.ccp4_environment = orig_ccp4_environment
+
+    captured_calls: list[dict[str, object]] = []
+    orig_run_logged = toolchain.run_logged
+    try:
+        def _fake_run_logged(arguments, log_p, *, cwd=None, timeout=None, env=None, ccp4=False, input_text=None):
+            captured_calls.append(
+                {
+                    "arguments": list(arguments),
+                    "log_path": log_p,
+                    "cwd": cwd,
+                    "timeout": timeout,
+                    "env": env,
+                    "ccp4": ccp4,
+                    "input_text": input_text,
+                }
+            )
+            return toolchain.subprocess.CompletedProcess(["refmac5"], 0, "", "")
+
+        toolchain.run_logged = _fake_run_logged
+        toolchain.run_refmac(
+            "in.pdb",
+            "in.mtz",
+            "out.pdb",
+            "out.mtz",
+            "refmac.log",
+            "END\n",
+            cwd=root,
+        )
+    finally:
+        toolchain.run_logged = orig_run_logged
+    check(
+        "default run_refmac call preserves legacy argv and ccp4=True",
+        len(captured_calls) == 1
+        and captured_calls[0]["arguments"]
+        == [
+            "refmac5",
+            "XYZIN",
+            "in.pdb",
+            "HKLIN",
+            "in.mtz",
+            "XYZOUT",
+            "out.pdb",
+            "HKLOUT",
+            "out.mtz",
+        ]
+        and captured_calls[0]["ccp4"] is True
+        and captured_calls[0]["env"] is None,
+    )
+
 print("\nall toolchain unit tests passed")

scripts/toolchain.py

@@ -1035,23 +1035,47 @@ def run_refmac(
     *,
     cwd: str | os.PathLike[str],
     timeout: float = 1800,
+    xml_output: str | os.PathLike[str] | None = None,
+    executable: str | os.PathLike[str] = "refmac5",
+    env: Mapping[str, str] | None = None,
+    ccp4: bool | None = None,
 ) -> subprocess.CompletedProcess[str]:
-    """Run REFMAC with its keyword block on stdin, without a heredoc shell."""
+    """Run REFMAC with its keyword block on stdin, without a heredoc shell.
+
+    Environment contract:
+    - When ``env`` is supplied (``env is not None``) and ``ccp4`` is omitted
+      (``None``) or ``False``, ``env`` is authoritative and ``ccp4_environment()``
+      is not called.
+    - Combining ``env is not None`` with ``ccp4=True`` raises ``ValueError`` so
+      a caller-supplied environment is never silently overwritten.
+    - Omitting both ``env`` and ``ccp4`` preserves the legacy default
+      ``ccp4=True`` (loading ``ccp4_environment()``).
+    """
+    if env is not None and ccp4:
+        raise ValueError(
+            "run_refmac cannot combine an explicit env with ccp4=True; "
+            "pass env with ccp4 omitted or False so the supplied environment is authoritative"
+        )
+    arguments: list[str | os.PathLike[str]] = [
+        executable,
+        "XYZIN",
+        model,
+        "HKLIN",
+        reflections,
+        "XYZOUT",
+        xyz_output,
+        "HKLOUT",
+        hkl_output,
+    ]
+    if xml_output is not None:
+        arguments.extend(["XMLOUT", xml_output])
+    use_ccp4 = (env is None) if ccp4 is None else bool(ccp4)
     return run_logged(
-        [
-            "refmac5",
-            "XYZIN",
-            model,
-            "HKLIN",
-            reflections,
-            "XYZOUT",
-            xyz_output,
-            "HKLOUT",
-            hkl_output,
-        ],
+        arguments,
         log_path,
         cwd=cwd,
         timeout=timeout,
-        ccp4=True,
+        env=env,
+        ccp4=use_ccp4,
         input_text=keywords,
     )

scripts/validate.sh

@@ -216,7 +216,13 @@ if ! "${PYTHON}" "${REPO_ROOT}/scripts/test_record_tools.py"; then
   fail "record tool unit tests"
 fi
 
-# 4i. Every remaining scripts/test_*.py, discovered rather than enumerated (#228).
+# 4i. 1SAR rerun input-admission, MTZ projection, and PHENIX R-factor
+#     reconciliation pure-logic unit tests (no PHENIX/CCP4/Gemmi execution).
+if ! "${PYTHON}" "${REPO_ROOT}/scripts/test_onesar_rerun.py"; then
+  fail "onesar_rerun unit tests"
+fi
+
+# 4j. Every remaining scripts/test_*.py, discovered rather than enumerated (#228).
 #     test_round_figures.py shipped in #188 with 18 checks and was never run here,
 #     because this file names each suite by hand and one was forgotten -- silently,
 #     as it always would be. A suite nothing executes is the repo's #2 class: a
@@ -226,7 +232,7 @@ for _suite in "${REPO_ROOT}"/scripts/test_*.py; do
   case "$(basename "${_suite}")" in
     test_qds_emit.py|test_t15_ss_agreement.py|test_t16_interface_quality.py| \
     test_t17_nmr_ensemble.py|test_t17_restraint_summary.py|test_bench_tolerances.py| \
-    test_guards.py|test_record_tools.py|test_summary_coverage.py) continue ;;
+    test_guards.py|test_record_tools.py|test_onesar_rerun.py|test_summary_coverage.py) continue ;;
   esac
   if ! "${PYTHON}" "${_suite}"; then
     fail "$(basename "${_suite}" .py)"