PR review is not a prompt, part two

Note: this follows straight on from part one, and like part one it keeps the wrong turns in, because they were more useful than the destination.

Part one is the context, but here is enough of it to read this on its own.

Last Light’s PR review workflow was approving nearly everything. Measured against eight real pull requests from a private repository carrying twenty five human-confirmed defects, it posted one finding and matched none of them. Three theories about why died on the way - that it needed verification, that it needed better prompting, that it needed a bigger model - and the one that survived was duller than all of them: the model’s question set does not contain the human’s questions.

So the fix was a multi-phase pipeline in which a deterministic layer reads the diff, mints concrete questions that each name both ends of a possible defect, and hands one family of them to each of five cheap model passes that are told to over-produce. A fresh-context adjudicator then decides which of the resulting claims a person should see. It worked: matched defects went from none of twenty five to seventeen found at least once, and on a public benchmark it gained seven findings against the old reviewer and lost none. Those ten public pull requests come back at the end of this post, so I will describe them properly there.

Every number below is over those same eight pull requests and twenty five defects unless it says otherwise, and every arm was run at least twice, because a single run of a language model is a story rather than a number.

Then I raised discovery again, and made the reviewer worse

At this point I was still convinced: discovery is the ceiling, so raise it. I spent the next stretch of work doing exactly that, in two batches. I made an efficiency / cost change, where a deterministic layer now stages the diff once instead of five survey passes each re-deriving it (this was burning tokens as each agent used git to the diff). Then, every question family gets its own budget, so the noisiest family cannot eat the whole allowance. Finally the instruction telling a survey pass what to look at was reworded from a boundary into a starting point.

Each change did what it was built to do, and I can show that it did. Shell calls inside the survey passes fell from 857 a run to 400, and the 306 of those that re-derived the same commit range fell to 14. The question sets came back byte identical across repeats with every family funded. And a defect that no earlier run had ever generated a question about - eight runs across four configurations, every one that recorded the generated surface - was both generated and posted.

Posted recall went 0.400, then 0.260, then 0.180. The scorer says REVERT to both batches.

The generated number says where it went. Twenty one of twenty five, then eleven, then eighteen: the first change to implement budgets really did break discovery and the reword really did repair it. What did not come back is the other end. Eighteen defects generated, four or five said out loud.

So I went to find which of my budgets was throwing them away, and the answer is none of them. I had built an attention boundary with six per-family thresholds, an inline cap and a body cap, and for these runs the body cap was pinned to unlimited so I could watch the whole funnel. Two hundred and fifty nine findings reached that boundary across eight pull requests. Two hundred and forty two of them arrived already marked internal. The adjudicator one step upstream had done all of the filtering. The boundary I had been planning to tune cut nothing at all.

Most of those 242 deserve it. They read like this: “SILENT_SIGN_IN_NONCE_COOKIE_NAME is properly used throughout”. That is a survey pass reporting that it looked and found nothing, which is exactly what it should do and exactly what nobody should be shown.

But the tier is being decided on the grammar of the sentence rather than its content, and some of them read like this: “getActiveUsers throws on empty roster; call site has try-catch at handler level but no explicit error case”. Same shape, same tier, and that one is a defect. Sixty two of the findings filed internal in a single run carried the adjudicator’s own Critical rating. Not all of those are defects - a confirmation that a critical path is correct gets rated the same way - but the tier does not tell me which, and it does not tell the reviewer either.

That moves the problem, and for once it moved somewhere I can point at. The rule doing the damage is one I wrote: a verification report is always internal, whatever its confidence. In this workflow, ‘Internal’ means that it will not be posted as a comment or finding in the final PR. It exists for a good reason - seventeen findings once reached a single pull request at maximum confidence saying nothing more interesting than that a constant was imported and enforced at its boundaries - and it was firing on the wording of a sentence rather than the direction of its claim. In the one adjudication I read line by line, both of the findings that matched a real defect were confirmations written in that grammar, and the rule buried both of them.

So the prompt now has to answer a different question before it files anything internal: if this sentence is true, is the code wrong? If yes, it is a finding, discharge language and all. A second rule was doing smaller damage in the same direction - one adjudication demoted a real spec violation because “since the prior reviewer didn’t block it, the issue might be acceptable”, which reads the independent reviewer’s silence as evidence, when that reviewer never saw the hypotheses in the first place. Both fixes landed, and the run that tests them is the last thing in this post.

And then the fix, which worked

The repair was a bundle again, and this time everything in it pointed at the same place. Both adjudication rules above. A merge rule, because collapsing several defect-shaped rows into one “verified correct” row is not merging, it is deleting without a transcript. And the same instinct pushed back up into all five survey passes: stop writing correctly handled without naming the bar you graded against, and when you cannot name it, record the mechanism instead of grading it away - a later pass can remove a risk you wrote down and will never see one you dismissed.

Eight pull requests, same model, same questions, two repeats:

