refactor(regression): collapse path-placeholder clump into PlaceholderMap

changelog/759.trivial.md

@@ -0,0 +1 @@
+Collapsed the regression-baseline path placeholders (`<OUTPUT_DIR>` / `<TEST_DATA_DIR>` / `<SOFTWARE_ROOT_DIR>`) into a single `PlaceholderMap` value object shared by the capture and verification paths, replacing the directory parameters previously threaded through both.

packages/climate-ref-core/src/climate_ref_core/output_files.py

@@ -15,7 +15,7 @@
 Only files in the results directory are accessed by the API/public.
 
 For some tests we must sanitise paths to files as well as the contents of text files
-(:func:`to_placeholders` / :func:`from_placeholders`).
+(:class:`PlaceholderMap`).
 This ensures that the regression data is machine independent.
 """
 
@@ -25,6 +25,8 @@
 from pathlib import Path
 from typing import TYPE_CHECKING
 
+from attrs import frozen
+
 from climate_ref_core.diagnostics import ensure_relative_path
 from climate_ref_core.logging import EXECUTION_LOG_FILENAME
 from climate_ref_core.pycmec.output import CMECOutput
@@ -109,85 +111,97 @@ def rewrite_tree(
                 file.write_text(rewritten, encoding="utf-8")
 
 
-def _placeholder_pairs(
-    output_dir: Path, test_data_dir: Path, software_root_dir: Path | None
-) -> list[tuple[str, Path]]:
-    """Return the ``(placeholder, absolute directory)`` pairs sanitised in committed artefacts.
-
-    Shared by :func:`to_placeholders` and :func:`from_placeholders` so the placeholder set is
-    declared once and each picks a substitution direction.
+@frozen
+class PlaceholderMap:
     """
-    pairs = [(PLACEHOLDER_OUTPUT_DIR, output_dir), (PLACEHOLDER_TEST_DATA_DIR, test_data_dir)]
-    if software_root_dir is not None:
-        pairs.append((PLACEHOLDER_SOFTWARE_ROOT_DIR, software_root_dir))
-    return pairs
-
-
-def to_placeholders(
-    directory: Path,
-    *,
-    output_dir: Path,
-    test_data_dir: Path,
-    software_root_dir: Path | None = None,
-    globs: tuple[str, ...] = SANITISED_FILE_GLOBS,
-) -> None:
+    Bidirectional map between absolute runtime directories and portable ``<TOKEN>`` placeholders.
+
+    A committed regression bundle is made machine-independent
+    by replacing host-specific absolute paths with stable tokens --
+    ``<OUTPUT_DIR>``, ``<TEST_DATA_DIR>`` and (optionally) ``<SOFTWARE_ROOT_DIR>``.
+    The *same* map drives every direction:
+
+    - :meth:`sanitise` rewrites absolute paths to tokens (capture / mint).
+    - :meth:`hydrate` rewrites tokens back to absolute paths (replay / rebuild).
+    - :meth:`as_replacements` yields the ``{absolute: token}`` mapping
+      the bundle and series comparators apply to a freshly regenerated artefact before diffing.
+
+    The token set is declared in one place (:meth:`for_baseline` then :meth:`with_output`),
+    so the capture side and the verification side cannot declare different sets and drift apart.
+    Adding a placeholder is a one-line change here,
+    not a new parameter threaded through every caller.
+
+    The two configuration-stable tokens (``<TEST_DATA_DIR>`` / ``<SOFTWARE_ROOT_DIR>``)
+    are fixed for a whole run;
+    the per-execution ``<OUTPUT_DIR>`` is late-bound with :meth:`with_output`.
     """
-    Rewrite absolute paths in committed artefacts to portable placeholders ("to").
 
-    Replaces the absolute ``output_dir`` with ``<OUTPUT_DIR>``, the absolute
-    ``test_data_dir`` with ``<TEST_DATA_DIR>``, and (when given) the absolute
-    ``software_root_dir`` with ``<SOFTWARE_ROOT_DIR>`` in every text artefact under ``directory``.
-    Binary files are never touched.
+    pairs: tuple[tuple[str, Path], ...]
+    """Ordered ``(token, absolute_directory)`` pairs.
 
-    Parameters
-    ----------
-    directory
-        The tree of committed artefacts to sanitise in place.
-    output_dir
-        The absolute execution output directory.
-    test_data_dir
-        The absolute provider test-data directory.
-    software_root_dir
-        The absolute shared-software root directory, if any. Substituted in provenance
-        command lines so the committed bundle stays machine-independent.
-    globs
-        File globs whose contents are rewritten.
+    Application order is not significant:
+    all rewriting goes through :func:`rewrite_tree`,
+    which re-sorts longest-match-first so an overlapping shorter path cannot shadow a longer one.
     """
