fix(goal_curation): progress-evidence gate fails closed on semantic verdict parse-miss (#2569)

The progress-evidence gate previously FAILED OPEN (Accept) whenever the
LLM/recipe reviewer produced no recognizable accept/reject verdict — even on a
successful, non-empty run — letting hallucinated "0%->100% with no verdict"
bumps land as false "done" states.

Adopt the merge judge's infra-vs-semantic split across both live tiers
(RecipeProgressChecker primary + LlmReviewerProgressChecker direct-LLM fallback):
- INFRA failure (transport error / spawn failure / non-zero exit / output that
  strips to empty) -> keep fail-OPEN, so goals aren't blocked on infra hiccups.
- SEMANTIC parse-miss (successful, non-empty response with no verdict, or an
  unknown verdict string) -> fail-CLOSED (Reject). Reject only keeps the prior
  percent + logs a hallucination alert; it does not stall the goal.

Also parse the structured {"verdict": ...} JSON first in the recipe tier (was a
naive substring scan that wrongly Rejected an `accept` whose rationale mentioned
"reject", and could Accept "unacceptable"), reusing the direct-LLM tier's
tolerant parse_reviewer_response — matching the merge judge.

The merge path reported in #2569 was already fail-closed on main
(#2486/#2490/#2504, merged before #2569 was filed against released v0.22.0);
this adds a regression test pinning the reporter's exact SUCCESS-with-no-verdict
banners (30s AND 102s) -> Verdict::Unclear, and fixes the analogous fail-open in
the progress path.

Docs (progress-evidence-api, text-parsing-wire-formats §2a,
progress-evidence-gating, text-based-brain-protocol) updated to the new policy
and corrected to reflect that progress_reviewer.rs is the live direct-LLM
fallback tier (not deleted). Adds a gadugi outside-in scenario plus
parser/decision regression tests.

Closes #2569

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: rysweet

docs/concepts/progress-evidence-gating.md

@@ -72,10 +72,15 @@ The reviewer **rejects** when the claimed percent looks hallucinated:
 - A 100% claim with no shipped artifact in the plan or WIP.
 - A claim that contradicts the plan (e.g. "blocked on review" but claims 90%).
 
-On **LLM infrastructure failure** (transport error, parse error, empty
-response), the reviewer **accepts** with a diagnostic rationale. The gate's
+On a **genuine infrastructure gap** (LLM transport error, or an *empty*
+response), the reviewer **accepts** with a diagnostic rationale — the gate's
 purpose is to catch hallucinated jumps, not to block goals on LLM
-availability.
+availability. But a **semantic parse-miss** — a *successful, non-empty*
+response that carries no recognizable `accept`/`reject` verdict (unparseable
+output, or an unknown verdict string) — is treated as evidence-absence and
+**rejected**, so a hallucinated "no verdict" bump cannot land. (`Reject`
+keeps the prior percent and logs a hallucination alert; it does not stall the
+goal.)
 
 > **History:** Prior to PR #2007/#2011, evidence was gathered via `git log`
 > and `gh pr list` shellouts (the `DefaultProgressEvidenceChecker`). Per user
@@ -91,9 +96,11 @@ time the brain makes a progress claim. An LLM reviewer can assess whether a
 claimed delta is *proportional to the described plan* — a judgment call that
 a fixed-rule state machine cannot make.
 
-The trade-off is that the reviewer depends on LLM availability. The fail-open
-design means an LLM outage degrades to pre-#1967 behavior (all claims
-accepted) rather than blocking all goals.
+The trade-off is that the reviewer depends on LLM availability. The
+fail-open-on-**infra** design means an LLM outage degrades to pre-#1967
+behavior (claims accepted) rather than blocking all goals — while a
+successful run that produces no parseable verdict still fails **closed**, so
+the outage tolerance never becomes a blanket "accept everything".
 
 ### What does *not* count as evidence
 

docs/concepts/text-based-brain-protocol.md

@@ -41,7 +41,7 @@ sites in the codebase parsed JSON from LLM or recipe output:
 | `decide.rs` | `DecideJudgment` from LLM response | `find('{')..rfind('}')` extraction — boundary attacks, partial objects |
 | `orient.rs` | `OrientJudgment` from LLM response | Same extraction pattern |
 | `rustyclawd.rs` | `EngineerLifecycleDecision` from LLM response | JSON fallback path after DECISION marker — two parsers, double failure surface |
-| `progress_reviewer.rs` | `ReviewerResponse` from LLM response | Dead code — replaced by recipe_progress_checker |
+| `progress_reviewer.rs` | `ReviewerResponse` from LLM response | Retained as the live direct-LLM fallback tier (used when `recipe-runner-rs` is unavailable); its tolerant `parse_reviewer_response` is now also reused by `recipe_progress_checker.rs` for JSON-first verdict extraction |
 | `merge_judge.rs` | `JudgeOutcome` from LLM response | Dead code — replaced by recipe_merge_judge |
 
 Every one of these was brittle and unnecessary:
@@ -227,9 +227,12 @@ Several modules were deleted or cleaned up as part of the text-migration changes
   `parse_orient_from_text`, `parse_lifecycle_from_text`) rewritten as trivial
   first-word/first-float extractors.
 