The scorer says KEEP. Paired defect by defect it is ten gained against five lost, a one-sided p of 0.15, so it is a real move and not a significant one.

Discovery did not move. Eighteen generated against seventeen. Everything the change bought is on the saying side, which is exactly where it was aimed, and that is the first time in this project I have said in advance which number would move and been right.

Set against the best arm part one had - the 0.400 - recall is 0.340 against it, which does not clear either band, at half the posted volume and double the precision for the same money. And the number the verdict actually turns on, the defects posted on every repeat rather than at least one, went from three of twenty five to four.

And then the part that matters more than any of it.

Everything so far is eight pull requests from one private repository, tuned against its own gold. Part one ends on the one result that is not: the same pipeline, unchanged, on ten cal.com pull requests from Martian’s public review benchmark - 31 defects, open-source code, gold written by other people, none of it used to build or tune any part of this, with the models wired the way production would wire them rather than forced onto one for comparability.

So I ran those same ten again with the fix, and nothing else touched:

Against the pre-fix pipeline that is KEEP, three gained and none lost, at a one-sided p of 0.125. Against the original reviewer it is now nine gained and none lost, at a p of 0.002, where before the fix it was seven and 0.008. Precision did not move at all - the two pipeline arms average 0.269 and 0.270 - so this is not the recall-for-noise trade every other change in this post has made.

That is the answer to the question the whole project was built around. The rules I changed came from reading transcripts of my own eight private pull requests, and they moved a benchmark that none of that reading touched, in the same direction, by roughly the same amount. The fix is not a fit to my repository.

Now the cautions, because there are plenty.

The original reviewer is far better on this set than on mine - 15 of 31 against 0 of 25. These are different difficulty regimes measuring different things, and I am not going to pool them or quote a combined figure. Whatever makes cal.com’s defects findable, my eight cases do not have it.

Its 0.484 is a single run, so its own noise is unbounded and some of that gap could be it. The scorer refuses to call the comparison anything better than indistinguishable for exactly that reason, and it is right to. Repeating that baseline is the next thing I owe this number.

But nothing was lost, again. Nine gained and none given up, on top of the seven gained and none given up before the fix. The claim I had been making on private data - that the pipeline never surrenders a finding the old reviewer already caught - has now held twice on a repository nobody tuned it against. That is the part I would defend, and it is not the size of the gain.

Some ranking context, since the benchmark supplies it. Twenty two published tools were measured on these exact ten pull requests. At 0.710 recall this pipeline would sit second of twenty three, behind CodeRabbit at 0.774. On F1 it ranks seventeenth, at 0.386 against Sourcery’s 0.618, entirely because its precision is 0.27 where the F1 leaders sit above 0.55. It is still below its own baseline’s F1 of 0.462. Both of those are measured with the body-comment budget deliberately switched off so I can see the whole funnel; what ships caps it at five, and nobody has measured that.

One repeat had a pull request the judge could not grade. It is named in the output rather than quietly shrinking the denominator, which is a fix from earlier in the week doing its job.

The gap is smaller and it is not closed. The adjudicator still files 88% of everything it is handed as internal, 260 findings of 295 on one repeat, which is barely down from the 93% it was doing before. It did not stop burying things. What changed is that what it does let through is roughly twice as likely to be a real defect.

An aside: ten configurations, and the one thing they agree on

If the constraint is what gets said rather than what gets found, the obvious question is how much of that is the model doing the saying. So I took four of the eight pull requests - 14 defects between three of them, the fourth being the control that contains none, and two of the three held-out cases among them - and ran ten configurations twice each. I did this purely from a cost / speed perspective - each run takes time and burns token, and I didn’t have an unlimited supply of either.

Same deterministic layer, same questions every time. Everywhere else the only things that varied were which model wrote the review and the adjudication, and how much thinking it was allowed to do.

Recall is the share of the 14 defects a run posted, averaged over the two repeats. Band is the gap between the better repeat and the worse one, in points of recall: it is the run-to-run noise of that configuration, and any difference smaller than it is not a difference.

* one of its two repeats lost a pull request to a grading failure, so that mean is over ten defects rather than fourteen and is flattered. The last row is not one of the ten: it is the configuration I ship, taken from the eight-case run and restricted to these four pull requests, so that the table has a reference point.

Read the found column first. It sits between 7 and 11 for every row, and it should, because every row was handed the same questions. Even the row that changed the survey model as well lands on 8, in the middle of the range. The model barely touches what the pipeline finds. What it decides is how much of that gets said, and that runs from 29% of what was found to 88% of it.

Then read two rows for the band alone. Haiku with the thinking budget raised averages 29% on a band of 29 points, which is to say its two runs scored 43% and 14% and the average describes neither. Sonnet on its default averages the same 29% on a band of zero. Same headline number, and only one of them is a number.

