Skip to content

Commit 033b4d9

Browse files
committed
Show what a review will examine before anyone pays for it
Work item V4 of the M3 plan: the pre-flight panel and the linked-project toggle, the two things the new-review screen was missing. The pre-flight endpoint is read-only and free. Git and arithmetic only: it fetches, diffs, parses, runs the mechanical sweep and estimates the heaviest prompt the run would send. No model is called and nothing is written. A review is expensive enough that seeing its size first, and noticing that a branch pair is empty or enormous, is worth a round trip. Its pins are advisory and the panel says so, because creating the review fetches and pins again. Sweep problems are reported separately from sweep hits. A pattern that could not run means an incomplete sweep, which the pipeline refuses outright, and seeing that before paying is worth more than the hit count. The panel also says plainly when a model has not been probed, so its context window is unknown and the review will run unsplit, rather than quietly guessing at a limit. The dependency toggle is never on by default. When the dependency has a branch of the same name as the one being reviewed it is preselected and labelled suggested, because that is almost always the other half of the change, and almost always is not always. Driven against a running server: primary alone reported ten files, ten hunks, one request; including the dependency reported eleven files and the two changed exported symbols the fixture plants, and creating the linked review pinned both sides. Asking for a dependency that is not registered as one is refused by name. Two corrections the work forced. A test asserting the fixture produces sweep hits failed, and measuring showed it genuinely produces none: six patterns run and none match, so the test now asserts that every pattern ran, which is the part that matters. And the screen server-rendered a blank page, because its Suspense boundary had a null fallback; it now falls back to the header it is about to show anyway.
1 parent 35ff978 commit 033b4d9

5 files changed

Lines changed: 554 additions & 3 deletions

File tree

docs/DECISIONS.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,3 +691,21 @@ verified evidence, in writing, here.
691691
the branch already chosen. Arriving at a form that has forgotten the choice
692692
just made is the kind of small friction that makes a tool feel like
693693
paperwork.
694+
- 2026-07-31 DECIDED (V4, D-37): the pre-flight endpoint is read-only and free.
695+
Git and arithmetic only: it fetches, diffs, parses, runs the sweep and
696+
estimates tokens, and writes nothing except the fetch timestamp. Its pins are
697+
advisory and the screen says so, because creating the review fetches and pins
698+
again (D-27); the numbers are a preview rather than a promise.
699+
- 2026-07-31 DECIDED (V4): the pre-flight reports sweep problems separately
700+
from sweep hits. A pattern that could not run means an incomplete sweep,
701+
which the pipeline refuses outright, and seeing that before paying is worth
702+
more than the hit count itself.
703+
- 2026-07-31 NOTED (V4): the seeded fixture produces zero sweep hits against
704+
the example protocol. Six patterns run and none match its changed lines.
705+
Measured rather than assumed: a test asserting hits were non-zero failed, and
706+
the expectation was wrong rather than the code. What the test asserts now is
707+
that every pattern ran.
708+
- 2026-07-31 DECIDED (V4): a dependency is never included in a review
709+
automatically. When the dependency has a branch of the same name as the one
710+
being reviewed it is preselected and labelled as suggested, because that is
711+
almost always the other half of the change, and almost always is not always.

