fix: isolate Zeo++ output runs

README.md

@@ -205,12 +205,14 @@ result = run_zeopp(
     analyses=["res", "sa"],
     radii_file="UFF.rad",
     num_samples=100000,
+    output_dir="zeopp_runs",
 )
 print(result["results"]["res"])   # {'Di': 18.5, 'Df': 8.0, 'Dif': 10.9, ...}
 print(result["results"]["sa"])    # {'ASA': 4004.7, 'ASA_m2_g': 3918.3, ...}
+print(result["output_dir"])        # unique persistent directory for this run
 
-# Parse existing Zeo++ output files
-result = get_output_data("output_dir/")
+# Parse output files for one structure/run
+result = get_output_data(result["output_dir"])
 
 # Agent-free, runtime-selectable MLIP execution
 from matkit.mlip import (

docs/plans/code-review-followups.md

@@ -0,0 +1,118 @@
+# Code review follow-ups
+
+Review date: 2026-09-05. Baseline revision: `5cd7a03` on
+`docs/calculation-examples-review`.
+
+This file records findings 2-7 from the repository review. They are deferred:
+none of the issues below are fixed by the accompanying Zeo++ stale-output
+change. Each item should receive a focused implementation and regression test
+before it is marked complete.
+
+## 2. gRASPA CLI uses the wrong default output columns
+
+**Severity:** High. **Status:** Deferred.
+
+The gRASPA setup templates and unified `AdsorptionRequest` default to PR-EOS,
+but `matkit graspa analyze` calls `get_output_data()` without `eos=True`. The
+legacy parser therefore reads the non-EOS column positions by default. In the
+review fixture, the same result is interpreted as uptake 12 by the unified API
+and uptake 7 by the CLI.
+
+**Impact:** A successful CLI command can report a valid-looking but incorrect
+scientific result.
+
+**Recommended remediation:** Persist the chosen fugacity treatment with each
+prepared simulation and make analysis consume it. Add an explicit CLI override
+for manually produced output, rather than relying on a silent Boolean default.
+Test default PR-EOS setup/analyze parity, an explicit non-EOS result, and
+missing or contradictory metadata.
+
+## 3. Legacy CLI failures return exit status zero
+
+**Severity:** High. **Status:** Deferred.
+
+Many legacy commands in `src/matkit/cli.py` catch broad exceptions, print an
+error to stderr, and return normally. Click consequently exits with status 0.
+This affects gRASPA, pygRASPA, RASPA2, plotting, legacy MLIP, PACMOF2, and
+Zeo++ command paths.
+
+**Impact:** Shell scripts, schedulers, and CI can treat failed work as
+successful and continue with missing or invalid artifacts.
+
+**Recommended remediation:** Convert operational failures to `ClickException`
+or an equivalent nonzero exit while retaining Click's exit 2 for invalid
+arguments. Add parameterized CLI tests asserting exit 1 and stderr for each
+command family, plus exit 0 for successful invocations.
+
+## 4. UMA multiprocessing can deadlock or lose results
+
+**Severity:** High. **Status:** Deferred.
+
+`run_opt_uma_batch()` joins every worker before draining `result_queue`, then
+uses `Queue.empty()` to decide whether results remain. A worker can block while
+flushing a full queue, preventing `join()` from completing, and
+`multiprocessing.Queue.empty()` is not reliable for synchronization.
+
+**Impact:** Large batches can hang indefinitely or produce an incomplete
+`results.jsonl` while appearing to have processed all jobs.
+
+**Recommended remediation:** Collect exactly one terminal result per submitted
+job while workers are active, with explicit worker-exit and timeout handling;
+then join and close queue resources. Synthesize failure records for jobs whose
+workers terminate without reporting. Test queue backpressure, abrupt worker
+exit, delayed delivery, and complete accounting for every input/model/run-type
+combination.
+
+## 5. Batch interruption overwrites preparation failures
+
+**Severity:** Medium. **Status:** Deferred.
+
+`_interrupt_batch()` marks every run with `run.json` but no `result.json` as a
+new batch interruption. That includes items which already reached a terminal
+preparation failure, so their original stage, exception, and diagnostic are
+replaced by the later batch-level error.
+
+**Impact:** Manifests lose the root cause of individual failures and make
+recovery decisions less reliable.
+
+**Recommended remediation:** Treat terminal item failures as immutable.
+Interruption should update only pending or actively executing items, while the
+batch stores its own interruption failure separately. Test a preparation
+failure followed by interruption and verify both item and batch diagnostics
+survive inspection and restart.
+
+## 6. Sanitized CIF stems can collide
+
+**Severity:** Medium. **Status:** Deferred.
+
+gRASPA and pygRASPA replace dots with underscores when constructing batch
+directories and copied CIF names. Distinct inputs such as `a.b.cif` and
+`a_b.cif` therefore map to the same output tree and overwrite one another.
+
+**Impact:** Batch manifests can point multiple structures at one simulation,
+silently attributing inputs or results to the wrong framework.
+
+**Recommended remediation:** Assign each input a collision-resistant identity,
+such as a readable sanitized stem plus a stable short hash of the original
+name/path, and reject any remaining duplicate destination before writing.
+Preserve the original name in the manifest. Test colliding names in both
+gRASPA and pygRASPA single-condition and sweep setup.
+
+## 7. RASPA3 conversion accepts malformed interaction sections
+
+**Severity:** Medium. **Status:** Deferred.
+
+`parse_raspa2_force_field()` slices at most the declared interaction count but
+silently skips records with fewer than four fields. It does not verify that the
+file contains the declared number of valid interactions before processing the
+remaining section.
+
+**Impact:** Conversion can emit an incomplete RASPA3 force field while
+reporting success, shifting later lines into the wrong logical section or
+omitting atom interactions.
+
+**Recommended remediation:** Require exactly the declared number of interaction
+lines and validate every record's name, type, and numeric parameters with a
+line-specific error. Test truncated sections, malformed records, invalid
+numbers, zero interactions, and a valid file whose parsed count exactly matches
+the declaration.

src/matkit/cli.py

@@ -1998,7 +1998,7 @@ def zeopp_cli():
     "--outdir",
     default=None,
     type=click.Path(),
-    help="Output directory for result files.",
+    help="Parent for a unique Zeo++ result directory.",
 )
 def zeopp_run(
     cif,

src/matkit/types.py

@@ -49,6 +49,7 @@ class ZeoppResult(TypedDict):
     success: bool
     results: dict
     error: Optional[str]
+    output_dir: Optional[str]
 
 
 class PACMOF2Result(TypedDict):

src/matkit/zeopp/zeopp.py

@@ -73,6 +73,20 @@ def _output_path(directory: Path, stem: str, analysis: str) -> Path:
     return next((path for path in candidates if path.is_file()), candidates[0])
 
 
+def _output_inventory(directory: Path) -> dict[str, dict[str, Path]]:
+    """Group recognized Zeo++ outputs by their structure stem."""
+    inventory: dict[str, dict[str, Path]] = {}
+    for analysis, suffixes in _OUTPUT_SUFFIXES.items():
+        for suffix in suffixes:
+            for path in sorted(directory.glob(f"*{suffix}")):
+                if not path.is_file():
+                    continue
+                stem = path.name.removesuffix(suffix)
+                # Suffix order is significant for PSD: prefer .psd_histo.
+                inventory.setdefault(stem, {}).setdefault(analysis, path)
+    return inventory
+
+
 def _find_network_binary(network_path: str | None = None) -> str:
     """Locate the Zeo++ network binary.
 
@@ -368,7 +382,8 @@ def get_output_data(
     """Parse pre-existing Zeo++ output files.
 
     Args:
-        output_path: Path to directory containing Zeo++ output files.
+        output_path: Path to one Zeo++ output file or a directory containing
+            output files for exactly one structure stem.
         analyses: Which analyses to parse. If None, auto-detects from
             available files (.res, .sa, .vol, .psd_histo, .psd, .chan).
 
@@ -377,7 +392,8 @@ def get_output_data(
 
     Raises:
         FileNotFoundError: If output_path does not exist.
-        ValueError: If specified analyses are invalid or parsing fails.
+        ValueError: If specified analyses are invalid, directory outputs have
+            multiple structure stems, or parsing fails.
     """
     outdir = Path(output_path)
     if not outdir.exists():
@@ -403,16 +419,21 @@ def get_output_data(
             results["success"] = True
         return results
 
-    # Directory mode: find output files
-    detect = analyses if analyses is not None else list(VALID_ANALYSES)
+    # Directory mode: all analyses must belong to one structure. Selecting
+    # matches independently can silently combine stale, unrelated results.
+    inventory = _output_inventory(outdir)
+    if len(inventory) > 1:
+        stems = ", ".join(sorted(inventory))
+        raise ValueError(
+            "Multiple Zeo++ structure stems found in output directory: "
+            f"{stems}. Use a single-structure directory or a direct file path."
+        )
+
+    available = next(iter(inventory.values()), {})
+    detect = analyses if analyses is not None else sorted(available)
     for analysis in detect:
-        matches = [
-            path
-            for suffix in _OUTPUT_SUFFIXES[analysis]
-            for path in sorted(outdir.glob(f"*{suffix}"))
-        ]
-        if matches:
-            results[analysis] = _parse_output(matches[0], analysis)
+        if analysis in available:
+            results[analysis] = _parse_output(available[analysis], analysis)
         elif analyses is not None:
             raise ValueError(f"Missing requested Zeo++ analysis: {analysis}")
 
@@ -450,12 +471,12 @@ def run_zeopp(
             Passed via -r flag. Defaults to the bundled
             UFF.rad if None.
         network_path: Explicit path to the network binary.
-        output_dir: Directory for output files. Uses a temp directory
-            if None.
+        output_dir: Parent directory for a unique, persistent run directory.
+            Uses an automatically deleted temporary directory if None.
 
     Returns:
-        Dict with 'success', 'results' (per-analysis sub-dicts), and
-        'error' keys.
+        Dict with 'success', 'results' (per-analysis sub-dicts), 'error',
+        and 'output_dir'. The output directory is None for temporary runs.
 
     Raises:
         FileNotFoundError: If the CIF file, radii file, or network
@@ -488,8 +509,11 @@ def run_zeopp(
     if use_temp:
         workdir = Path(tempfile.mkdtemp(prefix="zeopp_"))
     else:
-        workdir = Path(output_dir)
-        workdir.mkdir(parents=True, exist_ok=True)
+        output_parent = Path(output_dir).expanduser().resolve()
+        output_parent.mkdir(parents=True, exist_ok=True)
+        workdir = Path(
+            tempfile.mkdtemp(prefix=f"{cifpath.stem}-zeopp-", dir=output_parent)
+        )
 
     try:
         # Copy CIF to working directory
@@ -521,7 +545,12 @@ def run_zeopp(
             )
 
         # Parse output files
-        result = {"success": False, "results": {}, "error": None}
+        result = {
+            "success": False,
+            "results": {},
+            "error": None,
+            "output_dir": None if use_temp else str(workdir),
+        }
         stem = cif_dest.stem
         for analysis in analyses:
             out_file = _output_path(workdir, stem, analysis)
@@ -578,7 +607,8 @@ def run_batch(
 
     Args:
         cif_dir: Directory containing CIF files.
-        output_dir: Directory for output files and results.jsonl.
+        output_dir: Parent directory for unique per-structure run directories
+            and results.jsonl.
         analyses: Analysis types to run. Defaults to ['res'].
         probe_radius: Probe molecule radius in Angstrom.
         chan_radius: Channel radius in Angstrom.
@@ -628,6 +658,7 @@ def _process_one(cif_file: Path) -> dict:
             record = {
                 "structure": stem,
                 "status": "success",
+                "output_dir": result["output_dir"],
             }
             record.update(_flatten_results(result["results"]))
             return record