feat: support extended (12-char) PDB IDs in output CIF patching

analyses/all.toml

@@ -10,7 +10,7 @@ PATCH_CIF_PATTERN = "refined.cif"
 GRID_SEARCH_DEPTH = "4"
 PATCH_DEPTH = "${GRID_SEARCH_DEPTH}"
 PATCH_INPUT_PDB_PATTERN = "processed/{pdb_id}/{pdb_id}_single_001_density_input.cif"
-PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/([A-Za-z0-9]{4})"
+PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})"
 
 [shared_args]
 grid-search-results-path = "${GRID_SEARCH_RESULTS_DIR}"

analyses/analyze_grid_search.toml

@@ -10,7 +10,7 @@ PATCH_CIF_PATTERN = "refined.cif"
 GRID_SEARCH_DEPTH = "4"
 PATCH_DEPTH = "${GRID_SEARCH_DEPTH}"
 PATCH_INPUT_PDB_PATTERN = "processed/{pdb_id}/{pdb_id}_single_001_density_input.cif"
-PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/([A-Za-z0-9]{4})"
+PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})"
 
 [shared_args]
 grid-search-results-path = "${GRID_SEARCH_RESULTS_DIR}"

analyses/external_tools.toml

@@ -10,7 +10,7 @@ PATCH_CIF_PATTERN = "refined.cif"
 GRID_SEARCH_DEPTH = "4"
 PATCH_DEPTH = "${GRID_SEARCH_DEPTH}"
 PATCH_INPUT_PDB_PATTERN = "processed/{pdb_id}/{pdb_id}_single_001_density_input.cif"
-PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/([A-Za-z0-9]{4})"
+PATCH_RCSB_PATTERN = "${GRID_SEARCH_RESULTS_DIR}/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})"
 
 [shared_args]
 grid-search-results-path = "${GRID_SEARCH_RESULTS_DIR}"

scripts/eval/EVALUATION.md

