Just an idea which I haven't personally tried: AI agents understand technical limitations, such as CI failures. Maybe make a CI job which checks that a PR has a reasonable size, and auto-reject with a polite message if it's not? Something like, "This PR size exceeds the limit of N lines that we accept for review; if you implement a big feature please consider splitting it in several smaller PRs." There are chances that it won't help, but it might!
I know someone working on a smaller open source who has same thing. They have considered just blocking all PRs outside known contributors because AI spam even on their tiny open source project is too much.
At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
> why did you put it up for a human review at all then?
This seems to be the crux of the issue.
I'm guessing the most of the time, the answer is "because that's a mandatory gate to getting these changes into production". If the PR author doesn't see the value in review, it's going to be hard to convince them to write reviewable PRs.
If they're actually looking for human feedback, telling them how to submit PRs in a way that's amenable to human feedback is going to be a lot more successful.
In my experience the models perform substantially worse if asked to create small PRs or commits. They lack the ability to sequence work and understand dependencies efficiently enough to manage it – it's not that they can't do small PRs, it's that doing them takes vastly more resources which then hits context limits etc. And if you want to then go back and edit a stack of commits or PRs, rebasing work into the middle, that's even more. I don't think any of this scales linearly in the amount of code or number of commits.
This is all in addition to the fact that the models are generally poor at storytelling, because that requires a theory of mind of the person you're communicating with. Authoring for review is storytelling, it's making changes in such a way as to build confidence in the reviewer. I believe current LLMs are still years away from this.
In my opinion, if you can't do these things, you're just cosplaying software engineering. Vibe coding has its uses, as does LLM programming, I do a lot of this! But we're kidding ourselves and dropping our standards dangerously low if we think that this is software engineering.
> In my opinion, if you can't do these things, you're just cosplaying software engineering.
This standard would significantly reduce the number of software engineers I've worked with.
Not that I'm saying you're wrong about this; it was obvious even during my degree that "software engineers" were trying to have their cake and eat it with regards being counted as "being an engineer" without any of the real responsibility if the code you wrote was dangerously bad. Case study we went over at the time being https://en.wikipedia.org/wiki/LASCAD
> If your variable is not named well and you need a comment, name your variable better.
100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.
>I'm tired boss. I'm tired of reviewing one, two, three thousand line PRs because some agent was able to "one shot the whole issue." Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer.
100%
but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.
> Small PRs were never asked for because they're easier to write, it's always been for the benefit of the reviewer
I think they were asked before AI and even they were not easier to write.
Its same as with commits. Usually when implementing a new feature I'm just in flow, so I don't think how to properly separate changes to different commits.
I mean - not always, but usually maintaining git history in a beautiful and clean manner was extra work even before AI.
But the first thing I still check is consecutive comments and that goes very far as a signal whether the person sending it even tried to grok it or not
I really believe people who publish huge slop PRs (short of being fired) should have their tokens taxed on the basis that it's an unpriced cost on the colleagues and the firm
while we are at it, stop filling in the PR body with a mini novella of text generated by ai. they are hard to review and are unnecessarily verbose. the description should be there to benefit the reviewer.
I worked for a human for a while who complained the same way. Problem was: it was a religion for him, not based in any reasonable logic. The large PRs needed to be large because they were adding features that couldn't be half-pregnant. The feature needed to be implemented fully in order to demo to customers or management. Once you have the whole thing coded and working it makes no sense to artificially split it into smaller pieces. That's unnecessary work you're doing only to satisfy the bloke with the beef about large PRs.
Anyway, absolutely none of that had anything to do with LLMs -- it was a function of a person who liked to control other people as much as possible. With LLMs I find they positively like to attack problems in small pieces. I can't recall ever having to ask one to subdivide the work. They usually just do that anyway.
So ask the LLM to split it up into PRs of your preferred size. Or better yet, stop reviewing the code and review the working software instead. LLMs give far more substantive code reviews than humans and have for a while now.
It's possible to do a large amount of AI-assisted work, then do a second/third/fourth pass to break it up into a reviewable stack of self-contained PRs. But it takes time, and there's no such thing as one-shotting it. And it's basically impossible to continuously rebase manually without burning tokens. The way to merge the stack is more-or-less "stop the world".
However, I have yet to see how this will play out with upstream contribs:
Apparently I'm in the minority, but if a single chunk of functionality legitimately needs a thousand lines of code, I'd rather see it all up front than have to review 5 separate PRs that don't do anything until they're combined.
Y'all need to try PR review tools that split PRs into chapters. Stage was the best product in this space, but Linear Review has it, Graphite has it, and some other tools too. You get the full contexts while each piece is still reviewable individually.
It is reasonable to break PRs up into smaller chunks, but there is a limit. There are frequently reviewers who get very zealous about this and insist on breaking things up beyond what is reasonable, for example if breaking it up would destroy the intent, or if the "thousand line" PR just contains lots of tests (AI's love to write tests, and I love that they do that). Some tasks are just long, and its important to contextualise this when reviewing.
In the end though, these reviewers will die off like the dinosaurs. The article actually states that they find the idea of reviewing a large PR with AI bad because "it wastes your tokens reviewing a reingesting code that was already made by an AI". This doesn't make a whole lot of sense, AI will frequently reingest AI generated content, evals are a great example of this.
Just after this the article touches on the real issue at play: "okay, great, why did you put it up for a human review at all then?". Indeed, this is a good question to ask, why do we put it up for human review? I would wager that they don't actually want human feedback, a human has placed themselves as a gatekeeper and thus must be placated, and probably chooses the most inefficient way to keep that gate slowing everyone down who has kept up with the technology of our times.
Isn’t having humans review every PR only realistic for smaller systems? If AI is generating all these PRs, but humans still have to review every one of them, doesn’t that ultimately leave the humans responsible for everything?
I think this is more a symptom of the problem than the actual problem.
The issue is that we can generate tons of code using AI, but then are blocked on having humans review all of it.
I don’t think we should auto-approve all of this code without human review - that clearly doesn’t work either.
What I do think we need is probably at least two-fold
1) better ways to explain these big PRs to human reviewers.
2) better ways to verify the functionality of a piece of code. Things like auto generating walkthrough videos
I’m not sure that even this is enough. I’m sure there will be agents that try to solve this problem.
Agree with you. Going even further, I'm wondering whether the Diff alone is still the right review artifact...
On your 1), I see Entire.io is trying to build some useful building blocks (capturing all agent sessions and prompts to extract the human intent).
With a friend, we're currently working on a tool to ease the review using entire's agent session
38 comments
[ 0.20 ms ] story [ 29.5 ms ] threadFor OSS, my suggestion is to accept issues and specs do the implementation yourself. Warp.dev has a decent model of this in Github: https://github.com/warpdotdev/warp/blob/master/CONTRIBUTING....
At work, I've gotten into fights about PR approvals. If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
This seems to be the crux of the issue.
I'm guessing the most of the time, the answer is "because that's a mandatory gate to getting these changes into production". If the PR author doesn't see the value in review, it's going to be hard to convince them to write reviewable PRs.
If they're actually looking for human feedback, telling them how to submit PRs in a way that's amenable to human feedback is going to be a lot more successful.
This is all in addition to the fact that the models are generally poor at storytelling, because that requires a theory of mind of the person you're communicating with. Authoring for review is storytelling, it's making changes in such a way as to build confidence in the reviewer. I believe current LLMs are still years away from this.
In my opinion, if you can't do these things, you're just cosplaying software engineering. Vibe coding has its uses, as does LLM programming, I do a lot of this! But we're kidding ourselves and dropping our standards dangerously low if we think that this is software engineering.
This standard would significantly reduce the number of software engineers I've worked with.
Not that I'm saying you're wrong about this; it was obvious even during my degree that "software engineers" were trying to have their cake and eat it with regards being counted as "being an engineer" without any of the real responsibility if the code you wrote was dangerously bad. Case study we went over at the time being https://en.wikipedia.org/wiki/LASCAD
100% agree. While you are at it, consider naming and writing your functions in such a way that doesn't require a wall of comments. Clean Code uncle Bob style.
100%
but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.
I think they were asked before AI and even they were not easier to write.
Its same as with commits. Usually when implementing a new feature I'm just in flow, so I don't think how to properly separate changes to different commits.
I mean - not always, but usually maintaining git history in a beautiful and clean manner was extra work even before AI.
But the first thing I still check is consecutive comments and that goes very far as a signal whether the person sending it even tried to grok it or not
Anyway, absolutely none of that had anything to do with LLMs -- it was a function of a person who liked to control other people as much as possible. With LLMs I find they positively like to attack problems in small pieces. I can't recall ever having to ask one to subdivide the work. They usually just do that anyway.
However, I have yet to see how this will play out with upstream contribs:
https://github.com/moment/luxon/discussions/1796
https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches...
In the end though, these reviewers will die off like the dinosaurs. The article actually states that they find the idea of reviewing a large PR with AI bad because "it wastes your tokens reviewing a reingesting code that was already made by an AI". This doesn't make a whole lot of sense, AI will frequently reingest AI generated content, evals are a great example of this.
Just after this the article touches on the real issue at play: "okay, great, why did you put it up for a human review at all then?". Indeed, this is a good question to ask, why do we put it up for human review? I would wager that they don't actually want human feedback, a human has placed themselves as a gatekeeper and thus must be placated, and probably chooses the most inefficient way to keep that gate slowing everyone down who has kept up with the technology of our times.
The issue is that we can generate tons of code using AI, but then are blocked on having humans review all of it.
I don’t think we should auto-approve all of this code without human review - that clearly doesn’t work either.
What I do think we need is probably at least two-fold 1) better ways to explain these big PRs to human reviewers. 2) better ways to verify the functionality of a piece of code. Things like auto generating walkthrough videos
I’m not sure that even this is enough. I’m sure there will be agents that try to solve this problem.