Hacker News Reader

Comment by cadamsdotcom | Hacker News Reader

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.

View on HN

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.

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.

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.

Absolutely. It should simply not have permissions to delete your production database, can’t rely on prompting alone for that kind of safety.

These are still stochastic machines, guardrails must be inserted at the system level.

They are getting better every day about managing their own guardrails, so we will get there eventually.

I entirely agree, although I think this is counter to the narrative that we should just let the agent do everything that the labs want to sell.

There's a really major problem with the concept of human-in-the-loop though, which is just that humans are not built for that that kind of work. It's like all those tests against the TSA where they manage to sneak something through that should have been caught. But the problem is, a TSA agent sees probably like 1000 things they can ignore to the 1 thing they need to look at, and it's easy to just start rubber stamping things.

If you're relying on asking the LLM "pwease don't delete" then you're already in trouble. This kind of stuff doesn't work with people either and they generally exhibit actual signs of intelligence.

Sure; it's an example, I would never rely "Please don't delete" for real important data. However, we're being sold an idea that we should just let the agent do everything, so I think it's important to point out how utterly insane that idea actually is.

> How am I supposed to take this tool seriously if it struggles with the concept of "dont"?

Don't think of a pink elephant in a tutu.

What did you just think about? =)

This has been true since the very first GPT release, any words you say to the model will nudge it towards that word - it doesn't matter if you have a negative in front of the word.

Form the guidance using positive words like "always use the development database at..." instead of "don't access the production database" ... Now it "knows" that the prod database exists and "thinks" about it when processing.

I personally find that models are trending towards ignoring any instructions given to them, so depend more on vendor-instilled behavior. Anecdotal, but I had bad experiences with OAI's new 5.6.

Same for me. Not sure if its intentional (do they want to push people towards workflows that will burn more tokens?) or if it's just a side effect, but it's very annoying. I refuse to use tools that dictate how I develop, I'm just going to switch models to less opinionated ones.

I let it have elevated permissions and it started doing some crazy things like figuring out I had zsh and running zshell commands instead of the common ones.

I’m not a Claude code user, is there really no way to get it to not commit to git other than “asking it nicely in a prompt”? I thought there was some sort of permission system?

There are options, but they're not great.

I could ban it from using git, but having access to git logs helps it do work so I don't want to go that far. I could probably ban the subcommand, but as a convenience I do like to be able to ask it to make a commit. (I'm not super attached to this, I frequently commit myself just to avoid these issues). It does tend to write good commit messages, so sometimes I ask it to generate the commit message and then just do the operation myself, which kind of sucks in the sense of feeling like reverse-centaur (Cory Doctorow term). (The reason I do that is sometimes it gets confused where if I greenlit one commit, it assumes all future commits are greenlit)

I guess the other option would be to not use "auto" mode, but god there's just no way I want to sit around and it "yes" 50x a session. I'd rather just sandbox it and nuke it if it does something too stupid.

Run it in a Docker container and give readonly access to the .git folder.

This way the agent can read git logs (this is very useful) but cannot commit directly (I don't see any value in this).

If I’m going to be responsible for the code that Claude writes on my behalf, I’m also going to take full responsibility for the moment that it is added to the project history, so I’m going to at least review every line before I allow that to happen.

The automatic coauthor line is also just a pet peeve. It’s a tool, not a person. I don’t say that a commit was coauthored by vim - why should Claude get special treatment?

I want it to commit on my behalf, and I don't pay full attention before commits. But I do review every line at the PR level, just as I would if some other human were to submit a PR, and if I find something off, then I ask it to fix it before I will approve the PR.

re: automatic coauthoring -- you can turn that off, but I prefer to turn that on. Commit ownership is something that I want to make clear was not just a human, but a human + AI. Agentic coding does get special treatment from me, because the volition to make changes doesn't come from me. I want that to be clear in the history. The only time I do my own commits (without the coauthor line) is when I make some change myself, without a coding agent.

I wrote this to do that: https://github.com/deepbluedynamics.com/nemesis8, although I also added `gh` to it so it could do that if it needed to. It can be easily disabled during the container build. Supports at least 8 agent CLIs.

In opencode you can get granular, allow `git log *`, deny `git add *` and `git commit *`

Deny:

Bash(“git * commit *”)

And for bonus points you can have the agent write you a Pre Bash hook. Tell it to make the hook look in the command for anything that’s a git commit. The vaguer your instructions here the better. Let it pull some crazy regex out of its hat. You’ll be impressed.

I never ran into it making commits before but when I tried Fable it made a bunch of commits signed by Claude, but then in the next prompt I just said "don't make commits anymore" and it was like "okay" and saved its own little md file and it hasn't happened sense.

I want to be clear that I'm not saying this to discredit the people saying they can't get claude to stop making its own commits, I'm just adding data to the "wow this is inconsistent" collection.

I have commit signing enabled by default. I run my harness in a sandbox which doesn't have access to my signing key, and I always tell it not to commit. When it ignores me, the commit fails because it can't access the signing key... so the agent commits without signing. Tragic.

I do the opposite. I force LLM to "do jj new -m <desc>" after every logical batch of edits.

No signing is easy - make signing interactive and requiring a password.

Anyway, telling agent to NOT do something is always worse than telling agent to do something.

That's why you should say something like "Use constants instead of magic strings/numbers" rather than "Do not use magic strings/numbers".

Also whatever review its doing against the list of checks - must be a fresh context.

> Anyway, telling agent to NOT do something is always worse than telling agent to do something.

> That's why you should say something like "Use constants instead of magic strings/numbers" rather than "Do not use magic strings/numbers".

Pro tip: This is good guidance for communicating with humans as well. Don't just block people but give them a path forward. (But don't force them down that path, because people really hate feeling like their agency is being impinged.)

