How do you decide when to let an AI agent refactor working-but-ugly code vs just leave it alone?

by•

I've got a function in one of my projects that's been "temporary" for about 4 months now. It works, it's covered by tests, and it's ugly enough that every time an agent touches that file it asks if I want it cleaned up.

Every time I say yes, I regret it a little. Not because the refactor is bad, usually it's genuinely cleaner, but because now I've burned review time on code that wasn't broken, and there's a small chance the agent introduces a subtle behavior change I won't catch until it's in prod.

So now my rule is basically: only refactor if I'm already touching that code for a real feature reason, never as a standalone cleanup pass. Curious if other people have a clearer line than that, or if you just let the agent refactor opportunistically whenever it offers.

25 views

Add a comment

Replies

Best

This matches what we've landed on too. The rule that's worked best for us: never let an agent refactor code it wasn't already touching for a real reason. The moment "clean this up" becomes its own PR, you've traded a known-working system for review time spent verifying behavior didn't shift - and that trade is almost never worth it for code nobody's actively working in.

The one addition I'd make: how much I trust an opportunistic refactor scales with blast radius, not with how ugly the code looks. A messy pure function with full test coverage is low risk even as a standalone cleanup. A messy function that touches shared state or external calls, I want a feature reason before I'll even let the agent near it, tests or not - because the failure mode isn't "tests fail," it's "tests still pass but the edge case that mattered isn't covered."

My rule ended up almost identical to yours: never a standalone cleanup pass, only when I'm already in that file for a real reason. What pushed me there was your exact fear: a clean-looking refactor that silently changed an edge case with no test to catch it. The one thing I'd add: I let the agent refactor freely only where I trust the test coverage. If the ugly code is well-covered, the downside basically disappears. If it's not, "ugly but working" beats "pretty but unverified" every time. So it's less about the code and more about how much I trust my tests around it.

I follow a similar rule. If the code is stable, tested, and not slowing down development, I'd rather leave it alone. I usually refactor only when I'm already modifying that part of the code for a feature or bug fix.

It keeps the review focused and reduces the risk of introducing subtle regressions just for cleaner code.

I have seen the same pattern where agents always suggest cleaning tested code. How do you decide when the readability gain is worth another careful review cycle?

I usually hesitate after tests already pass. What kind of behavior changes have surprised you most after accepting a refactor?

I agree that working code deserves caution even when it looks messy because production surprise are expensive. Have you tried limiting agents to structural changes that guarantee identical behavior through stricter rules?

Really solid thread. On Jason's question - the readability gain has to come with a "future me/future agent will misread this" story attached, not just "this looks nicer." If I can't name a specific way the ugly version will bite someone later, I don't think a review cycle is worth spending. On Advin's question - the surprise that stuck with me was a refactor that flattened a nested conditional into early returns and quietly changed which branch ran first when two conditions were both true, tests passed because none of the existing tests covered that overlap case. Gal's blast-radius framing plus Esat's test-coverage-first rule both map onto that same incident for me.

@Gal Dayan the blast radius framing is better than mine honestly, I was thinking about it purely in terms of "how ugly" which is the wrong axis entirely. a messy pure function is basically free to clean up because a bad refactor there just fails its own tests loudly. the scary case is exactly what you said, shared state or external calls where the tests all stay green but some edge case silently changes behavior. I think I've been letting agents refactor based on annoyance rather than risk, going to flip that

Coming from a software developer background, I'm quite opinionated about what good code looks like. But just as with human teammates, you have to accept some compromises with AI agents. One thing I’ve found useful is instructing the agent to add new rules to its own instructions whenever I ask it to fix a recurring style or quality issue during code review. With enough of those rules, plus linters and style checkers, it can usually get about 90% of the way to what I expect. The remaining 10% would probably take another 90% of development time. Clearly, even LLMs follow the ninety-ninety rule :D.

The nice thing is that, in a situation like the one you described, the agent may already recognize the code as ugly and refactor it on its own. Just be careful not to let your CLAUDE.md file grow too long.