Rendered at 04:18:30 GMT+0000 (Coordinated Universal Time) with Cloudflare Workers.
danpalmer 4 hours ago [-]
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.
garfij 35 minutes ago [-]
I have a completely different experience than you, but I have also invested quite a bit of effort into the issue. I will agree that with simple prompting and without reminders, models do not do this sort of thing well or consistently. On the other hand I have two skills that I use regularly `/atomic-commits` and `/split-branch` that do exactly what you're describing. It has been an exercise in taking the time to put to words the concepts you're describing, which has been very interesting because it's forced me to put to words many thoughts on the subject I had never formalized, and you're correct, there's a lot of theory of mind and considering of other people's perspectives that go into it.
* Does the series of commits tell a cohesive story of the feature, each commit building on the last one? It's surprisingly easy to have an ordering of commits where each one builds, but the ordering is nonsense.
* Is each commit scoped to the right size? Not too big of course, describes a single idea or concept. That same rule can also lead to commits that are comically small or separates two changes that can obviously be argued are "two different things" but that make more sense when seen together as a single conceptual change.
* When is it appropriate to split up disparate changes within a single file, interwoven together, and how to correctly orchestrate that with git commands.
I care _deeply_ about commit hygiene and presentation because it's a foundation for making code review a smooth process and in the Before Times™ I would spend a _substantial_ amount of time reworking my own commits to ensure they were up to my own standards. I can count on two hands the number of commits I've authored on my own in the last year, and the process is immediately replicated and standardized across our entire engineering organization.
It is perfect 100% of the time? No, every once in a while I'll hit some weird case and need to issue some corrective instructions, or more likely ask the model why things are like that to see if there's tuning we need to do.
Commits are effectively like automated code-formatting now. This isn't years in the future, this is already _old news_.
packetlost 20 minutes ago [-]
I've also had a ton of luck getting agents to make focused, (mostly) well crafted commits using my home-grown git-based task manager: https://github.com/chiefnoah/tsk
It's very easy to be like: /tsk <big prompt> . Break down the problem into focused tasks using tsk, include all context necessary to complete a task in the tasks body, then prioritize them. Then begin working on them in priority order until complete.
Works 9/10 for me, though I often split up the instructions a bit so I have time to review the resulting tasks/design. tsk itself encourages creating single commits per task because it tracks the commit a task is closed on and the agents are pretty good about doing that.
gosolozero 3 hours ago [-]
I think LLMs are actually great at breaking up work into workstreams and tasks. The issue is that tasks do not equal good PR stacks on their own. You’ll need to use feature slices instead (like do backend part -> PR it, etc). LLMs can do that, but you need to harness them into it.
Twirrim 3 hours ago [-]
I've had _some_ success by asking Codex write a full plan, broken up in to logical phases, complete with git commit points along the way.
It's probably not quite how I would approach doing git commits, but they're at least logical boundaries, and make narrative sense for a reviewer.
singpolyma3 3 hours ago [-]
A human should be able to take the work and break it up properly during rework though.
socalgal2 3 hours ago [-]
I've had no trouble getting small PRs. No idea if I'm doing something different than you or the things I asked for just happened to be small. Next time I get a big one I'll consider trying to ask the AI to break it up after it's finished.
t-writescode 4 hours ago [-]
I mean, sometimes I don’t know how I want to write something until I’m finished. Huge refactors are often like this.
So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context.
It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing this back to you and you’re asked to review it.
jghn 4 hours ago [-]
This is exactly how I have always worked. I personally don't operate well breaking things up into small, bite sized PRs like people want. So instead I do the whole big project, and then break it apart into logical segments for my colleagues.
And this is often how I proceed when working with an LLM as well. We build the whole thing, and if I think it needs to be split apart, then in another session I work with the LLM to break it apart into reasonable chunks.
mattm 4 hours ago [-]
> sometimes I don’t know how I want to write something until I’m finished
This is knowledge that goes back to the beginning of software development - "Plan to throw [version] one away".
I think this could potentially become a good practice. LLMs make it so easy and cheap to just get it working and build that v1. Then you can play around with it and see if works and read the code about what could be better. Throw away the LLM generated version and now this is the part where human expertise comes in. Based on what you've learned from the v1, now guide the LLM more closely about how to write the thing and help guide it so that making small PRs that are easily reviewable and understandable are the output.
gavinhoward 1 hours ago [-]
This is the first time I've heard a use for LLMs that I think might be good.
trezm 3 hours ago [-]
Yes, totally. This is how I work as well! Frankly, it seems like LLMs are pretty good if you tell them after the fact to divide work like this too, "make stacked prs with model a, then model b, then service c" has done wonders for my mental health.
lokar 3 hours ago [-]
And then some of the time while breaking it up you realize you should have done it another way.
nradov 1 hours ago [-]
Most professional software developers (probably including you) never do any work that could legitimately be described as engineering. And that's fine because most software isn't very important anyway. There's very little software that requires true engineering practices. The exceptions are in domains like avionics and medical devices.
ls-a 1 hours ago [-]
I can't be happier seeing how many posts are about AI making PR terrorists unhappy. They can no longer bully their teammates so they rant about it here
usewik 4 hours ago [-]
Agreed. Asking for small PRs or commits can backfire, unless the work is deliberately scoped into smaller pieces from the beginning. This requires a human design review and planning and is one of the reasons I don't outsource that part to an agent.
jacquesm 2 hours ago [-]
That's just like humans though. Find me a developer that loves maintaining code and I'll show you 10 that love rewrites or greenfield.
gensym 4 hours ago [-]
> 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.
lokar 3 hours ago [-]
I would back up. If leadership is not committed to real reviews, it’s not your job to make them happen. Don’t try to fight an impossible fight no one cares about.
Personally, I would leave. But that’s not always an option for everyone.
skydhash 3 hours ago [-]
Pretty much this. In OSS, review is mostly about convincing the others that your change is good and useful enough to merge. In corporate, it's seen as a blocker to change the ticket status to done. The vibe of the latter is mostly "it's working on my computer, approve it so that we can reach the quota for the sprint".
dlevine 16 minutes ago [-]
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.
sackfield 1 hours ago [-]
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.
throwaway2037 35 minutes ago [-]
I like this comment. I also have a lot of experience with lazy (my accusation!) reviewers who ask me to split a PR into smaller PRs. At some jobs, I felt like it was a strategy to sabotage my work (slow me down).
In my experience, the best was to ensure your code will be approved (ok, maybe some minor tweaks) is to assign a code reviewer before you start writing code. You (the coder) performs some quick analysis, then formulates a plan for how to fix the bug or implement a change or new feature. You verbally discuss this plan using screenshare (or side-by-side in-person) so both of you can see the code that will be changed. The code reviewer needs to verbally approve your plan. This way, you don't spend a lot of time writing code and tests, only to have the reviewer rejected it very quickly: "You should have done it this way instead." (Please note: That process that I described is intentionally informal, casual, and unrigid. Why? This grants permission for the coder and reviewer to decide the plan as intelligent adults, not as "Children of JIRA" [hint: unnecessary formality].)
Another thing that I do: After I write the code, but before I write tests, I ask the reviewer to review, but not approve. After looking at their comments, I quickly add another commit to the PR to address their concerns, then start work on the tests. When the reviewer does the final review, it is very quick, like 10% of the initial review. All of this really helps to reduce coder/reviewer friction, and nearly eliminate "Tyranny of the Reviewer". One last gripe about code reviewing: It hardly makes sense for someone much more junior than the coder to do the review. I have seen this too many times.
bawolff 4 hours ago [-]
>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.
sipjca 3 hours ago [-]
As a maintainer just saying no and closing PRs is largely the solution
usewik 4 hours ago [-]
> 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.
throwaway2037 32 minutes ago [-]
> Clean Code [U]ncle Bob style
Without starting a flame war, in 2026, is this still really a thing? I cannot recall any developer that I thought was excellent ever quoting "Clean Code [U]ncle Bob style" as gospel for how to write your code. There are just so many silly rules that he touts.
t-writescode 4 hours ago [-]
Indeed. If you’re going to have an essay on top of a function or anywhere in code, earn the essay. That code better be operating on a ton of assumptions or using some creative logic to get to how it is that a simple reading doesn’t make sense.
I’ve done it myself on:
* engine definitions for complex workflows and DSLs
* heavy graph theory sections that included ASCII diagrams to clarify flow.
But those functions are probably 1 in 100 or rarer. Basically everything else is good enough with basic IDE-helping javadoc style comments at best, maybe with some input parameter clarification and business logic-clarifying 1-2 line comments sprinkled throughout.
ventana 5 hours ago [-]
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!
ok_computer 4 hours ago [-]
That message could also be generated by a PR line count rule and string replacement without the ai overhead.
Edit - apologies I misunderstood which side the ai agent should be on.
throwaway2037 30 minutes ago [-]
You raise a good point here. Why do we need a CI job? Just tell the AI/LLM: The max number of lines per PR is X. If you need more lines, please create a chain of PRs. That should work OK.
striking 4 hours ago [-]
Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.
wiml 4 hours ago [-]
Then reject them for being incomprehensible?
Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
striking 4 hours ago [-]
I do think code review is worthwhile, not sure how you read that from my comment.
A cap on PR size isn't inherently going to make an LLM do a good job of segmenting PRs. It requires careful prompting or manual action, the kind of effort typically exerted by people who already cared enough not to hit such a cap. You may as well just ditch the cap, to save yourself from having to reject a series of PRs rather than just the one.
paimapi 2 hours ago [-]
is there a reason why there's no standardization in orgs in terms of skills/harnesses/etc for AI-assisted development? for example, a rule of 'you must invoke ABC skill that contains all of the context for this part of the codebase if you plan on making changes there' with the codeowning team dedicated to maintaining it both for their own use and for the use of other teams that have up or downstream dependencies
t-writescode 4 hours ago [-]
How? If one PR builds off another, won’t either:
PR 1 is size 400
PR 2 is size 400 + 400 new
PR 3 is size 800 + 400 new
If they’re truly disjoint, would it be so bad to get them as unique? Because otherwise, when PRs depend on each other, you tend to get “one and then one and then one”.
At least that’s how it’s worked on teams I’ve worked on that have soft size limits.
eek2121 4 hours ago [-]
Size is an issue, but it isn't just about size. Ideally, agile development builds linearly in complexity. Rather than dumping a huge new feature, first introduce the building blocks and the reason you are introducing them, then the glue that ties them together, then the actual feature.
From what I've seen (not in software dev anymore, however I've been in it for close to 30 years), AI just tends to pile everything in, and it is very hard to review. No public model performs even average under the rules I've mentioned.
Also, simply breaking up a PR doesn't count if instead you dump all the PRs on maintainers at once. Humans are the bottleneck here, and can only review so much at once. If i were still involved in PR reviews, it doesn't matter if you gave me a single 4,000 line PR or 4 1,000 line PRs, I"d reject them.
What I want to see. Small, easily reviewable features with a build up to the main course, along with a good explanation for each. After that? I'd probably still reject it for a breach of code standards, or documentation, or because I don't like you sending me a PR at 4:59pm on a Friday. ;)
Humans also can't blindly rely on AI for review, so the models (more precisely, the folks building the underlying stuff) must adapt.
IshKebab 57 minutes ago [-]
Luckily GitHub supports stacked PRs now! So they have to at least pass CI individually.
dpc94 3 hours ago [-]
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.
leeoniya 2 hours ago [-]
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:
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.
sqemo 1 hours ago [-]
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?
orangecat 1 hours ago [-]
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.
stackskipton 4 hours ago [-]
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.
Seattle3503 32 minutes ago [-]
> If they are beyond us humans to review, screw it, remove the approver requirement and if CI passes, merge it.
Approvals are probably required by SOC 2 or other compliance requirements.
throwatdem12311 4 hours ago [-]
CI by itself is not got enough because LLMs are extremely good at writing vacuous tests that don’t actually test anything but look like the test something.
Even worse: they can write tests that make incorrect behavior part of your spec.
Tests matter.
Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application code anymore, you should at least be going over the tests with a fine toothed comb.
stackskipton 4 hours ago [-]
It's all we got at this point. Even as SRE, I just got 2000-line Golang change to something I think should be 150. However, the boss is already bouncing around happy we are going to deliver something that's been in Jira backlog for 9 months.
throwatdem12311 3 hours ago [-]
Why even bother then? Just feed Jira tickets into Claude Code and have it write the code, open the PRs have Claude in a GitHub action that does a code review on PRs, a routine that resolves the reviews, rebases the code and fixes conflicts and finally another that just merges anything that’s green in CI, no outstanding review and no conflicts. Then just spin in your chair whistling all day I guess. Surely your boss will be ecstatic.
stackskipton 3 hours ago [-]
Pretty sure that's what a group in my company is working on now.
Except, I won't be spinning in my chair, I'll be out of a job. At least until cost skyrockets and outages get much worse.
3 hours ago [-]
lokar 3 hours ago [-]
If the whole thing is really all or nothing (very rare), at least break it up into sensible commits that can be reviewed individually.
singpolyma3 3 hours ago [-]
If it can be broken into commits it's not all or nothing is it :)
lokar 2 hours ago [-]
You could allow commits in the same PR that must all go together, and then squash them before you merge
willswire 1 hours ago [-]
Paginate atomic commit diffs
thunkle 3 hours ago [-]
There's only one way out of this predicament. AI reviews. It's what we have to do.
the_sleaze_ 3 hours ago [-]
Who audits the reviewer?
ovao 3 hours ago [-]
Adversarial review is a thing, but it’s not the thing. Slop begets slop.
4lx87 3 hours ago [-]
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.
feelamee 3 hours ago [-]
> 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.
esikich 3 hours ago [-]
Yeah the one and only time I attempted an open source PR, it was for a performance improvement for one small part of the software but touched a zillion files. After looking at the PR I decided not to submit it because it just looked like a mess and I didn't really know how to split that sort of thing up at the time. AI might make this sort of thing more common, but it's certainly not new.
skydhash 3 hours ago [-]
Then you may need to improve your git-fu (or $vcs-fu). I use magit, so it's always easier to select only the lines/hunks/files that is for one specific change, stage and commit that. Before magit, I use sublime merge, Intellij vcs feature, and fugitive.
My flow state is for editing files. Once that's done and I've got something that work. It's always easy to convert those into sensible commits. Do not that the logs is not the like of "write database schema * add the index page * add the details page * add the new object form". They're more like "show the list of objects * allow object creation * show the details of a specific object". Those breaks to create the commits are more natural to the general flow state.
mhh__ 3 hours ago [-]
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
ls-a 1 hours ago [-]
There is spelling police, and there are PR terrorists.
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
esafak 4 hours ago [-]
A simple solution is to use a git hook that asks for confirmation if it is too big, with a suggestion to ask the user to have the agent split it up.
Yes could be a pipe failure. Commonly used for coverage or security concerns, it could be also used for PR size.
biglyburrito 2 hours ago [-]
"Yes, you're right to push back on that."
dboreham 3 hours ago [-]
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.
throwaway2037 25 minutes ago [-]
> The large PRs needed to be large because they were adding features that couldn't be half-pregnant.
I'm not convinced here. I have worked on a wide variety of large, complex software systems throughout my long career. Never once could we not stage a large new feature using multiple PRs and feature flags. And before you pushback, remember that Google Chromium, which is objectively one of the largest and most complex open source projects in history, makes extensive use of this strategy for rolling out large features. See: chrome://flags/
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.
* Does the series of commits tell a cohesive story of the feature, each commit building on the last one? It's surprisingly easy to have an ordering of commits where each one builds, but the ordering is nonsense. * Is each commit scoped to the right size? Not too big of course, describes a single idea or concept. That same rule can also lead to commits that are comically small or separates two changes that can obviously be argued are "two different things" but that make more sense when seen together as a single conceptual change. * When is it appropriate to split up disparate changes within a single file, interwoven together, and how to correctly orchestrate that with git commands.
I care _deeply_ about commit hygiene and presentation because it's a foundation for making code review a smooth process and in the Before Times™ I would spend a _substantial_ amount of time reworking my own commits to ensure they were up to my own standards. I can count on two hands the number of commits I've authored on my own in the last year, and the process is immediately replicated and standardized across our entire engineering organization.
It is perfect 100% of the time? No, every once in a while I'll hit some weird case and need to issue some corrective instructions, or more likely ask the model why things are like that to see if there's tuning we need to do.
Commits are effectively like automated code-formatting now. This isn't years in the future, this is already _old news_.
It's very easy to be like: /tsk <big prompt> . Break down the problem into focused tasks using tsk, include all context necessary to complete a task in the tasks body, then prioritize them. Then begin working on them in priority order until complete.
Works 9/10 for me, though I often split up the instructions a bit so I have time to review the resulting tasks/design. tsk itself encourages creating single commits per task because it tracks the commit a task is closed on and the agents are pretty good about doing that.
It's probably not quite how I would approach doing git commits, but they're at least logical boundaries, and make narrative sense for a reviewer.
So, just like you said, rewrite the whole thing, THEN break it apart into bite size chunks that tell the story and feed it to others with acceptable and reasonable context.
It’s a skill that engineers need, and it pays dividends to all on the team, including you, when your coworkers ALSO start doing this back to you and you’re asked to review it.
And this is often how I proceed when working with an LLM as well. We build the whole thing, and if I think it needs to be split apart, then in another session I work with the LLM to break it apart into reasonable chunks.
This is knowledge that goes back to the beginning of software development - "Plan to throw [version] one away".
I think this could potentially become a good practice. LLMs make it so easy and cheap to just get it working and build that v1. Then you can play around with it and see if works and read the code about what could be better. Throw away the LLM generated version and now this is the part where human expertise comes in. Based on what you've learned from the v1, now guide the LLM more closely about how to write the thing and help guide it so that making small PRs that are easily reviewable and understandable are the output.
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.
Personally, I would leave. But that’s not always an option for everyone.
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.
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.
In my experience, the best was to ensure your code will be approved (ok, maybe some minor tweaks) is to assign a code reviewer before you start writing code. You (the coder) performs some quick analysis, then formulates a plan for how to fix the bug or implement a change or new feature. You verbally discuss this plan using screenshare (or side-by-side in-person) so both of you can see the code that will be changed. The code reviewer needs to verbally approve your plan. This way, you don't spend a lot of time writing code and tests, only to have the reviewer rejected it very quickly: "You should have done it this way instead." (Please note: That process that I described is intentionally informal, casual, and unrigid. Why? This grants permission for the coder and reviewer to decide the plan as intelligent adults, not as "Children of JIRA" [hint: unnecessary formality].)
Another thing that I do: After I write the code, but before I write tests, I ask the reviewer to review, but not approve. After looking at their comments, I quickly add another commit to the PR to address their concerns, then start work on the tests. When the reviewer does the final review, it is very quick, like 10% of the initial review. All of this really helps to reduce coder/reviewer friction, and nearly eliminate "Tyranny of the Reviewer". One last gripe about code reviewing: It hardly makes sense for someone much more junior than the coder to do the review. I have seen this too many times.
100%
but also "no" is a two letter word and one of the most important and hardest parts of being a maintainer.
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’ve done it myself on:
* engine definitions for complex workflows and DSLs
* heavy graph theory sections that included ASCII diagrams to clarify flow.
But those functions are probably 1 in 100 or rarer. Basically everything else is good enough with basic IDE-helping javadoc style comments at best, maybe with some input parameter clarification and business logic-clarifying 1-2 line comments sprinkled throughout.
Edit - apologies I misunderstood which side the ai agent should be on.
Look, if you don't think code review is worthwhile, don't do it. Just give everybody unfettered permission to merge. But don't pretend to do review if you're not trying to maintain some standard of quality.
A cap on PR size isn't inherently going to make an LLM do a good job of segmenting PRs. It requires careful prompting or manual action, the kind of effort typically exerted by people who already cared enough not to hit such a cap. You may as well just ditch the cap, to save yourself from having to reject a series of PRs rather than just the one.
At least that’s how it’s worked on teams I’ve worked on that have soft size limits.
From what I've seen (not in software dev anymore, however I've been in it for close to 30 years), AI just tends to pile everything in, and it is very hard to review. No public model performs even average under the rules I've mentioned.
Also, simply breaking up a PR doesn't count if instead you dump all the PRs on maintainers at once. Humans are the bottleneck here, and can only review so much at once. If i were still involved in PR reviews, it doesn't matter if you gave me a single 4,000 line PR or 4 1,000 line PRs, I"d reject them.
What I want to see. Small, easily reviewable features with a build up to the main course, along with a good explanation for each. After that? I'd probably still reject it for a breach of code standards, or documentation, or because I don't like you sending me a PR at 4:59pm on a Friday. ;)
Humans also can't blindly rely on AI for review, so the models (more precisely, the folks building the underlying stuff) must adapt.
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...
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.
Approvals are probably required by SOC 2 or other compliance requirements.
Even worse: they can write tests that make incorrect behavior part of your spec.
Tests matter.
Writing tests can be hard, boring, tedious. But if anything should still be written by hand in the age of LLMs it’s the tests. If you’re not looking at the application code anymore, you should at least be going over the tests with a fine toothed comb.
Except, I won't be spinning in my chair, I'll be out of a job. At least until cost skyrockets and outages get much worse.
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.
My flow state is for editing files. Once that's done and I've got something that work. It's always easy to convert those into sensible commits. Do not that the logs is not the like of "write database schema * add the index page * add the details page * add the new object form". They're more like "show the list of objects * allow object creation * show the details of a specific object". Those breaks to create the commits are more natural to the general flow state.
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
For 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....
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.
"If there is a will, there is a way."