Skip to content

Build projects the record; plan review runs iff its verdict routes#54

Open
czpython wants to merge 3 commits into
mainfrom
commonzenpython/eng-734-build-project-the-record-plan-review-runs-iff-its-verdict
Open

Build projects the record; plan review runs iff its verdict routes#54
czpython wants to merge 3 commits into
mainfrom
commonzenpython/eng-734-build-project-the-record-plan-review-runs-iff-its-verdict

Conversation

@czpython

@czpython czpython commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Lands on #53's run record. Two halves, per the ticket.

The journal loses its mutation half. BuildJournal is now a frozen dataclass over self.record — pure typed projections, nothing stored, nothing to rebuild: plan = latest PlanData, implementations = list(ImplementationOutput, status="success") (a bailed delivery stays on the record but never counts), evaluations, and the assignee scan over recorded plans. PlanRecord, every add_*, the _reviews_at_plan windowing, and reviewer_requirements() are gone; body call sites read as plain agent awaits.

HumanFeedback died with it: it was a composed bag duplicating TriageOutput fields (body stored twice) with a status field that never left "pending". The journal now pairs each request_changes reply with the triage that digested it, positionally into plain dicts (zip strict=False — the newest reply is undigested while its own triage agent renders the projection). ReviewWork moved to contracts.py so the journal can select by its type without a cycle; on_wait is unchanged.

Plan review runs iff its verdict routes. _plan_phase computes plan_approval_gate(auto_dispatch_on_plan_approval) once. Gate mode is generate → park — the operator is the reviewer and their request_changes note is the critique channel; the machine reviewer never runs. Auto mode: generate → review → APPROVE → implement, or REQUEST_CHANGES → exactly one redraft (reviewer_notes=grade.body; the bound is a flag, so an empty critique can't loop) → re-review → park on the second rejection. The operator taking over resets the notes and re-arms a single fresh redraft. Questions always park, in either mode. ReviewOutput is binary (APPROVE | REQUEST_CHANGES + body), AWRC is deleted from the enum, and assignee_github_login moved to the planner's contract (PlanOutputPlanData; revisions carry None and the journal scan falls through) — the resolution prompt section moved from review_plan.md to generate_plan.md with it.

How the critique reaches the operator at the fallback park (the ticket left this open): as ask context. review() gains an optional context= that lands in the parked input_request; get_ask already spreads gate-declared fields, so the plan stays the ask's resolved document and the build page renders the critique beside it. ReviewOutput deliberately records no artifact — an artifact would displace the plan as the latest and hide the very thing the operator must judge.

Templates: generate_plan.md gains the top-level reviewer_notes block (StrictUndefined — the one call site always passes it, empty on plain paths); review_plan.md is rewritten for the binary verdict and the fold-once loop ("the plan will NOT be re-generated" is gone); the "Reviewer requirements (active)" section is deleted from _header.md; implement.md, evaluate_implementation.md (six sweeps now, plan-arrives-complete framing), and review_code.md drop every requirements mention. grep -ri "reviewer.requirement|APPROVE_WITH|AWRC" over templates/prompts is zero-hit.

Tests: the plan-phase suite drives both modes (reviewer-never-runs, fold-once, exhaustion + re-arm with the context assertion, questions-park, cancel); new test_build_journal.py covers the projections and the pairing; the durable stub now returns real contracts so the record's typed projections are exercised end-to-end, and the recovery test crashes twice — once at the plan park, once at the work park with an implementation on the record — asserting zero agent re-invocations across both replays; prompts tests cover the critique block and the assignee-section move.

Deploy note: this changes workflow structure. In-flight build runs parked at a question-free plan review recorded review_plan before their park; the new body skips it in gate mode, so those runs (and runs whose replay passes through that path) will not replay cleanly under the new image. Same treatment as the extension-identity deploy: drain or cancel in-flight build runs in the window.

codex-reviewed: the adversarial pass caught the artifact-displacement problem at the fallback park (fixed with the ask-context channel above), flagged the deployment cutover (the drain note), and called the durable stub's lookalikes too weak to catch a lost-projection regression (now real contracts + the second crash leg).

ENG-734

@czpython
czpython force-pushed the commonzenpython/eng-733-sdk-the-run-record-platform-recorded-typed-history-with branch 2 times, most recently from 6824c2d to 17f0477 Compare July 22, 2026 06:01
Base automatically changed from commonzenpython/eng-733-sdk-the-run-record-platform-recorded-typed-history-with to main July 22, 2026 06:01
@czpython
czpython force-pushed the commonzenpython/eng-734-build-project-the-record-plan-review-runs-iff-its-verdict branch from 0b6a106 to 3de3203 Compare July 22, 2026 06:06
@czpython
czpython marked this pull request as ready for review July 22, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant