4 ms·

Simon, if you're reading this, I'd be really curious to hear your thoughts on how to effectively conduct code reviews in a world where "code is cheap". One of

by slaye 7mo ago

Simon, if you're reading this, I'd be really curious to hear your thoughts on how to effectively conduct code reviews in a world where "code is cheap". One of the biggest struggles I have on my team is coworkers straight up vibing parts of the code and not understanding or guiding the architecture of subsystems. Or at least, not writing code in a way that is meant to be understood by others. Then when I go through the code and provide extensive feedback (mostly architectural and highlighting odd inconsistencies with the code additions) I'm met with much pushback because "it works, why change it"? Not to mention the sheer size of prs ballooning in recent months. The end result is me being the bottleneck because I can't keep up with the "pace" of code being generated, and feeling a lot of discomfort and pressure to lower my standards. I've thought about using a code review agent to review and act as me in proxy, but not being able to control the exact output worries me. And I don't like the lack of human touch it provides. Maybe someone has advice on a humane way to handle this problem.

simonw 7mo ago
This is genuinely one of the most interesting questions right now. I don't have solid answers yet, and I'm very keen to learn what people are finding works. If you accelerate the pace of code creation it inevitably creates bottlenecks elsewhere. Code review is by far the biggest of those right now. There may be an argument for leaning less on code review. When code is expensive to produce and is likely to stay in production for many years it's obviously important to review it very carefully. If code is cheap and can be inexpensively replaced maybe we can lower our review standards? But I don't want to lower my standards! I want the code I'm producing with coding agents to be better than the code I would produce without them. There are some aspects of code review that you cannot skimp on. Things like coding standards may not matter as much, but security review will never be optional. I've recently been wondering what we can learn from security teams at large companies. Once you have dozens or hundreds of teams shipping features at the same time - teams with varying levels of experience - you can no longer trust those teams not to make mistakes. I expect that the same strategies used by security teams at Facebook/Google-scale organizations could now be relevant to smaller organizations where coding agents are responsible for increasing amounts of code. Generally though I think this is very much an unsolved problem. I hope to document the effective patterns for this as they emerge.
- cma256 7mo ago
  > There may be an argument for leaning less on code review. When code is expensive to produce and is likely to stay in production for many years it's obviously important to review it very carefully. If code is cheap and can be inexpensively replaced maybe we can lower our review standards? Agree with everything else you said except this. In my opinion, this assumes code becomes more like a consumable as code-production costs reduce. But I don't think that's the case. Incorrect, but not visibly incorrect, code will sit in place for years.
  - simonw 7mo ago
    > Agree with everything else you said except this. Yeah, I'm not sure I agree with what I said there myself! > Incorrect, but not visibly incorrect, code will sit in place for years. If you let incorrect code sit in place for years I think that suggests a gap in your wider process somewhere. I'm still trying to figure out what closing those gaps looks like. The StrongDM pattern is interesting - having an ongoing swarm of testing agents which hammer away at a staging cluster trying different things and noting stuff that breaks. Effectively an agent-driven QA team. I'm not going to add that to the guide until I've heard it working for other teams and experienced it myself though!
    - Balgair 7mo ago
      This kinda gets into the idea of AIs as droids right? So, you have a code writing droid that is aligned towards writing good clean code that humans can read. Then you have an implementation droid that goes into actually launching and running the code and is aligned with business needs and expenses. And you have a QA droid that stress tests the code and is aligned with the hacker mindset and is just slightly evil, so to speak. Each droid is working together to make good code, but also are independent and adversarial in the day to day.
      - aprdm 7mo ago
        These are just agents with a different name ? People have been working like that today.
fantasizr 7mo ago
Code review is now a bit like Brandolini's law: "The amount of energy needed to refute bullshit is an order of magnitude bigger than that needed to produce it." You ultimately need a lot of buy in to spend more than 5 mins on something that took 5 seconds to produce.
- mistercheese 7mo ago
  Yes I thinks somehow we need a bulldog check gate before it even goes to a human reviewer
