Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1356,9 +1356,10 @@ size. Each size ranks its reviewers, the same for every repository:
| L | Codex `gpt-6.1-sol` xhigh | Claude Code `claude-opus-5-5` high |
| XL | Codex `gpt-6.1-sol` xhigh | Claude Code `claude-opus-5-5` xhigh |

Beside it, opencode `opencode-go/deepseek-v4.1-flash` at `high` reviews the
Beside it, opencode `opencode/deepseek-v4.1-flash-free` at `high` reviews the
same head for every size, in its own session, so each head is read by a model
family no writer runs, at about $0.05 a review. The shadow, the pick's model
family no writer runs. It is opencode's free tier of DeepSeek V4.1 Flash, so
reviews spend no paid allowance. The shadow, the pick's model
one effort lower (Sol low for an XS PR), reviews it too, in its own session, so
the routing can be retuned from what each effort step finds over the one below
it. It is skipped when the pick runs at low or at an `--effort` off the ladder
Expand All @@ -1369,7 +1370,10 @@ answered, and settled like any reviewer's. A pick with no step below it seats
a shadow at its own effort while the old shadow still has findings, so none
is left unanswered. The reviewers run at once;
the first to fail, such as on a usage limit, stops the others, and the round
saves none.
saves none. The one exception is the opencode reviewer's usage limit while it
has no findings of its own to settle: it is left out, as a `skipped` line and
a `Skipped:` line in the record, and the other reviewers' rounds count, so an
outage of the second reviewer never holds a head.

A quest branch takes its quest's size, read from the base branch's tip so a
change cannot lower its own review; any other PR, such as a skill's own, takes
Expand Down
3 changes: 2 additions & 1 deletion assets/skills/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ Step back before another patch: state the problem the PR solves without referenc
Report that comparison, recommending to continue, rethink the approach, merge as is, or drop the PR, and let the user decide.
Record each design change, including a rethink the user picks, in a PR comment that starts with `Design change:`, and start the next review with `quest review --new`, which restarts the count.

Either reviewer out of quota or logged out is an outage, not a skip: stop reviewing rather than go on with the other alone, keep the PR a draft, hand it back, and report the outage.
A reviewer out of quota or logged out is an outage, not a skip: stop reviewing rather than go on with the other alone, keep the PR a draft, hand it back, and report the outage.
The exception is the second reviewer's usage limit, which `quest review` reports as a `skipped` line and records itself: the other rounds count, so mention the skip in your summary and go on.
Its `## Waiting` item names the reviewer, its error, and the check, such as `Codex: out of quota until 2026-11-03 15:00 UTC; check that quest review runs`, with the reset time `quest review` prints when it exits 3 on a usage limit; a logged-out reviewer waits on `Maintainer:`, with the login command `quest review` prints.
Once the reset passes or the login is restored, the item has cleared: whoever finds it runs `/quest-review` on the PR.