@@ -46,7 +46,7 @@ You can run the following command, which assumes:
 ```shell
 pixi run -e analysis python scripts/patch_output_cif_files.py \
   --input-dir /home/ubuntu/grid_search_results \
-  --rcsb-pattern 'grid_search_results/(.{4})/...' \
+  --rcsb-pattern 'grid_search_results/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})/...' \
   --cif-pattern 'refined.cif' \
   --grid-search-input-dir /home/ubuntu/grid_search_inputs \
   --input-pdb-pattern '{pdb_id}/{pdb_id}_original.cif'

scripts/patch_output_cif_files.py

@@ -19,6 +19,18 @@
 SAMPLEWORKS_CACHE = Path("~/.sampleworks/rcsb").expanduser()
 
 
+# A valid PDB ID is either the extended 12-char form `pdb_` + 8 alphanumerics
+# (e.g. `pdb_00004hhb`) or the legacy 4-char form, whose first character is always a digit (0-9).
+# The leading-digit rule is what distinguishes a real legacy ID from a stray folder name like
+# `TEST`/`logs`.
+_VALID_RCSB_ID = re.compile(r"pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3}")
+
+# Default --rcsb-pattern: locate the id (one capturing group) right after the
+# grid_search_results/ folder. Its group IS _VALID_RCSB_ID, so the default pattern and the
+# validator can never drift apart.
+DEFAULT_RCSB_PATTERN = rf"grid_search_results/({_VALID_RCSB_ID.pattern})"
+
+
 def crawl_dir_by_depth(
     root_dir: str | Path,
     target_pattern: str,
@@ -66,7 +78,7 @@ def parse_args():
     )
     parser.add_argument(
         "--rcsb-pattern",
-        default="grid_search_results/(.{4})",
+        default=DEFAULT_RCSB_PATTERN,
         help="Regex pattern for rcsb ids in file paths. "
         "Must have only one group, surrounding the id",
     )
@@ -89,7 +101,7 @@ def main(
     input_dir: str | Path,
     grid_search_input_dir: str | Path,
     target_pattern: str,
-    rcsb_regex: str = r"grid_search_results/(.{4})",
+    rcsb_regex: str = DEFAULT_RCSB_PATTERN,
     depth: int = 4,
     input_pdb_pattern: str = "{pdb_id}/{pdb_id}_single_001_density_input.cif",
 ) -> int:
@@ -137,19 +149,96 @@ def main(
     return 0
 
 
+class InvalidRcsbIdError(ValueError):
+    """Raised when ``--rcsb-pattern`` matches a path but its capturing group captured a
+    string that is not a valid PDB id -- usually a sign the pattern targets the wrong part
+    of the path. Distinct from a plain no-match so the caller can warn specifically."""
+
+    def __init__(self, token: str, rcsb_regex: str) -> None:
+        self.token = token
+        self.rcsb_regex = rcsb_regex
+        super().__init__(f"--rcsb-pattern captured {token!r}, which is not a valid PDB id")
+
+
+def extract_rcsb_id(cif_path: Path, rcsb_regex: str) -> str | None:
+    """Extract and validate the RCSB id from a cif path.
+
+    ``rcsb_regex`` locates the candidate (it must contain exactly one capturing group around
+    the id; see ``--rcsb-pattern``). The id must be a **complete folder component**: a
+    capture that is only the prefix of a longer folder name is rejected, so a stray suffix
+    can't silently resolve to the wrong entry. The whole-component token is then checked
+    against the PDB-id grammar and returned *verbatim* (legacy ``4hhb`` or extended
+    ``pdb_00004hhb`` -- no normalization).
+
+    Examples with the default pattern (folder -> result)::
+
+        4hhb                  -> "4hhb"          (works)
+        pdb_00004hhb          -> "pdb_00004hhb"  (works)
+        1VME                  -> "1VME"          (works)
+        4hhb_final            -> None  (id is only a prefix of the folder)
+        protease_pdb_1000abcd -> None  (id is not at the component start)
+        1abc_pdb_1000abcd     -> None  (two id-like parts -- ambiguous)
+        logs                  -> None  (not a PDB id)
+
+    Possible problem this guards against: without the whole-component rule, ``4hhb_final``
+    would yield ``4hhb`` and ``1abc_pdb_1000abcd`` would yield ``1abc`` -- both silently
+    patching the wrong entry. Such folders are skipped with a warning instead. If your
+    folders embed the id in a larger name, pass a custom ``--rcsb-pattern`` (beware names
+    with more than one id-like substring).
+
+    Returns ``None`` when no complete PDB-id folder is found. Raises ``InvalidRcsbIdError``
+    when a whole component is captured but is not a valid PDB id (a likely sign the pattern
+    targets the wrong part of the path), and ``ValueError`` when the pattern does not have
+    exactly one capturing group. This is the main checkpoint: a wrong id here would patch
+    coordinates into the wrong template.
+    """
+    rcsb_re = re.compile(rcsb_regex)
+    if rcsb_re.groups != 1:
+        raise ValueError(
+            f"--rcsb-pattern must have exactly one capturing group, "
+            f"got {rcsb_re.groups}: {rcsb_regex!r}"
+        )
+    path_str = cif_path.as_posix()
+    m = rcsb_re.search(path_str)
+    if not m:
+        return None
+    # The id must be a whole folder component: reject a capture that is only a prefix/suffix of
+    # a longer name (e.g. "4hhb" out of "4hhb_final" or "4hhb" out of "foo4hhb"), which would
+    # silently resolve to the wrong entry. A whole-component id is bounded by path separators (or
+    # the start/end of the path).
+    start_ok = m.start(1) == 0 or path_str[m.start(1) - 1] == "/"
+    end_ok = m.end(1) == len(path_str) or path_str[m.end(1)] == "/"
+    if not (start_ok and end_ok):
+        return None
+    token = m.group(1)
+    if not _VALID_RCSB_ID.fullmatch(token):
+        raise InvalidRcsbIdError(token, rcsb_regex)
+    return token
+
+
 def patch_individual_cif_file(
     cif_file: Path, rcsb_regex: str, reference_dir: Path, input_pdb_pattern: str
 ) -> str | None:  # returns an error message if there was one
     cif_path = Path(cif_file)
-    m = re.search(rcsb_regex, str(cif_path))
-    rcsb_id = m.group(1) if m else None
-    if not m:
+    try:
+        rcsb_id = extract_rcsb_id(cif_path, rcsb_regex)
+    except InvalidRcsbIdError as exc:
         msg = (
-            f"Unable to parse an RCSB structure id: from path {cif_file} with pattern {rcsb_regex}"
+            f"--rcsb-pattern {rcsb_regex!r} matched {cif_file} but captured {exc.token!r}, "
+            f"which is not a valid PDB id (expected e.g. '4hhb' or 'pdb_00004hhb'; "
+            f"regex {_VALID_RCSB_ID.pattern!r}). Check that the capturing group targets the id."
+        )
+        logger.warning(msg)
+        return msg
+    if rcsb_id is None:
+        msg = (
+            f"--rcsb-pattern {rcsb_regex!r} did not find a complete PDB-id folder in "
+            f"{cif_file}. Expected a folder named exactly a PDB id (e.g. '4hhb' or "
+            f"'pdb_00004hhb'; regex {_VALID_RCSB_ID.pattern!r}). "
+            f"Check the directory layout or the pattern."
         )
         logger.warning(msg)
         return msg
-
     # Get the offset for residue numbering in the reference structure
     try:
         reference_path = reference_dir / input_pdb_pattern.format(pdb_id=rcsb_id)

tests/eval/script_loader.py

@@ -0,0 +1,31 @@
+"""Helper for loading standalone scripts as importable modules in tests.
+
+Scripts under ``scripts/`` live outside the installed ``sampleworks`` package, so they
+can't be imported normally. ``load_script`` imports one by filesystem path instead.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+from pathlib import Path
+from types import ModuleType
+
+
+def load_script(script_path: Path) -> ModuleType:
+    """Import a standalone ``.py`` script by path so tests don't require it on ``sys.path``.
+
+    Parameters
+    ----------
+    script_path : Path
+        Filesystem path to the script to import.
+
+    Returns
+    -------
+    ModuleType
+        The imported module, with the script's top-level names available as attributes.
+    """
+    spec = importlib.util.spec_from_file_location(script_path.stem, script_path)
+    assert spec is not None and spec.loader is not None
+    mod = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(mod)
+    return mod

tests/eval/test_patch_output_cif_files.py

@@ -0,0 +1,97 @@
+"""Unit tests for the RCSB id extraction in ``scripts/patch_output_cif_files.py``.
+
+This is the "main checkpoint" of the patching pipeline: the id is read from the folder
+segment of a path and drives both the RCSB ``fetch`` and the reference-input lookup, so a
+wrong id here would silently patch coordinates into the wrong template. These tests pin the
+exact capture for legacy and extended ids and assert that malformed folders are rejected.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from .script_loader import load_script
+
+
+_SCRIPT_PATH = Path(__file__).resolve().parents[2] / "scripts" / "patch_output_cif_files.py"
+
+
+@pytest.fixture(scope="module")
+def script():
+    return load_script(_SCRIPT_PATH)
+
+
+# An independent copy of the script's DEFAULT_RCSB_PATTERN, kept separate on purpose: the
+# test is a spec, not a mirror of the implementation. If the script's default ever changes,
+# this literal does not move with it -- the resulting behavior difference should surface as
+# a failure here for a human to review, rather than being silently tracked.
+_DEFAULT_REGEX = r"grid_search_results/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})"
+
+
+@pytest.mark.parametrize(
+    ("folder", "expected"),
+    [
+        # --- WORKS: the folder is exactly a PDB id -> extracted verbatim ---
+        ("4hhb", "4hhb"),  # legacy, lowercase
+        ("1VME", "1VME"),  # legacy, uppercase
+        ("9BN8", "9BN8"),  # legacy, mixed case + digits
+        ("pdb_00004hhb", "pdb_00004hhb"),  # extended transitional -- full id, NOT 0000 / 4hhb
+        ("pdb_1000axyz", "pdb_1000axyz"),  # extended, genuinely-new id with no legacy form
+        # --- REJECTED (-> None, skipped): not a bare PDB id, so we never mis-pick an entry ---
+        ("TEST", None),  # not a PDB id at all (letter-led)
+        ("logs", None),  # scratch folder
+        ("data", None),  # scratch folder
+        ("4hhb_final", None),  # id is only a PREFIX of the folder (would else capture "4hhb")
+        ("4hhbEXTRA", None),  # id prefix, no separator
+        ("pdb_1000abcd_final", None),  # extended-id prefix + suffix
+        ("protease_pdb_1000abcd", None),  # id embedded after a prefix (reviewer's example)
+        ("1abc_pdb_1000abcd", None),  # two id-like substrings -> ambiguous (would else pick "1abc")
+    ],
+)
+def test_extract_rcsb_id_from_folder(script, folder: str, expected: str | None) -> None:
+    """Only a folder that is *exactly* a PDB id is extracted; everything else returns None.
+
+    The rejected rows are the important part: a folder like ``4hhb_final`` or
+    ``1abc_pdb_1000abcd`` embeds an id-shaped substring, and without the whole-component rule
+    the extractor would silently capture ``4hhb`` / ``1abc`` and patch the wrong entry. These
+    are skipped instead. Extracting an id embedded inside a larger folder name is deliberately
+    NOT supported by the default pattern (it is ambiguous when several id-like parts appear).
+    """
+    path = Path(f"/data/results/grid_search_results/{folder}/trial_1/refined.cif")
+    assert script.extract_rcsb_id(path, _DEFAULT_REGEX) == expected
+
+
+def test_extract_rcsb_id_no_match_returns_none(script) -> None:
+    """A path that doesn't contain the anchor at all yields None (not an exception)."""
+    path = Path("/data/results/some_other_dir/4hhb/refined.cif")
+    assert script.extract_rcsb_id(path, _DEFAULT_REGEX) is None
+
+
+def test_loose_pattern_captures_non_id_raises(script) -> None:
+    """A permissive custom --rcsb-pattern that captures a non-id raises InvalidRcsbIdError.
+
+    This is distinct from a plain no-match (which returns None): the captured token matched
+    the pattern but isn't a real PDB id, so the user is told their pattern is the culprit.
+    A real id captured by the same loose pattern is still accepted.
+    """
+    loose = r"grid_search_results/(.{4})"
+    base = Path("/data/results/grid_search_results")
+    with pytest.raises(script.InvalidRcsbIdError, match="not a valid PDB id"):
+        script.extract_rcsb_id(base / "TEST" / "refined.cif", loose)
+    assert script.extract_rcsb_id(base / "4hhb" / "refined.cif", loose) == "4hhb"
+
+
+@pytest.mark.parametrize(
+    "bad_regex",
+    [
+        r"grid_search_results/pdb_[A-Za-z0-9]{8}",  # zero capturing groups
+        r"grid_search_results/(pdb_)([A-Za-z0-9]{8})",  # two capturing groups
+    ],
+)
+def test_extract_rcsb_id_requires_single_group(script, bad_regex: str) -> None:
+    """A misconfigured pattern (not exactly one group) fails loudly, not silently."""
+    path = Path("/data/results/grid_search_results/pdb_00004hhb/refined.cif")
+    with pytest.raises(ValueError, match="exactly one capturing group"):
+        script.extract_rcsb_id(path, bad_regex)

tests/runs/test_runner.py

@@ -311,7 +311,7 @@ def test_analysis_preset_builds_eval_script_invocations(monkeypatch: pytest.Monk
     assert patch_args["--input-dir"] == "/grid/results"
     assert patch_args["--grid-search-input-dir"] == "/grid/inputs"
     assert patch_args["--cif-pattern"] == "refined.cif"
-    assert patch_args["--rcsb-pattern"] == "/grid/results/([A-Za-z0-9]{4})"
+    assert patch_args["--rcsb-pattern"] == "/grid/results/(pdb_[A-Za-z0-9]{8}|[0-9][A-Za-z0-9]{3})"
     assert patch_args["--input-pdb-pattern"] == (
         "processed/{pdb_id}/{pdb_id}_single_001_density_input.cif"
     )