Review What Actually Runs

Part of Fourteen Automations That Run Without Me.

The obvious way to build an automated code reviewer is to point a model at your repository and let it read. This fails in a specific and boring way: the repository is mostly not the system. It contains experiments, abandoned branches of thought, scripts you wrote once, vendored code you did not write, and a mirror of somebody else's data. A reviewer that treats all of it equally spends most of its budget on files that could be deleted without anyone noticing, and reports findings you will never act on.

The system is the small subset the machine actually executes at 3 a.m. That subset is discoverable, it is much smaller than the repository, and it changes slowly. Find it first.

Your repository is not your system. Your crontab is closer.

Step one: the census

A zero-model script builds a census of executed code. It reads the scheduler's own tables — the crontab for each user it can read, and the service manager's unit list — and extracts the entry points. Then it follows references one level deep: a shell entry point that sources a cycle script that sources a shared library yields three files, not one. One level is deliberate. Two levels pulls in most of the standard library of your own making and the ranking stops discriminating.

The output is a ranked list. Rank comes from how many live paths reach a file: a shared library that three schedules depend on outranks a script that one monthly job calls. Nothing about this step needs a model, and it costs nothing to run every night, so it stays accurate as the schedule drifts.

The first time I ran this I learned two things I did not know. A file I had assumed was central was not referenced by any schedule at all. And a twelve-line helper I had written in a hurry sat on the critical path of four jobs. That helper had never been reviewed by anybody, including me.

Step two: the ledger

A second file records, per script, the commit hash and date at which it was last reviewed. From that you get a clean definition of due: a file needs review when it has commits newer than its last review, or has never been reviewed at all. Not "reviewed more than 30 days ago" — a file nobody has touched does not need re-reading, and a file that changed yesterday does, regardless of when you last looked.

Priority is then the product of the two signals:

priority = usage rank × staleness

A heavily-used file that changed last night goes to the top. A rarely-used file that changed last night, and a heavily-used file that has drifted unreviewed for months, compete in the middle. A leaf script nobody calls that has not changed stays at the bottom forever, correctly.

This is the whole idea of the system: the scarce resource is attention, and attention should be allocated by a formula you can inspect, not by whichever file the model happened to open.

Step three: three files a night

The reviewer takes the top three due files and makes exactly one model call per file, with a fixed rubric. The rubric names six things and nothing else:

1. Correctness bugs.
2. Unhandled failure paths.
3. Quoting and parsing hazards.
4. State-against-signal gaps — what happens when this message arrives in that state.
5. Concurrency with the other daemons that share this machine and this repository.
6. Missing tests.

Output is structured: file, line, severity, and the evidence. A per-run spend cap stops the run when the projected cost would exceed it, and the files that did not get reviewed stay marked due for tomorrow. Nothing is lost by stopping early; the ledger simply is not updated for a file that was not read.

Three is not a magic number. It is the number of findings per night I will actually read. Set yours by the same method.

The rule that kills most findings

One hard rule does more for signal quality than the rubric does: never file an issue about style alone. A finding needs a failure scenario — with input X, outcome Y happens.

Models are extremely willing to tell you that a variable name is unclear, that a function is long, or that error handling "could be improved". All of that is true of all code, which makes it worthless as a nightly signal. Requiring a concrete failure scenario forces the model to either demonstrate a real path to a bad outcome, or say nothing. It cuts volume hard, and what survives is worth reading.

Two supporting rules: never review a file outside the census, and if the model fails or returns unparseable output twice for a file, skip it and log it. Never file a finding you could not parse. A half-understood finding is a future hour wasted.

When the reviewer may fix it itself

Here the mechanical check test comes back. For a finding on a file that has a mechanical verifier — a syntax check for the language, plus a nearby self-test or a dry-run mode where one exists — the system generates a minimal fix with a coding-tier model, applies it to a backup-protected copy, and runs the verifier.

• Verifier passes: commit the fix, close the matching issue, file nothing. The work is done and the record shows it.
• Verifier fails: revert to the backup, and fall through to the ordinary reporting path, tagged as attempted. I see both the finding and the fact that an automatic fix was tried and did not hold.

Two limits on that path. A cap of five automatic fixes per run, for the same volume-bounding reason as everywhere else. And a hard exclusion: the reviewer never touches the code of another daemon while that daemon's service is in a failing state. Fixing a file underneath a crash-looping process is how you convert one incident into two.

Everything the fixer cannot prove flows to issues instead, where a nightly worker or I pick it up. The model writes the patch; the verifier decides whether the patch lives.

The failure-mode table

Every job I run carries a short table of state against signal. The reviewer's table is representative:

Note the third row in particular. "Exit successfully" when the tracker is unreachable is a deliberate choice: an infrastructure outage elsewhere should not turn into a failing job here, because a failing job trains you to ignore alerts. Log loudly, degrade, carry on.

Proving it before scheduling it

The reviewer does not get a schedule until its self-test passes, and the self-test runs without a model and without network writes. It checks that the census parses a fixture crontab and unit list; that the ledger arithmetic is right in both directions (a new commit makes a file due, a recorded review makes it not due); that deduplication against open issues works with a stubbed tracker; that the report renders; that the cap stops the run at the limit; that the kill switch short-circuits. For the automatic-fix path it additionally proves that a failing verifier restores the original file and leaves a backup, and that the per-run fix cap holds.

That last test is the one to write first. An automatic fixer whose revert does not work is not a productivity tool; it is a randomiser with commit access.

What to copy

1. Build the census before the reviewer. It is a few dozen lines, it needs no model, and it will surprise you.
2. Make priority a formula you can print. Usage times staleness. If you disagree with tonight's three files, you can look at the arithmetic instead of at a prompt.
3. Demand a failure scenario. This single rule is the difference between a signal and a mailing list.
4. Let it fix only what a verifier can judge, and prove the revert.

Read next: A Security Check With No Model In It — the same attention discipline, applied monthly, with no model calls at all.