Fix DataFrame truthiness crash in _validate_kinfile; surface pdp3 failures

packages/pride-ppp/src/pride_ppp/__init__.py

@@ -29,7 +29,12 @@
     read_kin_data,
     validate_kin_file,
 )
-from .factories.processor import PrideProcessor, ProcessingMode, ProcessingResult
+from .factories.processor import (
+    MissingProductsError,
+    PrideProcessor,
+    ProcessingMode,
+    ProcessingResult,
+)
 from .factories.rinex import merge_broadcast_files, rinex_get_time_range
 from .specifications.cli import PrideCLIConfig
 from .specifications.config import PRIDEPPPFileConfig, SatelliteProducts
@@ -40,6 +45,7 @@
     "PrideProcessor",
     "ProcessingMode",
     "ProcessingResult",
+    "MissingProductsError",
     # CLI / config
     "PrideCLIConfig",
     "PRIDEPPPFileConfig",

packages/pride-ppp/src/pride_ppp/factories/processor.py

@@ -108,6 +108,20 @@ class ProcessingMode(enum.Enum):
 # ---------------------------------------------------------------------------
 
 
+class MissingProductsError(RuntimeError):
+    """Raised when required GNSS products could not be resolved.
+
+    pdp3 cannot produce a usable solution without the required products,
+    so processing halts instead of launching a run that is guaranteed to
+    fail downstream.
+    """
+
+    def __init__(self, date: datetime.date, missing: list[str]):
+        self.date = date
+        self.missing = missing
+        super().__init__(f"Missing required products for {date}: {missing}")
+
+
 @dataclass(frozen=True)
 class ProcessingResult:
     """Immutable result from a single RINEX → kinematic processing run.
@@ -604,6 +618,15 @@ def _run_pdp3(
                 for line in result.stderr.strip().splitlines():
                     logger.warning(line)
 
+            if result.returncode != 0:
+                stderr_tail = "\n".join(result.stderr.strip().splitlines()[-5:])
+                logger.error(
+                    "pdp3 exited with code %d for site %s: %s",
+                    result.returncode,
+                    site,
+                    stderr_tail or "(no stderr)",
+                )
+
             # pdp3 writes outputs as e.g. "kin_2025254_ncc1" (no extension).
             # Search recursively in the working dir to find them.
             kin_files = list(Path(tmpdir).rglob(f"kin_*_{site.lower()}"))
@@ -621,6 +644,12 @@ def _run_pdp3(
                 shutil.move(str(src), str(dst))
                 kin_out = dst
                 logger.info("Generated kin file %s", dst)
+            else:
+                logger.error(
+                    "pdp3 produced no kin output for site %s (returncode %d)",
+                    site,
+                    result.returncode,
+                )
 
             if res_files:
                 src = res_files[0]
@@ -679,7 +708,7 @@ def _validate_kinfile(self, kin_path: Path, override: bool = False) -> bool:
                 return False
             # Attempt to parse the kinfile — only accept it if it yields data
             kin_df: pd.DataFrame | None = kin_to_kin_position_df(kin_path)
-            if kin_df and not kin_df.empty:
+            if kin_df is not None and not kin_df.empty:
                 return True
         return False
 
@@ -729,6 +758,9 @@ def process(
 
         Raises:
             FileNotFoundError: If *rinex* does not exist.
+            MissingProductsError: If required products could not be
+                resolved (unless a valid cached output short-circuits
+                the run first).
         """
         rinex = Path(rinex)
         if not rinex.exists():
