review-graph: reduce orchestration overhead and handle blocked validation reliably #72

Reviewing a modest staged change with $repo-review spent substantial effort on orchestration and repeated context loading, then could not finish its proof because of runtime contract defects. Please optimize the workflow and make predictable blocked validation cheap and reliable.

Observed during research-repo-tools #29, using the dotfiles skills at 7acf42f2b01ea6d1ec39eb45fda832fed35109d1. This is a workflow issue, not a request to weaken review coverage or evidence integrity.

Agreed scope and merge boundary

This issue covers the correctness/recovery fixes and a repeatable protocol-overhead baseline. Implementation is split between merged #76 and pending #80. Keep #72 open until #80 merges.

The remaining live routing measurements are assigned to #73. A full before/after review-quality comparison is deferred to #74 if proposed adoption changes dispatched review work. This explicitly revises the original live-benchmark acceptance scope; those measurements have not been performed and are not claimed as completed.

The protocol benchmark preserves required catalog routing and the conclusion-blind independent dispatch, projects a four-slot schedule, and checks exact payload binding and seeded finding preservation. It does not execute the independent review, validators, or syntheses, establish semantic recall, or measure model review time.

Measured shape of this run

• 18 changed paths, all staged and unchanged throughout this review.
• Initial plan: 14 nodes. Adding the permitted just check requirement expanded it to 15: nine specialist audits, one independent review, three validators, and two synthesis nodes.
• Four Python audits each owned the same eight changed Python files. The two documentation audits owned nine and eight documents, while the tooling audit owned 11 paths including those documents. Different review lenses are justified, but workers repeatedly loaded the same source and neighboring contracts.
• The original and expanded materializations produced 25 worker-input JSON files totaling 835,416 bytes before synthesis ran. This is serialized wrapper size, not measured model tokens; it includes superseded and retained materializations. Wrappers repeat large schemas, command policy, and persistence instructions.
• All ten audit/independent nodes compiled successfully. The review found four distinct P2 issues, with several independently corroborated. One routine validator failed before any check could run; two blocked validators encountered framework defects. Both synthesis nodes remained unstarted.
• Final runtime result: graph_proof_status=incomplete, repository_validation_status=failed, repository_readiness=blocked despite unchanged source and completed substantive audits.

Sources of avoidable work

1. Repeated source/context loading. The shared-read-only inspection profile did not prevent broad repeat reads across heavily overlapping ownership sets. Reuse a bounded, fingerprinted structural source packet and explicit contract excerpts while keeping semantic judgments independent. Keep the dedicated independent reviewer conclusion-blind.
2. Verbose per-worker contracts and protocol work. Fresh workers repeatedly read large wrappers, discover the same schema constraints, serialize a payload, review it, persist it, and return the complete bytes again. Provide a concise execution view plus lazy schema access, and a supported helper that handles the immutable review/persistence transaction. Preserve exact payload binding and approval receipts.
3. Late command-policy and environment discovery. The baseline just ci plan conflicted with repository Git policy, including Git mutations in disposable fixtures; the new directly invoked unittest consumer also bypasses pytest exclusions. A safe just check was added later, requiring plan expansion/rematerialization. Its inherited uv cache was sandbox-inaccessible, so it exited in 0.102609709s before checks began. Preflight recipe policy, executor/cache access, native-matrix availability, and expected outputs before launching the audit fan-out. Recovery from the cache launch failure is already tracked by review-graph: support recovery from validator launch failures #69.
4. Manual coordinator churn. This run also had coordinator mistakes: reusing the original next-ready output directory after expansion caused an immutable generation filename collision, and prose was supplied where expected_workspace_effects is interpreted as concrete paths. These are not all inherent specialist-review costs. Return one explicit continuation configuration and validate planning fields early so the coordinator does not need custom path plumbing or repeated contract reads.
5. Poor cost visibility. Expose projected node count, overlap, wrapper bytes, concurrency waves, source-read volume, and actual review-versus-orchestration time. Preserve this telemetry in the manifest so optimizations can be measured rather than inferred from elapsed conversation time.

