5 ms·

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. Wh

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.

deleted 3mo ago
[deleted]
ebiester 3mo ago
You should do an experiment of splitting that up to multiple reviews that are logically together. My hypothesis is that you may be losing signal due to the amount of text expected back.
- conception 3mo ago
  Or a dynamic workflow. $$$ but lots of coverage.
- Espressosaurus 3mo ago
  Yeah. I have a set of 5 review prompts attacking different problems, an adversarial review, and then a final synthesis, with the best results gotten by multiple passes using multiple models (the adversarial review stage combining all passes into one review per model and the synthesis picking the best of the two or three adversarial reviews). Expensive but it actually finds real problems that the single pass reviews rarely seem to find.
is_true 3mo ago
Nice. I had a file with code samples (old code I wrote), including formatting and I asked to use it as a reference. Will try your approach to distill the code to bullet points.
throwaw12 3mo ago
Can you share your list? I am curious what does it contain, for me a lot of times its a back and forth with agent until it "looks good to my eyes and taste", but haven't written any such list yet, because it is context dependant, in some projects I forgive minor issues, or allow magical numbers, but in other projects I force agent to use constants with meaningful names `SECONDS_IN_A_DAY = 24 * 60 * 60`
- willturman 3mo ago
  It is interesting that the output of code is associated with sight and taste, while the quality of the code itself is associated with smell. https://en.wikipedia.org/wiki/Code_smell https://en.wikipedia.org/wiki/Code_smell *edit: that wikipedia page ^ itself is a pretty answer to your request for a list of things to avoid when writing maintainable code.
  - blanched 3mo ago
    Pairs nicely with the fact that smell is the sense most likely to be experienced differently between two people :)
    - chowells 3mo ago
      Is that actually true? Just because English treats scents as only describable by analogy doesnt mean all languages do. There are a number of languages with scent descriptions for aspects of a smell that are transferrable. [1] That suggests everyone is capable of decomposing a scent into components the same way. If that's true, we're left with a question equivalent to "does everyone see the same red?". As far as I know, the pure version of that question cannot be answered because subjective experiences of sensation cannot be transferred. And at that point, I'd say the manner in which they're experienced differently is equivalent. 1. https://youtu.be/w3KswMaEBiI https://youtu.be/w3KswMaEBiI
      - blanched 3mo ago
        It’s not my area of expertise, but a friend in the field told me the fun fact originally. Looking into it, it seems like smell has the most potential for genetic variance: https://pmc.ncbi.nlm.nih.gov/articles/PMC3990440/ https://pmc.ncbi.nlm.nih.gov/articles/PMC3990440/ That said, I can’t find any direct research comparing perceived senses. So they (and now I) might have oversimplified. Thanks for the link, looks interesting!
wismwasm 3mo ago
I have good experience with using open-code-review: * https://github.com/alibaba/open-code-review https://github.com/alibaba/open-code-review ** https://layandreas.github.io/personal-blog/posts/beyond-videcoding/#step-3-ocrreview---review-the-proposal https://layandreas.github.io/personal-blog/posts/beyond-vide...
dugmartin 3mo ago
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.
soperj 3mo ago
Why wouldn't it already just do this though?
- notatoad 3mo ago
  It does. Claude has a built in review and it’s pretty good. But it doesn’t know exactly what you want. This is a way to tell it. Good way to double your token use though, if you’re concerned about that.
  - zahlman 3mo ago
    > But it doesn’t know exactly what you want. This is a way to tell it. What ever happened to communicating through code? Agents can follow examples and infer patterns, and they can read commit history and diffs. Real-world commit logs for human-only projects are dominated by short commits (well, at least the ones where the humans are skilled, appreciate version control, care about the project, etc.) with thoughtful commit messages. cf. https://wiki.c2.com/?JustCorrectDontPoint https://wiki.c2.com/?JustCorrectDontPoint
- kmoser 3mo ago
  Unless prompted to do so, why would it? From the article: "Every shortcut you merge into your codebase is a signal about how things are done here."
  - soperj 3mo ago
    Not duplicating code is literally best practice. Why would it deviate from that?
    - webjunkie 3mo ago
      This is far from a universal best practice and heavily depends on what code it is and for what reason you are duplicating it potentially.