-The daemon wiring in `operator_commands_ooda/daemon/mod.rs` was updated to
-match: the `LlmReviewerProgressChecker` fallback arm was removed. The chain
-is now `RecipeProgressChecker` → `NoopProgressEvidenceChecker`.
+The daemon wiring in `operator_commands_ooda/daemon/mod.rs` resolves the
+progress-evidence gate in three tiers: `RecipeProgressChecker` (recipe-runner
+backed, primary) → `LlmReviewerProgressChecker` (direct-LLM fallback, in
+`progress_reviewer.rs`) → `NoopProgressEvidenceChecker`. Both live LLM tiers
+share the same infra-fail-open / semantic-parse-miss-fail-closed verdict policy
+(see [Progress-evidence API](../reference/progress-evidence-api.md)).
 
 ## Why this matters for engineer spawning
 

docs/reference/progress-evidence-api.md

@@ -13,12 +13,14 @@ sibling `simard::goal_curation::operations` module.
 > **History:** Prior to PR #2007, the production implementation was
 > `DefaultProgressEvidenceChecker`, which shelled out to `git log` and
 > `gh pr list`. That struct and its helper traits (`GitRunner`, `GhRunner`)
-> were removed in PR #2011. The gate then delegated to `LlmReviewerProgressChecker`
-> (JSON-based LLM response parsing), which was replaced by
-> `RecipeProgressChecker` with text-based keyword verdict parsing in #1980.
-> The `LlmReviewerProgressChecker` and its module (`progress_reviewer.rs`)
-> have been deleted — they were dead code once the recipe-based checker
-> became the primary tier.
+> were removed in PR #2011. `LlmReviewerProgressChecker`
+> (`progress_reviewer.rs`, JSON-based LLM response parsing) was superseded as
+> the *primary* tier by `RecipeProgressChecker` (text-based keyword verdict
+> parsing) in #1980, but is **retained as the direct-LLM fallback tier**: at
+> daemon boot the gate resolves `RecipeProgressChecker` (recipe-runner-rs
+> backed) first, then `LlmReviewerProgressChecker` (direct LLM), then
+> `NoopProgressEvidenceChecker`. Both live tiers share the same
+> infra-fail-open / semantic-parse-miss-fail-closed policy.
 
 All public symbols below are re-exported from `simard::goal_curation`.
 
@@ -67,9 +69,13 @@ can be installed on `OodaBridges` and shared across all OODA actions.
   a few times per OODA cycle, only on progress-increase attempts.
 - `check` MUST return `Accept` when evidence supports the claim and
   `Reject` otherwise. The production `RecipeProgressChecker`
-  accepts on infrastructure failure (recipe not found, runner not
-  installed, non-zero exit) — the gate's purpose is to catch
-  hallucinated jumps, not to block goals on recipe availability. See
+  splits **infrastructure failure** from a **semantic parse-miss**:
+  it **accepts** on infra failure (recipe not found, runner not
+  installed, non-zero exit, or *empty* output) — the gate's purpose is
+  to catch hallucinated jumps, not to block goals on recipe availability
+  — but it **rejects** on a semantic parse-miss (a *successful,
+  non-empty* run whose output carries no `accept`/`reject` verdict), so
+  a hallucinated "no verdict" bump cannot land. See
   [`SIMARD_PROGRESS_EVIDENCE`](../operations/progress-evidence-kill-switch.md)
   for the operator escape hatch.
 - The `since` argument is provided by the caller; the trait does not
