tk3.biz
Back to Blog

I Had Three AIs Review 1418 Lines I Wrote, and They Found 11 Real Bugs

Introduction

A while ago, I added a local LLM as the third reviewer in dev-orchestra. I wrote then that “next I’ll run it on real development”, so this is the follow-up.

The subject is my own code: the scripts I wrote during this series of work, 13 files, 1418 lines. I wrote them and I run them, and that’s it. They had never been reviewed by anyone else.

The result: 11 real bugs. Three of them were ones I would almost certainly never have found on my own.

The Three Reviewers’ Results

The reviewers read the same diff in parallel, without seeing each other’s findings. I triaged every finding that came out.

How many of each reviewer's 6 findings survived triage. claude opus had 6 accepted, codex gpt-6-sol had 4 accepted and 2 duplicates, and localllm qwen had 1 accepted and 5 rejected

ReviewerRaisedAcceptedRejectedDuplicate
claude opus6600
codex gpt-6-sol6402
localllm qwen3.6-35B-A3B6150

The two commercial reviewers had zero misses. codex’s 2 duplicates described the same spots as claude in different words, and were correct findings in substance.

The local one got 1 out of 6. It’s one round of data, so I won’t draw firm conclusions, but looking at the content, the pattern was clear. More on that below.

Three I Couldn’t Have Found on My Own

1. continue Inside a PowerShell switch Doesn’t Go Back to the Loop

This is the code that resets the history when you type clear in interactive mode.

while ($true) {
    $line = Read-Host
    switch ($line) {
        'clear' { $messages.Clear(); continue }   # ← ここ
    }
    $messages.Add(@{ role = 'user'; content = $line })   # 到達してしまう
    ...
}

continue inside a switch only exits that switch; it doesn’t go back to the outer while. As a result, right after resetting the history, the string “clear” was sent to the model as is, and it dutifully answered.

Since the finding was about language semantics, I actually reproduced it.

[1] モデルに送信: msg1
[2] clear を検出 → continue
[2] モデルに送信: clear    ← 指摘どおり
[3] モデルに送信: msg3

Adding a :repl label and using continue repl fixed it. It looked like it was working, so I probably would never have noticed.

2. Streaming Didn’t Check the HTTP Status

The code that reads SSE only picked up lines starting with data: .

Error responses have no data: lines. They just return a JSON error object. So the loop exits without running even once, and it “succeeds” with nothing displayed.

This one hit home because I had run into this exact failure on the day it was flagged. When I tried to have the local LLM do a review and got a 400, the only message was “HTTP Error 400: Bad Request”. The server was returning the real cause (context exceeded) as JSON, and my side was throwing it away.

After the fix, it looks like this:

修正前: リクエストに失敗しました: HTTP Error 400: Bad Request
修正後: サーバがエラーを返しました: Context size has been exceeded.

3. The Script That Tallies Review Results Was Itself Broken

This one stung the most.

To record the three reviewers’ hit rates, I had written a script that reads consolidated.json and tallies results per reviewer. Mixing in untriaged rounds would throw the numbers off, so it was built to show a warning.

untriaged = [r for r in rows if r["status"] == "untriaged"]

The value dev-orchestra actually writes was needs-triage. The string untriaged doesn’t exist anywhere.

The warning would never have appeared, and it would have kept silently recording untriaged rounds. The tool for measuring hit rates had ended up distorting the measurement.

And this was a bug you could confirm just by looking at that very review’s output. consolidated.md says Triage: needs-triage.

The Local LLM’s 5 Rejections

Looking at what got rejected made the gap with the commercial models clear.

A finding that contradicts itself (security, HIGH)

It’s good that subprocess.Popen doesn’t use shell=True, but the script path is hardcoded. If this path were writable by an attacker, code could be injected.

It admits the path is “hardcoded” and then argues “it would be dangerous if it were dynamic”. There’s no concrete path to failure.

Raising designed behavior as a defect (MEDIUM)

main reads all of stdin into memory. A huge input could cause memory problems or timeouts.

It’s a CLI that receives a prompt, so that’s by design.

Calling a trade-off a defect (LOW, performance)

--sleep-idle-seconds is set to 43200. When idle, the model is unloaded, so the next request pays a cold-start penalty.

That’s a trade-off I chose on purpose in the previous post.

What they have in common is that they end with “it could …”. The two commercial reviewers were the opposite. How continue behaves in a switch, whether the string needs-triage is what actually gets written: they put it in a form you can verify.

The 1 I accepted (swallowing the body of error responses) was a sound finding. Rating it CRITICAL was overblown, but the defect itself is real.

A Defect All Three Missed

On the other hand, there was something everyone missed. And it was one with serious real-world impact.

After the fixes, I ran the cleanup script as a dry run, and it printed this:

[2] Lemonade Server を停止
  [dry-run] 停止対象: llama-server (PID 89960)

I wrote this script in part 1 to clean up the old environment. Back then, llama-server was the backend that Lemonade launched, and stopping it was correct.

Now llama-server is the new environment itself.

A defect all three missed. In the old environment, llama-server was Lemonade's backend and stopping it was correct, but after Lemonade was removed it came to mean the new environment itself. Not a single line of code changed

$procs = Get-Process | Where-Object { $_.Name -match 'LemonadeServer|llama-server|ryzenai' }

If I had run it with -Execute, the cleanup script would have killed the new environment.

You could say it’s natural that it was missed. Not a single line of code changed. What changed was a fact outside the repository: “what that name refers to”. Expecting that much from a review that reads a diff would be asking too much.

It was the obvious lesson: some things can’t be found just by reading the code statically. Actually running things is still worth it, even with three reviewers.

Cost vs. Benefit

To be honest, it’s a hassle.

  • I had to narrow the diff. Including posts and images, it came to 5535 lines, with a lot of waste
  • Reading all 18 findings and deciding on each takes a fair amount of time
  • The local one only added the work of reading 5 rejections

Still, in exchange for 11 real bugs, I thought it was cheap. The switch/continue one and the tally script one in particular were things I had little chance of finding myself.

As for the third, local reviewer, for now I see it as “free insurance”. Its hit rate is low, but it costs nothing, so the only cost is the work of rejecting findings. If the pattern doesn’t change after a few more rounds, though, I’ll consider removing it.

Summary

  • My own 1418 lines of code had 11 real bugs
  • 3 I couldn’t have found on my own: language semantics, a path that fails silently, and a bug in the measuring tool itself
  • The local LLM got 1 out of 6. Most of the rejections were guesses that end with “it could …”
  • There was also a defect all three missed. The code didn’t change, but what a name refers to did. A dry run caught it

Adding more reviews doesn’t mean you can relax. Having it read and actually running it are different things, and you need both.

Next, I’ll dig into widening the context length from 16K to 64K. In part 2, I decided “no KV cache quantization if f16 fits”, but I haven’t yet measured whether that assumption still holds at long context lengths.

Related Posts