Review this for me

After publishing Refactor this for me a colleague reached out:

“hey, I think the original code was better. Why did you need the effect for?”

Knowing him for a few years, I knew his point was: can we have a single source of state changes?

So we talked about what I was doing. I was beginning to migrate an existing screen to use the DataViews component. I wasn’t yet familiar with how the existing screen worked, but I explained what I knew: it filters some data based on dates. The dates state is updated differently depending on the UI component. Some components would trigger state changes that end up updating the URL. Some other components (auto-refresh), would trigger interval-based state changes but don’t update the URL.

As I was writing this down, I realized:

“Why doesn’t the interval trigger an URL update as well?”

I proceeded to refactor the existing code removing all intermediary state handlers: every component would trigger a URL state change directly. That got us a single source of truth for state, which prevents unnecessary re-renders, requires less code to maintain, and simplifies the data flow. It was just a little day-to-day thing, but it triggered a question.

Could a LLM provide an expert-level code review?

I was curious if there were ways for a LLM to give me feedback along the same lines an expert-level human would, so I experimented.

First, I tried open questions (“review and suggest changes”) and they didn’t provide anything meaningful or related to state management. All they offered was general or low-level suggestions: “split your components for better composability”, “use TypeScript enums instead of string literals”, etc.

I narrowed down the questions to performance:

“Me: I’ve noticed this code has multiple sources of state updates. For example, it’s using useInterval to trigger date range changes upon each interval tick. Can you suggest anything to reduce the number of re-renders in this component?”

This resulted in a bunch of “memo more things” and “batch state updates” — fine, it’s related. But suggestions were still generic and not really getting to the bottom of the issue. I asked other questions, until I finally became frustrated and wrote:

“Me: Are there ways to refactor this code to make it all dependant on URL changes (a single source of state updates)?”

“LLM: Yes! Converting the application to be URL-driven would create a single source of truth and simplify state management.”

The LLM response was “cute”.

Indeed, it suggested code changes that made sense, along the same lines I had already done. It wasn’t surprising because higher-level refactorings is something they are getting good at. But I wouldn’t consider this a success given it failed to provide impactful improvements to the data flow until I mentioned them explicitly — the whole point of a code review.

There are reasons to be positive about using LLMs in code reviews. For example, they perform fine at small-scope code changes that are structured and established such as TypeScript errors, or CSS flexbox issues. The question I don’t have an answer for is: how do we scale them up from these low-level problems to things more impactful such as application-level data flows and domain-specific code?


Comments

Leave a Reply

Your email address will not be published. Required fields are marked *