DEV Community

Blake Yang
Blake Yang

Posted on

Classify the Public Surface Before an OSS Crash Fix Hits Review

Consider a mid-size TypeScript CLI that received a three-line pull request stopping a null crash in flag parsing. The reporter attached a passing unit test, a screenshot of the local run, and a short stack note. Maintainers still closed the request after one pass because an exported helper changed return values for empty argv. Downstream packages had treated that helper as a stable contract, so the crash fix was a breaking change wearing patch-sized clothing.

Correctness is not the only gate on an upstream pull request for libraries that already have downstream dependents in late 2026. Maintainers also protect exported names, documented flags, config keys, and the promise implied by the next version tag. A contributor who only proves the bug is gone still leaves contract classification sitting on the reviewer. The packet below keeps a human in charge of reproduce, patch, and test, then records a Semver Impact Card before anyone files the diff.

Why small crash fixes still stall on versioning

Issue threads reward a green test and a short, readable diff against the reported file. Versioning rewards a boring, named claim about what downstream code may keep calling after the next tag. Those two rewards pull in opposite directions when a crash lives inside a helper that already ships in __all__, export, or a public header. The helper can start returning a documented empty object instead of throwing, which is kinder to interactive users and still a meaning change for callers that caught the exception.

Review time then migrates from the hunk to an argument about frozen intent. Maintainers compare README examples, type stubs, changelog footnotes, and the last tagged tarball. Contributors repeat that the crash was never written down as a feature. The Semver Impact Card moves that argument into a table before the pull request exists, so the first human review starts on a classified claim rather than on informal confidence.

Four stages before gh pr create

The working sequence is reproduce, patch, test, and classify. Each stage writes a file under .patch-packet/ so the pull request can link evidence instead of retelling the narrative. Model output appears only after the first three stages are green on a clean tag, because a model that reviews a still-failing tree invents compatibility stories.

  1. Reproduce the reported failure on the issue tag, not on a dirty local main.
  2. Patch the owning module without drive-by refactors or formatting sweeps.
  3. Run the project's test runner plus one new assertion that binds the contract.
  4. Fill the Semver Impact Card and invite a model to falsify the proposed bump.

Stage 1: Reproduce on the reported tag

Check out the tag named in the issue, not a local branch that already contains extra commits. A dirty tree hides whether upstream already shipped a fix, which makes any compatibility claim dishonest. Record the command, expected output, and observed output in repro.md beside a tiny shell wrapper that exits non-zero on the broken tag.

git fetch --tags origin
git switch --detach v4.2.1
python -m venv .venv && source .venv/bin/activate
pip install -e ".[dev]"
mkdir -p .patch-packet

cat > .patch-packet/repro.sh <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
# Documented contract: empty argv yields an empty object and exit 0.
# Observed on v4.2.1: TypeError from parse_argv([]).
python -m samplecli parse --argv '' --format json
EOF
chmod +x .patch-packet/repro.sh
./.patch-packet/repro.sh; echo "exit=$?"
Enter fullscreen mode Exit fullscreen mode

The wrapper must fail on the tagged release and later pass on the patched tree. If it cannot fail on that tag, the issue is not isolated enough for a compatibility claim, and the card should not be filled yet.

Stage 2: Patch the owning module only

Keep the diff inside the file that owns the crash path. Cross-cutting cleanups belong in a follow-up issue, not in a crash fix that also claims a version bump. After the edit, store a unified diff and a name-only file list next to the card so later review does not depend on a working tree.

git diff --stat
git diff -U3 -- src/samplecli/parse.py > .patch-packet/patch.diff
git diff --name-only > .patch-packet/files.txt
Enter fullscreen mode Exit fullscreen mode

A crash fix in this shape is usually a null guard plus a default that already appears in the docs. If the hunk set grows past the failing function, split the work before filling the card, because extra files inflate the public-surface table without extra evidence.

Stage 3: Bind the new behavior with the project runner

Call the same runner the CI workflow already uses. Add one assertion that encodes the old crash input and the intended contract, then keep neighboring tests unmodified unless they encoded the crash as expected behavior. Do not introduce a second framework for a single upstream patch, because reviewers will not install it.

# tests/test_parse_empty_argv.py
from samplecli.parse import parse_argv


def test_empty_argv_returns_empty_object_without_raising():
    result = parse_argv([])
    assert result == {"flags": {}, "positionals": []}
    assert "help" not in result["flags"]
Enter fullscreen mode Exit fullscreen mode
pytest -q tests/test_parse_empty_argv.py tests/test_parse.py
git add tests/test_parse_empty_argv.py src/samplecli/parse.py
Enter fullscreen mode Exit fullscreen mode