xhrpost 3mo ago
> 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.
- radlad 3mo ago
  I find project-wide or subpackage-wide (for sufficiently large projects) CLAUDE.md's that document patterns (including for tests) solves this. /init will make a project-wide one, or you can instruct it to "Create CLAUDE.md in any sub-directory that is sufficiently complex" then modify from there.
- selicos 3mo ago
  You're forced to document every check and thought related to development until you're wiring a manual on how to be laid off (unless self employed or otherwise critical). Which is fine. One should write quality docs and all that. But it seems exactly like training my offshore contract replacement instead of building tools for my own use. Except that income doesn't even go to a human just a black box datacenter company and shareholders.
- cadamsdotcom 3mo ago
  My review command isn’t an AI tool, it is is a text file. You don’t need a SaaS when you can use a text file instead. Just be incrementally aware of and closing gaps in the list of things you care about, and make it part of your day!
jhghbj 3mo ago
[dead]
zahlman 3mo ago
> 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.
- cadamsdotcom 3mo ago
  This technique incrementally codifies the subset of my personal definition of code quality that can be codified. I’ve found a discovery based approach is better than anticipatory, because it forces everything you spend tokens to prove its worth. I still do code review - how else would I know what to codify! But since starting this about a year ago, I don’t review menial dumb AI mistakes anymore.
- jason_s 3mo ago
  > there has been a library of information on HN by now about how to use agents effectively yes, but where?
  - zahlman 3mo ago
    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.
catzapd 3mo ago
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.
- cadamsdotcom 3mo ago
  There’s a saying for this - “don’t ship your first draft”. Its first attempt WILL be crap. You can refine its output in the loop as a human, or let it go and then do a quality pass post-hoc. The beauty of the latter is you can codify and automate a subset of the work, and the agent can loop until those bars are cleared then you get a higher quality work product to look at.
andai 3mo ago
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...
- cadamsdotcom 3mo ago
  Yes, and, write custom linters for things you want to prevent forever. Have a look at https://github.com/cadamsdotcom/CodeLeash/blob/main/scripts/check_dynamic_imports.py https://github.com/cadamsdotcom/CodeLeash/blob/main/scripts/... for an example (enforced before all commits via https://github.com/cadamsdotcom/CodeLeash/blob/main/.pre-commit-config.yaml#L40 https://github.com/cadamsdotcom/CodeLeash/blob/main/.pre-com...) Basically I got sick of telling the agent to put dynamic imports at the top of the file so I made them into an error that blocks commits. Now I don’t even need that in the review.md because it can’t make it though to /review.
overgard 3mo ago
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.
- ricardobeat 3mo ago
  Sometimes the fact you mentioned “signing commit messages” is exactly why it starts signing commit messages, and it’s better to start with no prompt at all. Negative prompting is very unreliable. Giving exact instructions on how you want commits made will give you better results.
  - overgard 3mo ago
    How am I supposed to take this tool seriously if it struggles with the concept of "dont"? if what you're suggesting is true, then more important instructions like "Don't delete the production database" are a problem. I shouldn't need to consider how to phrase "Don't delete the production database" in a positive manner. Isn't the point of an AI agent that it understands my intent and I don't need to hold its hand? I'm not saying your suggestion is wrong, I just think that suggests a limit to what these tools should be used for if that's the case. My guess though is that it probably ignores some of the positive instructions too, and the hardcore AI users mostly don't notice because they probably aren't reviewing the work.
    - 3mo ago
pipes 3mo ago
This sounds like gold to me, thanks. I'm going to try it.
yiyingzhang 3mo ago
> My list is like 200 items now 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 :)
- 0x457 3mo ago
  > 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. We (humans) don't do 200 point inspection, but we also don't review every code change in a spherical vacuum without prior knowledge of the project. However, we do check a lot of things. > What's hard for AI is also hard for human, if not more. That's just plain wrong. AI straight up suffers with counting things, and I'm not just talking about counting 'r' in strawberry. Some things are easier for AI some are easier for human.
- cadamsdotcom 3mo ago
  Humans pattern match against thousands upon thousands of criteria subconsciously. AI is a different intelligence - a 200 point list shoved in its face all at once doesn’t overwhelm it. That’s only 5-10k tokens! Bear in mind though, because I added all the items on the list I’ve seen the reasons they’re there & act as a backstop against it missing things. There’s always got to be a quality bar, it’s part of being a professional and signing your name on your work.
