radiocli Home Blog GitHub

All posts

Grading My Own Homework

I did something this week that would have been unthinkable for most of my career: I asked for a performance review, on purpose, from a reviewer with no reason to be polite. I pointed an AI at the radiocli repository, all 105,000 lines of Go plus the documentation and the research notes, and asked for a full audit with a letter grade for the repo and a second grade for the engineer behind it. That second ask is the one that takes a deep breath. The repo is code; the engineer is me.

I've been coding professionally since 1996, and in that time I've sat through every flavor of code review the industry has invented: the drive-by LGTM, the nitpick festival that argues about brace placement while a race condition sails through, the review that never happens because the senior person is busy and the junior person is scared. What I have almost never gotten, in thirty years, is the review I actually wanted: someone smart reading all of it, carefully, with no social pressure to go easy and no fatigue setting in around file forty. So I asked the machine for it.

Six reviewers walk into a repo

The audit didn't run as one long read. The model split the work into six parallel reviewers, each owning a slice: the core architecture and the daemon, the serial protocol and port-locking layer, the command packages split in half (there are 31 of them), the audio stack together with the end-to-end hardware test suite, and the documentation with the research notes. Each reviewer read its slice at depth, and, this is the part I care about, each one ran things instead of taking the code's word. They ran the tests. They ran the race detector on the concurrency-heavy broker. They checked whether the 100% statement coverage that every one of the 45 packages claims was real coverage or ceremony.

That last check matters more than any other, because coverage is the easiest metric in software to fake, sometimes by accident. You can hit every line of a function with a test that asserts nothing. Three of the six reviewers independently went looking for that, and all three came back saying the coverage survives inspection: the command tests drive a stateful fake radio that tracks menu position and cursor state, the protocol tests feed real XML documents through fakes that chunk reads the way a serial port actually does, and the audio ring buffer tests use position-dependent ramp data specifically so that lost, duplicated, or reordered bytes cannot masquerade as valid audio. When a reviewer says the tests are behavioral and not line-touching, and then cites the fake by file and line, I believe it.

The whole thing took about four minutes of wall clock. Fagan inspections at IBM in 1976 established that formal code reading finds defects cheaper than testing does, and the industry believed it, briefly, and then spent the next five decades not doing inspections because they cost a room full of senior engineers an afternoon per few hundred lines. The economics were the problem, never the idea. The economics just changed.

The report card

Five A's and one A-. The device and protocol layer, the two halves of the command layer, the audio stack with its test harness, and the documentation all came back A. The core architecture came back A-, and the A- is the interesting grade, because the reason for it is a real bug, the best find of the whole audit.

radiocli has a daemon mode: one process holds the scanner, and other invocations queue for turns instead of being refused at the port. A client can also take a lease, a time-limited exclusive claim, so a script can run several commands in a row without losing its place in line. The bug lives where those two features meet. If a lease's TTL expires while one of the leaseholder's commands is still executing, the expiry path releases the scanner and wakes the next waiter in the queue, whose command then starts running while the first one is still on the serial line. The scanner answers on one wire with nothing identifying who asked, so two concurrent commands read each other's replies. That is the failure mode the entire broker exists to prevent, stated in a comment in the runner itself, and the expiry path walks right past it. The reviewer found it, named the invariant it violates, cited the comment that states the invariant, and proposed the fix: hold expiry until the in-flight command finishes.

I want to be precise about why that finding impressed me, because it's not the severity. It's that the bug is an interaction between two features that are each individually correct. The scheduler is right. The lease bookkeeping is right; the reviewer called the FIFO scheduler's abandonment handling textbook-correct and rarely done right, which was nice to read. The bug only exists in the seam, in the one sequence of events where a timer fires during a window that no single component owns. Human reviewers miss seam bugs constantly, not because they're careless but because reviewing a diff shows you components and seams are not in any diff. A reviewer that reads the whole system at once has a shot at seams that a reviewer reading one pull request at a time structurally does not.

The rest of the findings ran from embarrassing to instructive. Two near-copy packages, systems and sites, have drifted apart in four separate behaviors, which is the tax you pay for a copy-paste convention, and I knew I was signing up for that tax when I made the rule that command packages never import each other. A no-op path in one command prints a human-readable table even when the caller asked for JSON. Empty lists serialize as JSON null instead of an empty array, which every strict decoder on earth treats differently. And two guard rails in the hardware test suite, ones the code proudly documents, turn out not to guard: the check that stops tests from sending the one command known to lock the scanner up can be bypassed by the JSON helper that most tests actually call, because the helper prepends flags and shifts the forbidden token out of the position the guard inspects.