Green tests still do not prove that exported meaning stayed compatible for downstream imports. That proof is the card in the next stage, not another assertion about private helpers or log lines.

Stage 4: Classify public surface before the pull request

Copy the template into .patch-packet/semver-impact.md and complete every field on the same day as the diff. Blank cells are a stop for the contributor, not a reminder for the maintainer to finish the taxonomy. The proposed bump must cite the project's own versioning document, even when that document is only a short README section.

Artifact: the Semver Impact Card

# Semver Impact Card

- Project / issue: samplecli#1842
- Base tag: v4.2.1
- Proposed bump: PATCH | MINOR | MAJOR
- Versioning source: VERSIONING.md (commit abc1234) or README "Versioning"

## Public surface touched
| Kind | Name | Before | After | Visible to downstream? |
| ---- | ---- | ------ | ----- | ---------------------- |
| function | parse_argv | raises TypeError on [] | returns empty object | yes, listed in __all__ |
| CLI flag | --format | unchanged | unchanged | yes |
| config key | (none) | | | |
| file format | (none) | | | |

## Classification rules used
- MAJOR: remove or rename an export, change return meaning, or reject previously valid input
- MINOR: add an export or optional flag whose default preserves old calls
- PATCH: fix a crash or wrong result without changing documented meaning

## Claim
PATCH. `parse_argv([])` now matches the documented empty-object example in README.md.
The TypeError was never listed as a contract in VERSIONING.md or the type stubs.

## Explicit non-claims
- Does not add --strict
- Does not change JSON key order
- Does not backport to the 3.x line

## Tests that bind the claim
- tests/test_parse_empty_argv.py (new)
- tests/test_parse.py (unchanged, still green)

## Items for maintainer confirmation
- Confirm parse_argv is public because __all__ includes it
- Confirm the README example, not the TypeError, is the frozen contract
Enter fullscreen mode Exit fullscreen mode

The table is the artifact a maintainer can scan without opening every hunk. The non-claims section matters as much as the bump, because it blocks extra behavior from sneaking into the pull request title. Store the filled card in git even if the project will not merge the .patch-packet/ directory, then paste a summary into the pull request body.

Extract candidate exports from the unified diff

Hand-filled tables miss aliases, re-exports, and definitions added only as +export. A small illustrative script greps added definition lines, then checks __all__ on the current file. The output is a candidate list for the card, not a legal API boundary and not a substitute for reading VERSIONING.md.

#!/usr/bin/env python3
"""semver_scan.py — list definition lines in a unified diff (illustrative)."""
from __future__ import annotations

import re
import sys
from pathlib import Path

DEF_RE = re.compile(
    r"^\+\s*(?:export\s+)?(?:async\s+)?(?:def|function|class|const|exports\.)\s*([A-Za-z_][\w.]*)"
)
ALL_RE = re.compile(r"__all__\s*=\s*\[([^\]]*)\]", re.S)


def definitions(diff: str) -> list[str]:
    found: list[str] = []
    for line in diff.splitlines():
        match = DEF_RE.match(line)
        if match:
            found.append(match.group(1))
    return found


def public_all(py_source: str) -> set[str]:
    match = ALL_RE.search(py_source)
    if not match:
        return set()
    return {tok.strip(" \"'") for tok in match.group(1).split(",") if tok.strip()}


def main() -> None:
    diff = Path(sys.argv[1]).read_text(encoding="utf-8")
    names = definitions(diff)
    print("definitions_added_or_modified:")
    for name in names:
        print(f"  - {name}")
    if len(sys.argv) > 2:
        src = Path(sys.argv[2]).read_text(encoding="utf-8")
        pub = public_all(src)
        print("in__all__:")
        for name in names:
            print(f"  - {name}: {name in pub}")


if __name__ == "__main__":
    main()
Enter fullscreen mode Exit fullscreen mode
python .patch-packet/semver_scan.py .patch-packet/patch.diff src/samplecli/parse.py
Enter fullscreen mode Exit fullscreen mode

Label the script illustrative, because languages disagree about the word public. Go uses capitalization, Rust uses pub and pub use, C uses installed headers, and TypeScript uses export. The card must accept or reject each candidate by name, including names the scanner never saw because they live only in documentation examples.

Falsify the bump with a free model after tests pass

A language model is a weak author of production hunks and a useful adversary for a versioning claim. The human still reproduces the crash, writes the change, and runs the project test runner. The model receives only the card, the unified diff, and VERSIONING.md, then tries to show that the bump is too low, too high, or unsupported by the files.