nextaccountic 3mo ago
Just one thing A model might be able to follow 200 different instructions at the same time, while another model will choke on it
- cadamsdotcom 3mo ago
  Yes and this is a really important point. I actually have no data on how other models do - feeling very spoiled by Claude to be honest. Would love to hear how it goes if you try it with other models!
tinasky 3mo ago
A couple of notes for anyone reading this comment and thinking of implementing something similar: - You should use skills instead of commands (commands still work, but they've been rolled into skills) https://code.claude.com/docs/en/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.
- cadamsdotcom 3mo ago
  Thanks for this, I’d agree and elaborate a little - Skills over commands: agreed - but they are more complex than a single file. I’m suggesting a command because it’s a great way to start and build up that checklist. Over time, migrate it to a skill. But one thing I’d say is keep it in your repo. That way, PRs can include new bullet points with the code that gave rise to them. Opinions will differ on this: it muddies the PR a little in the present. But OTOH future folks can git blame to see where that bullet came from. - Hardcoding against main: yes, this is a very good catch - a better prompt is to tell the agent to compare to the branch’s merge base. Again a detail left off for simplicity to encourage folks to start somewhere. Your suggestion is a quick win fast follow for correctness. (side note - 90% of the time Opus 4.8 guesses the intent and decides to use the merge base on its own! Opus 4.6 didn’t do that. These things are smart if you set them up for success!) - Plan mode for review I would say personal choice. It is crucial in my experience because some of my list makes it overzealous in planning cleanups and the like. YMMV. Yes plan mode is optional and I sometimes have to reread and discuss a review plan after a context switch in my day - but at least it didn’t spray bad cleanups all over a decent PR.
  - tinasky 3mo ago
    A skill is a single file too thought right? Just `.claude/skills/review/SKILL.md` instead of `.claude/commands/review.md`. I think the format is more or less identical as well? I may be missing some subtle difference though. As you say, either will work fine for this, but in general Anthropic's advice is to just use skills now. Agreed on the plan mode. I think it's personal choice and also situation-dependent. When I've already reviewed everything the agent has written and made changes along the way, I'm pretty confident a review is only going to throw up minor tweaks. When reviewing someone else's code or if you've got a load of changes you haven't reviewed yourself yet, plan mode is probably useful. I always think it's worth highlighting that plan mode isn't just 'don't make changes', it's a fundamentally different mode with a different prompt and objectives.

deleted 3mo ago

[deleted]

ebiester 3mo ago

You should do an experiment of splitting that up to multiple reviews that are logically together. My hypothesis is that you may be losing signal due to the amount of text expected back.

conception 3mo ago
Or a dynamic workflow. $$$ but lots of coverage.
Espressosaurus 3mo ago
Yeah. I have a set of 5 review prompts attacking different problems, an adversarial review, and then a final synthesis, with the best results gotten by multiple passes using multiple models (the adversarial review stage combining all passes into one review per model and the synthesis picking the best of the two or three adversarial reviews). Expensive but it actually finds real problems that the single pass reviews rarely seem to find.

is_true 3mo ago

Nice. I had a file with code samples (old code I wrote), including formatting and I asked to use it as a reference. Will try your approach to distill the code to bullet points.

throwaw12 3mo ago

Can you share your list? I am curious what does it contain, for me a lot of times its a back and forth with agent until it "looks good to my eyes and taste", but haven't written any such list yet, because it is context dependant, in some projects I forgive minor issues, or allow magical numbers, but in other projects I force agent to use constants with meaningful names `SECONDS_IN_A_DAY = 24 * 60 * 60`

willturman 3mo ago
It is interesting that the output of code is associated with sight and taste, while the quality of the code itself is associated with smell. https://en.wikipedia.org/wiki/Code_smell https://en.wikipedia.org/wiki/Code_smell *edit: that wikipedia page ^ itself is a pretty answer to your request for a list of things to avoid when writing maintainable code.
- blanched 3mo ago
  Pairs nicely with the fact that smell is the sense most likely to be experienced differently between two people :)
  - chowells 3mo ago
    Is that actually true? Just because English treats scents as only describable by analogy doesnt mean all languages do. There are a number of languages with scent descriptions for aspects of a smell that are transferrable. [1] That suggests everyone is capable of decomposing a scent into components the same way. If that's true, we're left with a question equivalent to "does everyone see the same red?". As far as I know, the pure version of that question cannot be answered because subjective experiences of sensation cannot be transferred. And at that point, I'd say the manner in which they're experienced differently is equivalent. 1. https://youtu.be/w3KswMaEBiI https://youtu.be/w3KswMaEBiI
    - blanched 3mo ago
      It’s not my area of expertise, but a friend in the field told me the fun fact originally. Looking into it, it seems like smell has the most potential for genetic variance: https://pmc.ncbi.nlm.nih.gov/articles/PMC3990440/ https://pmc.ncbi.nlm.nih.gov/articles/PMC3990440/ That said, I can’t find any direct research comparing perceived senses. So they (and now I) might have oversimplified. Thanks for the link, looks interesting!