jf22 7mo ago
How are the architecture changes you are proposing improving the end result? >but not being able to control the exact output worries me Why?
yonaguska 7mo ago
Can you document the hard architectural requirements of your codebase? And keep it up to date? If you can do that, you can force your coworkers to always use those requirements during their prompting /planning for their implementations and you can feed that to an agent and have that review the code. But more proactively, if people aren't going to write their own code, I think there needs to be a review process around their prompts, before they generate any code at all. Make this a formal process, generate the task list you're going to feed to your LLM, write a spec, and that should be reviewed. This is not a substitute for code reviews, but it tends to ensure that there are only nitpick issues left, not major violations of how the system is intended to be architected.
esafak 7mo ago
Code review should be mandatory and reviewers should ask big PRs to be broken up, and its submitters to be able to defend every line of code. For when the computer is generating the code, the most important duty of the submitter is to vouch for it. To do otherwise creates the bad incentive of making others do all your QA, and nobody is going to be rewarded for that.
- simonw 7mo ago
  Yeah, I think that's one of the biggest anti-patterns right now: dumping thousands of lines of agent-generated code on your team to review, which effectively delegates the real work to other people.
- xXSLAYERXx 7mo ago
  > Code review should be mandatory and reviewers should ask big PRs to be broken up Always, even before all this madness. It sounds more like a function of these teams CR process rather than agents writing the code. Sometimes super large prs are necessary and I've always requested a 30 minute meeting to discuss. I don't see this as an issue, just noise. Reduce the PR footprint. If not possible meet with the engineer(s)
- simonw 7mo ago
  I just added a chapter which touches on that: https://simonwillison.net/guides/agentic-engineering-patterns/anti-patterns/#inflicting-unreviewed-code-on-collaborators https://simonwillison.net/guides/agentic-engineering-pattern...
- mistercheese 7mo ago
  > the most important duty of the submitter is to vouch for it When shipping pressure comes, I’ve seen this to be the first thing to go. Despite formalizing ownership standards, etc… people on both the submitting and reviewing end just give up understanding Ai slop when management says they need to hit a deadline. Probably no company would actually do this, but I wonder if we should actually actively test the submitter’s understanding of the code submitted somehow as a prerequisite to moving a PR to ready for review. I’m not sure if it will be actually hopeful, enforcing people to understand the code, but maybe at least we’ll put the cultural expectation upfront and center?
ornornor 7mo ago
I’m running into this problem as well with juniors slinging code that takes me a very long time to understand and review. I’m iterating on an AGENTS.md file to share with them because they aren’t going to stop using AI and I’m a little tied of always saying the same things (Claude loves to mock everything and assert that spies were called X times with Y arguments which is a great recipe for brittle tests, for example) I know they won’t stop using AI so giving them a directives file that I’ve tried out might at least increase the quality of the output and lower my reviewing burden. Open to other ideas too :)
- esafak 7mo ago
  Have an AI reviewer take the first crack at it after pointing it to your rules file (e.g., AGENTS) so you don't have to repeat yourself. Gemini does this fairly well, for example. https://developers.google.com/gemini-code-assist/docs/review-github-code https://developers.google.com/gemini-code-assist/docs/review...
keithnz 7mo ago
Agent based code reviews is what you want. But you have to do set it up with really good context about what is wanted. You then review the reviews, keep improving the context it is working with. Make sure it's put into everyone's global context they work with as well. Weirdly this article doesn't really talk about the main agentic pattern - Plan (really important to start with a plan before code changes). iteratively build a plan to implement something. You can also have a colelctive review of the plan, make sure its what you want and there is guidance about how it should implement in terms of architecture (should also be pulling on pre existing context about your architecure /ccoding standards), what testing should be built. Make sure the agent reviews the plan, ask the agent to make suggestions and ask questions - Execute. Make the agent (or multiple agents) execute on the plan - Test / Fix cycle - Code Review / Refactor - Generate Test Guidance for QA Then your deliverables are Code / Feature context documentation / Test Guidance + evolving your global/project context
- ramoz 7mo ago
  > what testing should be built Yea, a big part of my planning has included what verification steps will be necessary along the way or at the end. No plan gets executed without that and I often ask for specific focus on this aspect in plan mode.
  - keithnz 7mo ago
    yeah, spending a bunch of time with the plan is really worthwhile, nearly all aspects of the plan are worth a bunch of attention. Getting it to think about edge cases and all the scenarios for testing is really worthwhile, what can be automated, what manual testing should be done. It's often working through testing scenarios that I often see gaps in the plan.
