Skip to content

Commit 0cf3170

Browse files
author
verify
committed
feat(review): retire dry chunks from reverse-audit rounds and pipeline verification
Rebuild of the retirement/pipelining feature as one commit on top of the merged reverse-audit budget gate (#8468). - retirement.ts: per-chunk scheduler over the CLI's own prompt records and the harness transcripts — a chunk whose two most recent audits are substantive dry receipts is cold-checked on the even rounds instead of audited on every one, and a cold check that yields returns it to every-round auditing. The certifying match is counted records per transcript (one launch matching several records certifies none), dry receipts are read structurally with the zh forms beside the English ones, and a filed finding requires the File+Severity pair so an echoed quotation cannot pin a chunk hot. Everything fails toward auditing. - agent-prompt --all-chunks: requireAuditableChunks, then the schedule (round >= 3, fail-open to all-due on any error), then CONVERGED exit 5 (nothing built, no stamp, no marker), then the budget gate (exit 4 + marker), then the build. The admission stamp keeps the #8468 ordering and lands only after the build succeeds: a cold-check-only round that builds still stamps, a converged round never does, and a build that throws leaves no stamp. - agent-prompt --chunk: a round holding an admission stamp is repaired without gates or scheduling; an unadmitted round answers to the same sequence as --all-chunks (convergence, then budget), and its first chunk build is the round's admission — stamped after the build. - prompt-record: optional sinceMs fence on readRecordedPrompts (history readers only; coverage's obligation reads stay unfenced), plus the flattenPrompt/deliveredVerbatim split so the scheduler flattens each launch once instead of once per (record, transcript) pair. - deadline: doc-comments rewritten for the pipelined cadence — the admission-to-admission measure no longer contains a verification pass, so the tail reserve is the terminal round's only cover (replacing the 'deliberate margin' overlap rationale), and the workflow's reserve cap is cross-referenced. runEpochMs fencing, the bilingual budget-stop marker and the stamp semantics are unchanged from #8468. - SKILL Step 5: builder-owned 3B scheduling, the CONVERGED exit-5 termination rule, verification launched alongside the next round's auditors, and the cumulative reported list with '— [unverified]' tagging (added at the admitting merge, cleared or removed after the verdict; anything still tagged is excluded from Step 6). Superseded pieces of the parallel branch were dropped in favour of the #8468 form now on main: the planMtimeMs-equality fence (runEpochMs stays), the budget-scaled round-1 estimate, stamping inside the admission helper, and the branch's variants of the budget-gate tests.
1 parent ac67de2 commit 0cf3170

7 files changed

Lines changed: 1698 additions & 84 deletions

File tree

packages/cli/src/commands/review/agent-prompt.test.ts

Lines changed: 531 additions & 8 deletions
Large diffs are not rendered by default.

packages/cli/src/commands/review/agent-prompt.ts

Lines changed: 240 additions & 43 deletions
Large diffs are not rendered by default.

packages/cli/src/commands/review/lib/deadline.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,27 @@ export const RESERVE_ENV = 'QWEN_REVIEW_DEADLINE_RESERVE_SECONDS';
5353
* verification of that round's findings, compose-review, anchor resolution
5454
* and the submission itself.
5555
*
56-
* The measured round estimate ALSO contains one verification pass — a round
57-
* is admitted only after the previous round's findings were verified and
58-
* merged (SKILL.md Step 5), so an admission-to-admission span includes the
59-
* verification between them — which means the gate holds back roughly one
60-
* verification more than the terminal round strictly needs. That overlap is
61-
* deliberate margin, not double-entry bookkeeping that slipped: round costs
62-
* trend UP (each round re-reads the diff against a longer findings list, and
63-
* repair relaunches land mid-loop), so the previous round's measurement
64-
* under-predicts the next in exactly the runs that end near the boundary —
65-
* and the two error directions are not symmetric. Over-reserving ends the
66-
* loop at most one round early, disclosed as a budget stop; under-reserving
67-
* is #8368 — killed mid-verification, holding every confirmed finding.
56+
* Under the pipelined loop (SKILL.md Step 5), a round's verification
57+
* launches WITH the next round's auditors instead of sitting between
58+
* admissions — so the admission-to-admission span the gate measures
59+
* contains no verification pass, and the terminal round's verification has
60+
* exactly one cover: this reserve. That makes the reserve's sizing the
61+
* whole margin, not a top-up on an overlap the measurement already
62+
* carried, and the two error directions are still not symmetric: round
63+
* costs trend UP (each round re-reads the diff against a longer findings
64+
* list, and repair relaunches land mid-loop), so the previous round's
65+
* measurement under-predicts the next in exactly the runs that end near
66+
* the boundary. Over-reserving ends the loop at most one round early,
67+
* disclosed as a budget stop; under-reserving is #8368 — killed
68+
* mid-verification, holding every confirmed finding.
6869
*
6970
* This is only the fallback: the budget itself is
7071
* chosen outside the CLI (a repository variable, a workflow input, a
7172
* `/review --timeout=N` comment), so the review workflow passes a reserve
7273
* scaled to the budget it resolved rather than trusting this constant to fit
73-
* an arbitrary one. A local run has no deadline and no reserve at all.
74+
* an arbitrary one. The workflow caps that scaled reserve at this same
75+
* number (`.github/workflows/qwen-code-pr-review.yml`) — keep the two in
76+
* sync. A local run has no deadline and no reserve at all.
7477
*/
7578
export const DEFAULT_RESERVE_SECONDS = 3600;
7679

@@ -176,11 +179,12 @@ export function stampRound(
176179

177180
/**
178181
* What the round about to be admitted is expected to cost, in seconds: the
179-
* observed cost of the previous round (admission-to-admission — its agents,
180-
* their verification, the orchestration between) when a stamp exists, else
181-
* the conservative constant. The span deliberately overlaps the tail
182-
* reserve by one verification pass — see `DEFAULT_RESERVE_SECONDS` for why
183-
* that margin is kept rather than netted out. A stamp of the SAME round is
182+
* observed cost of the previous round (admission-to-admission — its audit
183+
* fan-out and the orchestration around it; under the pipelined loop a
184+
* round's verification overlaps the NEXT round instead of sitting between
185+
* admissions, so it is not in this measure, and the terminal round's
186+
* verification is exactly what the deadline's reserve covers) when a stamp
187+
* exists, else the conservative constant. A stamp of the SAME round is
184188
* ignored — that is a rebuild, and measuring it would report a round as
185189
* cheap because its prompts were built twice quickly.
186190
*/

packages/cli/src/commands/review/lib/prompt-record.ts

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@
2727
// agent's actual launch prompt. The two artifacts have different authors, and
2828
// neither is the orchestrator.
2929

30-
import { mkdirSync, readFileSync, readdirSync, writeFileSync } from 'node:fs';
30+
import {
31+
mkdirSync,
32+
readFileSync,
33+
readdirSync,
34+
statSync,
35+
writeFileSync,
36+
} from 'node:fs';
3137
import { dirname, join, basename, resolve } from 'node:path';
3238

3339
/**
@@ -128,8 +134,24 @@ export function recordPrompt(
128134
}
129135
}
130136

131-
/** Every prompt this plan's builder emitted, keyed as it was recorded. */
132-
export function readRecordedPrompts(planPath: string): Map<string, string> {
137+
/**
138+
* Every prompt this plan's builder emitted, keyed as it was recorded.
139+
*
140+
* `sinceMs` is the same fence every other reader of this directory applies —
141+
* the plan's mtime. Nothing clears the record dir, and a run that dies
142+
* mid-review leaves its records beside the retry's (the CI retry re-runs the
143+
* review at the SAME plan path, under a freshly-captured plan): a record file
144+
* older than the plan belongs to that dead attempt. A caller that reads
145+
* records as OBLIGATIONS (coverage: "an agent was owed for this key") passes
146+
* nothing — an obligation is not less owed for being stale, and dropping one
147+
* would excuse the agent it demands. A caller that reads them as HISTORY
148+
* (retirement) must pass the fence, or the dead attempt's records shadow the
149+
* live ones.
150+
*/
151+
export function readRecordedPrompts(
152+
planPath: string,
153+
sinceMs?: number,
154+
): Map<string, string> {
133155
const out = new Map<string, string>();
134156
const dir = promptRecordDir(planPath);
135157
let names: string[];
@@ -150,7 +172,9 @@ export function readRecordedPrompts(planPath: string): Map<string, string> {
150172
} catch {
151173
continue; // Not a name this module wrote.
152174
}
153-
out.set(key, readFileSync(join(dir, name), 'utf8'));
175+
const file = join(dir, name);
176+
if (sinceMs !== undefined && statSync(file).mtimeMs < sinceMs) continue;
177+
out.set(key, readFileSync(file, 'utf8'));
154178
} catch {
155179
/* raced with a cleanup */
156180
}
@@ -186,6 +210,22 @@ export function readRecordedPrompts(planPath: string): Map<string, string> {
186210
export function wasDeliveredVerbatim(
187211
launchPrompt: string,
188212
built: string,
213+
): boolean {
214+
return deliveredVerbatim(flattenPrompt(launchPrompt), built);
215+
}
216+
217+
/**
218+
* `wasDeliveredVerbatim` with the launch prompt already put through
219+
* `flattenPrompt`. The pair exists for the one caller that pairs MANY records
220+
* against MANY transcripts (the retirement scheduler): flattening is the
221+
* expensive half of the check, and a caller that flattens each launch once
222+
* pays it per transcript instead of per (record, transcript) pair — a few
223+
* thousand full-prompt passes on the run the scheduler was built for, all
224+
* before the round is admitted. Same contract, same failure modes.
225+
*/
226+
export function deliveredVerbatim(
227+
flattenedLaunch: string,
228+
built: string,
189229
): boolean {
190230
// A zero-byte record is not a prompt, and the loop below would be vacuously true
191231
// for it — the check would pass every agent, and the roster would credit a role
@@ -194,25 +234,27 @@ export function wasDeliveredVerbatim(
194234
// *built*), so an empty file is exactly what a partial write leaves behind. It is
195235
// the one input that must fail closed.
196236
if (built.trim().length === 0) return false;
197-
const delivered = flatten(launchPrompt);
198237
let at = 0;
199238
for (const line of lines(built)) {
200-
const i = delivered.indexOf(line, at);
239+
const i = flattenedLaunch.indexOf(line, at);
201240
if (i === -1) return false;
202241
at = i + line.length;
203242
}
204243
return true;
205244
}
206245

207-
/** Whitespace collapsed to single spaces: a re-wrap is not an edit. */
208-
function flatten(s: string): string {
246+
/**
247+
* Whitespace collapsed to single spaces: a re-wrap is not an edit. What
248+
* `deliveredVerbatim` expects its launch side to have been put through.
249+
*/
250+
export function flattenPrompt(s: string): string {
209251
return s.replace(/\s+/g, ' ').trim();
210252
}
211253

212254
/** The built prompt's lines, whitespace-normalized, blanks dropped. */
213255
function lines(built: string): string[] {
214256
return built
215257
.split('\n')
216-
.map((l) => flatten(l))
258+
.map((l) => flattenPrompt(l))
217259
.filter((l) => l.length > 0);
218260
}

0 commit comments

Comments
 (0)