Expand Down
2 changes: 1 addition & 1 deletion quest/m1/model-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ checkpoints and outages to count by hand:
covers all three reviewers: opencode since
[opencode reviewers](https://github.com/moq-dev/quest/pull/603), on the
model `QUEST_LIVE_OPENCODE_MODEL` names (default
`opencode-go/deepseek-v4.1-flash#high`), with the dev shell's `srt`. Check
`opencode/deepseek-v4.1-flash-free#high`), with the dev shell's `srt`. Check
that opencode's `run` still takes no schema (it took none in 2.2.6, so the
findings turn's prompt carries it), and that `OPENCODE_DISABLE_PROJECT_CONFIG`
still skips the checkout's config and `AGENTS.md`.
Expand Down
19 changes: 14 additions & 5 deletions src/cli/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,8 @@ export function main(argv: string[], out: Output): number | Promise<number> {
"--effort override that reviewer's, or without --harness, the size pick's; a repository whose AGENTS.md names other reviewers " +
"passes them. opencode is no size's pick: --harness opencode needs --model, as provider/model, and " +
"--effort, its variant. A --harness run keeps the other reviewers' sessions for the next run of the pair, and refuses while one has findings to settle, as a run of the pair does while a reviewer only --harness runs has some. When the latest writer changes the size's pick to one with no session on the branch, its first session settles the open findings of the reviewer it replaces. The reviewers run at once, and the first to fail stops the others and fails the " +
"run, saving no round. The " +
"run, saving no round, except the opencode reviewer's usage limit while it has no findings of its own to settle: " +
"that leaves it out, printed as a `skipped` line and in the record's `Skipped:` line, and the other rounds count. The " +
"size is --size, else the size of the quest a quest/ branch names, read from the base's tip so a " +
`change cannot lower its own review, else the PR record's Size line, else ${DEFAULT_SIZE}. ` +
"The reviewer reaches its model as the user's own CLI " +
Expand Down Expand Up @@ -1339,19 +1340,27 @@ export function main(argv: string[], out: Output): number | Promise<number> {
},
);
pending = run.then(
async ({ rounds, tallies }) => {
async ({ rounds, tallies, skipped }) => {
// The rounds are saved in their sessions either way, so their report prints before a failed record.
let record: Recorded | undefined;
let failure: unknown;
try {
record = await recordRounds(root(), rounds, tallies, github);
record = await recordRounds(root(), rounds, tallies, skipped, github);
} catch (error) {
failure = error;
}
if (options.json)
print(JSON.stringify({ rounds, tallies, record: record ?? null, next: nextStep(rounds) }));
print(
JSON.stringify({
rounds,
tallies,
skipped,
record: record ?? null,
next: nextStep(rounds),
}),
);
else {
for (const line of roundLines(rounds)) print(line);
for (const line of roundLines(rounds, skipped)) print(line);
if (record !== undefined)
print("url" in record ? `record ${record.url}` : `record skipped: ${record.skipped}`);
print(nextStep(rounds));
Expand Down
5 changes: 3 additions & 2 deletions src/cli/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ export const REVIEWERS: Record<Size, readonly Model[]> = {

/**
* The second reviewer of every PR, beside its size's: DeepSeek through opencode, a model family no writer
* runs, for every size, since a review costs it about $0.05.
* runs, for every size. It runs on opencode's free tier, since on a paid plan a busy day's reviews
* spent its allowance and every review failed until the reset.
*/
export const SECOND_REVIEWER: Reviewing = {
harness: "opencode",
model: "opencode-go/deepseek-v4.1-flash",
model: "opencode/deepseek-v4.1-flash-free",
effort: "high",
};

Expand Down
33 changes: 29 additions & 4 deletions src/cli/review-record.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,17 @@ import { recorded } from "../github/models";
import { type ClaimDependencies, repositoryPath, upstream } from "./claims";
import { git } from "./git";
import { requireRemote, string } from "./github";
import { type PullModels, type Round, SEVERITIES, type Tally, located, parseBase, sameSeat } from "./review";
import {
type PullModels,
type Round,
SEVERITIES,
type Skipped,
type Tally,
located,
parseBase,
resetText,
sameSeat,
} from "./review";

const HARNESSES: Record<Round["reviewer"], string> = { claude: "Claude Code", codex: "Codex", opencode: "opencode" };

Expand Down Expand Up @@ -58,9 +68,10 @@ const counted = (tally: Tally): Counts => ({

/**
* The `## Review` section's text for a run's `rounds` of the same head, without its heading, with every tally
* on the branch: each round's reviewer's beside its round, and a reviewer the run left out on its own.
* on the branch: each round's reviewer's beside its round, and a reviewer the run left out on its own, with a
* `Skipped:` line when its usage limit left it out.
*/
export function reviewRecord(rounds: Round[], tallies: Tally[]): string {
export function reviewRecord(rounds: Round[], tallies: Tally[], skipped: Skipped[] = []): string {
const [shared] = rounds;
// A reviewer the run left out may still hold open findings, which keep the head from passing too.
const clean =
Expand Down Expand Up @@ -93,13 +104,26 @@ export function reviewRecord(rounds: Round[], tallies: Tally[]): string {
` - ${capitalized(finding.disposition)}: ${finding.severity}, ${code(located(finding))}: ${code(finding.title)}; ${code(finding.reason)}`,
);
}
const skip = (seat: Pick<Tally, "reviewer" | "shadow">) => {
const found = skipped.find((each) => sameSeat(seat, each));
return found === undefined ? [] : [`- Skipped: usage limit; resets ${resetText(found.until)}`];
};
for (const tally of tallies.filter((each) => !rounds.some((round) => sameSeat(round, each))))
lines.push(
"",
reviewerLine(tally),
// Its session, still named so `quest usage` counts it as reviewing.
...(tally.session === undefined ? [] : [`- Session: ${code(tally.session)}`]),
`- Rounds: ${tally.rounds}; ${findingsText(counted(tally))}`,
...skip(tally),
);
// A reviewer skipped before its first round has no tally yet.
for (const each of skipped.filter((seat) => !tallies.some((tally) => sameSeat(tally, seat))))
lines.push(
"",
reviewerLine(each),
`- Rounds: 0; ${findingsText({ findings: { high: 0, medium: 0, low: 0 }, fixed: 0, refuted: 0, settled: 0, open: 0 })}`,
...skip(each),
);
return lines.join("\n");
}
Expand Down Expand Up @@ -268,12 +292,13 @@ export async function recordRounds(
root: string,
rounds: Round[],
tallies: Tally[],
skipped: Skipped[],
deps: ClaimDependencies,
): Promise<Recorded> {
const [shared] = rounds;
const pull = await openPull(root, shared!.base.ref, shared!.branch, deps);
if ("skipped" in pull) return pull;
const next = withReview(pull.body, reviewRecord(rounds, tallies));
const next = withReview(pull.body, reviewRecord(rounds, tallies, skipped));
if (next !== pull.body) await pull.client.request(pull.path, { method: "PATCH", body: { body: next } });
return { url: pull.url };
}
62 changes: 53 additions & 9 deletions src/cli/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export class ReviewLimitError extends Error {
readonly until: number | null,
readonly model: string,
) {
const resets = until === null ? "unknown" : new Date(until).toISOString().replace(/\.\d{3}Z$/, "Z");
super(`${harness} hit its usage limit; resets ${resets}`);
super(`${harness} hit its usage limit; resets ${resetText(until)}`);
}
}

Expand Down Expand Up @@ -324,10 +323,22 @@ export interface Tally {
refuted: number;
}

/** What a run of `quest review` gives: each reviewer's round, and every tally on the branch, the run's first. */
/** The second reviewer, left out of a run because its account hit its usage limit, which resets at `until`. */
export interface Skipped {
reviewer: ReviewHarness;
model: string;
effort: string;
until: number | null;
}

/**
* What a run of `quest review` gives: each reviewer's round, every tally on the branch, the run's first, and the
* second reviewer when its usage limit left it out.
*/
export interface Reviewed {
rounds: Round[];
tallies: Tally[];
skipped: Skipped[];
}

/**
Expand Down Expand Up @@ -1724,23 +1735,46 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies):
env: deps.env,
signal,
};
// The second reviewer's spent usage limit leaves it out rather than failing the run, so an outage of the
// optional reviewer never blocks a head the size's pick reviewed. It still fails the run while it has findings
// of its own to settle, since only it can settle them.
const skippable = (seat: Seat, error: unknown): error is ReviewLimitError =>
error instanceof ReviewLimitError &&
seat.reviewer === SECOND_REVIEWER &&
!seat.shadow &&
(seat.session?.rounds.at(-1)!.findings.length ?? 0) === 0;
const results = await Promise.allSettled(
seats.map((seat) =>
reviewWith(shared, seat, options.responses).catch((error: unknown) => {
reviewWith(shared, seat, options.responses).catch((error: unknown): Skipped => {
if (skippable(seat, error)) {
const { harness: reviewer, model, effort } = seat.reviewer;
return { reviewer, model, effort, until: error.until };
}
controller.abort(error);
throw error;
}),
),
);
// The first failure stopped the others, so it is the one to report.
if (controller.signal.aborted) throw controller.signal.reason;
const done = results.map((result) => (result as PromiseFulfilledResult<Seated>).value);
const settled = results.map((result) => (result as PromiseFulfilledResult<Seated | Skipped>).value);
const ran = seats.flatMap((seat, i) => {
const value = settled[i]!;
return "round" in value ? [{ seat, done: value }] : [];
});
const skipped = settled.filter((value): value is Skipped => !("round" in value));
// A skipped reviewer's session, with nothing left to settle, waits for its next run.
const waiting = seats.flatMap((seat, i) => ("round" in settled[i]! || !seat.session ? [] : [seat.session]));
const tallies = tallied(
options.fresh ? dropped(saved) : (saved?.tallies ?? []),
done.map((each, i) => ({ round: each.round, settles: seats[i]!.session })),
ran.map(({ seat, done }) => ({ round: done.round, settles: seat.session })),
);
save(statePath, { branch, sessions: [...done.map((each) => each.session), ...kept], tallies } satisfies State);
return { rounds: done.map((each) => each.round), tallies };
save(statePath, {
branch,
sessions: [...ran.map(({ done }) => done.session), ...waiting, ...kept],
tallies,
} satisfies State);
return { rounds: ran.map(({ done }) => done.round), tallies, skipped };
}

/**
Expand Down Expand Up @@ -1995,7 +2029,7 @@ export function nextStep(rounds: Round[]): string {
* The plain report: the size, base, and head the reviewers share, then each reviewer's round, one record per
* line, and its findings, numbered across the reviewers in order, each with its body indented.
*/
export function roundLines(rounds: Round[]): string[] {
export function roundLines(rounds: Round[], skipped: Skipped[] = []): string[] {
const [shared] = rounds;
const lines = [
`size ${shared!.size} ${shared!.sizeFrom}`,
Expand Down Expand Up @@ -2033,5 +2067,15 @@ export function roundLines(rounds: Round[]): string[] {
);
}
}
for (const each of skipped)
lines.push(
"",
`skipped ${each.reviewer} ${each.model} ${each.effort} usage limit; resets ${resetText(each.until)}`,
);
return lines;
}

/** A usage limit's reset as `ReviewLimitError` names it: an ISO 8601 UTC time to the second, or `unknown`. */
export function resetText(until: number | null): string {
return until === null ? "unknown" : new Date(until).toISOString().replace(/\.\d{3}Z$/, "Z");
}
5 changes: 3 additions & 2 deletions src/github/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ export function recorded(body: string): {
}

// Display names and IDs compare alike: `Opus 5.5`, `Claude Opus 5.5`, and `claude-opus-5-5` share a key, as do
// `DeepSeek V4.1 Flash` and `opencode-go/deepseek-v4.1-flash`.
// `DeepSeek V4.1 Flash`, `opencode-go/deepseek-v4.1-flash`, and its free tier's `opencode/deepseek-v4.1-flash-free`.
const key = (name: string) =>
name
.toLowerCase()
.replace(/^.*\//, "")
.replace(/[\s.]+/g, "-")
.replace(/^claude-/, "");
.replace(/^claude-/, "")
.replace(/-free$/, "");
const KNOWN = new Map(
[...Object.values(WRITERS), ...Object.values(REVIEWERS), [SECOND_REVIEWER]]
.flat()
Expand Down
2 changes: 1 addition & 1 deletion test/live/review-sandbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { openSandbox } from "../../src/cli/sandbox";
* provider/model#variant, or a cheap one OpenCode Go serves.
*/
const [OPENCODE_MODEL, OPENCODE_EFFORT] = (
process.env.QUEST_LIVE_OPENCODE_MODEL ?? "opencode-go/deepseek-v4.1-flash#high"
process.env.QUEST_LIVE_OPENCODE_MODEL ?? "opencode/deepseek-v4.1-flash-free#high"
).split("#");

/** The model a sizeless review runs on `harness`. */
Expand Down
Loading
Loading