Ask HN: How do you code review? | Hacker News Reader

Ask HN: How do you code review?

9 pointsidose9 comments

In the past year, from my personal experience in 2 different companies, the amount of PRs has increased dramatically, and each PR is getting much bigger.

How do you approach code review when dealing with large and high volume AI-generated code?

View on HN

We did some measurement at the beginning of the year and found that our PRs were getting bigger and review time was coming down even without us doing anything, which is not a good sign.

We ended up moving peer review to the implementation plan rather than the PR, then having a loop that validates the code against the plan when the PR is raised. That way the agent gets a CI failure if it deviates from the plan, which it then fixes or acknowledges. Anything with no differences gets merged without human review, differences get approved by the original person who peer reviewed the plan.

I puled some stats the other day for a presentation I'm working on about what we did:

Matched plan on first pass: 25% (169/677) Had differences: 75% (508/677)

Differences per PR: Median 2 Mean 2.60 P90 6 Max 20

1,757 findings:

- Missing (skipped planned work): 44.6% of findings, 51.7% of PRs - Changed (done differently): 42.7% of findings, 51.3% of PRs - Beyond (extra, still in scope): 9.1% of findings, 19.8% of PRs - Scope (unplanned feature): 3.6% of findings, 6.6% of PRs

This is interesting, are you using internal custom made tools / protocols for this workflow or is it some publicly available product?

Review is the bottleneck in those scenarios - and it's a _good thing_.

AI-gen code is often subtly (or obviously) incorrect. Human eyeballs are the cure. There is no way to go faster without compromising the value of the review.

What I've been super annoyed about recently is the number of times I've seen a completely stupid question on a PR - eg "why are you doing this?" when there's a comment right above that line about the why. Or "where else is this used?" - we all have IDEs and reviewers obviously have access to the code - stop being a lazy potato and check out the code locally.

I try to avoid adding these lazy people to PRs as they create more work than they help. But sometimes they end up on there anyway, because they're "important".

Have you tried reviewing code on https://pyor.review instead of checking out the code locally?

One approach is to maximize test coverage and automate code review. That way you have confidence that things aren't breaking and that you are vetting the code going into the codebase. It's a lot of work to get a system you can trust up and running, but it's doable. As we move further towards "factories" this is going to be the way, in my opinion.

first the automated part, that makes a pr "review ready", this helps us alot to gain confidence.

1. Automated tests that actually test real cases 2. Automated tests to test for regressions 3. Automated end-to-end tests, to make sure all core functions (and new functions still work and ui flows are stable / correct) 4. automated ai reviews (we use strix for security and a small Copilot SDK harness) 5. linters (for go: gosec,govuln) and formatters

For the human review process, we try to "never do big refactorings out of the blue", normally we do this in pair, or in tight loops. We dont have special reviewer roles, every dev can review any pr, we think, that everyone in a team is responsible for the product (and its quality, this also helps alot with knowledge transfare, we have retros and dojos to keep everyone on the same levle and align our teams "taste").

My favourite way for relatively large PRs:

Read code, and then build it and run manually in my IDE against some ad-hoc test cases (unit test cases should be automated). Then check for styles and documentation.

My real way for any PR nowadays because we want vElOciTY:

"LGTM".

if this is a problem there is a problem in alignment in management or teams.

one team uses AI to create larger volume of work, but another does not use AI for whatever reason.

its really asking for problems.

either both should use it or neither.

if both is not perceived to be ok for quality reasons, then use neither.

From my experience all teams have used AI.

It’s just that the throughput and quality of review and implementation changes between teams and individuals.

Did you try any workflow that systematize this?

Reply on news.ycombinator.com