DEV Community

Blake Yang
Blake Yang

Posted on

Freeze the Issue Thread: Numbered Criteria Before an OSS Patch

A first-time contributor opened a three-year-old GitHub issue with forty-seven comments and three competing workarounds. The original report described a timeout, while later comments blamed a retry helper that landed in a different module. The contributor shipped a twelve-file refactor that matched the loudest comment and missed the reproduction steps in comment eighteen. Maintainers closed the pull request as out of scope, even though the local test suite stayed green.

Open-source issues rarely stay frozen after the first report, because later users paste new stacks and partial fixes. Comment order then starts to look like a specification, even though nobody voted on which workaround became canonical. A green test run on a laptop cannot prove that the change matches the issue the maintainers still intend to close. The missing artifact is a short contract that restates the thread as numbered, testable acceptance criteria.

Implicit specs collapse during review

Maintainers read a pull request against the issue title, the latest comment, and the code they remember shipping. Contributors often read the same thread as a pile of anecdotes and then patch the symptom that matched their local setup. Coding assistants amplify that mismatch when they treat the entire comment cloud as equally authoritative input. The workflow below treats the issue thread as raw material for a contract, not as a prompt dump.

This pattern sits next to current debates about assistants writing entire diffs without a frozen problem statement. The failure is not that models cannot emit patches; the failure is that the issue never named the observable result. A surgical open-source patch still needs an explicit MUST list before reproduction, implementation, or review begins.

Failure modes the packet is meant to catch

  • A workaround in a late comment silently replaces the original reproduction without maintainer confirmation.
  • A drive-by rename lands in the same commit as the behavioral fix and hides the contract change.
  • A test asserts an implementation detail that no comment required, so later refactors look like regressions.
  • A model-authored summary restates the loudest comment and drops environment pins from the original report.

Build the criteria packet before any source edit

The packet lives beside the clone, usually as ISSUE_CONTRACT.md, and it is committed only if the project already stores review notes. Otherwise the file stays in the contributor's notes directory and is pasted into the pull request body as a checklist. Every MUST line must name an observable result, an environment pin, and a source comment or issue section. SHOULD and WONTFIX lines exist so the patch can refuse popular requests that maintainers already rejected.

The following template is an illustrative packet, not a scraped issue from a live repository.

# ISSUE_CONTRACT.md
Issue: https://github.com/example/libretry/issues/1841
Head: main @ 9f3c1aa
Packet date: 2026-09-14

## MUST
- M1. A request that times out after 200ms returns `Retry-Exhausted` without calling the deprecated `sleep_loop` helper (issue body, env: Python 3.12, Linux).
- M2. `tests/test_retry.py` fails on current main with fixture `timeout_200ms.json` before the patch (comment 18).
- M3. Public module `libretry.api` keeps the same exported names; `git diff --stat` stays inside `libretry/retry.py` and `tests/test_retry.py` (CONTRIBUTING.md, small diffs).

## SHOULD
- S1. Log line on exhaustion includes the attempt count already used by `libretry.metrics` (comment 22, not confirmed).

## WONTFIX for this PR
- W1. Rewrite the backoff to jittered exponential across all clients (comment 31, maintainer: out of scope).
- W2. Rename `Retry-Exhausted` to `TimeoutError` (comment 9, closed as breaking).
Enter fullscreen mode Exit fullscreen mode

Numbering rules that keep the packet honest

  1. Bind each MUST to a comment number, issue body section, or CONTRIBUTING rule so the source stays auditable.
  2. Record the toolchain pin in the same line as the behavior, because a laptop default is not an observable.
  3. Put popular but rejected ideas under WONTFIX so a later model review cannot revive them as helpful extras.
  4. Freeze Head to a commit SHA before reproduction, so later main movement cannot silently rewrite the contract.

Reproduce against MUST lines only

Reproduction is the first implementation of the contract, and it should fail in the way M2 describes. Clone a clean tree, check out the frozen SHA, and install the project's documented toolchain instead of the contributor's global defaults. The commands below are a labeled example for a Python library; they are not claimed as a run from a production incident.

git fetch origin main
git checkout --detach 9f3c1aa
python -m venv .venv && . .venv/bin/activate
pip install -e ".[test]"
pytest tests/test_retry.py::test_timeout_200ms -q
# Expected on main: FAIL, matching M2
Enter fullscreen mode Exit fullscreen mode