- simonw 7mo ago
  I'm still trying to figure out how to write about planning. The problem is Claude Code has a planning mode baked in, which works really well but is quite custom to how Claude Code likes to do things. When I describe it as a pattern I want to stretch a little beyond the current default implementation in one of the most popular coding agents.
manquer 7mo ago
The way I am handing this - investing heavily in static and dynamic analysis aspects. - A lot more linting rules than ever before, also custom rule sets that do more org and project level validations. - Harder types enforcement in type optional languages , Stronger and deeper typing in all of them . - beyond unit tests - test quality coverage tooling like mutation testing(stryker) and property based testing (quickcheck) if you can go that precise - much more dx scripts and build harnesses that are specific to org and repo practices that usually junior/new devs learn over time - On dynamic side , per pull requests environments with e2e tests that agents can validate against and iterate when things don’t work. - documentation generation and skill curation. After doing a batch of pull requests reviews I will spend time in seeing where the gaps are in repo skills and agents. All this becomes pre-commit heavy, and laptops cannot keep up in monorepos, so we ended up doing more remote containers on beefy machines and investing and also task caching (nx/turborepo have this ) Reviews (agentic or human) have their uses , but doing this with reviews is just high latency, inefficient and also tends to miss things and we become the bottleneck. Earlier the coder(human or agent) gets repeatable consistent feedback it is better
pc86 7mo ago
"It works, why change it?" is a horrible attitude but is an organizational and interpersonal problem, not a technical one. They're only 1/3 of the way done according to Kent Beck.¹ There are plenty of orgs using AI who still care about architecture and having easily human-readable, human-maintainable code. Maybe that's becoming an anachronism, and those firms will go the way of the Brontosaurus. Maybe it will be a competitive advantage. Who knows? ¹ "Make it work, make it right, make it fast."
TeeWEE 7mo ago
We make the creator of the PR responsible for the code. Meaning they must understand it. Also, we only allow engineers to commit (agent generated) code. Designers just come up with suggestions, engineers take it and ensure it fits our architecture. We do have a huge codebase. We are teaching Claude Code with CLAUDE.md's and now also <feature>.spec.md (often a summary of the implementation plan). In the end, engineers are responsible.
layer8 7mo ago
> I'm met with much pushback because "it works, why change it"? This is an educational problem, and is unlikely to be easy to fix in your team (though I might be wrong). I would suggest to change to a team or company with a culture that values being able to reason about one’s software.
epolanski 7mo ago
Fire them. Easy. They have to be responsible for what they push.
scuff3d 7mo ago
In a business (or any large project setting) where there are real users and real risk involved, code can't move into a code base any faster then it can be reviewed by a human. Period. I apply the exact same standards to PRs for AI assisted code as I do for human written code. If the code is crap, the PR is too larger, or the dev can't explain it. it gets rejected. End of story. We are a long way away from the need for human review going away.
mightybyte 7mo ago
One plausible future I can see from here is that we see a shift in our relationship to code in high-level languages that is similar to what happened with code written in assembly language back when the first high level languages were introduced. Before them, software engineers operated in assembly language. They cared about the structure of assembly code. This happened before I started my professional software career, but I can imagine that a lot of the same things we are hearing from developers today were heard back then. Concern about devs producing code they didn't understand, the generated assembly not being meant to be understood by others, etc etc. Now, however, we know how that played out in the case of assembly language. The fact of the matter is that only a very tiny fraction of software engineers give the structure of the compiled assembly code even passing thought. Our ability to generate assembly code is so great that we don't care about the end result. We only care about its properties...i.e. that it runs efficiently enough and does what we want. I could easily see the AI software development revolution ending up the same way. Does it really matter if the code generated by AI agents is DRY and has good design if we can easily recreate it from scratch in a matter of minutes/hours? As much as I love the craft and process of creating a beautiful codebase, I think we have to seriously consider and plan for a future where that approach is dramatically less efficient than other AI-enabled approaches.

