Remix.run Logo
cadamsdotcom 4 hours ago

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.

andai 5 minutes ago | parent | next [-]

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

xhrpost 2 hours ago | parent | prev | next [-]

> 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 an hour ago | parent [-]

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.

throwaw12 3 hours ago | parent | prev | next [-]

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 2 hours ago | parent | next [-]

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.

blanched an hour ago | parent [-]

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

nicoty an hour ago | parent | prev [-]

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!

wismwasm 2 hours ago | parent | prev | next [-]

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

dugmartin 2 hours ago | parent | prev | next [-]

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.

ebiester 3 hours ago | parent | prev | next [-]

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.

Espressosaurus 2 hours ago | parent | next [-]

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.

conception 3 hours ago | parent | prev [-]

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

catzapd 9 minutes ago | parent | prev | next [-]

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.

3 hours ago | parent | prev | next [-]
[deleted]
soperj 2 hours ago | parent | prev | next [-]

Why wouldn't it already just do this though?

kmoser 2 hours ago | parent | next [-]

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 43 minutes ago | parent [-]

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

notatoad 2 hours ago | parent | prev [-]

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 37 minutes ago | parent [-]

> 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

zahlman 44 minutes ago | parent | prev | next [-]

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

is_true 3 hours ago | parent | prev | next [-]

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.

jhghbj 2 hours ago | parent | prev [-]

[dead]