@@ -88,18 +94,22 @@ can be installed on `OodaBridges` and shared across all OODA actions.
 
 The production implementation (`src/goal_curation/recipe_progress_checker.rs`)
 invokes a recipe that runs an LLM agent to review goal progress. The recipe
-stdout is parsed using the keyword verdict protocol — the checker scans for
-`"accept"` or `"reject"` keywords in the text output. See
+stdout is parsed **structured JSON verdict first** (`{"verdict":
+"accept"|"reject", …}`, via the shared `parse_reviewer_response` extractor),
+falling back to an `"accept"`/`"reject"` keyword scan for prose. Parsing the
+JSON object first avoids substring false-positives (e.g. an `accept` verdict
+whose rationale mentions "reject"). See
 [text-parsing wire formats § progress checker](../reference/text-parsing-wire-formats.md#2a-progress-checker-recipe_progress_checkerrs)
 for the full grammar.
 
 | Condition | Result | `reason` template |
 |---|---|---|
-| `new_percent <= old_percent` | `Accept` (auto, no recipe call) | `"progress-assessment-reviewer: downward / no-change (<old> -> <new>) auto-accepted"` |
-| Recipe stdout contains `"accept"` keyword | `Accept` | `"progress-assessment-reviewer: accept — <surrounding text as rationale>"` |
-| Recipe stdout contains `"reject"` keyword | `Reject` | `"progress-assessment-reviewer: reject — <surrounding text as rationale>"` |
-| No keyword found in recipe stdout | `Accept` (fail-open) | `"progress-assessment-reviewer: no verdict keyword found; accepting to avoid blocking goal"` |
-| Recipe invocation failure | `Accept` (fail-open) | `"progress-assessment-reviewer: recipe failed (<error>); accepting to avoid blocking goal"` |
+| `new_percent <= old_percent` | `Accept` (auto, no recipe call) | `"recipe-progress-checker: downward / no-change (<old> -> <new>) auto-accepted"` |
+| Recipe stdout contains `"accept"` keyword | `Accept` | `"recipe-progress-checker: accept — <surrounding text as rationale>"` |
+| Recipe stdout contains `"reject"` keyword | `Reject` | `"recipe-progress-checker: reject — <surrounding text as rationale>"` |
+| No keyword in **non-empty** recipe stdout | `Reject` (fail-closed: semantic parse-miss) | `"recipe-progress-checker: no verdict keyword in non-empty recipe output; rejecting unverified progress"` |
+| **Empty** recipe stdout on a successful run | `Accept` (fail-open: infra gap) | `"recipe-progress-checker: empty recipe output; accepting to avoid blocking goal on infra"` |
+| Recipe invocation failure (spawn / non-zero exit) | `Accept` (fail-open: infra) | `"recipe-progress-checker: recipe … accepting to avoid blocking goal …"` |
 
 The recipe template lives at
 `prompt_assets/simard/recipes/progress-assessment.yaml`. The prompt within
@@ -123,18 +133,23 @@ impl RecipeProgressChecker {
 ```
 
 The production checker. Invokes the progress-assessment recipe via
-`recipe-runner-rs` and parses the verdict from the text output using keyword
-scanning. No JSON parsing. No intermediate `ReviewerResponse` type.
+`recipe-runner-rs` and resolves the verdict **structured JSON first**
+(`{"verdict": …}`, via the shared `parse_reviewer_response` extractor), with an
+`"accept"`/`"reject"` keyword scan as a prose fallback.
 
 The checker:
 
 1. Auto-accepts downward/no-change moves without a recipe call.
 2. Resolves the recipe YAML (hot-reload path, then in-tree).
 3. Invokes `recipe-runner-rs` with goal context as variables.
-4. Scans stdout for `"accept"` or `"reject"` (case-insensitive).
-5. Maps the keyword to `EvidenceDecision`, using surrounding text as rationale.
-6. Fails open on any infrastructure error (recipe not found, runner not
-   installed, non-zero exit).
+4. Parses the structured `{"verdict": …}` JSON first; on a JSON miss, scans
+   stdout for `"accept"` or `"reject"` (case-insensitive).
+5. Maps the verdict to `EvidenceDecision`, using the rationale/surrounding text.
+6. Fails **open** on an infrastructure error (recipe not found, runner not
+   installed, non-zero exit, or empty output) — but fails **closed**
+   (`Reject`) on a semantic parse-miss: a successful, non-empty run whose
+   output carries no `accept`/`reject` verdict, so a hallucinated "no
+   verdict" progress bump is not silently accepted.
 
 ---
 

docs/reference/text-parsing-wire-formats.md

@@ -377,16 +377,35 @@ The parser scans the entire stdout for a verdict keyword. Everything else
 
 ### 2a. Progress checker (`recipe_progress_checker.rs`)
 
-**Keywords:**
+**Resolution order (structured verdict first, then keyword prose):** the
+recipe's prompt contract emits a JSON object
+`{"verdict": "accept"|"reject", "rationale": "..."}`, so the parser tries the
+**structured JSON verdict first** (via the shared
+`progress_reviewer::parse_reviewer_response` extractor — as-is / fenced /
+brace-balanced / outermost), mirroring the merge judge's
+`parse_judge_response`-first path. Parsing the object first avoids substring
+false-positives — e.g. an `accept` verdict whose *rationale* mentions "reject"
+(a naive `contains("reject")` scan would wrongly flip it to Reject), or
+"unacceptable" containing the `accept` substring. A valid JSON object with an
+*unknown* verdict string is a semantic parse-miss → `Reject` (fail-closed).
+Only when no verdict JSON parses does it fall back to the plain keyword scan.
+
+**Keywords (prose fallback):**
 
 | Keyword | Maps to | Priority |
 |---------|---------|----------|
 | `reject` | `EvidenceDecision::Reject` | Checked first |
 | `accept` | `EvidenceDecision::Accept` | Checked second |
 
-**Default (no keyword):** `EvidenceDecision::Accept` — fail-open. The gate's
-purpose is to catch hallucinated jumps, not to block goals on keyword-detection
-availability.
+**Default (no verdict):** splits on whether the run produced output. A
+**successful, non-empty** stdout with no `accept`/`reject` verdict is a
+*semantic parse-miss* → `EvidenceDecision::Reject` (**fail-closed**): the
+reviewer ran but gave no verdict, so the unverified progress bump is refused
+rather than silently accepted. Output that strips to **empty** is treated as an
+*infra gap* → `EvidenceDecision::Accept` (**fail-open**), so a lost-output
+hiccup does not block the goal. Genuine invocation failures (spawn / non-zero
+exit) likewise fail open. The gate's purpose is to catch hallucinated jumps
+without blocking goals on infrastructure availability.
 
 **Example recipe stdout:**
 
@@ -399,23 +418,18 @@ WIP summary references new test files in tests/integration/.
 
 The 8-point increase is proportional to the described work.
 
-accept
+{"verdict": "accept", "rationale": "8pt delta matches the described test work"}
 ```
 
-Parser result: `EvidenceDecision::Accept { reason: "After reviewing the plan and progress claims: ..." }`
-
-**Changes from prior implementation:**
+Parser result: `EvidenceDecision::Accept { reason: "recipe-progress-checker: accept — …" }`
 
-- `parse_reviewer_response` (which parsed JSON `ReviewerResponse`) is removed.
-- `RecipeProgressChecker::check()` now calls `parse_verdict_from_text()`
-  directly and returns `EvidenceDecision` without the intermediate
-  `ReviewerResponse` type.
-- The `progress_reviewer.rs` module (containing `LlmReviewerProgressChecker`)
-  is deleted. It was dead code — the daemon wiring already used
-  `RecipeProgressChecker` as the primary tier.
-- The daemon fallback chain is now: `RecipeProgressChecker` →
-  `NoopProgressEvidenceChecker` (was: `RecipeProgressChecker` →
-  `LlmReviewerProgressChecker` → `NoopProgressEvidenceChecker`).
+**Tier relationship:** `RecipeProgressChecker` is the primary tier;
+`progress_reviewer.rs`'s `LlmReviewerProgressChecker` is the live **direct-LLM
+fallback** tier (used when `recipe-runner-rs` is unavailable). The daemon
+resolution chain is `RecipeProgressChecker` → `LlmReviewerProgressChecker` →
+`NoopProgressEvidenceChecker`. Both live tiers share the same
+infra-fail-open / semantic-parse-miss-fail-closed policy and reuse the same
+`parse_reviewer_response` verdict extractor.
 
 ---