back
104 comments
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

I have had some success with asking the LLM to rewrite the commit history after the feature was written. I often have a good idea of a good conceptual split, and the LLM has no problem doing the soul-crushing work of manually pulling out individual files or hunks into a sequence of commits and making sure each builds.

I need to do this before I can even review the code myself, and I review everything before I send it to my colleagues for their review.

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.
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.
A human should be able to take the work and break it up properly during rework though.
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.
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.

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_.

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.
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
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.
> 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.

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.

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".
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.

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.

> why do we put it up for human review? I would wager that [...] a human has placed themselves as a gatekeeper

Well, yes, presumably because the maintainers have an obligation to their users (or whoever else they answer to), so I doubt they're overly willing to relegate themselves to being a rubber stamp for whatever some LLM hallucinates as looking good to it.

>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.

As a maintainer just saying no and closing PRs is largely the solution
If you generate PRs too big to review for others, then they are too big to review for yourself. This means you are delegating the task of understanding the code to an LLM, and the end result is inevitably that noone in the organization understands the code better than someone who just walked in the door. They can write the next LLM prompt just as well as you because they know as little about the system as you.

In that situation I ask you: what is your moat as a software company? Why would your customers keep paying you when companies like Anthropic can just do your-software-company-as-a-service and cut out the middle man and six figure salaries?

> 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.

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!
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.
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.
There's also another approach: Not all PRs should introduce new, functioning features or complete rewrites. Thus you can just introduce a handful of functions, some of the functionality behind a feature flag, a new db schema... and then introduce the rest in follow-up PRs. GitHub even does stacked PRs for this reason, so you can start big, organize your commits into these chunks and do the stacked PRs.

Having AI generate massive code and shoving it to your teammates' plates should be considered irresponsible. Yes, test code is verbose, but it should be accompanied with an adequate description of what is currently tested. Otherwise it's just AI having fun in your codebase.

Automated PR merges could still work, please just don't with multiple thousands of LoC changes. Both LLMs and humans have a context size limit.

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.

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:

https://github.com/moment/luxon/discussions/1796

https://github.com/leeoniya/luxon/tree/leeoniya/perf-patches...

The whole point of using AI is to generate more code more quickly. If you cant keep up, then step out of the way. Some people think being a reviewer is a privileged position. Well now you've got your work cut out for you.

Ultimately the solution will be to get rid of reviews and reviewers, and put the responsibility for the code on the "author" (prompter). If you're using AI to generate code you're already mostly just a reviewer, putting more reviewers on the same code just slows everything down. What's needed is more accountability.

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.

We were forced to merge 2 HUGE PRs this week because of a customer deadline. It didn't need to be this way, but our AI maximalist team member went full toxenmaxxing and we had 92 frontend files and 40ish backend.

Untestable, touched nearly the entire application stack, had several trivialities that were huge sticking points in review that obscured other more substantive issues we should have spent review time on.

I recently started stacking my PRs, makes life easier

https://docs.github.com/en/pull-requests/how-tos/stacked-pul...

I'm surprises GitHub haven't added a mechanism to donate tokens to let a project maintainer inspect, verify, explain and test PRs.

For big PRs it would make a difference if I didn't have to check everything for malicious links and junk without spending my own money or time.

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.
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 felt this. My majority of time correcting claude is making it stop the stupid verbose doc comments which are completely contemporary. I can add skills/memories and claude.md hints that i want, but every time it continues with those unnecessary doc comments of 5+ lines explaining some situation which should definitely not be in a doc comment.
If the whole thing is really all or nothing (very rare), at least break it up into sensible commits that can be reviewed individually.
There's only one way out of this predicament. AI reviews. It's what we have to do.
> 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.

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.
"Yes, you're right to push back on that."
This to me reads less as there being some objective level when a PR becomes "huge" and more about the tension between a system's ability to absorb change vs. our tools' ability to create change.

By way of analogy consider the relative impact on an ecosystem of one person fishing with a fishing line vs. a commercial fishing boat trawling the ocean. Of course, one person fishing is unlikely to have a huge impact on the ocean so it's generally permitted. Trawling (agentic coding) can be done in a way that's destructive to ecosystems but it can also be done sustainably!

So with the analogy in mind let's bring back the "sensible trawling" idea to agentic coding. What might it look like to solve the "huge PR bad" constraint in another way: by increasing our codebases' ability to absorb change, so what "a huge PR" is, becomes bigger?

Probably needs solves at many levels: assistance quickly comprehending the PR (AI driven walkthroughs, multiple media expected from the PR submitter not just text - eg. a screencast walkthrough of it), it requires rethinking how the code is read (better review tooling); it requires integrations with code-review automation tools (both you home-grown checklist and third-party tools) it requires rigorous testing (comprehensive automated e2e; test-driven; functional tests; etc); it requires putting the actual "in the loop" so post-release fast-follows can be expedited (eg. product signals and Sentry and metric anomalies are fed back in for quick follow up releases)

If you can be so much more responsive to the customer and market. Eg. you can unlaunch features just as easily as you launched them - and you can finally clean up all that tech debt. Better for the business better for the codebase and better for developer happiness.

Not all these strategies work for every situation - you can't do post-release in the loop if the shit needs to work first time! But the whole idea creates so much richness in applying human judgment and engineering solutions and it's all brand new because we never needed to deal with this much change before

Think of it as "releases in the loop".

If opportunities to rethink the stack to support MORE change excite you, congratulations! You're ready for the future that's coming. If you don't like this - get yourself into a job where you can say no a lot, or where shit needs to work first time, and you can be happy. Test-driven, strongly reviewed.. there's ways with agentic coding to also make super high quality stuff. But you can also shoot product from the hip more accurately and more often than ever before.

It won't be applied correctly everywhere - it's still heavily judgmental dependent and we're all fallible - but there'll be a much wider spectrum of options for how to build products. I think this is a really exciting future!

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
Paginate atomic commit diffs
Stumbled upon this the other day via my LinkedIn and now sanely look the jst code https://chromewebstore.google.com/detail/github-pr-focus/peb...

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

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.

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....

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.