wismwasm 3mo ago

I have good experience with using open-code-review: * https://github.com/alibaba/open-code-review https://github.com/alibaba/open-code-review ** https://layandreas.github.io/personal-blog/posts/beyond-videcoding/#step-3-ocrreview---review-the-proposal https://layandreas.github.io/personal-blog/posts/beyond-vide...

dugmartin 3mo ago

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.

soperj 3mo ago

Why wouldn't it already just do this though?

notatoad 3mo ago
It does. Claude has a built in review and it’s pretty good. But it doesn’t know exactly what you want. This is a way to tell it. Good way to double your token use though, if you’re concerned about that.
- zahlman 3mo ago
  > But it doesn’t know exactly what you want. This is a way to tell it. What ever happened to communicating through code? Agents can follow examples and infer patterns, and they can read commit history and diffs. Real-world commit logs for human-only projects are dominated by short commits (well, at least the ones where the humans are skilled, appreciate version control, care about the project, etc.) with thoughtful commit messages. cf. https://wiki.c2.com/?JustCorrectDontPoint https://wiki.c2.com/?JustCorrectDontPoint
kmoser 3mo ago
Unless prompted to do so, why would it? From the article: "Every shortcut you merge into your codebase is a signal about how things are done here."
- soperj 3mo ago
  Not duplicating code is literally best practice. Why would it deviate from that?
  - webjunkie 3mo ago
    This is far from a universal best practice and heavily depends on what code it is and for what reason you are duplicating it potentially.

xhrpost 3mo ago

> 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.

radlad 3mo ago
I find project-wide or subpackage-wide (for sufficiently large projects) CLAUDE.md's that document patterns (including for tests) solves this. /init will make a project-wide one, or you can instruct it to "Create CLAUDE.md in any sub-directory that is sufficiently complex" then modify from there.
selicos 3mo ago
You're forced to document every check and thought related to development until you're wiring a manual on how to be laid off (unless self employed or otherwise critical). Which is fine. One should write quality docs and all that. But it seems exactly like training my offshore contract replacement instead of building tools for my own use. Except that income doesn't even go to a human just a black box datacenter company and shareholders.
cadamsdotcom 3mo ago
My review command isn’t an AI tool, it is is a text file. You don’t need a SaaS when you can use a text file instead. Just be incrementally aware of and closing gaps in the list of things you care about, and make it part of your day!

jhghbj 3mo ago

[dead]

zahlman 3mo ago

> 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.

cadamsdotcom 3mo ago
This technique incrementally codifies the subset of my personal definition of code quality that can be codified. I’ve found a discovery based approach is better than anticipatory, because it forces everything you spend tokens to prove its worth. I still do code review - how else would I know what to codify! But since starting this about a year ago, I don’t review menial dumb AI mistakes anymore.
jason_s 3mo ago
> there has been a library of information on HN by now about how to use agents effectively yes, but where?
- zahlman 3mo ago
  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.

catzapd 3mo ago

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.

cadamsdotcom 3mo ago
There’s a saying for this - “don’t ship your first draft”. Its first attempt WILL be crap. You can refine its output in the loop as a human, or let it go and then do a quality pass post-hoc. The beauty of the latter is you can codify and automate a subset of the work, and the agent can loop until those bars are cleared then you get a higher quality work product to look at.

andai 3mo ago

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...

cadamsdotcom 3mo ago
Yes, and, write custom linters for things you want to prevent forever. Have a look at https://github.com/cadamsdotcom/CodeLeash/blob/main/scripts/check_dynamic_imports.py https://github.com/cadamsdotcom/CodeLeash/blob/main/scripts/... for an example (enforced before all commits via https://github.com/cadamsdotcom/CodeLeash/blob/main/.pre-commit-config.yaml#L40 https://github.com/cadamsdotcom/CodeLeash/blob/main/.pre-com...) Basically I got sick of telling the agent to put dynamic imports at the top of the file so I made them into an error that blocks commits. Now I don’t even need that in the review.md because it can’t make it though to /review.