-    pairs = _placeholder_pairs(output_dir, test_data_dir, software_root_dir)
-    rewrite_tree(directory, {str(abs_dir): placeholder for placeholder, abs_dir in pairs}, globs)
-
 
-def from_placeholders(
-    directory: Path,
-    *,
-    output_dir: Path,
-    test_data_dir: Path,
-    software_root_dir: Path | None = None,
-    globs: tuple[str, ...] = SANITISED_FILE_GLOBS,
-) -> None:
-    """
-    Rewrite portable placeholders back to absolute paths ("from").
-
-    Inverse of :func:`to_placeholders`: replaces ``<OUTPUT_DIR>`` with the absolute ``output_dir``,
-    ``<TEST_DATA_DIR>`` with the absolute ``test_data_dir``, and (when given) ``<SOFTWARE_ROOT_DIR>``
-    with the absolute ``software_root_dir`` in every text artefact under ``directory``.
-    Binary files are never touched.
-
-    Parameters
-    ----------
-    directory
-        The tree of artefacts to hydrate in place.
-    output_dir
-        The absolute execution output directory to substitute in.
-    test_data_dir
-        The absolute provider test-data directory to substitute in.
-    software_root_dir
-        The absolute shared-software root directory to substitute in, if any.
-    globs
-        File globs whose contents are rewritten.
-    """
-    pairs = _placeholder_pairs(output_dir, test_data_dir, software_root_dir)
-    rewrite_tree(directory, {placeholder: str(abs_dir) for placeholder, abs_dir in pairs}, globs)
+    @classmethod
+    def for_baseline(cls, *, test_data_dir: Path, software_root_dir: Path | None = None) -> PlaceholderMap:
+        """
+        Build the configuration-stable placeholder set for a committed baseline.
+
+        Holds every token except the per-execution output directory,
+        which is added with :meth:`with_output`.
+        ``software_root_dir`` is optional:
+        when ``None`` no ``<SOFTWARE_ROOT_DIR>`` substitution is applied --
+        a verification context that does not know the shared-software root relies on this,
+        and the omission is explicit and declared once.
+        """
+        pairs: list[tuple[str, Path]] = [(PLACEHOLDER_TEST_DATA_DIR, test_data_dir)]
+        if software_root_dir is not None:
+            pairs.append((PLACEHOLDER_SOFTWARE_ROOT_DIR, software_root_dir))
+        return cls(pairs=tuple(pairs))
+
+    def with_output(self, output_dir: Path) -> PlaceholderMap:
+        """Return a new map that also binds ``<OUTPUT_DIR>`` to the per-execution ``output_dir``.
+
+        Rebinding replaces any existing ``<OUTPUT_DIR>`` entry,
+        so a second call hydrates to the latest directory rather than the first.
+        """
+        rest = tuple((token, path) for token, path in self.pairs if token != PLACEHOLDER_OUTPUT_DIR)
+        return PlaceholderMap(pairs=((PLACEHOLDER_OUTPUT_DIR, output_dir), *rest))
+
+    @property
+    def is_output_bound(self) -> bool:
+        """Whether ``<OUTPUT_DIR>`` has been bound via :meth:`with_output`.
+
+        The committed-bundle writer requires this:
+        an unbound map would leave execution-specific output paths in the committed bundle.
+        """
+        return any(token == PLACEHOLDER_OUTPUT_DIR for token, _ in self.pairs)
+
+    def as_replacements(self) -> dict[str, str]:
+        """Return the ``{absolute_directory: token}`` mapping (real path -> placeholder).
+
+        This is what the bundle and series comparators apply to a regenerated artefact
+        before diffing it against the committed (already-placeholdered) baseline.
+        """
+        return {str(abs_dir): token for token, abs_dir in self.pairs}
+
+    def sanitise(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None:
+        """Rewrite absolute paths to ``<TOKEN>`` placeholders in every text artefact under ``directory``.
+
+        Binary files are never touched.
+        In-place.
+        """
+        rewrite_tree(directory, self.as_replacements(), globs)
+
+    def hydrate(self, directory: Path, globs: tuple[str, ...] = SANITISED_FILE_GLOBS) -> None:
+        """Inverse of :meth:`sanitise`: rewrite ``<TOKEN>`` placeholders back to absolute paths.
+
+        Binary files are never touched.
+        In-place.
+        """
+        rewrite_tree(directory, {token: str(abs_dir) for token, abs_dir in self.pairs}, globs)
 
 
 def copy_output_file(

packages/climate-ref-core/src/climate_ref_core/regression/capture.py

@@ -27,7 +27,7 @@
 
 from loguru import logger
 
-from climate_ref_core.output_files import copy_execution_outputs, to_placeholders
+from climate_ref_core.output_files import PlaceholderMap, copy_execution_outputs
 from climate_ref_core.paths import safe_path
 
 from ._quantise import round_floats
@@ -198,16 +198,14 @@ def write_committed_bundle(
     source_dir: Path,
     regression_dir: Path,
     *,
-    output_dir: Path,
-    test_data_dir: Path,
-    software_root_dir: Path | None = None,
+    placeholders: PlaceholderMap,
 ) -> dict[str, str]:
     """
     Write the sanitised committed CMEC bundle into ``regression_dir``.
 
     Copies each committed artefact present in ``source_dir`` into ``regression_dir``,
     then rewrites absolute paths to portable placeholders in place
-    (:func:`~climate_ref_core.output_files.to_placeholders`),
+    (:meth:`~climate_ref_core.output_files.PlaceholderMap.sanitise`),
     rounds floats (:func:`_round_committed_floats`),
     and redacts host/user-specific CMEC provenance fields (:func:`_redact_committed_provenance`).
     When a committed artefact is absent from ``source_dir``,
@@ -220,20 +218,30 @@ def write_committed_bundle(
         results directory).
     regression_dir
         The destination ``regression/`` directory (created if needed).
-    output_dir
-        The absolute execution output directory, for path substitution.
-    test_data_dir
-        The absolute provider test-data directory, for path substitution.
-    software_root_dir
-        The absolute shared-software root directory, for path substitution in provenance
-        command lines. When ``None``, no software-root substitution is applied.
+    placeholders
+        The placeholder map for this execution, already bound to the output directory via
+        :meth:`~climate_ref_core.output_files.PlaceholderMap.with_output`. Its absolute paths are
+        rewritten to portable ``<TOKEN>`` placeholders in the copied bundle.
 
     Returns
     -------
     :
         The committed digests ``{filename: sha256}`` of the bytes just written,
         suitable for :attr:`Manifest.committed`.
+
+    Raises
+    ------
+    ValueError
+        If ``placeholders`` is not bound to an output directory.
+        An unbound map would leave execution-specific output paths in the committed bundle
+        and digest those machine-specific bytes.
     """
+    if not placeholders.is_output_bound:
+        raise ValueError(
+            "placeholders must be bound to an output directory via with_output() "
+            "before writing a committed bundle"
+        )
+
     regression_dir.mkdir(parents=True, exist_ok=True)
 
     for filename in COMMITTED_BUNDLE_FILES:
@@ -245,12 +253,7 @@ def write_committed_bundle(
             # Drop a stale copy from a previous capture so it is not re-digested.
             dest.unlink(missing_ok=True)
 
-    to_placeholders(
-        regression_dir,
-        output_dir=output_dir,
-        test_data_dir=test_data_dir,
-        software_root_dir=software_root_dir,
-    )
+    placeholders.sanitise(regression_dir)
     # Round floats in place before digesting,
     # so the committed bytes (and their recorded digests) are the stable, rounded ones.
     # Placeholder substitution only rewrites path strings, so order relative to it does not matter for floats.
@@ -292,9 +295,7 @@ def capture_execution(  # noqa: PLR0913
     result: ExecutionResult,
     *,
     regression_dir: Path,
-    output_dir: Path,
-    test_data_dir: Path,
-    software_root_dir: Path | None = None,
+    placeholders: PlaceholderMap,
     # TODO: Unify the log handling
     include_log: bool = False,
 ) -> tuple[dict[str, str], dict[str, NativeEntry]]:
@@ -318,13 +319,9 @@ def capture_execution(  # noqa: PLR0913
         The successful execution result (must carry a metric bundle filename).
     regression_dir
         The test case ``regression/`` directory for the committed bundle.
-    output_dir
-        The absolute execution output directory, for path substitution.
-    test_data_dir
-        The absolute provider test-data directory, for path substitution.
-    software_root_dir
-        The absolute shared-software root directory, for path substitution in provenance
-        command lines. When ``None``, no software-root substitution is applied.
+    placeholders
+        The placeholder map for this execution, already bound to the output directory via
+        :meth:`~climate_ref_core.output_files.PlaceholderMap.with_output`.
     include_log
         If True, the execution log is included in the persisted/native set.
 
@@ -344,13 +341,7 @@ def capture_execution(  # noqa: PLR0913
         include_log=include_log,
     )
     base_dir = results_directory / fragment
-    committed = write_committed_bundle(
-        base_dir,
-        regression_dir,
-        output_dir=output_dir,
-        test_data_dir=test_data_dir,
-        software_root_dir=software_root_dir,
-    )
+    committed = write_committed_bundle(base_dir, regression_dir, placeholders=placeholders)
     native = build_native_snapshot(base_dir, relpaths)
     return committed, native
 

packages/climate-ref-core/src/climate_ref_core/testing.py

@@ -30,7 +30,7 @@
 from climate_ref_core.diagnostics import ExecutionDefinition, ExecutionResult
 from climate_ref_core.esgf.base import ESGFRequest
 from climate_ref_core.metric_values.typing import SeriesMetricValue
-from climate_ref_core.output_files import from_placeholders, ordered_replacements
+from climate_ref_core.output_files import PlaceholderMap, ordered_replacements
 from climate_ref_core.paths import safe_path
 from climate_ref_core.pycmec.metric import CMECMetric
 from climate_ref_core.pycmec.output import CMECOutput
@@ -567,8 +567,10 @@ def validate_cmec_bundles(diagnostic: Diagnostic, result: ExecutionResult) -> No
     # structurally validated here, so a diagnostic can change the metric/output
     # values without any regression test failing (see issue #703 for the series
     # equivalent). A content comparison must sanitise the regenerated bundle first
-    # via `ExecutionRegression.output_replacements`, since the committed bundles
-    # store `<OUTPUT_DIR>` / `<TEST_DATA_DIR>` placeholders.
+    # via `PlaceholderMap.for_baseline(...).with_output(...).as_replacements()`, since the
+    # committed bundles store `<OUTPUT_DIR>` / `<TEST_DATA_DIR>` / `<SOFTWARE_ROOT_DIR>`
+    # placeholders. diagnostic.json provenance carries the software root, so building the
+    # replacements from the map (rather than a hand-rolled dict) keeps this in step with capture.
     assert result.successful, f"Execution failed: {result}"
 
     # Validate metric bundle
@@ -701,7 +703,11 @@ def load_regression_definition(self, tmp_dir: Path) -> ExecutionDefinition:
         output_dir.mkdir(parents=True, exist_ok=True)
         shutil.copytree(regression_path, output_dir, dirs_exist_ok=True)
 
-        from_placeholders(output_dir, output_dir=output_dir, test_data_dir=self.test_data_dir)
+        # This verification context does not know the shared-software root, so the map omits
+        # <SOFTWARE_ROOT_DIR>; the omission is declared once on the map rather than open-coded here.
+        PlaceholderMap.for_baseline(test_data_dir=self.test_data_dir).with_output(output_dir).hydrate(
+            output_dir
+        )
 
         datasets: ExecutionDatasetCollection = load_datasets_from_yaml(catalog_path)
 
@@ -722,10 +728,9 @@ def validate(self, definition: ExecutionDefinition) -> None:
             expected_path=self.paths.regression / "series.json",
             actual_path=definition.output_directory / "series.json",
             slug=self.diagnostic.slug,
-            replacements={
-                str(definition.output_directory): "<OUTPUT_DIR>",
-                str(self.test_data_dir): "<TEST_DATA_DIR>",
-            },
+            replacements=PlaceholderMap.for_baseline(test_data_dir=self.test_data_dir)
+            .with_output(definition.output_directory)
+            .as_replacements(),
         )