fix(review): daily-review no-op when there are no ingest artifacts

.agent-plan.md

@@ -6,10 +6,13 @@
 
 ## Mainline Status
 
-- Last merged PR on main: `#224` (`FIX-CI-GITLEAKS-STATE-JOBS`) — install gitleaks in the
-  `setup-denbust-state-job` composite action so the fail-closed `scripts/state-run.sh` secret scan
+- Last merged PR on main: `#225` (`FIX-DAILY-REVIEW-NOOP`) — make `news-items-daily-review` a clean
+  no-op (return 0) when there are no `daily-state-run` ingest artifacts to review, instead of failing
+  the workflow; surfaced by the go-live dispatch (ingest is local-only/gated on #213, so the state
+  repo has no `news_items/ingest`). Prior: `#224` (`FIX-CI-GITLEAKS-STATE-JOBS`) — install gitleaks in
+  the `setup-denbust-state-job` composite action so the fail-closed `scripts/state-run.sh` secret scan
   has its binary on the runner; without it every scheduled state-writing workflow would have failed
-  at persist on its first scheduled run. The last `UNIFY-PR-06` prerequisite. Prior: `#223`
+  at persist on its first scheduled run. Earlier: `#223`
   (`FIX-DISCOVERY-RUNS-SCHEMA`; refs #150/#152/#156/#184/#186, closed on deploy) — add the missing
   `discovery_runs.skipped_query_count` column whose absence was failing every Supabase run-metadata
   write and blocking candidate persistence on daily-review runs; a guard test now asserts every
@@ -371,6 +374,16 @@
   first scheduled run. Pinned to v8.30.1 to match `.gitleaks.toml`. (`state-repo-squash` uses
   `state-squash.sh`, which does not scan, so it is unaffected.) Prerequisite for the `UNIFY-PR-06`
   go-live dispatch.
+- [done] `FIX-DAILY-REVIEW-NOOP` (#225): make `news-items-daily-review` a clean no-op when there are no
+  `daily-state-run` ingest artifacts to review, instead of failing the workflow. Surfaced by the
+  go-live dispatch: daily-review reviews ingest output, but ingest is local/dispatch-only (gated on
+  #213) and was not seeded, so the state repo has no `news_items/ingest` — a scheduled review with
+  nothing to review should not be a red run. The finder now raises a **dedicated**
+  `NoDailyReviewArtifacts` for the benign empty case (caught narrowly by the entrypoint, which emits
+  a `::warning::` annotation and returns 0 without constructing the Anthropic/GitHub clients), while a
+  genuinely missing `state_root` (broken checkout / misconfig) still raises `FileNotFoundError` so it
+  surfaces loudly rather than being swallowed. Tested: benign no-op makes zero client calls; missing
+  `state_root` propagates.
 - [next] `UNIFY-PR-06` (operational, go-live): the state repo is **seeded** (key-scrubbed after the
   incident below) with the recovered core state from local `data/news_items` (27,568 candidates +
   queues/attempts/verdicts/budget/yield + backfill_batches/runs/metrics; excluded: prefilter models

src/denbust/news_items/daily_review.py

@@ -168,6 +168,16 @@ def _render_source_diagnostics_section(source_diagnostics: dict[str, Any] | None
     )
 
 
+class NoDailyReviewArtifacts(Exception):
+    """No matching daily-ingest run exists yet to review.
+
+    This is a *benign* condition (e.g. ingest runs locally/on-demand and has not
+    produced state), distinct from a genuine filesystem/configuration failure
+    such as a missing ``state_root`` — which keeps raising ``FileNotFoundError``
+    so it surfaces loudly instead of being silently swallowed.
+    """
+
+
 def latest_daily_review_artifacts(
     *,
     state_root: Path,
@@ -176,7 +186,19 @@ def latest_daily_review_artifacts(
     workflow_name: str = "daily-state-run",
     source_diagnostics_path: Path | None = None,
 ) -> ReviewArtifacts:
-    """Find the latest daily-ingest artifacts from the state repo."""
+    """Find the latest daily-ingest artifacts from the state repo.
+
+    Raises:
+        FileNotFoundError: if ``state_root`` itself is missing or not a directory
+            (a broken state checkout / misconfigured ``DENBUST_STATE_ROOT`` — a
+            real failure that must not be mistaken for "nothing to review").
+        NoDailyReviewArtifacts: if the state tree is present but holds no
+            complete run matching ``workflow_name`` (the benign empty case).
+    """
+    if not state_root.is_dir():
+        raise FileNotFoundError(
+            f"state_root {str(state_root)!r} does not exist or is not a directory"
+        )
     runs_dir = state_root / dataset_name / job_name / "runs"
     logs_dir = state_root / dataset_name / job_name / "logs"
     candidates = sorted(logs_dir.glob("*.summary.json"), reverse=True)
@@ -204,7 +226,7 @@ def latest_daily_review_artifacts(
             debug_log=_load_json(debug_log_path),
         )
 
-    raise FileNotFoundError(
+    raise NoDailyReviewArtifacts(
         f"No complete {dataset_name}/{job_name} artifacts found for workflow '{workflow_name}'"
     )
 
@@ -374,6 +396,15 @@ def create_issue(self, candidate: IssueCandidate, artifacts: ReviewArtifacts) ->
         response.raise_for_status()
 
 
+def _emit_review_notice(message: str) -> None:
+    """Surface a non-fatal review notice. Under GitHub Actions, also emit a
+    ``::warning::`` annotation so a no-op run is visible at a glance rather than
+    buried in stdout."""
+    print(message)
+    if os.getenv("GITHUB_ACTIONS") == "true":
+        print(f"::warning title=daily-review::{message}")
+
+
 def review_latest_daily_run(
     *,
     state_root: Path,
@@ -385,12 +416,27 @@ def review_latest_daily_run(
     labels: list[str] | None = None,
     source_diagnostics_path: Path | None = None,
 ) -> int:
-    """Review the latest daily run and create any missing issues."""
-    artifacts = latest_daily_review_artifacts(
-        state_root=state_root,
-        workflow_name=workflow_name,
-        source_diagnostics_path=source_diagnostics_path,
-    )
+    """Review the latest daily run and create any missing issues.
+
+    Returns the number of issues created. When no matching ``daily-state-run``
+    ingest artifacts exist yet (e.g. ingest runs locally/on-demand and has not
+    produced state, so there is nothing to review), this is a clean no-op that
+    returns 0 rather than failing the workflow — a scheduled review with nothing
+    to review should not surface as a red run.
+    """
+    try:
+        artifacts = latest_daily_review_artifacts(
+            state_root=state_root,
+            workflow_name=workflow_name,
+            source_diagnostics_path=source_diagnostics_path,
+        )
+    except NoDailyReviewArtifacts as exc:
+        # Benign: nothing to review yet. Make it observable (not a bare stdout
+        # line) so a scheduled no-op is distinguishable from a healthy review,
+        # but do not fail the workflow. A genuinely missing state_root raises
+        # FileNotFoundError instead and is deliberately NOT caught here.
+        _emit_review_notice(f"No daily ingest artifacts to review: {exc}")
+        return 0
     reviewer = AnthropicDailyReviewer(api_key=anthropic_api_key, model=model)
     review = reviewer.review(artifacts)
     if not review.issues:

tests/unit/test_daily_review.py

@@ -13,6 +13,7 @@
     AnthropicDailyReviewer,
     GitHubIssueClient,
     IssueCandidate,
+    NoDailyReviewArtifacts,
     ReviewArtifacts,
     ReviewResult,
     _compact_for_prompt,
@@ -182,10 +183,11 @@ def test_load_json_rejects_non_object_payload(self, tmp_path: Path) -> None:
         with pytest.raises(ValueError, match="Expected JSON object"):
             _load_json(path)
 
-    def test_latest_daily_review_artifacts_raises_when_daily_artifacts_missing(
+    def test_latest_daily_review_artifacts_raises_no_artifacts_when_incomplete(
         self, tmp_path: Path
     ) -> None:
-        """Missing or incomplete daily artifacts should fail clearly."""
+        """A present state tree with no complete matching run raises the benign,
+        narrow NoDailyReviewArtifacts — not a bare FileNotFoundError."""
         logs_dir = tmp_path / "news_items" / "ingest" / "logs"
         _write_json(
             logs_dir / "2026-03-21T04-00-00-000000Z.summary.json",
@@ -195,9 +197,54 @@ def test_latest_daily_review_artifacts_raises_when_daily_artifacts_missing(
             },
         )
 
-        with pytest.raises(FileNotFoundError, match="No complete news_items/ingest artifacts"):
+        with pytest.raises(NoDailyReviewArtifacts, match="No complete news_items/ingest artifacts"):
             latest_daily_review_artifacts(state_root=tmp_path)
 
+    def test_latest_daily_review_artifacts_raises_filenotfound_when_state_root_missing(
+        self, tmp_path: Path
+    ) -> None:
+        """A missing state_root (broken checkout / misconfig) is NOT the benign
+        case — it raises FileNotFoundError so it surfaces loudly, and is not a
+        subclass of NoDailyReviewArtifacts."""
+        missing = tmp_path / "does-not-exist"
+        with pytest.raises(FileNotFoundError) as excinfo:
+            latest_daily_review_artifacts(state_root=missing)
+        assert not isinstance(excinfo.value, NoDailyReviewArtifacts)
+
+    def test_review_latest_daily_run_noop_makes_no_client_calls(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """The empty case returns 0 and never constructs the Anthropic or GitHub
+        clients (the no-network guarantee), so a scheduled run cannot fail."""
+
+        def _boom(*_args: object, **_kwargs: object) -> None:
+            raise AssertionError("no client should be constructed on the empty path")
+
+        monkeypatch.setattr("denbust.news_items.daily_review.AnthropicDailyReviewer", _boom)
+        monkeypatch.setattr("denbust.news_items.daily_review.GitHubIssueClient", _boom)
+
+        ingest = tmp_path / "news_items" / "ingest" / "logs"
+        ingest.mkdir(parents=True)  # state tree exists, but holds no runs
+
+        created = review_latest_daily_run(
+            state_root=tmp_path,
+            repository="DataHackIL/tfht_enforce_idx",
+            anthropic_api_key="unused",
+            github_token="unused",
+        )
+        assert created == 0
+
+    def test_review_latest_daily_run_propagates_missing_state_root(self, tmp_path: Path) -> None:
+        """A missing state_root must NOT be silently no-op'd — it propagates so a
+        broken state checkout fails the workflow loudly."""
+        with pytest.raises(FileNotFoundError):
+            review_latest_daily_run(
+                state_root=tmp_path / "nope",
+                repository="DataHackIL/tfht_enforce_idx",
+                anthropic_api_key="unused",
+                github_token="unused",
+            )
+
     def test_compact_for_prompt_truncates_large_lists_and_strings(self) -> None:
         """Large debug payloads should be compacted before prompting."""
         compact = _compact_for_prompt(