Hacker News Reader

Comment by ventana | Hacker News Reader

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!

View on HN

Yeah, that's a fun way to get massive stacks of PRs that are individually incomprehensible.

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.

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.

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.

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.

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

This is defeatism. Reject unacceptable PRs, full stop. It's on the author to work to break them up.

Luckily GitHub supports stacked PRs now! So they have to at least pass CI individually.

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.

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.

Reply on news.ycombinator.com