Two rows also settle a comparison I could not read earlier. Before any of this I had run the whole pipeline on gpt-5.4-mini instead of Haiku and got the same recall at a quarter of the volume, which looked like a clean and rather exciting result until I noticed the two runs carried different git hashes and a false-positive fix had landed between them. gpt-5.4-mini everywhere against Haiku 4.5 everywhere, same commit, same afternoon, same questions: both found 8 of the 14, gpt posted 7 against Haiku’s 4, and it cost 22% less to run. That 21-point recall gap clears both arms’ bands, which is as much as this dataset can say; paired defect by defect it is four gained against one lost, a one-sided p of 0.75 over four pull requests, so it is not significant and I am not calling it that.

A third row isolates the half that matters. Hold the surveys on Haiku and change only the review and the adjudication to gpt-5.4-mini, and precision goes from 22% to 80% - 29% and 19% against 83% and 75%, ranges that do not overlap - while it posts 6 of the defects instead of 4. Recall reads 29% against 18%, which is 11 points on a 14-point band, so by the rule above that one is not a difference. Taken together: the model choice is mostly a saying decision, and the part of it that pays is the review and the adjudication rather than the surveys.

Thinking is the other axis, and it behaved more legibly than anything else in this project. Sonnet emitted 64 thinking blocks on its default, 45 when I capped it and none at all when I turned it off, and recall did not move across the three: 29%, 29%, 31%. Volume and precision moved a great deal. On the default it posted 15 findings at 53% precision; capped, 33 findings at 24%; off, 21 at 38%. On the say side, thinking is a selectivity knob and not a recall knob. Haiku is less tidy: turning thinking off left recall exactly where the default had it, 18%, and turning it up produced the 29-point band above, which I cannot read as an improvement over anything.

Then the number I keep going back to. Across those ten configurations, all 14 defects were found by somebody and 13 of the 14 were posted by somebody, while any single configuration posted between 2 and 7 of them. The pipeline can already find and say nearly everything in this set. It cannot do it twice running from one configuration, and I have no way to know in advance which configuration will be the one that says it.

So: is it worth it? Closer than it was, and the reason is still reliability rather than price. Two or three dollars a pull request I can live with, and a review that used to take two and a half minutes and now takes twelve I can live with less easily, but neither is the thing stopping me. The arm I would ship today is the last one. Thirteen of twenty five posted at least once is a genuinely useful reviewer. Four of twenty five posted every time is not, and the second number is the one a person actually experiences. Four is better than the three I had a week ago and it arrives on half the noise, and it is still not the number that makes me turn this on for somebody else’s repository.

Where it goes next

The seeding change in part one is still the best thing that has happened to this project, and it is not where I would have guessed four months ago. It was not a better model, a longer prompt or another phase. It was noticing that a whole class of defect could never be asked about, and adding rules to a deterministic process. Ten defects appeared that no amount of prompting had ever surfaced.

Finding the defects nobody has ever found, working out what question would have caught them, and seeing whether that question can be minted from evidence rather than guessed at - that is still the discovery half, and it now has a measured limit. I raised discovery twice more after that, demonstrably both times, and the reviewer got worse both times, because what it found stopped being what it said.

The saying half is where the next work is, and it is now the half with a method attached. Read the transcripts of what the pipeline found and did not say, find the rule that buried it, change that rule, and measure whether the posted number moves while the generated one holds still. That loop ran once and returned +0.16. There is no reason to think it has run out: the adjudicator still files seven eighths of everything it is handed where nobody will read it, and I have examined the reasoning behind almost none of that.

Then more of what the cal.com run started. Ten public pull requests going the right way is the first thing here that is not measured on the material it was built from, and the cheapest thing I can do next is repeat that baseline so the comparison has two ends rather than one. After that, more repositories, and gold I have never read.

If you are building something in this shape, the thing I would press on you is smaller than a pipeline. Decide which parts of your system are allowed to be non-deterministic, make that boundary explicit, and put everything you can on the deterministic side of it - not because it is smarter, but because it is the only side you can hold still while you change the other one. Then measure the other side by running it more than once, because a single run of a language model is a story, not a number.

A word on what made any of this measurable, because it is not cleverness on my part. All of it runs on Last Light, and two properties of it did the work.

The first is that a workflow holds both kinds of phase in one declaration. The deterministic steps that read the diff and mint the questions, and the model steps that answer them, are nodes in the same graph with the edges written down rather than implied. That is what makes the seam this post keeps going on about a thing you can move: pushing work from the model side to the deterministic side is an edit to a file, not a rewrite.

The second is the evals harness. It runs the real workflow against a mocked GitHub, repeats it as many times as you ask, grades what it posted against gold, and keeps every transcript. Every number in both of these posts came out of that, and the change that mattered most came out of sitting and reading the transcripts of what the pipeline had found and then refused to say.

If you have an agent workflow you suspect is not working and no way to tell, that second part is the one I would go and look at.

I would genuinely like to compare notes if you are down the same road, especially if you have found a way to make either half of this stable. I have not … yet.

This is part two of PR review is not a prompt. Earlier posts in this thread: The harness is the product, What is harness engineering, and Meet agentic-pi.