by slaye 7mo ago

simonw 7mo ago

This is genuinely one of the most interesting questions right now. I don't have solid answers yet, and I'm very keen to learn what people are finding works. If you accelerate the pace of code creation it inevitably creates bottlenecks elsewhere. Code review is by far the biggest of those right now. There may be an argument for leaning less on code review. When code is expensive to produce and is likely to stay in production for many years it's obviously important to review it very carefully. If code is cheap and can be inexpensively replaced maybe we can lower our review standards? But I don't want to lower my standards! I want the code I'm producing with coding agents to be better than the code I would produce without them. There are some aspects of code review that you cannot skimp on. Things like coding standards may not matter as much, but security review will never be optional. I've recently been wondering what we can learn from security teams at large companies. Once you have dozens or hundreds of teams shipping features at the same time - teams with varying levels of experience - you can no longer trust those teams not to make mistakes. I expect that the same strategies used by security teams at Facebook/Google-scale organizations could now be relevant to smaller organizations where coding agents are responsible for increasing amounts of code. Generally though I think this is very much an unsolved problem. I hope to document the effective patterns for this as they emerge.

cma256 7mo ago
> There may be an argument for leaning less on code review. When code is expensive to produce and is likely to stay in production for many years it's obviously important to review it very carefully. If code is cheap and can be inexpensively replaced maybe we can lower our review standards? Agree with everything else you said except this. In my opinion, this assumes code becomes more like a consumable as code-production costs reduce. But I don't think that's the case. Incorrect, but not visibly incorrect, code will sit in place for years.
- simonw 7mo ago
  > Agree with everything else you said except this. Yeah, I'm not sure I agree with what I said there myself! > Incorrect, but not visibly incorrect, code will sit in place for years. If you let incorrect code sit in place for years I think that suggests a gap in your wider process somewhere. I'm still trying to figure out what closing those gaps looks like. The StrongDM pattern is interesting - having an ongoing swarm of testing agents which hammer away at a staging cluster trying different things and noting stuff that breaks. Effectively an agent-driven QA team. I'm not going to add that to the guide until I've heard it working for other teams and experienced it myself though!
  - Balgair 7mo ago
    This kinda gets into the idea of AIs as droids right? So, you have a code writing droid that is aligned towards writing good clean code that humans can read. Then you have an implementation droid that goes into actually launching and running the code and is aligned with business needs and expenses. And you have a QA droid that stress tests the code and is aligned with the hacker mindset and is just slightly evil, so to speak. Each droid is working together to make good code, but also are independent and adversarial in the day to day.
    - aprdm 7mo ago
      These are just agents with a different name ? People have been working like that today.

fantasizr 7mo ago

Code review is now a bit like Brandolini's law: "The amount of energy needed to refute bullshit is an order of magnitude bigger than that needed to produce it." You ultimately need a lot of buy in to spend more than 5 mins on something that took 5 seconds to produce.

mistercheese 7mo ago
Yes I thinks somehow we need a bulldog check gate before it even goes to a human reviewer

jf22 7mo ago

How are the architecture changes you are proposing improving the end result? >but not being able to control the exact output worries me Why?

yonaguska 7mo ago

Can you document the hard architectural requirements of your codebase? And keep it up to date? If you can do that, you can force your coworkers to always use those requirements during their prompting /planning for their implementations and you can feed that to an agent and have that review the code. But more proactively, if people aren't going to write their own code, I think there needs to be a review process around their prompts, before they generate any code at all. Make this a formal process, generate the task list you're going to feed to your LLM, write a spec, and that should be reviewed. This is not a substitute for code reviews, but it tends to ensure that there are only nitpick issues left, not major violations of how the system is intended to be architected.