Contributors without a private GPU can run that second pass on a free model endpoint. MonkeyCode currently offers free model access and a free server option for this kind of review host. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Teams that want a hosted loop for the review prompt can use the free server option as one of several possible hosts. The product is not required; any local endpoint that accepts a text prompt can execute the same checklist against the packet files.

Save the prompt as .patch-packet/review-prompt.md and paste the card plus git diff --name-only under a clear delimiter.

You are reviewing a Semver Impact Card for an upstream pull request.
Do not rewrite the patch. Do not invent symbols that are absent from the diff.

Inputs:
1) semver-impact.md
2) git diff --name-only and the unified diff
3) VERSIONING.md or the README versioning section

Tasks:
- List every added, removed, or retyped name that looks exported.
- State whether the proposed bump is too low, too high, or consistent.
- Quote the rule from VERSIONING.md that supports that judgment.
- Flag any line in Explicit non-claims that the diff already implements.
- Return at most 12 lines with sections: missed_exports, bump_judgment, rule_quote, overclaims.

If evidence is missing, write insufficient evidence instead of guessing.
Enter fullscreen mode Exit fullscreen mode

Run the prompt only after the wrapper fails on the base tag and passes on HEAD. Store the output in .patch-packet/model-review.txt and treat disagreements as contributor work, not as automatic edits to parse.py. A model that rewrites the hunk at this stage undoes the whole point of classifying a human patch.

Decision table for the proposed bump

Observation on the tagged tree Typical bump Stop filling the card if
Crash on input the docs already allow PATCH docs and tests disagree about the input
New optional flag with a preserving default MINOR the default changes old output
Return type or error class changes for valid input MAJOR the issue asked only for a crash fix
Rename of an exported helper MAJOR no alias remains for one release
Internal function only, absent from docs and __all__ PATCH or no release the name appears in a public example

The table is a starting filter for contributors who do not yet know the project's customs. The project's VERSIONING.md wins when it conflicts with this chart, and the card should quote the winning sentence. When VERSIONING.md is missing, the card should say so in the versioning-source field and propose PATCH only for undocumented crash behavior that existing tests already allowed.

Sample pull request body

## Summary
Crash on empty argv in parse_argv. Packet claims PATCH against v4.2.1.

## Evidence
- Repro: .patch-packet/repro.sh fails on v4.2.1, passes on this branch
- Test: tests/test_parse_empty_argv.py
- Surface: .patch-packet/semver-impact.md
- Adversarial pass: .patch-packet/model-review.txt (informational only)

## Maintainer confirmation requested
Please confirm parse_argv is public and that README empty-object example is the contract.
Enter fullscreen mode Exit fullscreen mode

Keep the model report informational. Maintainers did not ask for a generated review persona, and a contributor should not present model prose as CI.

Limitations

The scanner does not understand re-exports, decorator-wrapped APIs, or documentation-only contracts. A README sample can freeze behavior that never appears in __all__, and plugin entry points can expose names that live only in packaging metadata. Generated bindings need a human pass over the installed artifact, not merely over the source diff. Model review can miss domain meaning, such as a numeric default that looks compatible but breaks a wire protocol or a sort order that tests never pinned.

Free model access and a free server option do not make the review authoritative. They do not replace maintainer judgment, signed tags, or the project's CI matrix. The card also fails for embargoed security fixes that must not describe public surface in a public packet. Those patches follow the project's security policy rather than this template, and they should not be sent to a third-party endpoint.

Time-sensitive product quotas, model catalogs, and hardware lists change often and are omitted here on purpose. Operators should confirm current availability on the surface they actually use, rather than copying capacity claims from a blog post.

Who should skip this packet

Skip the card for typo-only documentation edits and for private forks with no downstream consumers. Skip it for contributors who have not reproduced the failure on a clean tag, because there is no honest base for a bump. Skip model review when the repository forbids sending source to third-party endpoints, including patches that contain secrets, customer data, or undisclosed vulnerabilities. Skip the whole packet when the maintainer template already requires a changelog with the same fields, because duplicate paperwork helps nobody and slows the merge.

What the pull request should link

A pull request that links the repro wrapper, the new test, semver-impact.md, and model-review.txt gives reviewers a contract claim they can accept or correct in one pass. The patch can still be wrong on performance or style, but the argument is about a named export and a named bump. Engineering work stays in reproduce, patch, and test; the model only pressure-tests the written claim after those stages are already green.

Top comments (0)