Require enumerated candidates on explicit-none ledgers; close the refine-to-plan deferral leak - #3563
Conversation
…ine-to-plan deferral leak (#3526) Backfill of the pipeline-state history confirmed the operator's report: refine-phase substantive decisions fell from ~8 per refine-running pipeline to ~0 within weeks of the #2796 carve-out landing, and plan registrations never rose in compensation (deferral-to-plan candidates simply evaporated; the plan prompt had no registration protocol at all). - no_decisions_rationale now requires candidates_considered: at least one {question, disposition, why} entry per open choice dispositioned away (dispositions: not_operator_grade, deferred_to_plan). Enforced in the shared decision_attestation_errors helper, the Pydantic model, and the propose-time signal validator; the #3462 confirm question renders the enumeration so the operator confirms dispositions, not a paragraph. CLI: repeated --considered "<disposition> :: <question> :: <why>". - Refine deferrals become a handoff: deferred_to_plan candidates are injected into the plan-phase prompts as pre-seeded items the planner must register or explicitly disposition, and plan-phase attestations may not use deferred_to_plan at all (plan is the last decision surface). The plan prompt and concurrent architect prompt gain an operator-decisions protocol section. - The refine carve-out is restated as exhaustive (slicing, strategy, API/schema shape) with an uncertain-cases-register counterweight. - The gate-time ledger summary is persisted on PhaseExecution.decision_ledger so decisions-surfaced-per-phase is queryable from pipeline state. Closes #3526
There was a problem hiding this comment.
Review: PR #3563 — enumerated candidates on explicit-none ledgers + refine→plan deferral handoff
I traced this end-to-end (validation shape, both prompt-builder call sites, the message-scan handoff, gate persistence, and CLI parsing) and independently exercised the core validator. No blocking issues. The change is coherent, the cross-module handoff is correctly wired, and coverage exercises production paths (no self-seeding goldens, no fixture-bypass). Approving with non-blocking suggestions below.
What I verified holds
- Shared shape check is the single source of truth.
decision_attestation_errors(shared/egg_contracts/decisions.py:281) is called by bothDecisionSurfacingAttestation.validate_ledger_shape(orchestrator/attestation_schemas.py:158) and the propose-time_validate_decision_attestation_shape(orchestrator/routes/signals/_validation.py:420), so the two layers can't drift. I ran the pure helper against 10 input combinations — rationale-without-candidates rejected, rationale+candidates accepted, ids-only accepted, ids+candidates accepted, both/neither rejected, candidates-alone rejected (falls through to "neither"), malformed disposition and non-dict entries each rejected. All correct. - Plan-phase
deferred_to_planrejection is reachable._DECISION_ATTESTING_PHASES = {"refine","plan"}and the roles set includesarchitect/task_planner/risk_analyst(_validation.py:366-367), and the plan-specific branch (_validation.py:420-436) runs only when the base shape check already passed, so it never masks a more basic error. - The handoff is not a silent no-op.
pipeline_idis a required param of both_build_phase_prompt(_prompt_phase.py:15) and_build_agent_prompt(_prompt_agent.py:99), and is threaded from the concurrent runner (_run_concurrent.py:217)._find_deferred_plan_candidatesfilters to refine proposals (message.phase != "refine"guard) anddisposition == "deferred_to_plan", iteratingreversed(messages)— consistent with the existing_find_explicit_none_attestationand withget_messagesreturning oldest-first (XRANGE), so it returns the latest refine proposal's deferrals. The plan/architect prompts inject the section (verified bytest_plan_phase_prompt_includes_deferred_section). - All changed-arity call sites updated.
_collect_decision_ledger_status(now 4-tuple),_find_explicit_none_attestation/explicit_none(now 3-tuple), and_extract_attested_decision_fields(now 3-tuple) have no un-updated external callers. The question-dedup key stays byte-identical for the no-candidates case (test_attestation_question_without_candidates_unchanged_shape), so in-flight pre-#3526 pipelines don't orphan a duplicate confirmation decision.
Non-blocking suggestions
-
Deferral closure is prompt-enforced, not gate-enforced. The leak is narrowed (the deferred candidate reliably reaches the plan producer's prompt) but not hard-closed: nothing cross-checks that the plan producer's attestation actually registers or dispositions each refine-deferred question. A plan producer could attest explicit-none with unrelated
candidates_consideredand the specific deferred question X is never addressed — and the operator's confirm question renders the plan's candidates, not the refine-deferred ones, so it wouldn't obviously flag the omission. Given the PR title says "close the leak," consider a follow-up that cross-references refine'sdeferred_to_planquestions against the plan attestation at propose time (NACK if a deferred question is neither registered nor explicitly re-dispositioned). Not blocking because the surfacing half genuinely works. -
decision_ledgersummary schema is non-uniform across branches (_ledger.py:494-535): themissingkey exists only on the missing path andattested_byonly on the explicit-none path. Since the field's stated purpose is a queryable signal over time (models/_execution.py:276), a consumer must defensively handle absent keys. Consider emitting all keys on every branch (e.g."missing": False,"attested_by": None) for a stable shape. -
Write amplification on the gate path.
_persist_decision_ledger_summary(_decision_candidates.py:346) adds aload → mutate → save(git commit on the pipeline-state branch) on every refine/plan gate pass. It's best-effort and correctly non-blocking, but it's a new write on a hot path — fine as-is, just flagging. -
Reviewer criteria text in
_prompt_*/_criteria.pystill describesno_decisions_rationalewithout mentioning the newcandidates_consideredrequirement (_criteria.py:686,695). Reviewers checking a producer's explicit-none attestation may want the updated contract. Minor prompt-completeness gap.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
|
Thanks for the thorough trace. Dispositions for the four non-blocking suggestions: Note on delivery: #3563 sits on a human-owned branch, so the gateway won't let me push onto it. The in-PR fixes below landed on egg-owned branch
— Authored by egg |
|
egg feedback addressed. View run logs 1 previous review(s) hidden. |
Address #3563 review: uniform ledger shape + reviewer candidates rubric
Summary
Investigation and fix for #3526 (HITL decisions declining: agents under-surfacing operator decisions, especially at refine).
Measurement (investigation step 1-2, confirmed)
Backfilled per-pipeline decision counts from the
egg/pipeline-statebranch history (227 pipelines, Feb-Jul 2026), classifying every HITL queue entry (phase gates, consensus timeouts, infra escalations, feedback, substantive agent-raised decisions):Both operator-suspected mechanisms are confirmed:
Fix (issue steps 3-5)
1. Structured explicit-none ledger (#3526).
no_decisions_rationalenow requirescandidates_considered: at least one{question, disposition, why}entry per open choice the producer weighed and dispositioned away. Dispositions:not_operator_grade(design call the planner/implementer owns) ordeferred_to_plan(potentially operator-grade, better asked at plan). A single free-form paragraph was trivially satisfiable; an enumeration is harder to satisfy vacuously, and it is what the operator confirms at the #3462 gate (the confirm question now renders the candidates as bullets). Enforced in the shareddecision_attestation_errorshelper, so the Pydantic model and the propose-time signal validator stay in lockstep. CLI: repeated--considered "<disposition> :: <question> :: <why>".2. Deferral becomes a handoff, not a disappearance. Refine's
deferred_to_plancandidates are injected into the plan-phase prompts (both the single-agent plan prompt and the concurrent architect prompt) as pre-seeded candidates the planner must register or explicitly disposition. A plan-phase attestation may not usedeferred_to_planat all; plan is the last decision surface, so propose-time validation rejects it. The plan prompt also gains its own operator-decisions protocol section (previously absent entirely, which explains plan's historical ~zero registrations).3. Refine carve-out narrowed. The prompt now states the carve-out is exhaustive (slice/PR packaging, implementation strategy, API/schema details) and that "implementation-flavored" is not the test; when genuinely uncertain, register with a recommended option. Dispositioned-away choices must be enumerated as structured candidates, never folded into draft prose alone.
4. Permanent metric. The gate-time ledger summary (registered cq-N ids, explicit-none flag, candidates) is persisted on
PhaseExecution.decision_ledger, so decisions-surfaced-per-phase is queryable from pipeline state and a future decline shows up in data rather than operator feel.Non-goals (per the issue)
No revert of #2796 / #3390 / #3462; the carve-out classes remain out of scope for refine registration; the explicit-none path remains available, just no longer free-form.
Test plan
make lintgreen.make test(changeset-aware) green: new coverage for the shared candidate validator, the propose-time rules (explicit-none without candidates rejected, plan-phasedeferred_to_planrejected, refine deferral accepted), CLI--consideredparsing and local error paths, the candidate-rendering confirm question, the refine-to-plan deferral scan and prompt injection, andPhaseExecution.decision_ledgerpersistence; existing Guarantee HITL decision registration: hard-fail on missing decision ledger + reviewer obligation against un-surfaced decisions #3390/Agents can self-attest zero HITL decisions instead of registering them — meaningful decisions must be registered #3462 suites updated to the new attestation shape.Closes #3526