The Tests Came With the Bug: Reviewing Agent PRs for Oracle Contamination

Agent-generated pull requests fail a specific review, not a generic one. The implementation can be wrong and the test suite can still go green, because the same model often writes both. Treat that coupling as the primary defect class. Style, naming, and even the production diff come second.

This article is a code-review protocol for that failure mode. It tells a reviewer what to trust, what to revert, and what to re-test when the agent ships its own proof. The artifact is a small classifier that reads a unified diff and prints a review queue. No production metrics are claimed; the sample PR is labeled as constructed.

The failure, in one sentence

If a test encodes the implementation instead of the ticket, CI is not evidence.

Agent PRs make that cheap. The model sees the function it just invented, then writes assert helper(x) == <the value the helper returns>. Humans scan the red/green diff, notice coverage went up, and merge. The bug is not hidden. It is certified.

Call this oracle contamination: the test oracle was derived from the code under test, in the same change set, by the same generator.

A constructed PR (not a war story)

Suppose the ticket is: reject empty user_id on POST /sessions with HTTP 400 and a stable error code SESSION_USER_REQUIRED.

An agent might open one PR that adds a helper, a handler, and a test:

# app/sessions.py  (added in the PR)
def normalize_user_id(raw):
    if raw is None:
        return "anonymous"
    return str(raw).strip() or "anonymous"

def create_session(payload):
    user_id = normalize_user_id(payload.get("user_id"))
    return {"ok": True, "user_id": user_id}

# tests/test_sessions.py  (added in the same PR)
from app.sessions import create_session, normalize_user_id

def test_empty_user_becomes_anonymous():
    assert normalize_user_id("") == "anonymous"
    assert create_session({"user_id": ""})["ok"] is True

CI is green. The ticket is not. Empty user_id was supposed to 400. The test proves the opposite behavior and will fail if a reviewer later restores the spec. That is the review problem: the failing test would be the correct one.

Review order (do not start with style)

Run the review in this sequence. Skipping a layer is how contaminated oracles survive.

1. External contract. Status codes, error codes, CLI flags, schema fields, log event names, file formats. These are the only oracles the ticket can usually supply.
2. Test oracle source. Did the new assertions come from the ticket, a fixture corpus, or the new helpers in this PR?
3. Failure paths. Empty input, timeout, permission miss, partial write. Agents overweight the happy path.
4. Production diff. Only after the oracle is independent.
5. Style and refactors. Last. Unsolicited renames are noise until the contract holds.

If layer 1 and layer 2 disagree, stop. Do not “fix the test to match the code.”

Decision table: trust, revert, test

Signal in the PR	Trust?	Revert?	What to test instead
New assertions quote ticket examples, status codes, or golden files that existed before the PR	Yes, as a spec fragment	No	Boundary values the ticket listed but the agent skipped
Test imports a helper that first appears in the same diff and asserts that helper’s return value	No	Revert the test, keep the production file only if the contract is independently proven	Rewrite the test against HTTP/CLI/schema, not the helper
Test mocks the production module that the PR also rewrote	No	Revert the mock; it cannot catch the regression	One in-process call through the public entrypoint
Production change is a rename with identical bytecode-level behavior and tests were not rewritten	Provisionally	Revert if the rename crossed a public API	Snapshot the public surface (git diff on exported names)
Error-path tests absent, happy-path coverage rose	No	Keep the files, do not merge	Empty, null, oversized, and unauthorized inputs from the ticket
Lockfile / dependency add unrelated to the ticket	No	Revert the lockfile hunk	Build with the previous lockfile; confirm the feature still compiles
Comments or commit messages restate the ticket, code does the inverse	No	Revert comments that launder the behavior	Execute the ticket’s examples as a separate job

The table is a queue, not a score. One “revert the test” row is enough to block merge even if the production diff looks small.

Artifact: classify a diff before you read it

The script below is a reviewer aid. It does not decide merge. It prints a review queue so humans start at the contaminated tests instead of the prettiest production hunk.

Save as oracle_review.py and run:

git fetch origin
git diff origin/main...HEAD | python3 oracle_review.py

#!/usr/bin/env python3
"""oracle_review.py — flag implementation-derived tests in a unified diff.

Reads stdin. Prints a review queue. Heuristic only; label as unexecuted proof.
"""
from __future__ import annotations

import re
import sys
from collections import defaultdict

TEST_HINT = re.compile(r"(test_|_test\.|/tests/|^tests/)", re.I)
ADD_LINE = re.compile(r"^\+[^+]")
FILE_HUNK = re.compile(r"^\+\+\+ b/(.+)$")
DEF_NAME = re.compile(r"^\+\s*def\s+([A-Za-z_][A-Za-z0-9_]*)\s*\(")
ASSERT_LIT = re.compile(r"assert\s+.+")
MAGIC = re.compile(r"\b(\d{3,}|0x[0-9a-fA-F]+|HTTP_[A-Z_]+)\b")
MOCK = re.compile(r"\b(mock|MagicMock|patch\(|monkeypatch)\b")


def classify(path: str) -> str:
    if path == "/dev/null":
        return "deleted"
    if TEST_HINT.search(path):
        return "test"
    if path.endswith((".md", ".rst", ".txt")):
        return "docs"
    return "prod"


def parse(diff: str):
    files = {}
    current = None
    for line in diff.splitlines():
        m = FILE_HUNK.match(line)
        if m:
            current = m.group(1).strip()
            files[current] = []
            continue
        if current and ADD_LINE.match(line):
            files[current].append(line[1:])
    return files