overgard 3mo ago

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.

ricardobeat 3mo ago
Sometimes the fact you mentioned “signing commit messages” is exactly why it starts signing commit messages, and it’s better to start with no prompt at all. Negative prompting is very unreliable. Giving exact instructions on how you want commits made will give you better results.
- overgard 3mo ago
  How am I supposed to take this tool seriously if it struggles with the concept of "dont"? if what you're suggesting is true, then more important instructions like "Don't delete the production database" are a problem. I shouldn't need to consider how to phrase "Don't delete the production database" in a positive manner. Isn't the point of an AI agent that it understands my intent and I don't need to hold its hand? I'm not saying your suggestion is wrong, I just think that suggests a limit to what these tools should be used for if that's the case. My guess though is that it probably ignores some of the positive instructions too, and the hardcore AI users mostly don't notice because they probably aren't reviewing the work.
  - 3mo ago

pipes 3mo ago

This sounds like gold to me, thanks. I'm going to try it.

yiyingzhang 3mo ago

> My list is like 200 items now 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 :)

0x457 3mo ago
> 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. We (humans) don't do 200 point inspection, but we also don't review every code change in a spherical vacuum without prior knowledge of the project. However, we do check a lot of things. > What's hard for AI is also hard for human, if not more. That's just plain wrong. AI straight up suffers with counting things, and I'm not just talking about counting 'r' in strawberry. Some things are easier for AI some are easier for human.
cadamsdotcom 3mo ago
Humans pattern match against thousands upon thousands of criteria subconsciously. AI is a different intelligence - a 200 point list shoved in its face all at once doesn’t overwhelm it. That’s only 5-10k tokens! Bear in mind though, because I added all the items on the list I’ve seen the reasons they’re there & act as a backstop against it missing things. There’s always got to be a quality bar, it’s part of being a professional and signing your name on your work.

nextaccountic 3mo ago

Just one thing A model might be able to follow 200 different instructions at the same time, while another model will choke on it

cadamsdotcom 3mo ago
Yes and this is a really important point. I actually have no data on how other models do - feeling very spoiled by Claude to be honest. Would love to hear how it goes if you try it with other models!

tinasky 3mo ago

A couple of notes for anyone reading this comment and thinking of implementing something similar: - You should use skills instead of commands (commands still work, but they've been rolled into skills) https://code.claude.com/docs/en/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.

cadamsdotcom 3mo ago
Thanks for this, I’d agree and elaborate a little - Skills over commands: agreed - but they are more complex than a single file. I’m suggesting a command because it’s a great way to start and build up that checklist. Over time, migrate it to a skill. But one thing I’d say is keep it in your repo. That way, PRs can include new bullet points with the code that gave rise to them. Opinions will differ on this: it muddies the PR a little in the present. But OTOH future folks can git blame to see where that bullet came from. - Hardcoding against main: yes, this is a very good catch - a better prompt is to tell the agent to compare to the branch’s merge base. Again a detail left off for simplicity to encourage folks to start somewhere. Your suggestion is a quick win fast follow for correctness. (side note - 90% of the time Opus 4.8 guesses the intent and decides to use the merge base on its own! Opus 4.6 didn’t do that. These things are smart if you set them up for success!) - Plan mode for review I would say personal choice. It is crucial in my experience because some of my list makes it overzealous in planning cleanups and the like. YMMV. Yes plan mode is optional and I sometimes have to reread and discuss a review plan after a context switch in my day - but at least it didn’t spray bad cleanups all over a decent PR.
- tinasky 3mo ago
  A skill is a single file too thought right? Just `.claude/skills/review/SKILL.md` instead of `.claude/commands/review.md`. I think the format is more or less identical as well? I may be missing some subtle difference though. As you say, either will work fine for this, but in general Anthropic's advice is to just use skills now. Agreed on the plan mode. I think it's personal choice and also situation-dependent. When I've already reviewed everything the agent has written and made changes along the way, I'm pretty confident a review is only going to throw up minor tweaks. When reviewing someone else's code or if you've got a load of changes you haven't reviewed yourself yet, plan mode is probably useful. I always think it's worth highlighting that plan mode isn't just 'don't make changes', it's a fundamentally different mode with a different prompt and objectives.