@@ -781,6 +813,7 @@ def process(
         if not resolution.all_required_fulfilled:
             missing = [r.spec for r in resolution.missing if r.required]
             logger.error("Missing required products: %s", missing)
+            raise MissingProductsError(start_date, missing)
 
         # --- 4. Write config --------------------------------------------------
         # Config is persisted at pride_dir/{year}/{doy}/config_file so it can
@@ -850,7 +883,10 @@ def process_batch(
             A ``ProcessingResult`` for each file as it completes.
             Cached results are yielded first, then pdp3 results in
             completion order.  Wrap in ``list()`` if you need all
-            results at once.
+            results at once.  Files whose date is missing required
+            products yield a failed result (``returncode == -1``,
+            ``success == False``) without running pdp3; other dates in
+            the batch are unaffected.
 
         Raises:
             ValueError: If *sites* length does not match *rinex_files*.
@@ -921,6 +957,30 @@ def process_batch(
                 )
                 continue
 
+            # Required products missing for this date: pdp3 would fail
+            # downstream, so yield a failed result instead of dispatching.
+            # Other dates in the batch still process normally.
+            if not resolutions[d].all_required_fulfilled:
+                missing = [r.spec for r in resolutions[d].missing if r.required]
+                logger.error(
+                    "Missing required products for %s on %s: %s — skipping pdp3 run",
+                    site,
+                    d,
+                    missing,
+                )
+                yield ProcessingResult(
+                    rinex_path=rinex,
+                    site=site,
+                    date=d,
+                    kin_path=None,
+                    res_path=None,
+                    config_path=config_paths[d],
+                    resolution=resolutions[d],
+                    returncode=-1,
+                    stderr=f"Missing required products: {missing}",
+                )
+                continue
+
             command = self._build_pdp_command(
                 rinex=rinex,
                 site=site,

packages/pride-ppp/tests/test_processor.py

@@ -7,6 +7,9 @@
 
 from __future__ import annotations
 
+import datetime
+import logging
+import os
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
@@ -20,7 +23,10 @@
 except ImportError as e:
     pytest.skip(f"gnss-product-management not installed: {e}", allow_module_level=True)
 
+from pride_ppp.factories import processor as processor_module
 from pride_ppp.factories.processor import (
+    MissingProductsError,
+    PrideProcessor,
     _resolution_to_satellite_products,
     _resolution_to_table_dir,
 )
@@ -173,3 +179,134 @@ def test_resolution_to_table_dir_with_none_local_path() -> None:
 
     table_dir = _resolution_to_table_dir(resolution)
     assert table_dir is None
+
+
+@pytest.fixture
+def processor() -> PrideProcessor:
+    """A PrideProcessor without running __init__ — _validate_kinfile is self-free."""
+    return object.__new__(PrideProcessor)
+
+
+class TestValidateKinfile:
+    """Regression tests for PrideProcessor._validate_kinfile.
+
+    The original implementation used `if kin_df and not kin_df.empty`, which
+    raises `ValueError: The truth value of a DataFrame is ambiguous` for any
+    kin file that parses into a DataFrame.
+    """
+
+    def test_valid_kinfile_returns_true(self, processor: PrideProcessor, kin_file: Path) -> None:
+        # Raised ValueError before the truthiness fix
+        assert processor._validate_kinfile(kin_file) is True
+
+    def test_missing_path_returns_false(self, processor: PrideProcessor, tmp_path: Path) -> None:
+        assert processor._validate_kinfile(tmp_path / "kin_missing.kin") is False
+
+    def test_override_skips_cache_check(self, processor: PrideProcessor, kin_file: Path) -> None:
+        assert processor._validate_kinfile(kin_file, override=True) is False
+
+    def test_unparseable_kinfile_returns_false(
+        self, processor: PrideProcessor, tmp_path: Path
+    ) -> None:
+        garbage = tmp_path / "kin_2021220_bako.kin"
+        garbage.write_text("not a kin file\nno header here\n")
+        assert processor._validate_kinfile(garbage) is False
+
+
+class TestRunPdp3:
+    """Subprocess handling in _run_pdp3, exercised via a fake pdp3 on PATH."""
+
+    @pytest.fixture
+    def fake_pdp3(self, tmp_path, monkeypatch):
+        """Return a factory that installs a fake pdp3 shell script on PATH."""
+        bin_dir = tmp_path / "bin"
+        bin_dir.mkdir()
+
+        def install(script_body: str) -> None:
+            pdp3 = bin_dir / "pdp3"
+            pdp3.write_text(f"#!/bin/sh\n{script_body}\n")
+            pdp3.chmod(0o755)
+            monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}")
+
+        return install
+
+    def test_outputs_moved_with_extensions(self, fake_pdp3, tmp_path: Path) -> None:
+        fake_pdp3("touch kin_2025254_ncc1 res_2025254_ncc1")
+        out = tmp_path / "out"
+
+        kin, res, rc, _ = PrideProcessor._run_pdp3(command=["pdp3"], site="NCC1", output_dir=out)
+
+        assert rc == 0
+        assert kin == out / "kin_2025254_ncc1.kin" and kin.exists()
+        assert res == out / "res_2025254_ncc1.res" and res.exists()
+
+    def test_nonzero_exit_and_missing_output_are_logged(
+        self, fake_pdp3, tmp_path: Path, caplog
+    ) -> None:
+        fake_pdp3("echo boom >&2; exit 2")
+        out = tmp_path / "out"
+
+        with caplog.at_level(logging.ERROR, logger="pride_ppp.factories.processor"):
+            kin, res, rc, stderr = PrideProcessor._run_pdp3(
+                command=["pdp3"], site="NCC1", output_dir=out
+            )
+
+        assert rc == 2
+        assert kin is None and res is None
+        assert "boom" in stderr
+        assert any("pdp3 exited with code 2" in m for m in caplog.messages)
+        assert any("produced no kin output" in m for m in caplog.messages)
+
+
+def _unfulfilled_resolution() -> DependencyResolution:
+    return DependencyResolution(
+        spec_name="test",
+        resolved=[
+            ResolvedDependency(spec="ORBIT", required=True, status="missing", local_path=None)
+        ],
+    )
+
+
+class TestMissingRequiredProducts:
+    """Processing must not launch pdp3 when required products are missing."""
+
+    def test_process_raises_missing_products_error(self, tmp_path: Path, monkeypatch) -> None:
+        proc = object.__new__(PrideProcessor)
+        proc._output_dir = tmp_path / "out"
+        monkeypatch.setattr(proc, "_resolve", lambda dt: _unfulfilled_resolution())
+
+        rinex = tmp_path / "test.rnx"
+        rinex.write_text("")
+
+        with pytest.raises(MissingProductsError, match="ORBIT"):
+            proc.process(rinex, site="ncc1", date=datetime.date(2025, 9, 11))
+
+    def test_process_batch_yields_failed_result_without_running_pdp3(
+        self, tmp_path: Path, monkeypatch
+    ) -> None:
+        proc = object.__new__(PrideProcessor)
+        proc._output_dir = tmp_path / "out"
+        proc._pride_dir = tmp_path / "pride"
+        monkeypatch.setattr(proc, "_resolve", lambda dt: _unfulfilled_resolution())
+        # Keep the orchestration test hermetic: no real RINEX headers, no
+        # installed PRIDE config template, and pdp3 must never be invoked.
+        start = datetime.datetime(2025, 9, 11, tzinfo=datetime.timezone.utc)
+        monkeypatch.setattr(processor_module, "rinex_get_time_range", lambda p: (start, start))
+        monkeypatch.setattr(processor_module, "_write_config", lambda sp, td, dest: dest)
+
+        def _fail(*args, **kwargs):
+            raise AssertionError("pdp3 must not run for unfulfilled dates")
+
+        monkeypatch.setattr(proc, "_run_pdp3", _fail)
+
+        rinex = tmp_path / "test.rnx"
+        rinex.write_text("")
+
+        results = list(proc.process_batch([rinex], sites=["ncc1"]))
+
+        assert len(results) == 1
+        result = results[0]
+        assert result.returncode == -1
+        assert result.success is False
+        assert "Missing required products" in result.stderr
+        assert "ORBIT" in result.stderr