The one that made me wince was none of those. The audit found a leftover skill directory from a predecessor project, still sitting in the repo, documenting a binary that doesn't exist, a command that was never built, and a config behavior the current tool explicitly forbids. Dead documentation that reads as authoritative is worse than no documentation, and shipping it was half-assed housekeeping on my part. Everything else on the list is engineering tradeoffs; that one is just a mess I didn't clean up.

Writing the ticket before the fix

Verdicts evaporate. A grade is a feeling by Friday, and a four-minute audit that produces only a feeling is a demo, not a tool. So the next thing I had the model do was convert everything it found into a findings document: one Markdown file, every finding numbered, sorted into severity bands from critical down to insignificant, each with a one-sentence technical summary ending in a file and line reference, then three sentences of detail. Forty items. The critical band is empty, and the file says so out loud rather than omitting the heading, because an explicit zero is information and a missing section is a shrug.

The shape of the list tells its own story. One high, the lease race. Eight mediums, which are mostly contract violations: output that breaks the JSON promise, guard rails that don't hold, the dead skill. Twenty-one lows, which are the edges: a protocol parse that hangs on a self-closing XML root the firmware has never sent, menu-walk wrap detection that breaks if a user names two entries identically, a lock-file scenario requiring macOS's temp cleaner to fire mid-session. Ten insignificants, dead fields and naming slips and a comment that describes a one-pass hash when the code does the stronger two-pass version. Nothing in the pile is mysterious. Every item has an address and most have a fix already sketched.

Here's the thing about that document: it's the review artifact I spent years trying to get human processes to produce and almost never got. Human review output is a conversation thread, comments scattered across a diff, half of them resolved with a reply instead of a commit, the whole thing unfindable three months later. A numbered list with severities and line references is a work order. I've written plenty of these by hand after incident postmortems. Getting one generated in minutes, from a fresh read of the entire codebase, with the numbering done so I can burn it down item by item, changes what a solo project can afford.

What the grade is actually worth

The skeptical read of all this is that I let the machine grade its own homework and it gave itself an A, since AI was in the loop when plenty of this code got written. It's a fair jab and I want to take it seriously rather than wave at it. Two things give the grade weight for me. First, the findings are checkable. A vibes review says "solid work, minor issues"; this one says the guard at a specific line inspects args[0] while the helper at another specific line prepends two flags, and I can open both files and watch the claim be true. Every finding I've spot-checked so far holds up. A review is only as trustworthy as its citations, whoever wrote it, and this one is dense with them.

Second, the review found the thing it was structurally disposed to miss. If the model were grading generously, the lease-expiry race is the finding that doesn't get made: it requires holding the scheduler, the lease timer, and the runner's invariant in mind at once and noticing that a timer callback written months apart from the runner violates a comment inside it. Flattery doesn't do that work. And the grades weren't uniform; the architecture slice took an A- with a specific, correct reason while easier slices took A's. A reviewer that differentiates is a reviewer that read the material. I've graded enough intern code to know the difference between an evaluation and a rubber stamp, and rubber stamps don't come with forty line-referenced defects attached.

The engineer grade came back A as well, and the model's reasoning for withholding the plus was the same as for the repo: the one uncovered invariant in the broker, and copy-paste drift I could have prevented with two hours of refactoring. I'll take that. It's a more honest performance review than most humans ever get, because most humans are reviewed by people who have read a fraction of their work and remember less of it.

Banging out the list

So now the plan, and it's the least glamorous plan in software: start at finding number one and work down. Fix the lease expiry so it waits out the in-flight command. Route the daemon's expiry warning through the logger that doesn't get swapped to a client's stream. Close the JSON gaps so a script gets parseable output on every path, including the no-op ones. Fix the two guard rails so they enforce what their comments promise. Delete the dead skill directory entirely; it documents a tool that doesn't exist and nothing that reads as authority should be allowed to lie. Then down through the lows and into the insignificants, the dead fields and the naming slips, because the difference between an A and an A+ codebase isn't any single fix. It's the absence of the pile.

Then I'll run the audit again, fresh reviewers, no memory of this round, and see what grade comes back. Maybe it finds new things; audits usually do, and a second pass that found nothing new would make me trust the first pass less, not more. But that's the loop now, and it's a loop one person can actually run: audit, list, burn down, audit again. The room full of inspectors that IBM couldn't afford to convene weekly in 1976 now convenes in four minutes, and it never gets tired, and it reads everything.

An A is a fine grade. I didn't spend thirty years at this to be fine.