Outdated Means the Lines Moved, Not That You Fixed It

My merge gate said zero. Zero unresolved review threads, everything green, ready to ship. I was one command away from merging when GitHub itself refused — and it turned out GitHub was right and my own tool was wrong.

Here’s the setup in plain terms. I have an agent that decides whether a pull request — a proposed batch of code changes — is safe to merge. To do that it reads the comments left by CodeRabbit, an automated reviewer that reads your diff and flags problems. My agent counted the still-open comment threads. If the count was zero, it called the PR clean.

The count was zero. It was lying.

There was a CRITICAL finding sitting right there: a fail-open bug, the kind where a security check quietly passes when it should block. CodeRabbit had caught it. So why did my gate miss it?

Because of one word in my filter: isOutdated. I had counted a thread as unresolved only if isResolved == false and isOutdated == false. That second clause felt obvious when I wrote it — if a comment is outdated, surely it’s been dealt with, right? Wrong. Outdated doesn’t mean addressed. It means the line numbers moved. An earlier commit had shifted the file down a few lines, so GitHub re-tagged the finding as outdated. The bug was still there, word for word. My filter had quietly decided “outdated ⇒ handled,” and that assumption was the whole failure.

What saved me was a dumber, coarser gate I didn’t write. GitHub branch protection has a setting called require_conversation_resolution, and it counts every unresolved thread — it doesn’t know or care about outdated. So even with every check mark green, the merge state stayed BLOCKED. I couldn’t merge. That friction is the only reason I looked deeper instead of shipping a fail-open security hole.

The lesson that stuck with me is about where bugs hide. I’d been looking for bugs in the code under review. The worst one was in the thing doing the reviewing. A filter you write to certify something as “clean” is really a list of your own assumptions, and it inherits every blind spot you had the day you wrote it. My blind spot was a plausible-sounding equivalence I never questioned.

# Per review thread on the PR
isResolved   # true only if a human/bot explicitly resolved it
isOutdated   # true when line anchors moved — says NOTHING about validity

# WRONG — encodes "outdated means addressed"
unresolved = [t for t in threads
              if not t.isResolved and not t.isOutdated]

# RIGHT — outdated is not an exclusion
unresolved = [t for t in threads if not t.isResolved]

# And trust the structural gate over your own count:
ready = pr.mergeStateStatus == "CLEAN"   # branch protection already
                                         # counted ALL unresolved threads

So when you build any filter that stamps something “good to go,” stop and name the assumption inside it — then ask whether a coarser, structural gate would catch what that assumption hides. Keep the dumb gate. It doesn’t share your blind spots.