Two runtime defects observed while recording legitimate blockers

A. Commandless blocked validation generates an invalid payload contract

A required hosted-matrix unit had commands: [], working_directories: [], no canonical recipe, and an explicit planning blocker because staged bytes have no corresponding hosted commit.

The planner creates coverage with (unit.canonical_recipe or " + ".join(unit.commands),), yielding [""]. Materialization copies that into owned_paths. The bound review-worker-payload-write then rejects $.owned_paths[0] with minLength >= 1, before any payload write. A truthful blocked payload with empty executions cannot be published.

Relevant locations: review_graph_plan.py around line 1979 and review_graph_runtime.py materialization around lines 3418/3492. Model commandless blocked requirements explicitly; do not generate invalid fake paths.

B. An absent allowed artifact makes accurate blocked validation uncompilable

The policy-blocked just ci unit had approved ignored build/cache output roots, including target, which did not exist. Before/after snapshots correctly recorded it as absent; no validation command ran.

_validation_artifacts_body skips approved paths absent from the trusted after-snapshot. _validation_artifact_blockers nevertheless requires the rendered artifact paths to equal every path in allowed_artifacts, rejecting compilation with:

native validation result artifact paths do not match its dispatch

This appears applicable beyond blocked commands whenever an allowed output is not actually produced. Distinguish permitted outputs from required outputs and consistently represent verified absence. Do not require creating unused directories or editing dispatch-bound evidence to make a report acceptable.

Acceptance criteria (revised scope)

• Add a repeatable protocol benchmark for an approximately 18-file Python/tooling/docs change, preserving required catalog coverage and the conclusion-blind independent dispatch, with a four-slot concurrency projection.
• Report before/after serialized context bytes, scripted direct and packet reads, coordinator operations, and measured protocol durations. Demonstrate reduced protocol overhead while preserving exact payload binding, approval receipts, and seeded findings. Clearly distinguish projections and scripted observations from live worker measurements; leave unobserved worker reads and model review time unknown. Do not infer review quality or a reliable wall-clock speedup from this evidence.
• Preflight policy-blocked, commandless hosted, missing optional artifact, inaccessible cache, and malformed workspace-effect cases before expensive fan-out where possible.
• Add regressions for both runtime defects above and for safe continuation after a plan expansion without next-ready generation collisions.
• Keep failed/blocked attempts and their reasons immutable. Produce an accurate partial review/resume report without pretending validation passed or discarding accepted audits.
• Keep framework failures distinct from repository findings and from coordinator configuration errors.

Deferred evaluation and evidence

• review-graph: evaluate TypeSafe/Jev routing in shadow mode before integration #73 owns live routing-cost instrumentation and the comparison of normal coordinator routing with Jev shadow suggestions on the same captured cases. It must distinguish measured observations from review-graph: reduce orchestration overhead and handle blocked validation reliably #72's protocol baseline and preserve the unchanged normal graph.
• review-graph: adopt TypeSafe routing only after the shadow evaluation passes #74 owns a full before/after review-quality comparison if adoption would change dispatched work: execute required specialist and conclusion-blind independent reviews, compare adjudicated findings, and retain observed worker/orchestration costs and validation/continuation evidence. If review-graph: evaluate TypeSafe/Jev routing in shadow mode before integration #73 produces a no-go decision, this adoption-specific comparison is unnecessary.
• review-graph: reevaluate future Jev revisions before reconsidering adoption #75 remains the later model/question upgrade evaluation; no additional work is added there.
• Baseline method, measured values, and limitations: dispatch overhead documentation in the proposed implementation. Scripted finding preservation is publication evidence, not proof of semantic recall.

Related: #69 covers bounded launch-failure recovery; #68 covers publication/compiler coverage-partition parity. This issue covers the broader efficiency path and the two additional blocker-contract defects.

Local evidence retained by the reporting session: /private/tmp/research-repo-tools-review-29.jNgfxC/, including expanded-result.json, materialized wrappers, validator snapshots, journal, and final-proof.json. That temporary local directory is supporting evidence, not a portable attachment; the reproduction shapes and errors above are self-contained.