A case on a string prefix that JScript refuses to compile

We sent a feature scope to a read-only AI reviewer with the repo mounted. The first pass came back with six objections, every one citing a file and line we could open and check. We fixed them. The second pass found two more things wrong. We fixed those. The third pass found three more. None of the eleven were hallucinated. We had been skeptical that re-running a design through an AI reviewer would give us anything new, and what we got was compounding signal, not the same vague pushback reworded. Here is the run that changed our minds.

The setup

The ticket was a custom-columns feature for a SaaS platform that has been running for years. Admins wanted to attach arbitrary extra columns to the member list that belongs to an account (a number, a label, whatever). Those columns had to render on the public member list, the admin member list, CSV exports and print views, and they had to sort correctly. Accounts can also be linked under a parent organization, which matters below. The codebase is an older web application, JScript flavor, deep. In code like that, the right design depends on how three or four existing subsystems actually behave, and a greenfield design tells you little.

Before writing the implementation we wrote a scope doc and handed it to Codex running at effort=high with the production codebase mounted read-only. We did not ask it to review a diff.

The three passes ran at roughly 325K, 454K, and 518K tokens. The reviewer graded each with a verdict: REVISE FIRST, then REVISE AGAIN, then MINOR FIXES. That progression is the whole story. The reviewer never got stuck. Each pass was materially different from the last because each one was reviewing a different, less-wrong design.

Pass one: six objections, two that mattered most

Two of the six would have shipped a broken feature.

One was an ID-scheme bug. Our plan keyed custom columns as c1, c2, c3. Looked fine. But the reviewer pointed at three real code paths in the organization-linking and account-packaging routines and showed that those positional keys silently rebind their meaning when an account gets linked to an organization or detached from one. We had designed the IDs to be stable across a rename. The actual requirement was that they stay durable across a change of owner, because the ownership operations in those files reshuffle exactly the thing our keys depended on. That is a corruption bug that would only show up after a customer linked their account to an organization months later.

The other was a prefs-cascade mismatch. Our design assumed custom columns would carry down through a nested merge, with account values layered over organization defaults. The reviewer pointed at the prefs layer and showed that the set and get-types helpers are flat, top-level overwrite. There is no nested merge in this system. Our sparse-override model described a mechanism that did not exist.

We could verify both in under a minute each, because the objections said “here is the file, here are the lines, here is the operation that breaks your assumption.” They did not say “consider whether ownership changes affect your IDs.” That specificity is what a reviewer that actually read the code gives you.

Pass two: a collision-math sign-off and one thing still impossible

We rewrote the ID scheme to 16-hex-character c_ IDs and sent it back.

Pass two did the collision math instead of waving at it. 64 bits of entropy: the move-pair collision probability works out to about 9/2^64, roughly 4.9e-19. Even minting a million IDs, the birthday-style risk is about 2.7e-8. Verdict: operationally fine, and use a full GUID if you want it to round to zero. We will take a reviewer that runs the numbers over one that says “consider collision risk” every time.

The same pass also found a feature we had written into the plan that the codebase simply cannot support. We had specced a “copy from former organization” option. The reviewer pointed at the organization-edit handler and showed that the code overwrites the parent organization in place and never persists the old one. There is no “former organization” to copy from. The data isn’t kept. We had promised a capability the storage model makes impossible, and it was far better to learn that from a reviewer than from a support ticket.

Pass three: a render path we’d missed and a syntax error

The third pass is where we expected diminishing returns. It found three more real things.

One was an under-scoped surface. We had treated the non-responsive member list as just another render path we could gate with a simple prefix check. The reviewer showed it is a SQL-backed table assembled through a column-builder helper rather than a sibling template, so a prefix check alone is wrong there. It needed a real column-definition strategy, and we would have found that out the hard way when the old-layout member list rendered garbage.

Another was a flat-out syntax bug in our own pseudocode. We had written a case on a string prefix in a switch. That isn’t valid JScript. You can’t case on a prefix. It has to be a prefix check before the switch. Small, but it is the sign of an author who hasn’t mentally compiled the code, and the reviewer had.

The third pushed back on a migration step we had called safe. Our plan deleted the legacy custom-columns pref whenever an account saved a new-system config, and the previous pass had signed off on that. This pass corrected it: both renderers still honor that pref as a full-layout override, so deleting it on every save is a one-way migration that happens to look like cleanup. If that is the behavior we want, the plan has to say so on purpose and not slip it in as housekeeping.

After that came MINOR FIXES. The well ran dry at the point where the design was actually right. When a fresh pass returns only nits, you are done, and that is the signal we now trust over our own tiredness.

What shipped

Server-side landed on a branch, seven commits, and we verified it live end to end. The custom column renders in the admin member list, the visitor member list, the CSV export and the print view, and sorting by custom-column ID works. The legacy positional-column data clears on save as intended. The client-side work (settings UI, per-member edit popup, sort dropdown) got parked for the next session because it needs a JS build-process discovery we didn’t want to rush. The part the three passes hardened, the data model and the render contract, shipped correct.

What to run before a cross-cutting feature

Have a read-only reviewer check the plan against the real codebase, and keep re-running it until a pass returns only minor fixes. In this run that surfaced an ID scheme that would have broken after an account was linked to an organization, and a promised feature the storage model could not support. Each finding cited a file and line, so each could be checked in about a minute.

dxdev LESSON: paste into your AI coding agent
LESSON: Multi-pass AI review compounds: each pass reviews a less-wrong design
SOURCE: dxdev.com/blog/three-pass-ai-code-review-kept-finding-real-bugs

WHAT HAPPENED:
A feature scope was reviewed by a read-only AI with the full codebase mounted, not a diff.
Pass one found six objections, all citing real files and lines. Fixes changed the design.
Pass two reviewed the updated design and found two new problems the original design had hidden.
Pass three reviewed the twice-revised design and found three more real issues, including one the previous pass had incorrectly signed off on.

THE RULE: Run the reviewer again after each revision; each pass is reviewing a different, less-wrong design, so the signal does not repeat.

CHECK MY CODE, then report PASS / FAIL with file:line for each:
  1. Any ID or key scheme that assumes stability across ownership or parent-relationship changes (linking, detaching, reparenting).
  2. Any inheritance or merge behavior in a config or prefs layer assumed to be nested when the actual implementation is a flat top-level overwrite.
  3. Any feature in the plan that references data the storage model does not retain (former state, pre-change snapshots, deleted records).

THEN PRINT: a table (check, PASS/FAIL, evidence, fix) + a verdict (applies / partially / OUT_OF_SCOPE / no) + the single most important next action.