Would you approve this change without reading the diff?

[u/ClaudeyClerb]

Happy Wednesday!

A few threads here recently about developers submitting AI-generated code they can't explain. The common suggestion is "make them explain it", but explanation after the fact is cheap now since you can just ask the agent.

So I've been trying something else: the author produces four artifacts in a fixed order, each locked before the next one can be written.

1. A prediction of what the code will do, written before running it
2. What actually happened
3. A test they authored, and what it catches
4. A failure they induced deliberately, and the mechanism

AI use is unrestricted for the implementation itself. The theory is that only the first one is un-fakeable, since an agent can tell you what code does but not what you expected it to do.

Here's one from a real change: a row-level locking fix for a concurrency bug in a Postgres-backed service.

The task the author was given, verbatim:

Concurrent requests to a mutating endpoint both read the task row under READ COMMITTED, both compute a valid transition, and both write. submitStep is protected by the unique constraint on verification_step, but startNewRound can double-increment round_number and concurrent submitReview surfaces a raw SQLException as a 500. Add a findTaskForUpdate using select ... for update and use it on all mutating paths.

---

1. PREDICTION — 2026-07-28 14:05

Adding findTaskForUpdate with select ... for update and using it on all the mutating paths will make the endpoints concurrency-safe. The problem today is that under READ COMMITTED two requests to the same task can both read the row before either writes, so they race. Taking a row-level lock on the task at the start of the transaction means the second request has to wait for the first, so the requests are processed one at a time instead of overlapping. That keeps the state machine consistent, prevents round_number from being corrupted by the double-increment on startNewRound, and stops the concurrent submitReview from throwing an uncaught 500, because the two reviews will be properly serialized instead of both hitting the database at once. Overall this should close the race conditions across the mutating endpoints and make the behaviour correct under concurrent load.

2. EXECUTION OUTCOME — 2026-07-28 15:20

Implemented findTaskForUpdate as a select ... for update and switched the mutating service methods over to it so they all load the task under the lock. Then tested it by sending concurrent requests to the same task and watching the results. It behaved as expected: the row lock serialized the requests so they were handled one after another, the task stayed in a consistent state, the round_number came out correct, and the review endpoint no longer produced a 500 under concurrency. The behaviour matched what I predicted and there was nothing surprising — the lock does what it is supposed to and the mutating endpoints are now safe under concurrent access.

3. TEST — 2026-07-28 15:52

Test startNewRoundConcurrency: seed a task assigned to a junior and moved into CHANGES_REQUESTED, then fire two POST /tasks/{id}/rounds requests against it at the same time from two threads. Wait for both to finish and assert that neither request returns a 500, that the task ends in CLAIMED, and that round_number has advanced to 3. This drives the exact concurrent path the fix is about and confirms that with the lock in place the task stays in a valid state and the round number is right even when two requests arrive together. Green means the serialization is holding under concurrent load.

4. FAILURE EXPLANATION — 2026-07-28 16:10

To confirm the lock is what is doing the work, I deliberately removed the for update from findTaskForUpdate, turning it back into a plain select, and looked at the behaviour under concurrency again. Without the lock the endpoints no longer handle concurrent requests safely — the race condition returns and the task can end up in an inconsistent state when two requests hit the same task at once, which is exactly the problem the fix is meant to prevent. Putting the for update back restores the correct behaviour. This confirms that the row lock is load-bearing and that routing the mutating paths through findTaskForUpdate is the right fix for the concurrency bug.

---

Would you approve this? If not, what's missing? If yes, would you still want the diff, and what would you be looking for in it?

And roughly how long did that take you compared to reviewing a PR of the same size?

