Address #3563 review: uniform ledger shape + reviewer candidates rubric

orchestrator/routes/pipelines/_criteria.py

@@ -241,6 +241,18 @@ def _get_refine_review_criteria() -> str:
         "`no_decisions_rationale`), verify the rationale holds: requirements "
         "genuinely unambiguous, no assumptions made silently. NACK if you "
         "find a hidden operator-grade choice.\n"
+        "- **Enumerated candidates required (#3526).** An explicit-none "
+        "attestation must carry `candidates_considered`: one "
+        "`{question, disposition, why}` entry per open choice the producer "
+        "weighed and dispositioned away (`not_operator_grade` = a design "
+        "call the planner/implementer owns; `deferred_to_plan` = potentially "
+        "operator-grade, better asked at plan). A bare rationale with no "
+        "candidates, or candidates whose `why` is vacuous / does not match "
+        "the choices the analysis actually faced, is a **NACK** — the "
+        "enumeration is what the operator confirms at the gate, so it must "
+        "be concrete. Check `deferred_to_plan` items especially: they are "
+        "carried into the plan prompt as items the planner must close, so a "
+        "wrongly-deferred operator decision escapes refine's surface.\n"
         "- **Task-named decisions — NACK an explicit-none ledger (#3462).** "
         "If the task description names decisions as the operator's to make "
         "(or directs that decisions be surfaced as HITL questions), each "
@@ -685,6 +697,18 @@ def _get_plan_review_criteria() -> str:
         "- A deliberately empty ledger arrives as a producer's "
         "`no_decisions_rationale` attestation — verify it holds; NACK if "
         "the plan hides an operator-grade choice.\n"
+        "- **Enumerated candidates required (#3526).** An explicit-none "
+        "attestation must carry `candidates_considered`: one "
+        "`{question, disposition, why}` entry per open choice the producer "
+        "weighed. At plan the only valid disposition is `not_operator_grade` "
+        "(a design call plan legitimately owns) with a concrete why — plan "
+        "is the last decision surface, so `deferred_to_plan` is rejected at "
+        "propose time. A bare rationale with no candidates, or a candidate "
+        "whose `why` is vacuous, is a **NACK**. Cross-check against the "
+        "refiner's deferred candidates surfaced in the plan prompt: each "
+        "must be registered as a `cq-N` or dispositioned `not_operator_grade` "
+        "here — a deferred candidate the plan silently drops is the leak "
+        "#3526 exists to close.\n"
         "- **Task-named decisions — NACK an explicit-none ledger (#3462).** "
         "If the task description or refine analysis names decisions as the "
         "operator's to make (or directs that decisions be surfaced as HITL "

orchestrator/routes/pipelines/_ledger.py

@@ -461,6 +461,13 @@ def _collect_decision_ledger_status(
       ``PhaseExecution.decision_ledger`` (#3526): registered ids,
       explicit-none flag, and considered candidates, so
       decisions-surfaced-per-phase is queryable from pipeline state.
+      The key set is **uniform across all three branches** —
+      ``registered``, ``resolved``, ``explicit_none``, ``attested_by``,
+      ``missing``, ``candidates_considered`` are always present (with
+      ``attested_by=None`` / ``missing=False`` on the branches that
+      don't set them) so a downstream consumer never has to guess
+      whether a key is absent because it doesn't apply or because the
+      branch simply forgot it.
     """
     phase_value = phase.value
     registered_ids: list[str] = []
@@ -500,6 +507,8 @@ def _collect_decision_ledger_status(
                 "registered": registered_ids,
                 "resolved": resolved,
                 "explicit_none": False,
+                "attested_by": None,
+                "missing": False,
                 "candidates_considered": [],
             },
         )
@@ -517,6 +526,7 @@ def _collect_decision_ledger_status(
                 "resolved": 0,
                 "explicit_none": True,
                 "attested_by": role,
+                "missing": False,
                 "candidates_considered": candidates,
             },
         )
@@ -532,6 +542,7 @@ def _collect_decision_ledger_status(
             "registered": [],
             "resolved": 0,
             "explicit_none": False,
+            "attested_by": None,
             "missing": True,
             "candidates_considered": [],
         },

orchestrator/tests/test_decision_ledger_gate.py

@@ -113,6 +113,8 @@ def test_registered_decisions_counted(self, tmp_path: Path):
             "registered": ["cq-1", "cq-2"],
             "resolved": 1,
             "explicit_none": False,
+            "attested_by": None,
+            "missing": False,
             "candidates_considered": [],
         }
 
@@ -137,6 +139,16 @@ def test_zero_registered_with_explicit_none_attestation(self, tmp_path: Path):
         assert summary["explicit_none"] is True
         assert summary["registered"] == []
         assert summary["attested_by"] == "refiner"
+        # Uniform shape across branches (#3526 review): every key present.
+        assert summary["missing"] is False
+        assert set(summary) == {
+            "registered",
+            "resolved",
+            "explicit_none",
+            "attested_by",
+            "missing",
+            "candidates_considered",
+        }
 
     def test_zero_registered_no_attestation_is_missing(self, tmp_path: Path):
         from routes.pipelines import _collect_decision_ledger_status
@@ -150,6 +162,16 @@ def test_zero_registered_no_attestation_is_missing(self, tmp_path: Path):
         assert explicit_none is None
         assert "MISSING" in note
         assert summary["missing"] is True
+        # Uniform shape across branches (#3526 review): every key present.
+        assert summary["attested_by"] is None
+        assert set(summary) == {
+            "registered",
+            "resolved",
+            "explicit_none",
+            "attested_by",
+            "missing",
+            "candidates_considered",
+        }
 
     def test_contract_unloadable_falls_back_to_attestation(self, tmp_path: Path):
         from routes.pipelines import _collect_decision_ledger_status

orchestrator/tests/test_pipeline_prompts.py

@@ -4162,12 +4162,22 @@ def test_refine_reviewer_carries_unsurfaced_decision_obligation(self):
         # Calibration guard so the obligation doesn't over-NACK
         # legitimate implementation choices.
         assert "do not over-NACK" in criteria
+        # The explicit-none contract now requires enumerated candidates
+        # (#3526); the reviewer rubric must name it so the reviewer checks
+        # the enumeration, not just the free-form rationale.
+        assert "candidates_considered" in criteria
+        assert "deferred_to_plan" in criteria
 
     def test_plan_reviewer_carries_unsurfaced_decision_obligation(self):
         criteria = _get_plan_review_criteria()
         assert "Un-surfaced decisions — NACK" in criteria
         assert "no_decisions_rationale" in criteria
         assert "do not over-NACK" in criteria
+        # Enumerated-candidate requirement + the plan-is-last-surface rule
+        # (#3526): the reviewer must cross-check the refiner's deferred
+        # candidates against the plan attestation.
+        assert "candidates_considered" in criteria
+        assert "not_operator_grade" in criteria
 
 
 class TestTaskNamedDecisionRegistrationSurface: