Right now the comments that upset me the most are LLM TMI-style comments that break encapsulation by talking about the behavior of specific current callers of a function right above the function definition.
I recently reacted angrily in a PR review comment after encountering one for the umpteenth time... that caught me off guard. I didn't know I was capable of that.
Used to work with a guy who would frequently say "a comment is an apology" i.e. the comment is there because the code itself is not clear. That can be the case, but I generally find more comments better than fewer, especially if they relate the code to actual business or functional requirements and don't just restate what the code is doing.
Years ago I would often write comments first. I.e. start with describing the overall goals. Then break it down into routines and order of operations, all still in plain english. Once I was happy with that, I'd break up the comments with blocks of code. I guess this is sort of like "literate programming" though I was doing it long before I ever heard that term and I still have never read much about it. It's almost more like I was prompting myself towards the end goal. The downside of this approach is that the comments do end up more or less just explaining in english what the code is doing, so maybe aren't quite as useful to future maintainers.
AI is trained on a various range of code quality, including very low one.
I'm paid to provide good quality code and not flood my company with more average code than it should.
In my previous job, I could regularly reduce a PR code down to 10-20%% of its size because someone overlooked something or was just "overengineer" a feature.
AI are such "bullshiters" that they produce more text than necessary.
Code bloat was already real, but from my personal experience it becomes realer with AI.
The outcome of this will likely be apparent when no one can dive into any code base because of the amount of fluff in it (and you will obviously need more AI to deal with this).
It's interesting that the author didn't mention considering updating their agentic code review prompt to keep an eye out for repetitive/duplicate code.
AI isnt taking my job. my company is supporting local AI for development. who ever comes after me will have the same hardware and models or better. unless a MBA is put in charge, my boss and predecessors can maintain and build out as needed.
bottom up AI use seems a godsend compared to the corporate AI rat race.
i setup some slop reporting systems and ensured my boss knows theyre great starting points but serious use requires real time investment.
That sounds like a good idea, but shipping 10x as many features and bugfixes sounds better.
I started using AI with the best intentions. Checking everything before committing. Improving output by hand if it didn't quite follow the existing code style guidelines or variables were not named as well as they should be. Or if it did something sloppy or hacky.
Now, AI GOES BURRRRRRRRRRRR! If the tests pass it's good to ship. AI can deal with the problems it may create. No problems so far.
Are you my colleague? It's fine if it's your own personal app, but please don't do this in a large complex codebase in a team. It's entirely depressing. You can use AI and still write good code. I think it's actually probably easier to write maintainable code with AI.
I absolutely will not write corporate code like humans are maintaining it anymore, because I don’t have any confidence actual humans will be maintaining it.
For personal projects, I can trust that I myself will be maintaining things so I still write things like it matters, but I do not extend the trust to others.
I continually run codebases through different models to have them look for bad code smells like repeated code. That's been pretty effective. You do have to maintain over time or else you end up with a sloppy mess which I can only imagine compounds.
I don't have a scientific answer to this but I'm just assuming a different model, and more specifically, the frontier latest models like Fable/5.6 would catch things the author model might skip over.
What do you think of traditional AST-based static analyzers (like SonarQube, Rubocop, etc.) that catch cyclomatic complexity and code duplication deterministically?
You can even put them on the build pipeline, even. No tokens spent at all!
I haven't used them but I'm sure they're good and I'm sure they're even using LLMs themselves now as a non-deterministic pass. For my use cases I'm just trying to keep vendors/costs to a minimum.
Have a bit of a contrarian view on this tl;dr don’t write code for human consumption if you use AI; BUT you have to accept AI coding lock in and change how you work.
I’m pretty sure many people who use AI to write emails or blog posts add "make it sound like a human wrote it" to their prompts. We all know what the result usually looks like.
If AI is writing my code, I'd rather have it focus purely on correctness and efficiency than on making the code easy to read.
heck! I might even ask it to imitate Arthur Whitney’s style.
Write yourself a /review command. That is an empty markdown file at `.claude/commands/review.md`. In it, put a checklist of things the agent should look for. When you’re ready to have your agent review the code, type `/review`. The checklist will be examined and it’ll plan out some findings to ask you if you want them fixed.
Mine starts with “Enter plan mode. Examine the differences on this branch vs. main. Consider: ...” and proceeds to a bullet list of things.
Any time I notice something in code review and have to get the agent to fix it.. I throw it on the list!
My list is like 200 items now. Know what? Agents don’t care that they just got a wall of generic feedback, they happily look into all the bullet points.
I added “ensure the new things aren’t duplicating code that already exists elsewhere” and it gave me such a surprise - it really truly started planning cleanups!
We are just scratching the surface. We have to give tools to our tools so they can use them to be better tools for us.
I have my own review skill (I think it predates when Claude added theirs) and one thing I'd add to your description is tell it to examine all the code and then, based on the changes, do a multi-role review of the code again using the most appropriate N of the following roles based on the changes: ... (where ... is a long list I have like Senior Engineer, Security Engineer, WCAG specialist, etc). Claude will spawn those reviews in parallel and then consolidate the feedback. I do spec based development so I just have my skill append the issues to the spec so I have a trail of issues and decisions.
> Any time I notice something in code review and have to get the agent to fix it.. I throw it on the list! My list is like 200 items now.
This is a gripe I've had with AI tools for a while now. Though it's gotten somewhat better in time, but we don't really know what to expect from the tool in terms of quality. Ex. I'd expect a human engineer to probably not use a brand new assertion library for a new test when there are 200 tests using an existing one. But Claude has done this to me multiple times. So I have to add yet another item to the list, like you have, and tell it to look for testing conventions before writing. But, there is plenty we don't have to tell it, like what a function is or a test should probably cover the change in the diff. But we don't really have a list of what things are on each side so we're just left to sort of hunt and peck to build a viable solution.
> My list is like 200 items now. Know what? Agents don’t care that they just got a wall of generic feedback, they happily look into all the bullet points.
Yes, yes, there has been a library of information on HN by now about how to use agents effectively. (And I'm grateful for that, because I can keep current and in the loop without feeling enslaved to the new style of development.)
None of that is a reason not to do what the title of TFA says. If your review process is doing the right thing, you should observe that it results in your agent moving the code in the "human-maintainable" direction. If you, for whatever reason, actually directly make commits yourself any more (read this ironically; I genuinely can't understand why anyone would want to give up on that, no matter how good the generated code gets, because "the LLM could do better" is not the point), then of course you should write it to be human-maintainable.
The reason humans find "human-maintainable" code to be maintainable is because maintainability is one of the precious few worthwhile at-least-vaguely-objective metrics of code quality we have.
Every time I see someone try to make a point about the fact that some code actually is just better than other code, only to be met with more of this sort of advice, I start to wonder whether I was alone in ever actually enjoying programming.
In the discussion, mostly. It's been months; so if you find the general topic interesting (as I do) and check HN regularly (as I do) you'll have seen quite a bit that can at least be tried out, or thought about.
Maybe you can get an agent to identify and summarize such threads.
Given all the hype that LLMs have got and the valuations these AI companies are getting, I am disappointed at how even the "best" coding agents degrade over time.
WTF - I need to implement a review command to guide to do its job properly.
Can you imagine any other industry charging people money for a product like this ?
When you are charging people money - scratching the surface cannot be an excuse.
I've been finding that they can write surprisingly elegant code, you just have to ask them to. It seems to be kind of like the old generative AI art days where you still had to add "+quality -bad" to the prompt.
Failing that they default to the most common style they were trained on. Which, at this point, is mostly code they wrote...
My experience is the more things you add to the list, the worse agents perform (I'm not exactly the first person to notice this)
I actually have a pretty simple set of instructions right now and Claude still regularly messes them up. Like, my first instructions are:
- Never commit to git without permission.
- Never sign commit messages
You know what it constantly does? Commits without permissions and signs commit messages! And then I ask it, and it's like "oh, you're right, I have that instruction and I ignored it". If you were working with a junior developer, you'd expect at some point they get it, but with Claude even if I tell it to stuff the instructions into it's memory, it will do it, and STILL mess it up.
Then it gets even more fun, because if I politely correct it in the session, it will understand, but then there's a decent chance it will be completely unable to commit anything in the session going forward. Phd level intelligence!
Anyway, my point is, if you're asking it to review a list of 200 items I guarantee it's quietly messing up on a lot of that list.
My git commits are ssh key verified and the key is behind a TouchID gate. I've told agents that they shouldn't try committing because I need to be on the computer to activate Touch ID.
Do human developers check for 200 items when they do code review? How long would that take? It's quite clear that AI code review could be better even with some error.
What's easy for human to review is also easy for AI (small code base, small PRs). What's hard for AI is also hard for human, if not more. But the time cost is so different that it's almost a nobrainer to choose AI to code and review, especially considering most software out there is not so critical :)
- Hardcoding git diff against main isn't always ideal (depending on your git workflow). If you branch off of branches, the agent will figure it out, but it often has to go through a few hops to determine what to actually diff. You're better to ask it to diff 'the commits on this
branch' or similar.
- You might not necessarily want to enter plan mode for a review. I don't personally as I don't see any benefit to it writing a markdown plan file for a review.
AI is so miserable for this. It's so focused on doing what you ask, it forgets that there's stuff worth doing that you didn't ask for, like defining reasonable abstractions.
Getting away from stuff like this is exactly why I want to use AI. When I say "implement this for idle but active users," I _want_it to define isUserActiveIdle() and stuff these 4 conditionals in it. Having to check the generated code for stuff like this undoes, like .... all the benefit of using AI.
AI makes all these little decisions for us. I can about some of these decisions. I just want to notice when it's doing this without having to make my eyes bleed reading 10k lines of generated code a day.
I run various forms of workflows to run dedicated QA, code review (of various flavors) simplification and text simplification agents. Especially the simplification goes a long way to remove dumb padding, duplication and efficiency. Dedicated docs/comment simplification is also becoming more and more necessary on recent models. For things like feature development in my workflow, the majority of time the agents run and tokens spent is critiquing the code from various perspectives and it's not close.
Of course, this doesn't solve the overall issue that agents don't write code like you and still requires a lot of human attention in planning and code review out to clean up leftover issues, and e.g. challenge bad assumptions about architecture and real-world context. A human is still very much needed to cull the slop (or, more gratuitously: align the agent). But IME it does help avoid a lot of pitfalls and makes the code high quality a lot more quickly.
The key idea here is that your codebase is context that will be used for future changes. And context determines the model’s output, so it’s still worth having a well-designed codebase.
Easier said than done to be honest, especially if there are many people (and their agents) pushing code. It’s hard to keep up these days.
I’m not so sure this matters. My team manages a couple pretty new projects and I still see LLM tools doing this. I’m starting to suspect that vendors are building in these behaviors to ensure the output compiles (never throw an exception, null check every variable no matter what, never change a function or method but copy or inline its code and change that, etc.)
I think I would prefer code that is clear, understandable and simple even if it doesn’t compile and needs some straightforward polishing.
Just run weekly cron job to assess code quality and highlight candidates for refactoring. In addition to doing the same in each PR, of course, but things can get through.
I have good results with this prompt after every larger change: Now do a final code check. Is everything tidy and do the components adhere to the principle of separations-of-concerns. Is everything in an understandable and maintainable state? Do we make any assumptions that may not be true anymore? Is any code left over from previous edits or experiments that does not belong into the codebase? Is the documentation still representing the current state of code?
good, but this is just a verbose "make no mistakes"; it'd probably make more sense to just setup a nightly cron job that loops through the prior days' work and writes some morning tasks of the same character.
The models will interpret this willynilly; but nonetheless, it's often a better than doing nothing.
I usually just say “make sure this code is professional and ready to deliver as a senior engineer” and it usually infers all that stuff you said plus more things as well. I try to give it the goal and let it decide what to do.
One thing I usually keep having to point out directly is to remove all “progress tracking” code comments and make sure all comments are appropriate for long term maintenance in the code base. Claude tends to leave comments like “button click causes save now, no longer uses onBlur” when the code really never used onBlur, that was just a thing Claude wanted to do earlier in the same task/branch and I redirected it at some point.
118 comments
[ 5.0 ms ] story [ 84.9 ms ] thread"Add comments to your code under the assumption that the next person to maintain it is a homicidal maniac who knows where you live"
I recently reacted angrily in a PR review comment after encountering one for the umpteenth time... that caught me off guard. I didn't know I was capable of that.
Years ago I would often write comments first. I.e. start with describing the overall goals. Then break it down into routines and order of operations, all still in plain english. Once I was happy with that, I'd break up the comments with blocks of code. I guess this is sort of like "literate programming" though I was doing it long before I ever heard that term and I still have never read much about it. It's almost more like I was prompting myself towards the end goal. The downside of this approach is that the comments do end up more or less just explaining in english what the code is doing, so maybe aren't quite as useful to future maintainers.
I'm paid to provide good quality code and not flood my company with more average code than it should.
In my previous job, I could regularly reduce a PR code down to 10-20%% of its size because someone overlooked something or was just "overengineer" a feature.
AI are such "bullshiters" that they produce more text than necessary.
Code bloat was already real, but from my personal experience it becomes realer with AI. The outcome of this will likely be apparent when no one can dive into any code base because of the amount of fluff in it (and you will obviously need more AI to deal with this).
bottom up AI use seems a godsend compared to the corporate AI rat race.
i setup some slop reporting systems and ensured my boss knows theyre great starting points but serious use requires real time investment.
I started using AI with the best intentions. Checking everything before committing. Improving output by hand if it didn't quite follow the existing code style guidelines or variables were not named as well as they should be. Or if it did something sloppy or hacky.
Now, AI GOES BURRRRRRRRRRRR! If the tests pass it's good to ship. AI can deal with the problems it may create. No problems so far.
Maybe someone has the perfect claude.md that solves this problem but I have not seen it.
For personal projects, I can trust that I myself will be maintaining things so I still write things like it matters, but I do not extend the trust to others.
Or is it more about the review process and a context reset?
You can even put them on the build pipeline, even. No tokens spent at all!
Funny enough, discussed this yesterday
Stop Optimizing Code for Humans https://youtube.com/live/eLn4-XA-KdQ?feature=share
— John F. Woods (1991)
I’m pretty sure many people who use AI to write emails or blog posts add "make it sound like a human wrote it" to their prompts. We all know what the result usually looks like.
If AI is writing my code, I'd rather have it focus purely on correctness and efficiency than on making the code easy to read.
heck! I might even ask it to imitate Arthur Whitney’s style.
/s
Mine starts with “Enter plan mode. Examine the differences on this branch vs. main. Consider: ...” and proceeds to a bullet list of things.
Any time I notice something in code review and have to get the agent to fix it.. I throw it on the list!
My list is like 200 items now. Know what? Agents don’t care that they just got a wall of generic feedback, they happily look into all the bullet points.
I added “ensure the new things aren’t duplicating code that already exists elsewhere” and it gave me such a surprise - it really truly started planning cleanups!
We are just scratching the surface. We have to give tools to our tools so they can use them to be better tools for us.
* https://github.com/alibaba/open-code-review
** https://layandreas.github.io/personal-blog/posts/beyond-vide...
This is a gripe I've had with AI tools for a while now. Though it's gotten somewhat better in time, but we don't really know what to expect from the tool in terms of quality. Ex. I'd expect a human engineer to probably not use a brand new assertion library for a new test when there are 200 tests using an existing one. But Claude has done this to me multiple times. So I have to add yet another item to the list, like you have, and tell it to look for testing conventions before writing. But, there is plenty we don't have to tell it, like what a function is or a test should probably cover the change in the diff. But we don't really have a list of what things are on each side so we're just left to sort of hunt and peck to build a viable solution.
Yes, yes, there has been a library of information on HN by now about how to use agents effectively. (And I'm grateful for that, because I can keep current and in the loop without feeling enslaved to the new style of development.)
None of that is a reason not to do what the title of TFA says. If your review process is doing the right thing, you should observe that it results in your agent moving the code in the "human-maintainable" direction. If you, for whatever reason, actually directly make commits yourself any more (read this ironically; I genuinely can't understand why anyone would want to give up on that, no matter how good the generated code gets, because "the LLM could do better" is not the point), then of course you should write it to be human-maintainable.
The reason humans find "human-maintainable" code to be maintainable is because maintainability is one of the precious few worthwhile at-least-vaguely-objective metrics of code quality we have.
Every time I see someone try to make a point about the fact that some code actually is just better than other code, only to be met with more of this sort of advice, I start to wonder whether I was alone in ever actually enjoying programming.
yes, but where?
Maybe you can get an agent to identify and summarize such threads.
WTF - I need to implement a review command to guide to do its job properly.
Can you imagine any other industry charging people money for a product like this ?
When you are charging people money - scratching the surface cannot be an excuse.
Failing that they default to the most common style they were trained on. Which, at this point, is mostly code they wrote...
I actually have a pretty simple set of instructions right now and Claude still regularly messes them up. Like, my first instructions are:
You know what it constantly does? Commits without permissions and signs commit messages! And then I ask it, and it's like "oh, you're right, I have that instruction and I ignored it". If you were working with a junior developer, you'd expect at some point they get it, but with Claude even if I tell it to stuff the instructions into it's memory, it will do it, and STILL mess it up.Then it gets even more fun, because if I politely correct it in the session, it will understand, but then there's a decent chance it will be completely unable to commit anything in the session going forward. Phd level intelligence!
Anyway, my point is, if you're asking it to review a list of 200 items I guarantee it's quietly messing up on a lot of that list.
Non-deterministic behavior via code.
Every harness I've tried has obeyed that.
Do human developers check for 200 items when they do code review? How long would that take? It's quite clear that AI code review could be better even with some error.
What's easy for human to review is also easy for AI (small code base, small PRs). What's hard for AI is also hard for human, if not more. But the time cost is so different that it's almost a nobrainer to choose AI to code and review, especially considering most software out there is not so critical :)
A model might be able to follow 200 different instructions at the same time, while another model will choke on it
- You should use skills instead of commands (commands still work, but they've been rolled into skills) https://code.claude.com/docs/en/skills
- Hardcoding git diff against main isn't always ideal (depending on your git workflow). If you branch off of branches, the agent will figure it out, but it often has to go through a few hops to determine what to actually diff. You're better to ask it to diff 'the commits on this branch' or similar.
- You might not necessarily want to enter plan mode for a review. I don't personally as I don't see any benefit to it writing a markdown plan file for a review.
Getting away from stuff like this is exactly why I want to use AI. When I say "implement this for idle but active users," I _want_it to define isUserActiveIdle() and stuff these 4 conditionals in it. Having to check the generated code for stuff like this undoes, like .... all the benefit of using AI.
AI makes all these little decisions for us. I can about some of these decisions. I just want to notice when it's doing this without having to make my eyes bleed reading 10k lines of generated code a day.
Of course, this doesn't solve the overall issue that agents don't write code like you and still requires a lot of human attention in planning and code review out to clean up leftover issues, and e.g. challenge bad assumptions about architecture and real-world context. A human is still very much needed to cull the slop (or, more gratuitously: align the agent). But IME it does help avoid a lot of pitfalls and makes the code high quality a lot more quickly.
Easier said than done to be honest, especially if there are many people (and their agents) pushing code. It’s hard to keep up these days.
I think I would prefer code that is clear, understandable and simple even if it doesn’t compile and needs some straightforward polishing.
Taken from: https://github.com/zakirullin/cognitive-load/blob/main/READM...
* define the software layers, their function, and the max depth allowed
* establish a corp code formatter for each language, along with a process to PR it
* establish a business vocabulary and what the terms mean
* establish a data dictionary, make it part of the database schema/table/col comments
Are far more successful with LLMs. You _should_ have been doing this years ago, but with LLMs its a super power.
I am following similar steps from this article https://www.lucasfcosta.com/blog/backpressure-is-all-you-nee...
The models will interpret this willynilly; but nonetheless, it's often a better than doing nothing.
One thing I usually keep having to point out directly is to remove all “progress tracking” code comments and make sure all comments are appropriate for long term maintenance in the code base. Claude tends to leave comments like “button click causes save now, no longer uses onBlur” when the code really never used onBlur, that was just a thing Claude wanted to do earlier in the same task/branch and I redirected it at some point.