As I continue working on projects of my own, as well as those in my day job, I am learning more about the best ways to use AI to create software. Mostly Claude and sometimes Codex.
One thing I cannot help noticing is the sheer number of comments AI adds to the code.
In January 2025, I wrote a blog post called Comments in Code: Just Don’t Do It, in which I quoted Uncle Bob:
It is well known that I prefer code that has few comments. I code by the principle that good code does not require many comments. Indeed, I have often suggested that every comment represents a failure to make the code self explanatory. I have advised programmers to consider comments as a last resort.
I went on to say:
- Comments should always be “why”, never “what”, and only when absolutely necessary.
- If you feel the need to write a “what” comment, put the code in a well-named, meaning descriptive, function instead.
I stand by this view. Comments should be few and far between and, when they are necessary, should explain “why”. I have had long and sometimes heated discussions with senior people about this. I have stood firm for more than 20 years. However, I am now allowing AI to litter the code it writes for me with comments.
This made me wonder whether I was ignoring my own advice. So, with Claude, I looked at the codebase of one of my recent projects to see what sort of comments it had actually added.
The codebase has an unusually consistent, essay-like comment style. It is clearly deliberate rather than incidental. However, almost none of the comments describe what the code does. The identifiers already tell you what the code does. Instead, they explain why the code is the way it is.
A few patterns stood out.
Why this design and not the obvious alternative
From Logs.tsx:
// Deliberately not reported. The row fetch is asking the
// same BFFabout the same window, and has already bannered
// whatever wentwrong; a second banner for one outage would
// name a chart nobody was looking at. LOGS-14.
const counted = histogram.data;
The obvious implementation would check histogram.isError and display an error banner.
The comment explains why that error path was deliberately not built. The row fetch has already reported the same underlying failure, so displaying another banner would tell the user about a chart they were not looking at and make a single outage appear to be two problems.
It also references LOGS-14, where the decision presumably came from.
Without the comment, a future software engineer, or AI, might see the missing error handling as an oversight and helpfully put it back.
A non-obvious gotcha
From client.ts:
for (const [key, value] of Object.entries(params)) {
// Before the emptiness test rather than inside it:
// `String(null)` is "null", which is a param whose
// value is the word rather than a param with no value,
// and it is wrong in a way only the wire would show.
if (value === null) search.set(key, "");
else if (value !== undefined && value !== "")
search.set(key, String(value));
}
The code is reasonably clear about what it does. The comment explains why null is handled in its own branch before the generic String(value) path.
Without it, null would silently become the literal string "null" on the wire. That is the sort of bug which looks perfectly reasonable in the code and only becomes obvious when inspecting the network traffic.
Again, the comment is not narrating the code. It is recording a trap. However, you could argue that a software engineer, and an AI, should already know how String(null) works.
A cross-repository contract
From the top of metrics.ts:
// The contract for the read calls is the OpenAPI block
// above each handler in the sibling `omnilog-query`
// repo (`src/handlers/metrics-*.mts`, and `services.mts`
// for the shared services route); the BFF proxies them
// verbatim, query string included. Nothing here is
// cursor-paginated, so nothing here goes through `toPage`
/ — each answer is one envelope holding the whole of it.
//
// `fetchMetricDescription`/`updateMetricDescription` are
// the one pair thatisn't a proxy: a metric's description
// is domain data the BFF owns itself, the same precedent
// dashboards set (pjgrenyer/omnilog-frontend#8) ...
This is a file-level comment rather than one attached to a particular function.
It tells the reader that the real API contract lives in a different repository entirely. It also explains that the BFF proxies those calls verbatim and that they are not cursor-paginated, which is why they do not use the otherwise familiar toPage path.
Finally, it identifies the only two functions that break the pattern and links that exception back to the pull request that established the precedent.
None of this could be expressed adequately through a better function name.
Why a test is shaped this way
From stalled-session-check.test.tsx:
// The failure the shell had no answer for. `App` shows
// the splash and nothing under it while GET /api/me is
// in flight — deliberately, because the sign-in gate
// would tell a signed-in user they are signed out — and
// that is only safe if the request is certain to settle.
// A BFF that stalls rather than errors settles nothing,
// and the user is left watching the mark with no explanation
// and no retry, forever.
//
// The bound is `SESSION_TIMEOUT_MS`, so these run the clock
// rather than the wall: waiting the real interval out would
// make this the slowest file in the suite and prove exactly
// the same thing.
The test itself includes:
it("when the session check has not yet run out of time, then the shell has not given up on it", async () => {
// The other half of the bargain. Falling through
// early would reintroduce the flash of the wrong screen
// that waiting behind the splash exists to avoid.
vi.useFakeTimers();
stubBff({ "GET /api/me": stalls });
The file-level comment explains why the test exists at all.
The application deliberately shows only the splash screen while GET /api/me is in flight. Showing the sign-in screen could briefly tell a signed-in user that they had been signed out; it is safe only if the request is guaranteed to finish.
A BFF which fails produces an error. A BFF which stalls produces nothing, leaving the user staring at the splash screen forever.
The rest explains why the test advances fake time and guards both boundaries: the application must eventually give up, but not so early that it flashes the wrong screen.
Have I changed my mind?
Not really.
These comments all follow the rule I set out last year. They explain why, not what. They record design decisions, hidden traps, external contracts and failure modes. Removing them would remove information which the code alone cannot express.
What has changed is the cost of writing and maintaining comments. Traditionally, software engineers aren’t great at keeping them up to date. Code changes, the corresponding comment does not, and the comment becomes inaccurate and useless, if not dangerously misleading. In my experience, AI is better at keeping comments in sync, provided it has the relevant context. It never complains about updating the documentation.
Software engineers often understand why they have written something but do not take the time to capture that reasoning. When AI has been part of the discussion that led to the code, it can turn that reasoning into a comment. Perhaps too readily. Left unchecked, it will happily write an essay beside every decision.
That still needs judgement. A long comment which merely translates the code into English is still noise. An incorrect or out-of-date comment is still worse than no comment. AI-generated comments need reviewing just as much as AI-generated code does.
But a comment which stops the next software engineer from “fixing” a deliberate decision, exposes a bug that would otherwise only appear in production, or records a contract which lives in another repository is useful.
So I still believe comments should be a last resort.
It turns out AI can be rather good at finding the occasions when that last resort is justified.
Comments
Post a Comment