DEV Community

Blake Yang
Blake Yang

Posted on

The Review Cited a Helper That Never Shipped: Assumption Audits Before an OSS Patch

A mid-size Python library merged a cache around its parser after a polite, confident review. Continuous integration stayed green because the new helper lived only inside the incoming diff. Two days later a maintainer bisected a production traceback and found the praised name only in review notes. The model had recommended load_cached_grammar(), a function that never existed on main, and the contributor had implemented the suggestion. Tests asserted against the invented helper, public docs never mentioned it, and callers had no honest way to discover the private surface.

That failure is common when a model reviews an open-source patch against a vague issue title. Plausible names, error codes, and fixtures appear in the comment thread even though git grep cannot find them. The workflow below treats invented symbols as a merge blocker. It reproduces the bug, drafts a minimal patch, and then audits every review claim against the tree before anyone clicks merge.

Why invented helpers survive CI

Green CI does not prove that a review comment described the real codebase. A test imported from the patch can exercise a helper that the patch itself just created. Maintainers reading the model output often treat fluent advice as local knowledge. The result is a private API that no release tag, changelog, or type stub can explain.

Open-source patches also travel with incomplete context. The model may see a hunk, a stack trace, and a one-line issue title. It rarely sees neighboring modules, historical names, or the public export list. Filling those gaps with confident guesses is the default behavior, not a rare hallucination.

Pin the tree before any model reads the diff

Start from a clone at the issue’s reported revision, not from an already edited working tree. Record the remote, the default branch, and the exact commit that still fails. The commands below are a worked example against a fictional parser library named tokparse; treat paths as placeholders, not as a published project.

git clone https://example.com/oss/tokparse.git
cd tokparse
git fetch origin
git switch --detach origin/main
git rev-parse HEAD > /tmp/repro.sha
git status --porcelain
Enter fullscreen mode Exit fullscreen mode

If the report names a release tag, check that tag out instead of main. Mixed trees hide whether a failure belongs to the bug or to local edits. Keep secrets, tokens, and untracked editor files out of the worktree so later review prompts cannot leak them.

Reproduce, then freeze the failing command

Write the smallest command that fails on the pinned commit and save it as a script. Do not let a model invent a test runner, extra pytest flags, or a Docker image that the repository does not already document. Read CONTRIBUTING, tox.ini, pyproject.toml, and .github/workflows first.

# example: documented in the repo's CONTRIBUTING.md
python -m venv .venv
. .venv/bin/activate
pip install -e ".[dev]"
pytest tests/test_parser.py::test_trailing_comma -q
echo $? > /tmp/repro.exit
Enter fullscreen mode Exit fullscreen mode

If that test does not exist yet, add one failing assertion that uses only public APIs already imported by neighboring tests. Label the new test as a proposal until it fails for the reported reason. A passing characterization of the wrong behavior is useful; a passing test of an invented helper is not.

# proposal: tests/test_parser.py
# Uses only Parser and ParseError, both imported by existing tests.
from tokparse import Parser, ParseError

def test_trailing_comma_is_a_parse_error():
    parser = Parser()
    try:
        parser.parse("{"a": 1,}")
    except ParseError as exc:
        assert "trailing" in str(exc).lower()
        return
    raise AssertionError("expected ParseError for a trailing comma")
Enter fullscreen mode Exit fullscreen mode

Run the test twice on the pinned commit. If the result flips, stop and isolate fixtures before any review model sees the tree. Flaky reproduction turns invented helpers into noise that nobody can falsify.

Draft the smallest patch that makes the frozen command pass

Edit only the module that already owns the failing path. Avoid drive-by renames, formatter-only hunks, and new files whose names the issue never required. After the frozen command passes, produce a unified diff against the recorded SHA.

git add tests/test_parser.py src/tokparse/parser.py
git diff --cached > /tmp/oss.patch
git diff --cached --stat
Enter fullscreen mode Exit fullscreen mode

Keep the patch nearby as a file, not only as a chat attachment. File-backed diffs can be grepped, hashed, and reapplied. Chat-backed diffs silently pick up extra prose that later looks like source.

Artifact: an assumption log plus a symbol gate

The original artifact in this article is a two-file gate. The first file is a human-edited assumption log. The second is a small checker that extracts added names from the patch and rejects review comments that cite missing symbols.

Assumption log template

Copy the block below into assumption-log.md beside the patch. Fill every row before a model is allowed to comment. Mark unknown items as unknown instead of guessing a library convention.

# Assumption log for /tmp/oss.patch
# Pinned SHA: (paste from /tmp/repro.sha)

| ID | Claim | Evidence command | Status |
| --- | --- | --- | --- |
| A1 | Public parser API is `Parser.parse` | git grep -n "class Parser" src | verified |
| A2 | Trailing commas raise `ParseError` | git grep -n "class ParseError" src | verified |
| A3 | No cache helper exists on main | git grep -n "load_cached_grammar" . | absent |
| A4 | Tests run under pytest from tox.ini | sed -n "1,80p" tox.ini | verified |
| A5 | Issue asks for a public cache | unknown; issue body never says cache | blocked |
Enter fullscreen mode Exit fullscreen mode

Status values are only verified, absent, or blocked. blocked means the patch must not add the claimed behavior. Models are not asked to resolve blocked rows; humans are.

Extract names the patch itself introduces