If the named test does not exist yet, add a failing test that encodes M1 before any production edit. Keep that commit separate from the fix so reviewers can read failure, then repair, as two logical steps. Do not ask a model to make tests pass until M2 is visible on the frozen SHA. A passing suite at this stage means the contract is wrong, not that the project is healthy.

Constrain the diff to contract paths

Most rejected OSS patches fail review because the diff teaches the project a second lesson the issue never asked for. After M2 fails on main, create a branch whose only job is M1, and refuse files that M3 did not name. The shell snippet below is a local gate the contributor can run before every git push.

git checkout -b fix/1841-retry-exhausted
# edit only libretry/retry.py after the failing test exists
ALLOWED='^(libretry/retry.py|tests/test_retry.py|ISSUE_CONTRACT.md)$'
bad=$(git diff --name-only origin/main...HEAD | grep -Ev "$ALLOWED" || true)
if [ -n "$bad" ]; then
  printf 'Paths outside M3:\n%s\n' "$bad"
  exit 1
fi
git diff origin/main...HEAD --stat
Enter fullscreen mode Exit fullscreen mode

A second commit may add S1 if the metrics log is cheap and already patterned in the code. W1 and W2 stay out of the branch even when an assistant proposes them as cleanup. Surgical diffs remain easier to revert, bisect, and backport than the comment-driven refactors that usually follow a noisy thread.

Bind tests to criterion identifiers

Reviewers should not hunt through the issue thread for the sentence that a test claims to cover. Name the test after the MUST id, and restate the observable in the assertion message. The example below is pseudocode for illustration, not a patch against a real library.

# tests/test_retry.py
def test_m1_timeout_returns_retry_exhausted_without_sleep_loop():
    """ISSUE_CONTRACT M1: 200ms timeout -> Retry-Exhausted, no sleep_loop."""
    client = RetryClient(timeout_ms=200)
    with pytest.raises(RetryExhausted):
        client.request("https://example.invalid/slow")
    assert "sleep_loop" not in client.helpers_called
Enter fullscreen mode Exit fullscreen mode
Criterion Observable Test id Allowed paths Status on frozen main
M1 Retry-Exhausted, no sleep_loop test_m1_timeout_returns_retry_exhausted_without_sleep_loop libretry/retry.py not implemented
M2 named test fails before the fix same test on 9f3c1aa tests/test_retry.py FAIL required
M3 export surface unchanged check_issue_contract.py those two files only gate
S1 attempt count in log optional libretry/retry.py skip if noisy
W1–W2 no extra diff git diff review none reject extras

The small checker below walks the contract file and the merge diff. It is a local gate, not a claim about any project's CI. Save it as check_issue_contract.py and treat it as a labeled example rather than a published OSS tool.

#!/usr/bin/env python3
from __future__ import annotations

import re
import subprocess
import sys
from pathlib import Path

CONTRACT = Path("ISSUE_CONTRACT.md")
MUST_RE = re.compile(r"^- (M\d+)\. ", re.M)
PATH_RE = re.compile(r"`([^`]+)`")


def must_ids(text: str) -> list[str]:
    return MUST_RE.findall(text)


def allowed_paths(text: str) -> set[str]:
    block = text.split("## MUST", 1)[-1].split("## SHOULD", 1)[0]
    return {p for p in PATH_RE.findall(block) if "/" in p or p.endswith(".py")}


def diff_names() -> list[str]:
    out = subprocess.check_output(
        ["git", "diff", "--name-only", "origin/main...HEAD"], text=True
    )
    return [line for line in out.splitlines() if line]


def main() -> int:
    text = CONTRACT.read_text(encoding="utf-8")
    ids = must_ids(text)
    if not ids:
        print("No MUST ids found", file=sys.stderr)
        return 2
    pr_path = Path("PR_BODY.md")
    pr_body = pr_path.read_text(encoding="utf-8") if pr_path.exists() else text
    missing = [i for i in ids if i not in pr_body]
    if missing:
        print("PR body missing MUST ids:", ", ".join(missing))
        return 1
    allowed = allowed_paths(text) | {"ISSUE_CONTRACT.md", "PR_BODY.md"}
    bad = [p for p in diff_names() if p not in allowed]
    if bad:
        print("Paths outside contract:")
        print("\n".join(bad))
        return 1
    print("Contract ids:", ", ".join(ids))
    print("Diff paths inside M3.")
    return 0


