Repository navigation
Make plan and code reviews faster and need fewer rounds - #99
Merged
DocksDocks merged 15 commits intoOct 23, 2026
Merged
DocksDocks merged 15 commits into
DocksDocks merged 15 commits into
Conversation
A code-review round will post every lens block in one comment (#98). `parseReviewComment` took only the first verdict line, so a later `repair` or `blocked` block would disappear behind a first `pass`. The helper now reads every verdict line of the comment's kind and takes the most severe: blocked, then repair or fixes-required, then pass. A verdict line of the other kind is prose and does not count; an unknown verdict word still makes the comment unreadable as a review; the severity upgrade for a code pass stays. Red baseline: the two new cases failed on their assertions (a three- block comment read `pass` instead of `blocked`), and the other six cases in reviews.test.mjs passed. Refs #98
5512b45 counted every verdict line of a comment. A prose line such as "- Code review: missing test" then had an unknown verdict word and made the whole comment unreadable, so an older trusted pass could stand; and a delta block that quoted the previous round's "Code-review: fixes-required" read as fixes-required although it passed. A comment now splits into blocks at its first line and at each later Markdown heading that names the same review kind. Each block's verdict is its first verdict line of that kind; later verdict-like lines are prose. A later heading block without a verdict belongs to the block before it. The most severe block verdict still wins. Red baseline against 5512b45: the prose-line case read undefined and the quoted-verdict case read fixes-required (both should read their own block verdict); the list-item case already passed and guards the block rule. After the fix, the plan-CLI suite passes 66 of 66. Refs #98
…cklists The reviewers run on GPT-6-family models in omp, and current guidance for those models (and practice at Anthropic, Cursor, and OpenAI's Codex review) points the same way: a short router document, details in references, no emphasis, a clear finding bar, every qualifying finding in one pass, evidence with input, expected, and actual behavior, and a named defect class so the author fixes every instance (#98). - New internal `code-reviewer` skill: precedence, finding bar, evidence rules, the round modes `full`, `panel`, and `delta`, defect classes, severities, and the output contract, with one reference per lens (correctness, spec-and-tests, text-and-safety) and one for delta rounds. Lenses load the matching docks code-review reference when docks is installed. The `code-reviewer` agent loads the skill the way `plan-reviewer` loads its skill and can still run a full round alone. - `plan-reviewer`: the same finding bar and evidence rules, a checklist of checkable plan facts, and a delta mode for re-reviews; the Output Format block stays identical in the agent and the skill. Step 6 of #98 benchmarks these prompts against the previous ones and keeps them only where they win. Refs #98
Benchmark 1 of #98 replayed plan #96: three lens reviewers found 4 to 6 real defects per round where one reviewer found 0 to 3, and a delta re-review of a 94-line repair took 108 to 123 s and about 0.35 M tokens against 180 s and 1.5 M tokens for the full re-review, while still catching a planted bug in 2 of 2 runs. - Contract: a round posts all its blocks in one comment; a block's verdict is its first verdict line and the most severe block wins. New "Code-review rounds": commit the candidate before each round (the reviewed head); `panel` (three lenses in one message, shared prompt prefix, lens line last) when the full diff changes 50 or more lines, else `full`; `delta` only after a completed round, for repairs only, and when the delta diff is at most half the full diff; focus diffs by path class; re-dispatch a failed reviewer; fix every instance of a named defect class; every lens runs as the `code-reviewer` agent, so per-agent model settings apply. Rejection evidence is a comment in every loop, and saved round files feed the next delta reviewer. A plan re-review may use the plan delta mode. - plan-manager phases 4 and 6 and yolo follow these rules; in yolo the self-review runs while round 1 runs, and other work overlaps the waits. - Codex wrappers, their templates, the Roles list, and the README name the new inputs and the internal `code-reviewer` skill. Refs #98
Every reviewer change in #98 has to show that it is better. The new `review-benchmark` skill replays recorded review cases through baseline and candidate reviewer setups and scores them, so later changes can keep being measured. - `prepare.mjs` builds each case workspace without git history (the reviewed tree, full, focus, and delta diffs, the plan, and one prompt per reviewer and arm), and writes arm agents for omp or Claude Code with the same model and only the instructions differing. - `score.mjs` parses the returned blocks, builds pairs for a blind judge from another model family (a match pass, then an adjudication pass that rates unmatched findings valid, wrong, or unknown), and reports recall, extras, wrong findings, time, and tokens per arm. - Cases: dev replays of #96 and #89 (code, delta, and plan), and two held-out cases with seeded defects whose keys only the judge reads. - A unit test covers parsing, pairs, report math, path classes, and the transcript reader; it is registered as `unit-review-benchmark`. Refs #98
omp's eval `agent()` returns a reviewer's text JSON-wrapped (an object with one string field, or a JSON string), so `score.mjs findings` marked every such block unreadable in the first benchmark wave. The scorer now unwraps both shapes before it parses a block, and the procedure says to save plain Markdown. The plan delta mode had no planted repair defect to catch, so its keep rule could not apply. `plan-96-delta-planted` replays the #96 round-2 repair with one defect planted in a repaired Steps row. Refs #98
Two dev iterations that plan #98's decision rule allows (the held-out cases stayed closed): - it1: a `full` round serves small diffs, so it no longer loads all three lens references; that cut its time on code-89 from 217 s to 167 s without a recall loss. - it2: the reviewer's job in a round is coverage. It reports every defect, also a LOW one, and an evidenced finding whose reach is uncertain, because the manager reproduces each finding before it acts; the finding line asks only for the defect and the input or scenario that shows it. The single reviewer then ran within 9% of the old prompt's time on dev and found one more known defect. Refs #98
The results log now holds the dev and held-out tables for every arm (old and new code and plan prompts, the lens panel, and the code and plan delta modes), the decision under the pre-registered rule for each candidate (all kept), and the limits and leads for the next run: the held-out code case sat at the ceiling, the dev cases favor the lens setups, lens text in the dispatch beat lens text in a reference on dev, and reasoning effort changes both recall and time. Refs #98
…tion The first panel decision of #98 credited the delta rounds' savings to the panel. Like for like, with delta follow-ups in both policies, a panel round 1 costs about 88% more loop tokens than one reviewer, and it found more only on the hardest dev case (code-96); it tied on code-89 and on the held-out code-H1, and the old prompt with lens text in the dispatch beat the new prompt as a panel. Round 1 of code review is now a single `full` reviewer. It is a `panel` round when the user asks for one or Research records the change as high risk with its reason; yolo records that decision in Research. The new code prompt, the code and plan delta modes, and the new plan prompt stay, as the benchmark kept them. The results log records the like-for-like loop costs as a correction. Refs #98
Plan review round 6 found that the panel correction said the panel tied on code-89. That holds for B1 and the first candidate panel, not for the tuned one: with the new skill, the panel found more than one reviewer on both dev cases (code-96 0.21 against 0.14, code-89 0.83 against 0.67). The results log gets an appended correction, and the contract now says that a panel found somewhat more defects for about twice the tokens. The policy stays: one full reviewer by default, a panel for a high-risk change. Refs #98
Code review round 1 and the self-review of #98 found three scorer defects: - An `unknown` pair judgment vanished, so a known defect that a run may have found counted as a plain miss. Each run now reports the unresolved known defects: not found, but judged `unknown` for a pair with one of the run's findings. - The summary gave only the mean seconds and tokens, while the decision rule compares medians and the procedure promises ranges. It now gives the median, minimum, and maximum of both, so one slow run cannot hide a regression. - Dispatch names drop separators and case, so `code-7` and `code_7` shared one name and one transcript. `names` and `transcripts` now stop on a collision before they write anything. Refs #98
…tion rule Code review round 1 of #98 found that the GitHub publication reference still told the manager to post each reviewer's block as its own comment. With the latest review comment deciding the verdict, a later lens pass could then hide an earlier lens failure. The reference now posts the blocks of one round in one comment, as the contract says. Refs #98
Code review round 2 of #98 found that an `unknown` pair answer still vanished when its known defect was found by another finding: F1 matches D1, F2 is `unknown` against D1 and rated valid, and the report showed one valid extra with nothing unresolved, although F2 may repeat D1. Each run now also reports `Unknown pairs`, the count of every `unknown` answer for its findings; `Unresolved` stays the subset that can change recall. Refs #98
After the merge question of #98, the user removed the token limit: more tokens are acceptable when the reviews find more defects or finish sooner. In the benchmark, the panel found more defects than one reviewer (with the new skill on both dev cases, and with the old prompt on code-96) at about the same round time, for about twice the tokens. Round 1 is a panel again when the full diff changes 50 or more lines; a later full round uses the same size rule. A user who wants to save tokens can ask for a full round. yolo no longer decides a panel in Research. The results log records the decision change. Refs #98
…nswer Code review round 4 of #98 found three benchmark defects: - The snapshot left out the benchmark folder, but the full, focus, and delta diffs did not. A replay whose range changes that folder, such as #98 itself, gave reviewers the held-out answer keys. One shared pathspec now leaves the folder out of the snapshot, the changed-path list, and every diff. - Scoring did not check that the judge answered every candidate pair, so an omitted pair counted as a miss and a valid extra with nothing unjudged. The report now stops and names the missing pairs. - The prompts allowed reads only in the snapshot and the named files, which conflicts with loading the reviewer's own instructions and checklists. They now keep the evidence boundary and allow those reads. Refs #98
DocksDocks
deleted the
98-make-plan-and-code-reviews-faster-and-need-fewer-rounds
branch
October 23, 2026 19:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Yolo run: #98 Make plan and code reviews faster and need fewer rounds
Pull request: #99. Checks: 5 of 5 passed (CI plugin-targeting contracts, resolve validation shards, validation shard core, validation shard repo, validate). Reviewed head:
4db65007.What changed
plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs).code-reviewerskill holds the finding bar, the round modes (full,panel,delta), the output contract, and one checklist reference per lens and for the delta round. Thecode-revieweragent loads it (plugins/plan-lifecycle/skills/productivity/code-reviewer/,plugins/plan-lifecycle/agents/code-reviewer.md).plan-reviewerskill gets one-pass finding rules, a checklist of checkable plan facts, and a delta mode (plugins/plan-lifecycle/skills/productivity/plan-reviewer/).plan-contract.md,github-issue-publication.md,plan-manager/SKILL.md,yolo/SKILL.md, the Codex wrappers)..agents/skills/review-benchmark/).Decisions and corrections
orchestrateis a one-turn prompt keyword, not an API, so the manager dispatches parallel reviewers in every runtime. Plan review gets a better prompt, a checklist, and a delta mode, but no lenses.node: consumers run the shipped files with Node, and Bun gave no meaningful gain for a network-bound helper (plan decision 9).Reviews
Plan review passed in round 8: round 6 found one research gap, round 7 (a delta round of 50 s) passed the repair, and round 8 passed the token-limit change. Code review passed in round 5: round 1 was a three-lens panel, rounds 2 and 3 were delta rounds of 159 s and 53 s, round 4 was a full panel round after the panel-default change and found 4 benchmark defects, and round 5 was a delta round of 74 s.
Open findings: none.
Follow-up: #100 builds a larger benchmark from real defects to judge the prompts.
Verification
bun scripts/ci.mjs: passed for the repo-wide checks and both plugins.node scripts/tests/plan-cli.mjs: 66 of 66 passed;plan.mjsline coverage 99.68%.bun test ./scripts/tests/unit/review-benchmark.test.mjs: 24 of 24 passed. At4db65007,prepare.mjsbuilt all 10 case workspaces, and a replay of this branch's own range put no benchmark-folder path in any diff.code-reviewer15 andplan-reviewer15; skill scores 15 and 15; 31 identical report sections..agents/skills/review-benchmark/results.mdand in the plan's Verification Results.code-reviewerdescription: 9 of 9 should-trigger queries chose it, and 9 of 9 near-misses routed away (held-back 3 of 3 each).omp -p --plugin-dir ./plugins/plan-lifecycle:skill://code-reviewerresolved to the local plugin directory, and the task tool listedcode-reviewerandplan-reviewer.Limits and failures
Kept resources
Merge now: the remote branch98-make-plan-and-code-reviews-faster-and-need-fewer-rounds(the repository does not delete merged branches), and the untracked review scratch directory$(git rev-parse --git-path docks-review), which goes with the clone.Leave pull request open: the local and remote branch, the open issue and pull request, and the review scratch directory.After merge
Closes #98