Repository navigation
feat(review): worker self-check gate before review_step; reviewer round semantics (#657) - #661
Merged
Merged
Conversation
…nd semantics (#657) Review cycles dominated wall time (penster TP-1921: ~52 min authoring vs ~117 min review; every gate to the 2-round cap; round-2 findings were the worker's own Step 0 invariants). - review_step (code/test) refuses unless STATUS.md carries a '### Self-check (Step N)' section AFTER the step's last checked box with at least one data row (selfCheckRefusal). Opt-out: taskRunner.worker.requireSelfCheck=false → TASKPLANE_REQUIRE_SELF_CHECK=0. - Every review request states its round (reviewRoundForGate: 1 + prior files for the gate) and, from round 2, the prior review path. Round 1 is exhaustive; round ≥ 2 verifies the fold and raises NEW blocking findings only at the top severity label (taskRunner.reviewer.round2NewFindings 'p0-only' default | 'any' → TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS). Code-review requests also point the reviewer at the self-check table. - Templates: task-worker.md gains the self-check step (3) in the Order of Operations; task-reviewer.md gains 'Round semantics'; reviewer-extension promptGuidelines carry the round line. - Config: schema/types/loader/env for both keys; task-runner config doc. Tests: review-self-check.test.ts (10): refusal matrix (missing / empty / stale-before-last-checkbox / step-scoped), round counting per gate, round lines both modes, config→env, wiring, templates. 4132 pass.
…und counts only prior REVISE/RETHINK; settings reach the worker
1. Round ≥ 2 no longer downgrades genuine correctness defects: a regression
introduced by the fold, a bug or an unmet requirement is BLOCKING at any
severity label; only non-defect observations (hardening, style, extra
tests) are subject to the p0-only/any scope. The unconditional '→ APPROVE'
line is gone ('never manufacture an APPROVE'). Templates + reviewer tool
guidance updated to match.
2. reviewRoundForGate counts only prior files whose verdict parses as
REVISE/RETHINK — an APPROVE, an incomplete file or an unparseable verdict
is not evidence of a completed exhaustive review and does not narrow scope.
3. Settings now reach the worker on every path: engine maps
requireSelfCheck (+ exitInterceptTimeoutSec/holdTimeoutMinutes, which had
the same gap) into the executeWave worker config at both sites; execution
consumes TASKPLANE_REQUIRE_SELF_CHECK / TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS
into LaneRunnerConfig; lane-runner forwards them into the worker env.
4. Refusal text starts with 'REFUSED:' so the disposition parser classifies
it (not a review verdict); self-check data-row detection is structural
(header = row above a |---| separator), not keyword-based.
Tests: round counting matrix, defect-blocks lines, end-to-end env chain;
review-step-guard-runtime fixture carries a self-check. 4134 pass.
…657 Sage round 2) attemptModelFallbackRetry built its env without buildWorkerEnv, so a model-access retry silently reset requireSelfCheck to true and dropped the configured exit-intercept / hold timeouts. Forward every worker setting via buildWorkerEnv with the configured model withheld (the retry must use the session model, not the one that just failed). Test asserts the built env.
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.
Closes #657.
Problem: review cycles, not authoring, dominate wall time (penster TP-1921: ~52 min authoring vs ~117 min in review; every gate to the 2-round cap; round-2 findings were invariants the worker had written in its own Step 0 design). Same pattern in this repo's TP-198 run (7 code-review rounds).
Worker self-check (gate).
review_step(code/test) now refuses —REFUSED: self-check missing — …— unless STATUS.md carries a### Self-check (Step N)table placed after the step's last checked box, with ≥1 data row: one row per outcome / Step 0 design decision / Completion Criterion (and prior finding on round ≥ 2), each withfile:lineevidence. Structural row detection (header = row above|---|). Opt out:taskRunner.worker.requireSelfCheck: false.Reviewer round semantics. Every review request states its round (= 1 + prior REVISE/RETHINK reviews of the gate; an APPROVE or unparseable file never narrows scope) and, from round 2, the prior review path. Round 1 exhaustive. Round ≥ 2 verifies the fold; a new finding that is a genuine defect (regression from the fold, bug, unmet requirement) blocks at any severity; other new observations block only at the top label (
taskRunner.reviewer.round2NewFindings:"p0-only"default |"any") and otherwise go to Suggestions. Never "manufacture an APPROVE" — an exhausted budget is the worker's escalation.Wiring. Both settings flow config →
buildWorkerEnv/buildReviewerEnv→ lane config → worker env on every path (waves, stale-worktree recovery, crash retry, model-fallback retry — which also previously droppedexitInterceptTimeoutSec/holdTimeoutMinutes, now fixed). Templates (task-worker.mdOrder of Operations step 3;task-reviewer.md"Round semantics") and reviewer tool guidance carry the contract.Sage blockers-only: round 1 (defects downgraded in round 2 → fixed; settings not forwarded → fixed), round 2 (model-fallback path still bare → fixed), round 3 sign-off. Suite 4135/0, lint 283, typecheck/format clean.
Intended for the next penster batch before external release.