if __name__ == "__main__":
    raise SystemExit(main())
Enter fullscreen mode Exit fullscreen mode

Run python check_issue_contract.py after filling PR_BODY.md with the same MUST list that will appear on GitHub. The script fails closed when a drive-by file appears, which is the usual review killer. It does not prove semantic correctness; it only proves the packet and the diff still talk about the same files and ids.

Score the diff against the packet, not against vibes

Once M2 fails on the frozen SHA and the fix plus tests pass, the remaining risk is semantic drift. A second human pass remains the standard of record, especially for security-sensitive or cryptography-adjacent code. A model pass is useful when scored against ISSUE_CONTRACT.md and the merge diff, rather than a request to improve the patch.

Disclosure: This article was prepared as part of MonkeyCode's product outreach. MonkeyCode's free model access can host that contract-aware review without pasting the issue thread into an ad-hoc chat window. The free server option is relevant when the review command and the project's test extras need a longer-lived workspace than a laptop session. No model name, quota, or benchmark is claimed; the workflow only needs a model that can read a diff and a packet.

The review prompt below is a template for operators to adapt. It is not an executed evaluation and it is not a ranking of products.

You are scoring a patch against ISSUE_CONTRACT.md.
Read the contract first, then the git diff.
For each MUST id, answer: satisfied / violated / not evidenced.
For each WONTFIX id, answer: absent / revived.
List files in the diff that the contract did not allow.
Do not suggest extra refactors, renames, or dependency bumps.
If the diff includes files outside M3, fail the review.
Enter fullscreen mode Exit fullscreen mode

Keep that prompt next to the packet so the same words gate both the human description and the model pass. Contributors can run the same prompt against MonkeyCode's free model access when they want a second reader that cannot invent new scope. The packet still works if that second reader is a teammate instead of a hosted model.

Decision table for common issue shapes

Issue shape Contract emphasis Patch allowed to touch Model review question
Timeout or retry bug environment pin plus exact exception one module and one test Did the deprecated helper return in traces?
Docs versus behavior mismatch quote the documented example docs file or code, not both unless M1 says so Which side is canonical in CONTRIBUTING?
Closed WONTFIX revived by users WONTFIX list copied from maintainer comment no files, or an issue comment only Did the diff revive W1?
CI-only failure job name, image tag, and seed CI config plus the failing test Was the local command the same as CI?

The table is a planning tool for triage, not a complete taxonomy of every issue shape. If an issue mixes a public API break with a private helper change, split it into two packets and two pull requests. Mixing those concerns is how comment eighteen disappears under a rename that no MUST line requested.

Limitations

This workflow assumes the issue thread contains at least one reproduction hint and one maintainer signal about scope. It will not invent a root cause for a crash that nobody on the thread can trigger. The path gate cannot see semantic breakage inside an allowed file, so M1 still needs a real assertion. Automated model review will still miss domain invariants that the written contract forgot to capture in MUST form.

The checker also trusts origin/main...HEAD, which is wrong for stacked branches or for repositories whose default branch is not main. Teams that squash every commit still need the failing test visible in review, even if history becomes one commit later. None of the examples above are performance claims, and none should be copied into a security advisory without a separate process.

Who should skip this workflow

  • Drivers who already have a maintainer-written design doc should follow that doc rather than re-deriving MUST lines from noisy comments.
  • Emergency incident patches that need to land in minutes cannot afford a packet ceremony and should use a revert or a feature flag.
  • Contributors who cannot run the project's tests at all should not use a model review as a substitute for reproduction.
  • People looking for a fully automated issue-to-merge agent will not find that here, because the contract is a human freeze.

What maintainers see

A pull request that pastes the packet and shows M2 failing on the named SHA is easier to trust than a restated comment. The follow-up commit should show M1 passing on the branch with the same identifiers in the test names. The diffstat matches M3, WONTFIX items stay absent, and the citations still point at issue comments. That packet is the original artifact; the optional model score is only a second reader with the same sheet.

Top comments (0)