Review rounds produce findings no step acts on, and round 2 has never found a blocking issue #126
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Split off from #121 (finding 4 of the scratch-run analysis,
docs/design/measurements/2026-10-02-scratch-run-efficiency.md).Nine review rounds in the scratch feature runs produced 87 findings: 6 blocking (all in round 1), 45 should-fix and 36 notes.
openspec.revisefixes blocking findings only, and the gate auto-approves, so no step is asked to handle a should-fix finding. A few surfaced later by chance: run 25's unit-test step picked up the test lens's gaps, and run 37's code review noted one and left it.Round 2 re-runs all four lenses over everything. It ran three times (runs 22, 23 and 25), took 10–13 min each, and never found a blocking issue. The review phase took 13–47 min per run, 12% of the wall clock, and reviewers spend 91% of their output on reasoning.
Ideas:
Related: #114 (how the spine draws the review rounds).
Shipped and live on production (
9ae119c, deployed 2026-10-03). Against the three ideas:1. Round 2 as one targeted check — shipped.
agent.review.r2and.r3are single nodes in place of four lenses. The check is handed the previous round's blocking findings with their ids, the reviser's account of what it changed, and the revision's own diff of the change's files (or a line saying the reviser changed nothing), and writes.osf/review/rN/recheck.json: a verdict for each blocking finding (resolved,unresolvedorwrong, with a sentence) and any new problem the revision itself introduced. It is asked not to read afresh what the revision did not touch. A finding it does not answer stands: the digest drops what was resolved, lists what was called wrong with the check's reason (the gate prints it, so a person can put it back), keeps what is unresolved, and names the blocking findings it gave no verdict on; a verdict outside the three counts as none, and a round in which nothing was written leaves the previous findings standing and ends at the gate. The loop's bounds are as they were, and unattended approval after a check is still judged on round 1's lenses (three of four); the check does not count as a lens. A run admitted before this keeps its four-lens rounds: a round with lens files and no check file is folded as it always was.2. Should-fix findings handed on — shipped to the apply steps and the unit-test step, not to revise. The digest of a later round carries round 1's should-fix findings and notes forward, so the gate and the steps after it still see them. The apply steps are handed the should-fix findings (at most eight, the ones more lenses agreed on first) and the unit-test step those the test lens raised, as notes that do not block, with the instruction not to edit the change's documents for them.
openspec.revisestill acts on blocking findings only: handing it polish would mix required fixes with cosmetics and make the revision harder to check.3. Fewer lenses for a small change — not done; it waits for data. No run had kept what each lens contributed. The digest now records what each reader raised by severity (
by_lens), andoutcomes.jsonsums it asfindings_by_lens. After a few runs that says which lens earns its slot for which size of change.Checked: unit tests for the digest folds (every verdict, no verdict, a verdict outside the three, a new finding, two rounds, the old four-lens fold, a round in which nothing was written), the prompts, the schema and its completion predicate, and the pipeline shape in every built-in pipeline; the fast and full lanes; the whole review section end to end in a local osfd with scripted stand-ins for the agent steps (a clean first round, one revision that resolves, one that does not, two revisions); and on production through the deployed code (the pipeline loads with single check nodes; the check's prompt rendered on a past run's worktree carries the real blocking id, the reviser's account and the revision's diff). To measure in the next scratch run: the REVIEW phase's duration against 10–13 minutes for each later round, and what the check says about each blocking finding (the gate brief,
findings_by_lens).Related: #114 draws either shape of a later round. Change record:
openspec/changes/archive/2026-10-03-targeted-rereview.