The commit signatures are a setting, you have to disable it in config

I have dedicated checkpoint and commit skills. Checkpoint has it run my review skill, then run my commit skill. The commit skill just asks it to suggest commit messages for my review. Having these skills in place has resulted in claude preferring to use the commit skill instead of committing on its own, and suggesting commit messages to me before actually doing the commit.

Why are you having it commit or write commit messages unless you have no idea what it’s doing?

Who likes writing commit messages? In the abstract, perfectly spherical developer mindset, I'd spend hours writing the best commit message to go with the most elegant and perfect, beautiful code that I'm comitting, but the real world with meetings and life demands is greasier and grimer than that. Letting the LLM start off with the commit messages for me to edit with additional details has been a boon to productivity. Staring at an empty $EDITOR my brain goes blank, but with a framework of what's going on, I find the details for me to add flow more easily.

Use hooks.

My assumption is somewhere in the harness toolchain there's a prompt inject that says:

    - Always sign commit messages

In which case, Claude is being given conflicting instructions, which is a different class of problem than too many instructions.

> You know what it constantly does? Commits without permissions

You know what solves this? A deny rule. That’s right, you can solve it committing without permissions by.. not giving it permissions.

> it's quietly messing up on a lot of that list.

If only there was some way to know. But alas, code review won’t be invented until 50 years ago.

>and then I ask it

Why? It has no idea why it does what it does…

You should set things like this in Claude's settings.json rather than just asking the model and hoping it will obey. That way it will be enforced by the harness.

As some other comments have said, there are various other options like hooks which are run by the harness and can be used to always enforce certain things (running a formatter for example).

But for your examples, settings are the correct solution. Set an ask permission on git commit and it will always prompt you before committing. For commit messages, just set attribution commit to an empty string and it will stop adding its own attribution. https://code.claude.com/docs/en/settings#attribution-setting...

Two tiny changes; problem solved permanently.

Thanks, that's helpful, although I can't help but appreciate I had to get this suggestion from a person rather than the AI..

Totally. For anything like this, I find asking people or just reading the docs way quicker and much, much more reliable than asking AI. It's interesting that the models seem to know very little about their own abilities.

I've found the only thing that will actually work for cases like this is a programmatic hook to block Claude from the action.

Non-deterministic behavior via code.

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.

Every harness I've tried has obeyed that.

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

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.

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.

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!

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`

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

*edit: that wikipedia page ^ itself is a pretty answer to your request for a list of things to avoid when writing maintainable code.

Pairs nicely with the fact that smell is the sense most likely to be experienced differently between two people :)

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

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/

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!

I think that's because you can usually smell when food is close to being rotten before you can see it...

EDIT: reading the WardsWiki reference from that Wikipedia page, there's also the point made by early users of the term that smells are something you have to check out, but don't always mean something needs fixing - e.g. a bad smell may be a gas leak, or it may just be a rubbish bin.

I've codified mine into a reusable workflow https://github.com/nothingnesses/agent-scaffold . To be honest, this isn't fool-proof though, since the agents can simply choose to ignore them, so I also like to pair this with deterministic linting and compile time checking.

For a Rust project, I created macros that output compiler errors when documentation and tests are not in a shape I want them to be, like missing function invocations or assertions, which forces the agent to address them, where otherwise they would've just worked around them by adding stupid trivial assertions like `assert_eq!(true, true)`.

That still isn't fool-proof either, but it helps minimise those instances. I'm bullish on the idea of integrating formal methods and model-checking with AI. I think that combo feels like a promising avenue for constraining the stochastic side of AI-generated code with something closer to deterministic verification. Provided you can write correct specs of course!

Somewhat out of date (it’s grown a lot!)

https://github.com/cadamsdotcom/CodeLeash/blob/main/.claude/...

I have good experience with using open-code-review:

* https://github.com/alibaba/open-code-review

** https://layandreas.github.io/personal-blog/posts/beyond-vide...

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.

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.

Or a dynamic workflow. $$$ but lots of coverage.

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.

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

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

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.

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.

Why wouldn't it already just do this though?

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.

> 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

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

Not duplicating code is literally best practice. Why would it deviate from that?

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.

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

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.

> there has been a library of information on HN by now about how to use agents effectively

yes, but where?

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.

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.

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.

> 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 :)

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

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.

Just one thing

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

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!

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

Yes, and, write custom linters for things you want to prevent forever. Have a look at https://github.com/cadamsdotcom/CodeLeash/blob/main/scripts/... for an example (enforced before all commits via 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.

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

Reply on news.ycombinator.com