ai newsMo., 19. Okt.
EilmeldungInside Prometheus: Meta’s Journey to a Gigawatt-Scale Supercluster

Our AI reviewer caught two bugs in the fix I wrote for it

Muhannad Salkini, co-founder of Tently This week I fixed a bug in Tently, our PR reviewer. The tests passed: 77 in the database package, 251 in the API, 190 in the worker. Then Tently reviewed the pull request and found two problems the tests didn't cover. One was in the change's effect on a caller

dev.toMontag 13:16 · 19.10.4 Min. Lesezeit
Bild: dev.to

This week I fixed a bug in Tently, our PR reviewer. The tests passed: 77 in the database package, 251 in the API, 190 in the worker. Then Tently reviewed the pull request and found two problems the tests didn't cover. One was in the change's effect on a caller I wasn't thinking about. The other was in a file the diff never touched.

This is the kind of miss I think agent-written PRs make more often, so here is what happened.

Tently keeps a store of decisions a team has made ("no I/O in the core and reviewer packages", "LLM calls go through the router package"). Each decision has a scope: a set of path globs like packages/reviewer/**.

While recording a demo, I asked Cursor to call the GitHub API directly from our reviewer package. Cursor called Tently's search first and passed the folder it was about to edit: packages/reviewer. The search came back with process rules and missed the one decision that mattered, which was marked BLOCKING.

The cause was one line. A glob like packages/reviewer/** compiles to a regex that needs something after the slash. A bare folder name never matched, so every decision scoped to that folder disappeared from the search.

The fix seemed simple. If a path has no file name, treat it as a folder, and match any decision whose globs reach inside it. My rule for "no file name" was "the last segment has no dot", so packages/reviewer counted as a folder and src/app.ts as a file.

I wrote tests for folders, parents, siblings and excludes. Everything passed. I wrote in the PR description that file paths were unchanged.

Tently's review came back with one medium finding. Paraphrased:

Extension-less files such as Makefile, Dockerfile and LICENSE are now classed as directories. Their exclude paths are tested against path/ instead of the path, so exact-name excludes stop vetoing them. This affects the PR reviewer, which sends real file paths.

That was right, and it was worse than it sounds. Teams use exclude paths for ratified exceptions: "this rule applies to apps/api/**, except apps/api/Makefile ". After my change, the reviewer would read apps/api/Makefile as a folder and check whether the exclude covered apps/api/Makefile/. It didn't, so the exception stopped working. The function I changed is shared by the search the coding agent calls and the search the PR reviewer runs. I had been thinking only about the first.

The tests passed because none of them used an extension-less file with an exclude on it.

The fix: a path without a dot can be either a folder or a file, so it matches if either reading says so, and an exclude on it works if either reading says so. Only a trailing slash forces folder mode.

The second review found something outside the diff entirely. The dashboard has its own copy of the same matcher, because the web app can't import the database package. The copy's header says it is "ported verbatim" and must be kept in sync.

My PR changed the original and not the copy. Tently flagged the gap, and pointed out that the copy had already drifted before my PR: it applied excludes across all of a PR's files instead of each file. When I went to mirror the change, I found a second drift as well: its **/ handling let a/**/b match a/cab. So the PR page could list different decisions from the ones the reviewer enforces, and had been doing so for a while.

No line of my diff touched that file. A review that reads only the diff can't see it. Tently found it because a comment in that file stated a rule ("keep the two in sync"), and the change broke the rule.

Neither bug was exotic. Both are the kind a careful reviewer catches if they hold the rest of the system in their head: who else calls this function, and what else promised to stay in sync with it. That's the part an author forgets first, whether the author is a person or an agent, because the diff looks finished.

The fix PR ended with three commits instead of one, and the dashboard copy is now byte-identical to the original, tests included.

Tently reviews pull requests against the decisions your team already made and the code the diff doesn't touch. It's free for 30 PRs a month, no waitlist from this link: try Tently.

Quelle: dev.to (dev.to). Original öffnen