The checker below is a proposal script. It reads a unified diff and prints added identifiers so a later review cannot treat them as pre-existing APIs.

# proposal: tools/extract_added_symbols.py
import re
import sys

IDENT = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]{2,})\b")
KEYWORDS = {
    "def", "class", "return", "import", "from", "self", "None",
    "True", "False", "with", "as", "try", "except", "raise",
}

def added_lines(diff_text: str):
    for line in diff_text.splitlines():
        if line.startswith("+++") or line.startswith("---"):
            continue
        if line.startswith("+") and not line.startswith("+++"):
            yield line[1:]

def main(path: str) -> None:
    text = open(path, encoding="utf-8").read()
    found = []
    for line in added_lines(text):
        for match in IDENT.findall(line):
            if match not in KEYWORDS:
                found.append(match)
    unique = sorted(set(found))
    for name in unique:
        print(name)

if __name__ == "__main__":
    main(sys.argv[1])
Enter fullscreen mode Exit fullscreen mode
python tools/extract_added_symbols.py /tmp/oss.patch > /tmp/added.syms
sort -u /tmp/added.syms
Enter fullscreen mode Exit fullscreen mode

Verify every review claim with git grep

After a model (or a human) writes review comments, save them as plain text. Then run each cited identifier against the tree at the pinned SHA, not against the index that already contains the patch, unless the comment explicitly talks about new code.

# proposal: tools/verify_review.sh
set -euo pipefail
SHA=$(cat /tmp/repro.sha)
COMMENTS=${1:-/tmp/review.comments}

# Extract backtick-quoted names from review prose.
grep -oE '`[A-Za-z_][A-Za-z0-9_]+`' "$COMMENTS" \
  | tr -d '`' | sort -u > /tmp/cited.syms

while read -r name; do
  [ -n "$name" ] || continue
  hits=$(git grep -n -F "$name" "$SHA" -- src tests || true)
  added=$(grep -Fx "$name" /tmp/added.syms || true)
  if [ -z "$hits" ] && [ -z "$added" ]; then
    echo "INVENTED: $name"
  elif [ -z "$hits" ] && [ -n "$added" ]; then
    echo "NEW_IN_PATCH: $name"
  else
    echo "PRESENT: $name"
  fi
done < /tmp/cited.syms
Enter fullscreen mode Exit fullscreen mode

Any INVENTED line is a failed gate. Delete that review comment, or rewrite it until git grep can show a file and a line. NEW_IN_PATCH lines are allowed only when the assumption log already accepted that new name.

Decision table for keeping a review comment

Review comment type Evidence required Action
Mentions a function, class, or constant git grep hit on the pinned SHA, or a NEW_IN_PATCH row already accepted keep or rewrite
Mentions a test file or fixture path exists in git ls-tree -r --name-only $SHA keep or rewrite
Asks for a new helper not in the issue assumption log row is blocked or absent discard
Style-only advice that rewrites unrelated hunks git diff --stat would grow beyond the failing module discard
Claims CI already covers a branch job name appears in .github/workflows keep only with a file citation
Security or secret-handling advice human maintainer review; do not run extra scanners on a shared machine escalate, do not auto-apply

The table is the review policy. Models do not get a vote on discarded rows. Humans may still apply a discarded idea later, but only after a new assumption row is verified.

Where a free model actually helps

Disclosure: This article was prepared as part of MonkeyCode's product outreach. After the assumption log and symbol files exist, a free model can review /tmp/oss.patch against those files instead of against an empty prompt. MonkeyCode’s free model access and free server option are relevant here only as a place to run that constrained pass and to execute pytest plus verify_review.sh on a clean checkout. The model prompt should attach the patch, the assumption log, the added-symbol list, and a hard rule: cite a path or mark the comment as speculation.

You are reviewing an OSS patch. You may cite identifiers only if they
appear in assumption-log.md, added.syms, or the attached diff.
If you need a helper that is not listed, write SPECULATION and stop.
Do not invent test runner flags. Do not invent CI job names.
Return comments as: path:line: severity: text.
Enter fullscreen mode Exit fullscreen mode

The server side of the loop is just a clean clone, the frozen test command, and the grep gate. It is not a substitute for the project’s own CI. If the repository contains credentials, private submodules, or production data dumps, skip shared machines and run the same scripts locally.

Limitations

Symbol presence is not semantic correctness. A helper can exist and still be the wrong layer for the bug. Grep also false-positives on comments, dead code, and similarly named tests. Binary files, generated protobufs, and vendored trees need a different inventory than the script above. The workflow does not prove license, CLA, or maintainer-tone issues. It only blocks a frequent class of fluent, wrong review comments.

The loop also costs a checkout and a full test run before any model token is spent. Tiny typo patches do not repay that cost. Neither do issues that already include a failing test, a one-line fix, and a maintainer who asked only for a rebase.

Who should not use this approach

Skip the assumption audit when the change is a spelling fix in documentation with no code hunks. Skip it when the project forbids AI-assisted review in CONTRIBUTING. Skip shared free servers when the reproducer needs customer fixtures or secrets. Skip it when the reporter already attached a bisect SHA, a failing test, and a two-line patch that git apply --check accepts. In those cases a human maintainer is faster than a gated model pass.

Contributors who already pin a SHA, freeze a failing command, and grep review comments against the tree can run the same constrained pass with MonkeyCode’s free model access on the free server option, then throw away every INVENTED line before opening the pull request.

Top comments (0)