Call for a style guide meeting every time you see feedback like this, and write down what everybody agrees on. You’ll never have to do it again after 3-4 of those. Problem solved.
Still not solved? Guess it was really about the commas and not the value delivered anyway, so do whatever you feel like.
That meeting sounds like it would be the most bikeshed-y thing imaginable.
What you actually need to do is have a manager lead assert that it’s happening in a top down way and just run the formatter with the defaults. Let people argue case-by-case on what to change after that.
There's been a lot of talk about the purpose of code review recently. It makes sense in the face of AI. Heres a link that was submitted a little while ago: https://mathstodon.xyz/@mjd/115096720350507897
And in response I wrote a non-exhaustive checklist of things that a code review can look for:
- Does it functionally achieve what it sets out to (as per tacker issue or PR description)?
- Does it have extraneous code? Leftover debug prints, private API keys etc...
- Does it have any obvious defects? Memory leaks, un-handled edge cases, security flaws, obsolete API calls, etc...
- Could it be more understandable? Add/remove abstractions, better variable/method names, more/less functional etc...
- Is the style consistent with the codebase and/or style guidelines?
- Are there obvious performance improvements? Hashset instead of list, lazy evaluations, etc...
- Is it sufficiently well tested?
I think LLMs are okay at most of these, and worst at the first.
The thing is, leadership relies on the people actually building the software to provide concrete, accurate feedback about cost. Without that they have no chance to do a decent cost/benefit analysis.
But AI has engendered a collapse in developers’ ability to actually do that. Those of us who are stuck on the vibecoding bandwagon have lost the comprehensive understanding of the systems under our care that we need to understand and explain the quality and maintenance implications of a change.
Worse, if you happen to lose your mind and suggest the initial development cost is anything more than ~zero, your friendly neighborhood Claude keener will publicly shame you for not having sufficient faith in the Glorious Agentic Future. Product leadership will then have no choice but to side with them, not necessarily because they agree, but because they, too, are aware that we’re still in the phase of the hype cycle where openly questioning said hype is a career-limiting move.
“Net new” is one it seems to be particularly bad at.
I don’t think I have ever even once seen an LLM solve a problem related to overengineering by simply removing the overengineering. They always choose to add more epicycles and further compound the complexity.
I actually think this might be a false positive. I think it got tripped up by the higher than average use of jargon (which I don't mind here because the article itself flowed well and raised good points).
Gpt-zero scores "human", and I've always found it to be a better judge
code reviews are just a gateway that can be whatever you want it to be, and is kind of legacy human coder thing now. At its basics it was a point to catch problems that humans were likely to make / would more likely make if they knew there wasn't a review. Now you can target it for AI mistakes. You can build your code review skills (AI skill) to be incredibly thorough. The points made in the article don't really seem like things you need to do at the "legacy" gateway of code review. Things are different now. Code is cheap. Validation, Product Coherence, Governance need to be done early and throughout.
In my experience, automated code review is more pointless than ever.
We have all the linters, tests, and AI writing code for us. I don’t need the left hand to tell the right hand it did a good job. I’m very certain my code runs when I push the PR.
What I need now is architectural, long-horizon and business perspective.
No, no they can’t. I use AI a lot and I get a lot of value from it but they are still terrible at programming “in the large”, by which I mean slotting features into the place meant for them in the existing code base. When coding and reviewing their view is too local. They will implement a change in the first place that looks feasible when coding. When reviewing they will not look for code duplication or fit. They will review for correctness, performance, security and style but not for architectural coherence.
They also have shockingly weak ability to identify business acceptance criteria that are completely missing in the implementation or test coverage.
For those of us who do utilize the pre-existing tools and the coding agents effectively, yeah automated code reviews eventually become redundant. But they're still around because there are a lot of devs who don't even glance at what the agents are writing for them. Maybe partly because they don't care to streamline their workflow, or maybe because they've never been particularly good at writing code, and don't actually understand what the agent is giving them.
Lately I've seen some pretty glaringly obvious issues caught during the preliminary automated code review, and the issues seem to be coming from individuals who don't actually understand what the code is doing.
For the rest of us who are actually using the whole stack effectively, the automated code review is essentially a CI gate to protect the repo from the devs who don't know what they're doing.
But they're still around because there are a lot of devs who don't even glance at what the agents are writing for them
But how does automated AI code review help, here? Doesn’t it just reinforce that they don’t need to look at it (or change their habits), because the AI review will catch the issues?
I disagree. Most of my prompts are “implement <issue-id>”. The issue has a user story and acceptance criteria that the LLM uses to write a plan and ultimately produce code. None of that matters since the code artifact is what actually gets pushed in the repo, thus the code is what should be reviewed.
I believe code review is important (the article articulates some of the reasons) but I have to admit, having different models review a PR before passing to a human has proven valuable.
A defense of human code review I wish I saw more often, especially in light of the concerns people have about cognitive/comprehension debt: comprehension redundancy. At the end, if taken seriously, at least two people understand how the feature works (even if that number is, on average, trending closer to between one and zero). Ideally at least one of the two also comes away with a better understanding of the wider system and how the feature fits into or stands out from that landscape.
If only one person knows the code, then PR time isn't going to save you.
I'm tech lead and I basically don't review PRs, and I tell people this, with a caveat - if you can tell me what you specifically want me to review, for what specific purpose, I'm happy to!
So "can you review this bit for race conditions" is great, love it. This forces people to actually think about what in their code they should be suspicious of, if anything.
"Can you review this" [link to PR] is getting a rubber stamp because humans have never been good enough at "just spotting bugs" to make this worth it and now any LLM is better than a human.
And for the purpose of understanding - PR time is too late. I have not reviewed a PR ("for real") in a long time and yet I could tell you how every system my people have built works down to a very fine level of detail. And it's because _we talk to each other!_ We don't just chill in the same slack channel and code independently, we all value each others brains and want each others inputs because we know it will improve our product and we value what perspectives others will bring.
Trying to learn via PR is a sad substitute for real collaboration and teamwork.
I consider PRs to be primarily a defense of the architecture, and to a lesser degree a general sanity check. It’s also a useful opportunity to enforce automations are being run
I consider 99% of my "defense of the architecture" strategy to be teaching my team why the architecture is important, how to think about it, and invite they commentary on it as we own and evolve it together. And of course if they are doing something and want input or are uncertain, then my door is open.
If PRs are a notable part of my architecture defense, I'm going to work on investing in the team instead of reviewing PRs.
I follow mailing lists (emacs and openbsd) and sending a diff is kinda the boundary between wishing for something and making the something into a thing. It’s the difference between discussing a plot and discussinf a draft.
Sneding a PR should not be for understanding or just for rubber stamping. It’s about getting someone to look at your approach and helping you find flaws or proposing ideas that could make it better.
When I review PR, the primary question is: For the stated problem, is the diff a good solution? Sometimes I don’t know enough about the problem, so I just try to see if the code has glaring mistakes (mispellings, styles,…) but those are just comments, not suggestions.
THANK YOU. I’ve always felt this way about PR review. I feel like people
should write a natural
language description of the change, and every section of it should link to part of the diff, and every part of the diff should be linked to by part of the description. That or just leave a comment on every chunk of the diff.
What makes you think so?
It's not my experience, so I'd like to hear what kind of software you work with/teams and what is leading you to understand that code doesn't need review.
I am contemplating code review within my own organization, and the question I return to is:
Does this organization prioritize human learning?
That has been my primary motivator for code reviews. I want to teach and learn from others, especially given the decreasing levels of collaboration due to increased AI usage.
The sad truth is that all of my feedback just goes straight to agents. Maybe 10% is reacted to by a human, so I’m left wondering if there’s any value to a real review aside from poorly training robots to do my job, and further atrophying the abilities of my team members.
If you will tell me precisely what it is that my machine cannot do, then I can always prompt my machine to do just that.
- John von Altman
Articulating “what humans can do, that AI cannot” is a mug’s game. If you specify it well enough, they just paste your text into their /goal prompt box and ralph loop their agent swarm until it produces something too exhausting to distinguish from doing the thing. If you don’t specify it well enough, then you’re just doing human-centric magical thinking to move the goalposts etc etc.
The true reason why code review is universal is that it provides a liability shield for negligence. Negligence is interesting. It has nothing to do with whether or not you ship something broken. As long as you follow a process that attempts to not ship something broken, then you are not negligent.
Engineers played along with this farce because code review served valuable team collaboration, coordination and management functions, about which the author of the article is correct.
Understanding a system by reading code is harder than understanding a system by writing code.
If AI can generate code at 100X, 1000X, or 10000X human capacity (no ceiling here), and you are gated on code review as your mechanism for system understanding, then a team's productive output will barely increase.
If companies want to compete in the world of AI generated code, human code review has to go. The only question is, what replaces it?
Continuing to apply human code review to AI generated code is negligent, if you are shipping at AI generation speed, with that as your only gate, and no other systems and processes to validate correctness and limit risk.
On the engineering side we can adapt easily.
Code review was never about finding bugs. When we do code review the first thing we check is: "do the tests pass?" Then we look at the change and the test coverage added for it and ask: "does the test coverage adequately demonstrate the functionality of the code?" The we ask: "What is the scope and potential impact of this change?" "What is the deployment and rollback plan and how will we monitor and detect defects after deployment?"
Code review was never about the code. It made the lawyers happy and provided a vehicle for doing the things that actually make systems work.
I think this applies to the writing of code as well
Unfortunately this often represents the only feedback given by the people in those „higher“ positions.
„The indent is wrong here“
„Comments should end with a period“
Because this kind of feedback is and was always easy.
If you get feedback like that, it’s time for your team to get automated linting/formatting
Call for a style guide meeting every time you see feedback like this, and write down what everybody agrees on. You’ll never have to do it again after 3-4 of those. Problem solved.
Still not solved? Guess it was really about the commas and not the value delivered anyway, so do whatever you feel like.
That meeting sounds like it would be the most bikeshed-y thing imaginable.
What you actually need to do is have a manager lead assert that it’s happening in a top down way and just run the formatter with the defaults. Let people argue case-by-case on what to change after that.
There's been a lot of talk about the purpose of code review recently. It makes sense in the face of AI. Heres a link that was submitted a little while ago: https://mathstodon.xyz/@mjd/115096720350507897
And in response I wrote a non-exhaustive checklist of things that a code review can look for:
- Does it functionally achieve what it sets out to (as per tacker issue or PR description)?
- Does it have extraneous code? Leftover debug prints, private API keys etc...
- Does it have any obvious defects? Memory leaks, un-handled edge cases, security flaws, obsolete API calls, etc...
- Could it be more understandable? Add/remove abstractions, better variable/method names, more/less functional etc...
- Is the style consistent with the codebase and/or style guidelines?
- Are there obvious performance improvements? Hashset instead of list, lazy evaluations, etc...
- Is it sufficiently well tested?
I think LLMs are okay at most of these, and worst at the first.
- Do we want this? Cost/Benefit etc
- Is the change architecturally right?
Particularly the latter LLMs seem still pretty useless at.
The former feels more like a product leadership problem.
Although I do think that LLMs have made it much easier to justify writing low-value code which can make this more common now.
I work on an open source project, so to-be-reviewed work can come in without any involvement by anyone :)
The thing is, leadership relies on the people actually building the software to provide concrete, accurate feedback about cost. Without that they have no chance to do a decent cost/benefit analysis.
But AI has engendered a collapse in developers’ ability to actually do that. Those of us who are stuck on the vibecoding bandwagon have lost the comprehensive understanding of the systems under our care that we need to understand and explain the quality and maintenance implications of a change.
Worse, if you happen to lose your mind and suggest the initial development cost is anything more than ~zero, your friendly neighborhood Claude keener will publicly shame you for not having sufficient faith in the Glorious Agentic Future. Product leadership will then have no choice but to side with them, not necessarily because they agree, but because they, too, are aware that we’re still in the phase of the hype cycle where openly questioning said hype is a career-limiting move.
Missing my biggest issues as you ask the agents to do larger tasks with less up front planning.
Is there already a pattern or code on in in the existing codebase that handles this functionality,
Do we really need net new code to achieve this functionality?
Can existing code be extended or abstracted to more cleanly implement this feature or functionality.
“Net new” is one it seems to be particularly bad at.
I don’t think I have ever even once seen an LLM solve a problem related to overengineering by simply removing the overengineering. They always choose to add more epicycles and further compound the complexity.
Code review also transfers knowledge to the reviewers!
I couldn’t agree more. Code review is integral to engineering, to sharing system understanding, to building sustainable systems.
Something is missing in the new ai bot review paradigm we’ve all sleepwalked into.
I’ve been building Archme.io for this reason. PR reviews for the age of AI
Pangram check on the article: 94% of this text is AI
it's really obvious too
Pangram check on this comment: 142% of this text is AI
I actually think this might be a false positive. I think it got tripped up by the higher than average use of jargon (which I don't mind here because the article itself flowed well and raised good points).
Gpt-zero scores "human", and I've always found it to be a better judge
code reviews are just a gateway that can be whatever you want it to be, and is kind of legacy human coder thing now. At its basics it was a point to catch problems that humans were likely to make / would more likely make if they knew there wasn't a review. Now you can target it for AI mistakes. You can build your code review skills (AI skill) to be incredibly thorough. The points made in the article don't really seem like things you need to do at the "legacy" gateway of code review. Things are different now. Code is cheap. Validation, Product Coherence, Governance need to be done early and throughout.
In my experience, automated code review is more pointless than ever.
We have all the linters, tests, and AI writing code for us. I don’t need the left hand to tell the right hand it did a good job. I’m very certain my code runs when I push the PR.
What I need now is architectural, long-horizon and business perspective.
What kind of code review tools did you try?
That's exactly what these tools are now good at. They have a huge gap when fixing these issues properly but they can spot these issues no problem
No, no they can’t. I use AI a lot and I get a lot of value from it but they are still terrible at programming “in the large”, by which I mean slotting features into the place meant for them in the existing code base. When coding and reviewing their view is too local. They will implement a change in the first place that looks feasible when coding. When reviewing they will not look for code duplication or fit. They will review for correctness, performance, security and style but not for architectural coherence.
They also have shockingly weak ability to identify business acceptance criteria that are completely missing in the implementation or test coverage.
For those of us who do utilize the pre-existing tools and the coding agents effectively, yeah automated code reviews eventually become redundant. But they're still around because there are a lot of devs who don't even glance at what the agents are writing for them. Maybe partly because they don't care to streamline their workflow, or maybe because they've never been particularly good at writing code, and don't actually understand what the agent is giving them.
Lately I've seen some pretty glaringly obvious issues caught during the preliminary automated code review, and the issues seem to be coming from individuals who don't actually understand what the code is doing.
For the rest of us who are actually using the whole stack effectively, the automated code review is essentially a CI gate to protect the repo from the devs who don't know what they're doing.
But how does automated AI code review help, here? Doesn’t it just reinforce that they don’t need to look at it (or change their habits), because the AI review will catch the issues?
To get the context that isn't in the code, maybe it would be better to ask for a review of the prompt?
I disagree. Most of my prompts are “implement <issue-id>”. The issue has a user story and acceptance criteria that the LLM uses to write a plan and ultimately produce code. None of that matters since the code artifact is what actually gets pushed in the repo, thus the code is what should be reviewed.
Thank you, well put! Bots reviewing code written by bots is a self licking ice cream cone.
I believe code review is important (the article articulates some of the reasons) but I have to admit, having different models review a PR before passing to a human has proven valuable.
A defense of human code review I wish I saw more often, especially in light of the concerns people have about cognitive/comprehension debt: comprehension redundancy. At the end, if taken seriously, at least two people understand how the feature works (even if that number is, on average, trending closer to between one and zero). Ideally at least one of the two also comes away with a better understanding of the wider system and how the feature fits into or stands out from that landscape.
If only one person knows the code, then PR time isn't going to save you.
I'm tech lead and I basically don't review PRs, and I tell people this, with a caveat - if you can tell me what you specifically want me to review, for what specific purpose, I'm happy to!
So "can you review this bit for race conditions" is great, love it. This forces people to actually think about what in their code they should be suspicious of, if anything.
"Can you review this" [link to PR] is getting a rubber stamp because humans have never been good enough at "just spotting bugs" to make this worth it and now any LLM is better than a human.
And for the purpose of understanding - PR time is too late. I have not reviewed a PR ("for real") in a long time and yet I could tell you how every system my people have built works down to a very fine level of detail. And it's because _we talk to each other!_ We don't just chill in the same slack channel and code independently, we all value each others brains and want each others inputs because we know it will improve our product and we value what perspectives others will bring.
Trying to learn via PR is a sad substitute for real collaboration and teamwork.
Well articulated. Ty
I consider PRs to be primarily a defense of the architecture, and to a lesser degree a general sanity check. It’s also a useful opportunity to enforce automations are being run
I consider 99% of my "defense of the architecture" strategy to be teaching my team why the architecture is important, how to think about it, and invite they commentary on it as we own and evolve it together. And of course if they are doing something and want input or are uncertain, then my door is open.
If PRs are a notable part of my architecture defense, I'm going to work on investing in the team instead of reviewing PRs.
I follow mailing lists (emacs and openbsd) and sending a diff is kinda the boundary between wishing for something and making the something into a thing. It’s the difference between discussing a plot and discussinf a draft.
Sneding a PR should not be for understanding or just for rubber stamping. It’s about getting someone to look at your approach and helping you find flaws or proposing ideas that could make it better.
When I review PR, the primary question is: For the stated problem, is the diff a good solution? Sometimes I don’t know enough about the problem, so I just try to see if the code has glaring mistakes (mispellings, styles,…) but those are just comments, not suggestions.
THANK YOU. I’ve always felt this way about PR review. I feel like people should write a natural language description of the change, and every section of it should link to part of the diff, and every part of the diff should be linked to by part of the description. That or just leave a comment on every chunk of the diff.
This sounds like a fussier version of what Donald Knuth was doing with literate programming.
Which, incidentally, is a really enjoyable way to work.
I'm pretty sure human code review is already on the way out for > 90% of code generated outside of critical systems.
What makes you think so? It's not my experience, so I'd like to hear what kind of software you work with/teams and what is leading you to understand that code doesn't need review.
I am contemplating code review within my own organization, and the question I return to is:
That has been my primary motivator for code reviews. I want to teach and learn from others, especially given the decreasing levels of collaboration due to increased AI usage.
The sad truth is that all of my feedback just goes straight to agents. Maybe 10% is reacted to by a human, so I’m left wondering if there’s any value to a real review aside from poorly training robots to do my job, and further atrophying the abilities of my team members.
Articulating “what humans can do, that AI cannot” is a mug’s game. If you specify it well enough, they just paste your text into their /goal prompt box and ralph loop their agent swarm until it produces something too exhausting to distinguish from doing the thing. If you don’t specify it well enough, then you’re just doing human-centric magical thinking to move the goalposts etc etc.
The true reason why code review is universal is that it provides a liability shield for negligence. Negligence is interesting. It has nothing to do with whether or not you ship something broken. As long as you follow a process that attempts to not ship something broken, then you are not negligent.
Engineers played along with this farce because code review served valuable team collaboration, coordination and management functions, about which the author of the article is correct.
Understanding a system by reading code is harder than understanding a system by writing code.
If AI can generate code at 100X, 1000X, or 10000X human capacity (no ceiling here), and you are gated on code review as your mechanism for system understanding, then a team's productive output will barely increase.
If companies want to compete in the world of AI generated code, human code review has to go. The only question is, what replaces it?
Continuing to apply human code review to AI generated code is negligent, if you are shipping at AI generation speed, with that as your only gate, and no other systems and processes to validate correctness and limit risk.
On the engineering side we can adapt easily.
Code review was never about finding bugs. When we do code review the first thing we check is: "do the tests pass?" Then we look at the change and the test coverage added for it and ask: "does the test coverage adequately demonstrate the functionality of the code?" The we ask: "What is the scope and potential impact of this change?" "What is the deployment and rollback plan and how will we monitor and detect defects after deployment?"
Code review was never about the code. It made the lawyers happy and provided a vehicle for doing the things that actually make systems work.
You’re going a bit hand wavy for an answer by redefining the term into something that fits what you’re promoting.