I've always had a tough time reviewing pull requests, and now that most of the code is generated by AI and then reviewed by AI, I feel like I have even less feedback than before.
We still do code review in my team, because we build systems that handle customers core business data and infrastructure. Other teams are generating and reviewing entirely by AI and their codebases are incredibly sloppy and verbose with little understanding, and these problems compound over time.
Our code review step achieves 3 things - check that it does what's intended, make sure it fits into the broader picture, and build human understanding of the codebase.
The question is: Do you require human understand what they're working on?
I'm watching dark factories and all-AI projects being developed elsewhere to see whats working, and thus far I don't think the evidence is there that for our constraints fully AI-developed and checked solutions pass the quality bar and have the traits we need. We have customers, they depend on us. From what I can see, unconstrained AI-based development of PoCs and implementations works for demos, startups, throwaway projects, but not for products you need to work and keep working.
I download the change into my local context and use AI to give me a high level overview of the change because people always sucked at describing their changes anyway
Then I go through a review it as normal and flag issues
Then I do a pass asking my own local agent to find any issues, I sanity check/confirm those and comment them if necessary
We still do code review but it does bottleneck how much we can ship.
I'm looking at what we can do to alleviate this. Here's what I think the near future may look like, as far as getting more value per time spent out of human code review. I'm coming at this from the POV of an org that still mandates human code review and I think a lot of this advice doesn't hold if you are all-in on the black box software factory approach.
If you're reviewing code you're responsible for, your job now is to provide stewardship of its long term technical direction even if you are not personally reading and writing every line of code. This means preserving the coherence of designs past and present, and ensuring that they are not being worked around in service of a short-term goal.
Stop reading every line of every file to look for minor gotchas. Focus on core business logic and whether invariants are modeled well & to the fullest extent possible by the type system. Agents are better than most humans now at spotting insane gotchas. At least one frontier model should review most PRs.
In the before times, you could argue that TODO comments and small nits on PRs should be addressed, because most likely they'd never get attention again once the PR merges. Now, small issues should instead be automatically recorded and followed up on quickly by another agent. Minor code quality issues should become rules, and old code should be constantly revisited by agents to bring it up to par with new code.
Provide the transcripts of the session with the PR. Look for evidence of thoughtful direction & solid premise behind the conversation that led to the PR being made. In addition to providing more mentorship opportunities to the author, this also discourages meat proxying/low-value-added uses of AI. Asking someone to rewrite or completely discard a PR that's just a bad idea should no longer be the ego bruising event it once was (at least with respect to the cost sunk to make the PR).
Require a higher bar for proving that code works and stays working. The testing pyramid IMO should now be more of a testing hourglass. Or an upside-down pyramid, even. Automated E2E tests should be abundant. Every feature should receive adequate manual testing by an agent, with proof of work.
In the vein of "stays working," look for good observability and guardrails around a feature. If you're doing a good job of leveraging agents to keep code up to par, this means adding (and cycling out) feature flags, ensuring that there's adequate logging, having a cohesive monitoring solution with good coverage, and a plan for what to do if there's an issue in prod.
Reviewing the code is that hard part, yes, but what kills me is that GitHub is still expecting us to review PRs the same way, and it’s increasingly stressful due to GitHub’s poor support for a good PR review experience, I ended up building my own solution (https://pyor.review) to make reviewing the code a lot less tedious and closer to how I want to look at code.
I built a review skill that has a bit of custom project knowledge, our team's agreed engineering standards & tools, a standardised format of how it should respond, and instructions to check for PR description accuracy as well as meeting all ticket requirements.
This kind of offloads the responsibility of deep code review to the planning phase: ensure the ticket spec is correct for what needs to be done on a product/team/technical level, and verifying daily CRUD app code is a breeze.
Human judgement of the review is still required, because the biggest way issues slip through is through one-slice "it looks fine in this issue" solutions that actually violate undocumented project knowledge.
To remedy that, we've started storing architectural decision records in the repo as well.
Basically, the more context you can give it (duh), the better agents are at review of day to day work.
Taking this approach helps in teams of varying skill levels to standardise the kind of expected feedback on PRs, as well as letting developers run a self-review whether they wrote some code with AI or not. There's still plenty of review feedback going back and forth on PRs, but our defect rates have gone down and clients have been happy with the output.
we're in a transition phase at my company. some teams already use AI reviewers, though humans still do the final review.
my team still relies entirely on human reviews, and I feel like it's becoming a bottleneck for shipping features.
interestingly, after some incidents, people ran AI reviews on the PRs that caused them, and even relatively lightweight models were able to catch the issues.
of course, that's with hindsight, but it makes me think AI review could be a pretty useful extra safety net rather than a replacement for human reviewers.
You can standardize your entire review flow across your team and have it match your QA requirements with zeroshot.sh - could be worth checking out. I am one of the maintainers.
19 comments
[ 2.2 ms ] story [ 6.1 ms ] threadOur code review step achieves 3 things - check that it does what's intended, make sure it fits into the broader picture, and build human understanding of the codebase.
The question is: Do you require human understand what they're working on?
I'm watching dark factories and all-AI projects being developed elsewhere to see whats working, and thus far I don't think the evidence is there that for our constraints fully AI-developed and checked solutions pass the quality bar and have the traits we need. We have customers, they depend on us. From what I can see, unconstrained AI-based development of PoCs and implementations works for demos, startups, throwaway projects, but not for products you need to work and keep working.
Then I go through a review it as normal and flag issues
Then I do a pass asking my own local agent to find any issues, I sanity check/confirm those and comment them if necessary
I'm looking at what we can do to alleviate this. Here's what I think the near future may look like, as far as getting more value per time spent out of human code review. I'm coming at this from the POV of an org that still mandates human code review and I think a lot of this advice doesn't hold if you are all-in on the black box software factory approach.
If you're reviewing code you're responsible for, your job now is to provide stewardship of its long term technical direction even if you are not personally reading and writing every line of code. This means preserving the coherence of designs past and present, and ensuring that they are not being worked around in service of a short-term goal.
Stop reading every line of every file to look for minor gotchas. Focus on core business logic and whether invariants are modeled well & to the fullest extent possible by the type system. Agents are better than most humans now at spotting insane gotchas. At least one frontier model should review most PRs.
In the before times, you could argue that TODO comments and small nits on PRs should be addressed, because most likely they'd never get attention again once the PR merges. Now, small issues should instead be automatically recorded and followed up on quickly by another agent. Minor code quality issues should become rules, and old code should be constantly revisited by agents to bring it up to par with new code.
Provide the transcripts of the session with the PR. Look for evidence of thoughtful direction & solid premise behind the conversation that led to the PR being made. In addition to providing more mentorship opportunities to the author, this also discourages meat proxying/low-value-added uses of AI. Asking someone to rewrite or completely discard a PR that's just a bad idea should no longer be the ego bruising event it once was (at least with respect to the cost sunk to make the PR).
Require a higher bar for proving that code works and stays working. The testing pyramid IMO should now be more of a testing hourglass. Or an upside-down pyramid, even. Automated E2E tests should be abundant. Every feature should receive adequate manual testing by an agent, with proof of work.
In the vein of "stays working," look for good observability and guardrails around a feature. If you're doing a good job of leveraging agents to keep code up to par, this means adding (and cycling out) feature flags, ensuring that there's adequate logging, having a cohesive monitoring solution with good coverage, and a plan for what to do if there's an issue in prod.
This kind of offloads the responsibility of deep code review to the planning phase: ensure the ticket spec is correct for what needs to be done on a product/team/technical level, and verifying daily CRUD app code is a breeze.
Human judgement of the review is still required, because the biggest way issues slip through is through one-slice "it looks fine in this issue" solutions that actually violate undocumented project knowledge.
To remedy that, we've started storing architectural decision records in the repo as well.
Basically, the more context you can give it (duh), the better agents are at review of day to day work.
Taking this approach helps in teams of varying skill levels to standardise the kind of expected feedback on PRs, as well as letting developers run a self-review whether they wrote some code with AI or not. There's still plenty of review feedback going back and forth on PRs, but our defect rates have gone down and clients have been happy with the output.
my team still relies entirely on human reviews, and I feel like it's becoming a bottleneck for shipping features.
interestingly, after some incidents, people ran AI reviews on the PRs that caused them, and even relatively lightweight models were able to catch the issues.
of course, that's with hindsight, but it makes me think AI review could be a pretty useful extra safety net rather than a replacement for human reviewers.