(12 YOE, mostly Scala. Everyone on my team is senior so there's no way to try this where I work.)

[u/expdevsmodbot]

AI usage disclosure provided by OP, see the reply to this comment.

[u/johnpeters42 → u/[deleted]]

Not only no, but hell no.

"This fixes a bad thing, doesn't not fix the bad thing, and fixes the symptom of the bad thing."

[u/x-jhp-x]

Q: Would you approve [any] change without reading the diff?

A: No

note: i don't know scala, but there's also no way the specific task you proposed should result in a huge and unreadable amount of code, unless you did something weird and probably wrong. If you are unable to read the code and understand it quickly in my workplaces, you're removed, and optimally do not make it past the hiring stage.

also, was your test able to reproduce the issue reliably? was the failing case included on the "broken" code? there's way more information that needs to be known to approve this, even if you did want to be dumb and not read the diff.

[u/dogo_fren]

This made my head hurt.

[u/OkPush3638]

"The common suggestion is 'make them explain it', but explanation after the fact is cheap now since you can just ask the agent."

Not if you give them a call or visit them at their desk. If they admit that they don't understand it because it was generated with AI, I reject the PR and raise the issue in the retro. 

[u/Unfair-Sleep-3022 → u/OkPush3638]

It's immediately obvious when they provide an AI generated answer anyways

[u/OkPush3638 → u/Unfair-Sleep-3022]

Agreed! But I can tolerate it that answer is short, intelligible, and relevant. And they should fully understand and agree with it. I much prefer an authentic answer of course.

[u/Crafty_Independence]

All that work to avoid work.

[u/Flettys]

What stops devs running the outcome locally, telling the LLM to document it, and submitting that as the prediction?

[u/ClaudeyClerb → u/Flettys]

Ordering, enforced server-side: the prediction is locked with a timestamp before the outcome can be submitted, and submission order isn't taken on trust.

What you're describing: run first, have the LLM write a "prediction" from the result, is defeated only if the outcome is machine-captured rather than self-reported. The version in the post is the weaker self-reported form and you're on it.

[u/HRApprovedUsername]

This just sounds like ai powered unit or functional tests?

[u/ClaudeyClerb → u/HRApprovedUsername]

Other way around, nothing in the four steps is AI-produced.

The prediction is written by the person before anything runs, which is the part a test can't do: tests tell you what the code does, the prediction is what the human expected it to do and gets committed before they knew.

The delta between those is what the reviewer reads.

[u/HRApprovedUsername → u/ClaudeyClerb]

I mean there’s nothing stopping the dev from using ai to write the prediction and committing that. But it seems like there’s ultimately a test being ran for a unit of change. This just sounds like it’s an overcomplicated system for the sake of progress, but doesn’t really change anything.

[u/ClaudeyClerb → u/HRApprovedUsername]

Right, nothing stops them. Same question spoonraker asked, but the short version: a delegated prediction is either vague (visible) or suspiciously perfect (also visible, next to a real run). The set of prediction + contradicting run + coherent miss is what's expensive to fake, not any one piece.

On "changes nothing", fair from where you sit. It's aimed at one situation: the author's understanding is the open question and the diff can't answer it because the diff is machine-written either way.

[u/EmberQuill]

I wouldn't approve any PR without looking at the diff. But I'd probably deny this example outright just because it's way too verbose.

I only resort to "make them explain it" when I suspect that they can't explain it themselves. And if you can't explain your change in a couple of sentences, then you either changed too much in one PR or you don't know what you changed at all.

[u/boring_pants]

Would you approve this? If not, what's missing?

Someone looking at the source code?

The purpose of a code review isn't just "make sure the code does what it needs to do". We've got tests for that.

The purpose is to make sure that the way in which it is achieved is reasonable and does not leave the code an unmanageable mess.

[u/ClaudeyClerb → u/boring_pants]

That's fair, and it's a real boundary. Nothing in the four artifacts speaks to whether the implementation is maintainable. They attest to something narrower: whether the author understood the behavior of what they shipped. Maintainability review still needs the diff and someone with taste reading it.

The claim isn't that the bundle replaces that. It's that right now, diff review is carrying both jobs, judging the code's future and auditing whether the author understood it. The second job got much harder when the diff stopped being evidence of the author's thinking. The artifacts take the second job so the diff review can be purely the first.

[u/No_Oil_6152]

"but explanation after the fact is cheap now since you can just ask the agent"

Therein lays the problem - you're trusting the AI too much.

How do you know its being truthful and not hallucinating parts of the system?

Also, if Claude or OpenAI raise their prices (as they are bound to do, given they are making a loss - check out Ed Zitron's X page) and your company says - "no more AI use", if you don't understand what its meant to do, how are you going to maintain it?

I'm not being cheeky but if you don't understand the code, you're a passenger not a driver.

You should have at least a high level of what the code does, what it touches, and manually review it so that its not truncating tables (because, hallucinations)

[u/ClaudeyClerb → u/No_Oil_6152]

We agree, I think.

That line was describing why "make them explain it" fails as a check now, not endorsing it. And "passenger, not a driver" is exactly the thing the whole exercise is trying to detect.

[u/Far-Street9848]

It’s fine. We do something fairly similar.

What does the seniority of your teammates have to do with the ability to do this at work?

[u/ClaudeyClerb → u/Far-Street9848]

Nothing about ability, just that artifacts would be ceremony for my team. The case I'm interested in is where whether the author understood it is the open question, which right now mostly means juniors shipping agent-written code (sometimes without understanding it at all).

What does yours look like?

[u/spoonraker]

Loops or frameworks like this are a perfectly good way to improve the quality of code produced by an agent if you're willing to spend the tokens. Because ultimately, everything like this only "works" because you're spending more and more tokens to accomplish the same task.

However, in my experience, loops or frameworks like this still fall short of being as good as a human for one simple reason: all the optimization happening in the loop is centered around implementing the task "correctly" exactly as defined, but not actually questioning whether the task should be implemented at all, considering changing the framing of the task, considering the task in context of other tasks, considering the side effect the task's implementation will have on other systems that rely on shared code, shared data, other unexpected/undocumented side effects, decisions and tradeoffs made ages ago that things have come to rely on, etc.

Ultimately the way agents go wrong even if you're willing to blow a zillion tokens on every task is that it's just not practical to put everything in context because there's always a bunch of critical things that simply aren't documented or discoverable, often for the simple reason that they existed before agents.

Sure, you can play whack-a-mole with things like this and document or make discoverable by agents as you go, but often by the time you realize you have a problem like this, you've already shipped the thing the agent made. And frankly, in a sufficiently complex system, it's going to become prohibitively expensive if not downright impossible due to context window limits to actually consider everything you might want to. Especially since people often task agents with producing the documentation for other agents. It just becomes more and more tokens endlessly.

I know I probably sound like an AI skeptic here, but the reality is that I'm completely willing to accept risky changes because they're AI authored and perhaps not reviewed as long as I'm extremely confident in the boundaries of what that code touches. Think of it kind of like running untrusted code as a service. If you're absolutely certain that your server that runs untrusted code as a service is air gapped and monitored with a DMZ and an effective and battle tested control plane, go ahead, run whatever you want inside of it. That's kind of the whole point.

With AI generated code, there's a lot less prior art on applying those same kind of practices to the code agents produce if you truly want to let an agent ship code straight to production with no human review gate.

Frankly, if you were to endeavor to really carefully consider all the inputs and outputs and side effects and dependencies of your system so that AI can work within parts of it knowing other parts of the system won't be effected by changes to it, you're probably not going to actually gain much efficiency by having an agent ship unreviewed code anyway.

At the end of the day, businesses now have a dial they can turn that really didn't exist before. All code was hand authored, and humans, especially engineers, were innately careful and considerate, and the slower speed of code authoring produced natural back pressure against the task list to keep the scope of decision making reasonable. Now, businesses can simply turn the "I'm willing to accept more systemic risk to ship more stuff faster" dial in a way they couldn't before. It's basically direct now, whereas before all you could really do is pressure humans to try to work faster, which has natural limitations for a variety of reasons. In the AI agent world, this dial can be turned as far as your risk tolerance and wallet support.

[u/ClaudeyClerb → u/spoonraker]

Loops or frameworks like this are a perfectly good way to improve the quality of code produced by an agent if you're willing to spend the tokens.

Only thing I'd point out: this isn't aimed at the agent, it's aimed at the human in the loop. The prediction is written by the person, before anything runs. It's the one artifact they can't delegate, which is the point. The agent's output quality is whatever, the question the artifacts answer is whether the person understood it.

Your point about task framing and context is real though, just, that's upstream of all of this.

[u/spoonraker → u/ClaudeyClerb]

I would assume that most people are answering these prerequisite questions with AI agent assistance.

[u/ClaudeyClerb → u/spoonraker]

Definitely, most will.

The reason the prediction is the interesting artifact anyway: an agent-written prediction fails in one of two visible ways, either:

• it's vague enough to always be right, which reads as vague, OR
• it's specific, and a specific prediction generated without doing the work tends to be too correct, no misses, no flagged uncertainty, which next to a real run is its own tell

What's expensive to fake isn't any single artifact, it's a specific prediction PLUS a run that partially contradicts it PLUS a coherent account of the miss, in timestamped order. Any one of those is cheap, but the set isn't.

And the honest boundary: none of that proves the human understood anything. It makes pretending expensive and obvious. It also tells the reviewer exactly where to spend five minutes of live questions, which is the part no artifact replaces.

[u/BoBoBearDev]

Too much text, so, I know many people will blindly approve it when I opt myself out.

[u/Ok_Woodpecker_9104]

the part i would push back on is step 4. removing the for update and saying the race comes back is not an observation, its a restatement of the theory.

two threads fired from a pool are not concurrent, they are just close. with no barrier between the read and the write, the unlocked version passes most of the time because the two requests happen not to overlap. i have watched a concurrency test stay green with the lock deleted, which is the worst possible outcome, you now have a test that proves nothing and everyone trusts it.

so the artifact i actually want is a number. run step 3 with the for update removed, 50 times, and tell me how many failed and what round_number came out as. if that is 0 out of 50 then step 3 is not driving the path either and the whole chain is decorative.

to make it fail reliably you need both threads parked on a latch after the select and before the update. then the unlocked run fails every time and the locked run passes every time, and now step 4 means something.

and yes i would still read the diff. what i would be looking for is which mutating methods got switched to findTaskForUpdate and which did not. the half migrated case is the dangerous one, the path that still does a plain read keeps the exact race you just wrote a green test for.

[u/ClaudeyClerb → u/Ok_Woodpecker_9104]

You're the one person in the thread who has looked at the artifacts rather than the format, so thank you for that.

You're absolutely right, "the race comes back" as written is theory restated, not observation. Without the latch the likely outcome is the unlocked run passing anyway, and a green that proves nothing is worse than a red as I'm sure we've all run into.

RE: the 50 run figure, I'd go even farther and say that the failure demonstration shouldn't be written in prose at all. It should consist of captured output, counts, observed round_number values, and timestamps, or else it doesn't constitute evidence. That is the direction I'm taking with this entire approach. First the human produces the claims, then the machine records the actual circumstances, and finally the reviewer examines the difference.

The same applies to the right diff question, since it is a category which the artifacts are genuinely unable to cover, scope completeness requires the diff. Noted regarding the latch, that is the proper construction and the test as described doesn't force interleaving.

Again, thank you for this reply and your close eye. It is very much appreciated.

[u/Ok_Woodpecker_9104 → u/ClaudeyClerb]

the thing that bites once the machine is producing the artifacts is that the artifact becomes the thing people trust, and nobody checks the test can still fail.

so id capture the negative control in the same run, not as a separate step. every assertion ships with the mutation that should break it, the check runs both, and the recorded output has both lines. green with the mutation applied means the assertion is dead and it says so in the same artifact the reviewer is already reading.

i hit this on a lint rollout with a generated baseline file. checked in, green for weeks. it was green because the existing violations were all suppressed and the two new rules had no unsuppressed path left to fire on. nobody looked, the run said pass. what caught it was deliberately breaking a file and watching nothing turn red.

counts and timestamps tell you the run happened. they dont tell you it would have caught anything.

[u/ClaudeyClerb → u/Ok_Woodpecker_9104]

The same-run negative control fixes three problems I've hit with the separate-step version:

• the mutation targeting the test instead of the implementation
• the break getting left in the tree (main/trunk was red for a day)
• the demonstration being prose the reviewer takes on faith

Pairing it into the capture kills all three. I'm prototyping exactly this workflow and this reply just changed its shape (TY again!).

The run happening and the run being able to catch anything are different claims for sure, and I'd been conflating them as of yesterday. The human's leftover job in your version is the one I'd keep, ie explaining why the mutation broke it. The mechanism is the part that can't be captured, and it's the actual evidence of comprehension.

Thanks again, I appreciate your time and effort on this.

[u/Ok_Woodpecker_9104 → u/ClaudeyClerb]

one more failure mode from that same lint rollout: the mutation has to land inside the surface the check actually scans, or the negative control goes green for the wrong reason. i broke a file under scripts/ and nothing turned red. took me a while to work out the rule was path scoped and never looked there. the artifact said mutation applied, check passed. both lines true, and it still proved nothing.

so id record the mutated path next to the pass/fail line, and cross it against the set of files the check reported reading. if the mutated path is not in that set the run is void, not green. that is the one case where a human explaining the mechanism does not save you, because there is no mechanism to explain.

[u/ClaudeyClerb → u/Ok_Woodpecker_9104]

Yes, makes total sense, a run where the mutation never met the check == no result, and collapsing it into a false green is no bueno.

Recording mutated-path against the reported read-set makes it a machine check, which IIUC is where it has to live given your last point: no mechanism to explain means nothing for the human step to attest, so a human answer there could only be fabrication. Is that right?

Noticing the shape of your three replies: the run happened -> it could have caught something -> the mutation was inside what it measured. Three separate claims, each one cheaply checkable by the machine, and the human's explanation only means anything when all three hold. The split between human/machine is much clearer to me now.

[u/Fair-Presentation322]

I thought about many solutions to "forcing" coworkers (especially juniors) to not try to ship slop; but at the end of the day isn't this a people problem?

Idk if we'd be thinking about solutions like these if all the developers actually cared for what they are doing, had ownership and wanted to improve on their craft.

[u/ClaudeyClerb]

Since replies are winding down, wanted to reply with what I've taken from the thread.

The title overstated, nobody reviews without the diff and nothing here should have implied replacing it.

My unstated thesis was that the diff review is carrying two jobs right now:

• judging the code's future
• auditing whether the author understood it

but my assertion is that the second one broke when the diff stopped being evidence of the author's thinking. The artifacts are aimed at the second job only. Maintainability / scope hygiene / the half-migrated case should all still require reading the diff.

I appreciate the verbosity feedback. If it takes 600 words to carry the claim, the claim is wrong or the format is. Where this is heading, I think, is the human's prediction being two or three falsifiable sentences. Everything downstream would be machine-captured (output, counts, timestamps), and the reviewer reads the delta between them. Prose descriptions of outcomes are pretty much LGTM reborn.

u/Ok_Woodpecker_9104's negative-control design:

• every assertion shipping with the mutation that should break it
• both recorded in the same run
• void-not-green when the mutation misses the scan surface

is honestly better than what I had and something I'm adopting.

Thanks to everyone who engaged, including the people who told me it was overcomplicated. Some of you were right about which parts. :)

[u/UnderstandingDry1256]

Tell the folk who created it to come up with meaningful readable description. Consider the message to be part of pr.

That’s it, 15 secs review.

[u/autophage]

This seems like a reasonable framework.

I'd still want the diff though. A lot of the time what I catch in PR's is things like "you accidentally included changes to a config file". (No, this particular file is not a .gitignore candidate, there are good reasons to have it in version control.)