esafak 7mo ago

Code review should be mandatory and reviewers should ask big PRs to be broken up, and its submitters to be able to defend every line of code. For when the computer is generating the code, the most important duty of the submitter is to vouch for it. To do otherwise creates the bad incentive of making others do all your QA, and nobody is going to be rewarded for that.

simonw 7mo ago
Yeah, I think that's one of the biggest anti-patterns right now: dumping thousands of lines of agent-generated code on your team to review, which effectively delegates the real work to other people.
xXSLAYERXx 7mo ago
> Code review should be mandatory and reviewers should ask big PRs to be broken up Always, even before all this madness. It sounds more like a function of these teams CR process rather than agents writing the code. Sometimes super large prs are necessary and I've always requested a 30 minute meeting to discuss. I don't see this as an issue, just noise. Reduce the PR footprint. If not possible meet with the engineer(s)
simonw 7mo ago
I just added a chapter which touches on that: https://simonwillison.net/guides/agentic-engineering-patterns/anti-patterns/#inflicting-unreviewed-code-on-collaborators https://simonwillison.net/guides/agentic-engineering-pattern...
mistercheese 7mo ago
> the most important duty of the submitter is to vouch for it When shipping pressure comes, I’ve seen this to be the first thing to go. Despite formalizing ownership standards, etc… people on both the submitting and reviewing end just give up understanding Ai slop when management says they need to hit a deadline. Probably no company would actually do this, but I wonder if we should actually actively test the submitter’s understanding of the code submitted somehow as a prerequisite to moving a PR to ready for review. I’m not sure if it will be actually hopeful, enforcing people to understand the code, but maybe at least we’ll put the cultural expectation upfront and center?

ornornor 7mo ago

I’m running into this problem as well with juniors slinging code that takes me a very long time to understand and review. I’m iterating on an AGENTS.md file to share with them because they aren’t going to stop using AI and I’m a little tied of always saying the same things (Claude loves to mock everything and assert that spies were called X times with Y arguments which is a great recipe for brittle tests, for example) I know they won’t stop using AI so giving them a directives file that I’ve tried out might at least increase the quality of the output and lower my reviewing burden. Open to other ideas too :)

esafak 7mo ago
Have an AI reviewer take the first crack at it after pointing it to your rules file (e.g., AGENTS) so you don't have to repeat yourself. Gemini does this fairly well, for example. https://developers.google.com/gemini-code-assist/docs/review-github-code https://developers.google.com/gemini-code-assist/docs/review...

keithnz 7mo ago

Agent based code reviews is what you want. But you have to do set it up with really good context about what is wanted. You then review the reviews, keep improving the context it is working with. Make sure it's put into everyone's global context they work with as well. Weirdly this article doesn't really talk about the main agentic pattern - Plan (really important to start with a plan before code changes). iteratively build a plan to implement something. You can also have a colelctive review of the plan, make sure its what you want and there is guidance about how it should implement in terms of architecture (should also be pulling on pre existing context about your architecure /ccoding standards), what testing should be built. Make sure the agent reviews the plan, ask the agent to make suggestions and ask questions - Execute. Make the agent (or multiple agents) execute on the plan - Test / Fix cycle - Code Review / Refactor - Generate Test Guidance for QA Then your deliverables are Code / Feature context documentation / Test Guidance + evolving your global/project context

ramoz 7mo ago
> what testing should be built Yea, a big part of my planning has included what verification steps will be necessary along the way or at the end. No plan gets executed without that and I often ask for specific focus on this aspect in plan mode.
- keithnz 7mo ago
  yeah, spending a bunch of time with the plan is really worthwhile, nearly all aspects of the plan are worth a bunch of attention. Getting it to think about edge cases and all the scenarios for testing is really worthwhile, what can be automated, what manual testing should be done. It's often working through testing scenarios that I often see gaps in the plan.