def main() -> int:
    diff = sys.stdin.read()
    if not diff.strip():
        print("no diff on stdin")
        return 1

    files = parse(diff)
    buckets = defaultdict(list)
    prod_defs, prod_magic = set(), set()
    findings = []

    for path, added in files.items():
        buckets[classify(path)].append(path)
        if classify(path) != "prod":
            continue
        for ln in added:
            d = DEF_NAME.match(ln)
            if d:
                prod_defs.add(d.group(1))
            prod_magic.update(MAGIC.findall(ln))

    for path in buckets["test"]:
        added = files[path]
        text = "\n".join(added)
        imported_new = [n for n in prod_defs if re.search(rf"\b{n}\b", text)]
        asserts = [ln for ln in added if ASSERT_LIT.search(ln)]
        if imported_new and asserts:
            findings.append(
                f"CONTAMINATED? {path} asserts against new prod symbols: {imported_new}"
            )
        if MOCK.search(text) and imported_new:
            findings.append(f"MOCKED-UNDER-TEST {path} mocks symbols born in this PR")
        overlap = prod_magic.intersection(set(MAGIC.findall(text)))
        if overlap:
            findings.append(f"SHARED-LITERALS {path} reuses prod literals {sorted(overlap)[:8]}")
        if not asserts:
            findings.append(f"NO-ASSERT {path} grew without a new assertion")

    print("== review queue ==")
    print("1. contract/docs:", buckets["docs"] or "(none in diff)")
    print("2. tests:", buckets["test"] or "(none in diff)")
    print("3. production:", buckets["prod"] or "(none in diff)")
    print("== contamination flags ==")
    if not findings:
        print("none fired; still review layer 1 against the ticket by hand")
    else:
        for f in findings:
            print("-", f)
    print("== stop condition ==")
    print("If a test flag fires, rewrite or revert tests before reading prod style.")
    return 0


if __name__ == "__main__":
    raise SystemExit(main())

The script is a filter. Shared literals and new-symbol imports are clues, not guilt. A status code 400 appearing in both handler and test can be correct. The reviewer still has to ask: did that 400 come from the ticket, or from the function the agent just wrote?

What to trust

Trust artifacts that could have been written without seeing the new implementation:

• Ticket examples copied verbatim into a fixture file that does not import new helpers.
• Contract tests that speak HTTP, a CLI, or a serialized schema.
• Golden files that existed on main before the agent branch.
• Type or OpenAPI snapshots generated from a committed spec, not from runtime reflection on new classes.

Those can still be wrong. They are at least not circular.

What to revert immediately

Revert is cheaper than debate when the hunk cannot be an independent oracle:

• Tests whose only asserts target helpers introduced in the same PR.
• Mocks of the rewritten module.
• “Anonymous user” style defaults that invert a required-field rule.
• Drive-by dependency and formatter commits mixed into the feature.
• Comments that describe the ticket while the code implements a fallback the ticket never allowed.

Revert the test and the fallback, not necessarily the whole branch. Keep a thin production stub if it helps the next review, but do not keep a green suite that encodes the stub as policy.

What to test after the revert

Replace contaminated tests with checks the implementation cannot satisfy by agreeing with itself.

# proposed replacement — still an example, not a measured suite
def test_empty_user_id_is_400(client):
    res = client.post("/sessions", json={"user_id": ""})
    assert res.status_code == 400
    assert res.json()["code"] == "SESSION_USER_REQUIRED"

def test_missing_user_id_is_400(client):
    res = client.post("/sessions", json={})
    assert res.status_code == 400
    assert res.json()["code"] == "SESSION_USER_REQUIRED"

Then add one negative that the agent usually skips: oversized payload, unknown field if the API is closed, and a second call that proves the rejected body did not create a row. If the service is not HTTP, use the public CLI or the message schema. Do not unit-test normalize_user_id until the contract tests exist. Helpers are free to change; status codes are not.

A minimal command set after rewriting tests:

git diff origin/main...HEAD --stat
git diff origin/main...HEAD -- tests | python3 oracle_review.py
# run only the contract file, not the whole suite, on the first pass
pytest -q tests/test_sessions.py -k "400 or SESSION_USER"

If contract tests fail and helper tests pass, the helper tests are the contamination. Delete them. Do not “update expected values.”

Where a scratch coding environment fits

The protocol is local git plus one Python filter. Some teams still want a model in the loop to draft the replacement contract tests after a revert, not to bless the original PR.

Disclosure: This article was prepared as part of MonkeyCode's product outreach. MonkeyCode is an open-source coding assistant with free model access and a free server option. That is relevant when the review prompt and oracle_review.py should run on a throwaway branch without a paid API bill. It does not supply an oracle. Do not paste the agent’s own tests back into the prompt and ask “are these correct?” — that recreates the contamination. Feed the ticket examples and the public surface only.

No model names, token quotas, or hardware claims are made here. Availability can change; keep the script usable without any hosted product.

Limitations, and who should not use this

The classifier will false-flag legitimate tests that import a new pure function listed in the ticket. It will miss tests that reimplement the production algorithm in the test file without importing it. It cannot see runtime behavior, flaky time, or data races. It is not a security review.

Do not use this protocol as a merge bot. Do not use it as the only gate on payment, auth, or privacy code. Teams that already write contract tests before any implementation have less need for the script; they still need the decision table when an agent bypasses that order. If the ticket itself is ambiguous, stop and amend the ticket. A contaminated oracle plus a vague ticket is how silent product changes ship.

The method also assumes the reviewer can name the external contract. If nobody on the team can state the status code or the error identifier without reading the new source, the PR is not ready for automated classification. Write the contract down first.

Close

Start every agent PR review at the tests, and ask whether those tests could exist without the new production symbols. If they could not, they are not tests. They are a second copy of the bug. Revert that copy, re-test the ticket’s public examples, and only then read the implementation for style.