docs/plans/M3-FINISH-PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ confirms the wiring.
346346
| V1 | budget cap, engine note, no-auto-probe, worktree cleanup, 404 unlink, activity cap | - | DONE |
347347
| V2 | confirm/dismiss/complete/context routes, confirmation UI, keyboard map | V1 | DONE |
348348
| V3 | project detail page, fetch-now, links CRUD, project delete | V1 | DONE |
349-
| V4 | preflight route and panel, linked toggle with suggestion | V3 | |
349+
| V4 | preflight route and panel, linked toggle with suggestion | V3 | DONE |
350350
| V5 | report renderer, report/export routes, report UI | V2 | |
351351
| V6 | resume/queued/merged/delete UI, settings editor, probe buttons | V2 | |
352352
| V7 | rulesets detail, enable toggle, snapshot filter, export | V1 | |
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/**
2+
* What a review would examine, before anyone pays for it.
3+
*
4+
* Read-only and free: git and arithmetic, no model calls, nothing written.
5+
* That is the whole point. A review is expensive enough that being able to see
6+
* the size of it beforehand, and to notice that a branch pair is empty or
7+
* enormous, is worth a round trip.
8+
*
9+
* The pins here are advisory. Creating a review fetches and pins again (D-27),
10+
* because the answer must be taken at the moment the review is made, not the
11+
* moment someone looked at a panel. The screen says so.
12+
*/
13+
14+
import { z } from "zod";
15+
import { reviewProfileSchema } from "@/lib/domain/enums";
16+
import { parseUnifiedDiff, type ParsedFile } from "@/lib/git/diff";
17+
import { changedExportedSymbols } from "@/lib/git/symbols";
18+
import { repoSlug } from "@/lib/git/url";
19+
import { estimateTokens, budgetFor } from "@/lib/review/budget";
20+
import { outputContractFor, stageSchemaFor } from "@/lib/review/stage-schemas";
21+
import { runSweeps } from "@/lib/review/sweep";
22+
import { composeSystemPrompt, planRuleBatches } from "@/lib/rulesets/compose";
23+
import { availabilityOf, getModel } from "@/server/db/repositories/models";
24+
import { recordFetch, requireProject } from "@/server/db/repositories/projects";
25+
import { loadRuleset } from "@/server/db/repositories/rulesets";
26+
import { diffText, fetchAll, mergeBase, resolveCommit } from "@/server/gitops/repo";
27+
import { git } from "@/server/gitops/run";
28+
import { renderAdversarialPrompt, type ChangedFileEntry } from "@/server/review/content";
29+
import { failed, handler, ok, readJson } from "@/server/api/respond";
30+
import { runtime } from "@/server/runtime";
31+
32+
export const dynamic = "force-dynamic";
33+
34+
const body = z.object({
35+
projectId: z.string().min(1),
36+
fromBranch: z.string().min(1),
37+
intoBranch: z.string().min(1),
38+
rulesetId: z.string().min(1),
39+
model: z.string().min(1),
40+
profileId: reviewProfileSchema.default("full-context"),
41+
linked: z
42+
.object({
43+
projectId: z.string().min(1),
44+
fromBranch: z.string().min(1),
45+
intoBranch: z.string().min(1),
46+
})
47+
.optional(),
48+
});
49+
50+
interface Side {
51+
slug: string;
52+
files: ParsedFile[];
53+
fromCommit: string;
54+
intoCommit: string;
55+
mergeBaseCommit: string;
56+
subject: string;
57+
hunks: number;
58+
}
59+
60+
export function POST(request: Request): Promise<Response> {
61+
return handler(async () => {
62+
const { db } = runtime();
63+
const input = await readJson(request, body);
64+
const project = requireProject(db, input.projectId);
65+
const ruleset = loadRuleset(db, input.rulesetId);
66+
67+
let primary: Side;
68+
let linkedSide: Side | undefined;
69+
try {
70+
primary = await inspect(project.clonePath, repoSlug(project.name), input);
71+
recordFetch(db, project.id);
72+
73+
if (input.linked) {
74+
const dependency = requireProject(db, input.linked.projectId);
75+
const slug =
76+
repoSlug(dependency.name) === primary.slug
77+
? `${repoSlug(dependency.name)}-dep`
78+
: repoSlug(dependency.name);
79+
linkedSide = await inspect(dependency.clonePath, slug, input.linked);
80+
recordFetch(db, dependency.id);
81+
}
82+
} catch (error) {
83+
// A branch that vanished, or a remote that will not answer. Git's own
84+
// words, because they say which of those it was.
85+
return failed(error, 400);
86+
}
87+
88+
const entries: ChangedFileEntry[] = [
89+
...primary.files.map((file) => ({ repo: "primary" as const, slug: primary.slug, file })),
90+
...(linkedSide?.files ?? []).map((file) => ({
91+
repo: "linked" as const,
92+
slug: linkedSide!.slug,
93+
file,
94+
})),
95+
];
96+
97+
const sweep = runSweeps(
98+
entries.map((entry) => ({ repo: entry.repo, file: entry.file })),
99+
ruleset.rules,
100+
);
101+
102+
const changedSymbols = linkedSide ? changedExportedSymbols(linkedSide.files) : [];
103+
const plan = planRuleBatches(
104+
ruleset.rules,
105+
entries.map((entry) => `${entry.slug}/${entry.file.path}`),
106+
input.profileId,
107+
);
108+
109+
// The heaviest prompt the run would send, which is what decides whether it
110+
// has to be split at all.
111+
const systemPrompt = composeSystemPrompt({
112+
directives: ruleset.directives,
113+
rules: ruleset.rules,
114+
stage: "s3_adversarial",
115+
includeFullRules: true,
116+
outputContract: outputContractFor(stageSchemaFor("s3_adversarial")),
117+
});
118+
const prompt = renderAdversarialPrompt({
119+
files: entries,
120+
sweepHits: sweep.hits.map((hit) => ({ ...hit, path: hit.path })),
121+
...(changedSymbols.length === 0 ? {} : { changedSymbols }),
122+
});
123+
const estimate = estimateTokens(systemPrompt) + estimateTokens(prompt);
124+
125+
const model = getModel(db, input.model);
126+
const window =
127+
model && availabilityOf(model) === "available" ? (model.contextWindow ?? null) : null;
128+
129+
return ok({
130+
pins: {
131+
primary: sidePins(primary),
132+
...(linkedSide ? { linked: sidePins(linkedSide) } : {}),
133+
},
134+
files: entries.length,
135+
hunks: primary.hunks + (linkedSide?.hunks ?? 0),
136+
sweepHits: sweep.hits.length,
137+
// A pattern that could not be run means the sweep is incomplete, which
138+
// the pipeline refuses outright. Better seen here than after paying.
139+
sweepProblems: sweep.problems.map((problem) => problem.reason),
140+
changedSymbols: changedSymbols.length,
141+
estimatedTokens: estimate,
142+
contextWindow: window,
143+
withinWindow: window === null ? null : estimate <= budgetFor(window),
144+
requests: plan.batches.length,
145+
excludedPairs: plan.excluded.length,
146+
profile: input.profileId,
147+
});
148+
});
149+
}
150+
151+
function sidePins(side: Side) {
152+
return {
153+
slug: side.slug,
154+
fromCommit: side.fromCommit,
155+
intoCommit: side.intoCommit,
156+
mergeBaseCommit: side.mergeBaseCommit,
157+
subject: side.subject,
158+
files: side.files.length,
159+
hunks: side.hunks,
160+
};
161+
}
162+
163+
async function inspect(
164+
repoDir: string,
165+
slug: string,
166+
branches: { fromBranch: string; intoBranch: string },
167+
): Promise<Side> {
168+
await fetchAll(repoDir);
169+
const fromCommit = await resolveCommit(repoDir, branches.fromBranch);
170+
const intoCommit = await resolveCommit(repoDir, branches.intoBranch);
171+
const mergeBaseCommit = await mergeBase(repoDir, branches.intoBranch, branches.fromBranch);
172+
const files = parseUnifiedDiff(await diffText(repoDir, mergeBaseCommit, fromCommit));
173+
174+
return {
175+
slug,
176+
files,
177+
fromCommit,
178+
intoCommit,
179+
mergeBaseCommit,
180+
subject: (await git(["log", "-1", "--format=%s", fromCommit], { cwd: repoDir })).trim(),
181+
hunks: files.reduce((total, file) => total + file.hunks.length, 0),
182+
};
183+
}

0 commit comments

Comments
 (0)