simonw 7mo ago
I'm still trying to figure out how to write about planning. The problem is Claude Code has a planning mode baked in, which works really well but is quite custom to how Claude Code likes to do things. When I describe it as a pattern I want to stretch a little beyond the current default implementation in one of the most popular coding agents.

manquer 7mo ago

The way I am handing this - investing heavily in static and dynamic analysis aspects. - A lot more linting rules than ever before, also custom rule sets that do more org and project level validations. - Harder types enforcement in type optional languages , Stronger and deeper typing in all of them . - beyond unit tests - test quality coverage tooling like mutation testing(stryker) and property based testing (quickcheck) if you can go that precise - much more dx scripts and build harnesses that are specific to org and repo practices that usually junior/new devs learn over time - On dynamic side , per pull requests environments with e2e tests that agents can validate against and iterate when things don’t work. - documentation generation and skill curation. After doing a batch of pull requests reviews I will spend time in seeing where the gaps are in repo skills and agents. All this becomes pre-commit heavy, and laptops cannot keep up in monorepos, so we ended up doing more remote containers on beefy machines and investing and also task caching (nx/turborepo have this ) Reviews (agentic or human) have their uses , but doing this with reviews is just high latency, inefficient and also tends to miss things and we become the bottleneck. Earlier the coder(human or agent) gets repeatable consistent feedback it is better

pc86 7mo ago

"It works, why change it?" is a horrible attitude but is an organizational and interpersonal problem, not a technical one. They're only 1/3 of the way done according to Kent Beck.¹ There are plenty of orgs using AI who still care about architecture and having easily human-readable, human-maintainable code. Maybe that's becoming an anachronism, and those firms will go the way of the Brontosaurus. Maybe it will be a competitive advantage. Who knows? ¹ "Make it work, make it right, make it fast."

TeeWEE 7mo ago

We make the creator of the PR responsible for the code. Meaning they must understand it. Also, we only allow engineers to commit (agent generated) code. Designers just come up with suggestions, engineers take it and ensure it fits our architecture. We do have a huge codebase. We are teaching Claude Code with CLAUDE.md's and now also <feature>.spec.md (often a summary of the implementation plan). In the end, engineers are responsible.

layer8 7mo ago

> I'm met with much pushback because "it works, why change it"? This is an educational problem, and is unlikely to be easy to fix in your team (though I might be wrong). I would suggest to change to a team or company with a culture that values being able to reason about one’s software.

epolanski 7mo ago

Fire them. Easy. They have to be responsible for what they push.

scuff3d 7mo ago

In a business (or any large project setting) where there are real users and real risk involved, code can't move into a code base any faster then it can be reviewed by a human. Period. I apply the exact same standards to PRs for AI assisted code as I do for human written code. If the code is crap, the PR is too larger, or the dev can't explain it. it gets rejected. End of story. We are a long way away from the need for human review going away.

mightybyte 7mo ago

One plausible future I can see from here is that we see a shift in our relationship to code in high-level languages that is similar to what happened with code written in assembly language back when the first high level languages were introduced. Before them, software engineers operated in assembly language. They cared about the structure of assembly code. This happened before I started my professional software career, but I can imagine that a lot of the same things we are hearing from developers today were heard back then. Concern about devs producing code they didn't understand, the generated assembly not being meant to be understood by others, etc etc. Now, however, we know how that played out in the case of assembly language. The fact of the matter is that only a very tiny fraction of software engineers give the structure of the compiled assembly code even passing thought. Our ability to generate assembly code is so great that we don't care about the end result. We only care about its properties...i.e. that it runs efficiently enough and does what we want. I could easily see the AI software development revolution ending up the same way. Does it really matter if the code generated by AI agents is DRY and has good design if we can easily recreate it from scratch in a matter of minutes/hours? As much as I love the craft and process of creating a beautiful codebase, I think we have to seriously consider and plan for a future where that approach is dramatically less efficient than other AI-enabled approaches.