The Extra Argument Is a Release: Reviewing Agent PRs That Grow Signatures

An optional parameter is a release. Coding agents add them so a new branch compiles, then attach a default that every old caller inherits. Review that default as shipped behavior, not as leftover style.

This article is a review method for agent-generated pull requests that grow public signatures, exports, and CLI flags. It states what to trust, what to revert, and what to test. It also includes a proposed AST diff you can run on two worktrees.

The failure mode

Agents optimize for the call site they can see. They do not inventory every downstream importer, wrapper script, or dashboard that shells out to your CLI.

A defaulted parameter changes behavior for callers that never pass it. A newly public name is a support promise. A flag that defaults to on rewrites production argv you did not grep.

This is not hidden I/O, swallowed exceptions, or tests that encode the bug. The diff lives in the signature. The regression shows up two services away.

Define the contract before arguing about taste

For a Python package, treat the following as public unless the repo already documents a narrower rule:

• Module-level names that do not start with _
• Names listed in __all__
• Symbols re-exported from __init__.py
• argparse / click / typer options and their defaults
• Dataclass fields and typed-dict keys that serialize to JSON or ORM rows

Private helpers (_parse) can move. Public names cannot. If the agent promotes _parse to parse so a unit test can import it, that is a product decision. Revert the promotion or accept the support cost.

Artifact: two-tree signature diff

The script below is a proposed, unexecuted example. It walks two source trees (base vs PR) and prints added public callables, added parameters, and changed defaults. Run it on a checkout. Do not treat its output as a security audit.

#!/usr/bin/env python3
"""signature_diff.py — compare public function signatures between two trees."""
from __future__ import annotations

import ast
import sys
from dataclasses import dataclass
from pathlib import Path


@dataclass(frozen=True)
class Param:
    name: str
    kind: str
    default: str | None


@dataclass(frozen=True)
class FuncSig:
    qualname: str
    params: tuple[Param, ...]
    returns: str | None


def _rel(path: Path, root: Path) -> str:
    return path.relative_to(root).as_posix()


def _is_public(name: str) -> bool:
    return not name.startswith("_")


def _dump_default(node: ast.expr | None) -> str | None:
    if node is None:
        return None
    return ast.dump(node, include_attributes=False)


def _params(fn: ast.FunctionDef | ast.AsyncFunctionDef) -> tuple[Param, ...]:
    args = fn.args
    out: list[Param] = []

    def take(items: list[ast.arg], kind: str, defaults: list[ast.expr | None]) -> None:
        padded: list[ast.expr | None] = [None] * (len(items) - len(defaults)) + list(defaults)
        for arg, default in zip(items, padded):
            out.append(Param(arg.arg, kind, _dump_default(default)))

    take(args.posonlyargs, "posonly", [])
    take(args.args, "pos_or_kw", list(args.defaults))
    if args.vararg:
        out.append(Param(args.vararg.arg, "vararg", None))
    take(args.kwonlyargs, "kwonly", list(args.kw_defaults))
    if args.kwarg:
        out.append(Param(args.kwarg.arg, "kwarg", None))
    return tuple(out)


def _skip_path(path: Path) -> bool:
    parts = set(path.parts)
    if parts & {"venv", ".venv", "site-packages"}:
        return True
    if "tests" in parts or path.name.startswith("test_"):
        return True
    return False


def walk(root: Path) -> dict[str, FuncSig]:
    found: dict[str, FuncSig] = {}
    for path in root.rglob("*.py"):
        if _skip_path(path):
            continue
        try:
            tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
        except SyntaxError:
            continue
        mod = _rel(path, root).replace("/", ".").removesuffix(".py")
        for node in tree.body:
            if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
                if not _is_public(node.name):
                    continue
                q = f"{mod}:{node.name}"
                ret = ast.dump(node.returns, include_attributes=False) if node.returns else None
                found[q] = FuncSig(q, _params(node), ret)
            elif isinstance(node, ast.ClassDef) and _is_public(node.name):
                for item in node.body:
                    if not isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)):
                        continue
                    if not _is_public(item.name):
                        continue
                    q = f"{mod}:{node.name}.{item.name}"
                    ret = ast.dump(item.returns, include_attributes=False) if item.returns else None
                    found[q] = FuncSig(q, _params(item), ret)
    return found


def diff(base: dict[str, FuncSig], pr: dict[str, FuncSig]) -> None:
    print("## added public callables")
    for k in sorted(set(pr) - set(base)):
        names = ", ".join(p.name for p in pr[k].params)
        print(f"+ {k}({names})")
    print("## removed public callables")
    for k in sorted(set(base) - set(pr)):
        print(f"- {k}")
    print("## signature changes")
    for k in sorted(set(base) & set(pr)):
        if base[k].params == pr[k].params and base[k].returns == pr[k].returns:
            continue
        b = {p.name: p for p in base[k].params}
        p = {p.name: p for p in pr[k].params}
        added = [n for n in p if n not in b]
        dropped = [n for n in b if n not in p]
        changed = [n for n in p if n in b and p[n].default != b[n].default]
        print(f"* {k}")
        if added:
            print(f"    added params: {added}")
            for n in added:
                print(f"      default={p[n].default}")
        if dropped:
            print(f"    dropped params: {dropped}")
        if changed:
            print(f"    changed defaults: {changed}")
            for n in changed:
                print(f"      {b[n].default} -> {p[n].default}")
        if base[k].returns != pr[k].returns:
            print(f"    return: {base[k].returns} -> {pr[k].returns}")


def main(argv: list[str]) -> int:
    if len(argv) != 3:
        print("usage: signature_diff.py BASE_TREE PR_TREE", file=sys.stderr)
        return 2
    diff(walk(Path(argv[1])), walk(Path(argv[2])))
    return 0


if __name__ == "__main__":
    raise SystemExit(main(sys.argv))

Checkout both sides, then run:

git fetch origin pull/123/head:pr-123
git worktree add /tmp/pr-base origin/main
git worktree add /tmp/pr-head pr-123
python3 signature_diff.py /tmp/pr-base /tmp/pr-head

Pair the AST walk with a grep for CLI and env-shaped contracts the parser will miss:

git diff origin/main...HEAD -- '*.py' \
  | rg -n 'add_argument\(|click\.(option|command)|typer\.Option|os\.environ|getenv\('

The script will not see C extensions, *args forwarding that later binds names, or module-level __getattr__. Use it as a net, not as a proof.

Decision table: trust, revert, test

Diff class	Trust?	Revert?	Test
New _-prefixed helper, no re-export	Yes, if old public paths still have tests	No	Existing callers only
New public function used only by tests	No	Yes, or keep it private	Import linter: tests must not introduce new public names
New param whose default is None and skips a prior required path	No	Yes, unless the skip is an intentional product change	Omitted arg equals old behavior; explicit None is documented
New param whose default changes a timeout, limit, batch size, or retry	No	Revert the default; require explicit opt-in	Bound test with the old numeric value as the implicit default
Default value changed (limit=100 → limit=1000)	No	Restore the old default; add a named constant if needed	Snapshot callers that omit the arg
New required param	No, unless this is an intentional major	Restore a default or split a new function	Compile downstream examples
New CLI flag defaulting to on	No	Default off, or drop the flag	Run the documented command line with no new flags
Return type widened (list[User] → `list[User] \	None`)	No	Keep the old type; raise or return empty
Re-export from __init__.py	Only if it was already public	Drop the re-export	Freeze from package import name

"Trust" means: leave it in the merge. It does not mean the agent understood production.

Review sequence that stays on the signature

1. Generate the signature diff. If it is empty and the CLI grep is empty, stop this checklist and review other classes of bug.
2. For each added parameter, write the implied call in the review comment: foo(a, b) before, foo(a, b, c=DEFAULT) after. If DEFAULT is not observationally equal to the old body, the PR is a behavior change.
3. Reject DEFAULT=None as a stand-in for "do the new thing." None is a value. Callers pass it by accident.
4. Reject promotions made for test convenience. If a test needs a hook, keep the hook private or add a supported testing entrypoint.
5. Freeze one documented import list and one documented command line in CI. Agent PRs that change either must update the freeze file in the same commit.

Proposed freeze test (unexecuted example):

# tests/test_public_names_freeze.py
import json
from pathlib import Path
import myservice as pkg

FREEZE = Path(__file__).parent / "public_names.json"


def test_public_names_match_freeze():
    names = sorted(n for n in dir(pkg) if not n.startswith("_"))
    expected = json.loads(FREEZE.read_text())
    assert names == expected, (
        "Public names changed. If intentional, update public_names.json "
        "and the changelog in the same PR."
    )

That test is boring. Boring is the point. Agents will edit the freeze file if the instructions allow it. Treat a freeze-file edit as a product change, same as a schema migration.

Where a scratch model and server help

Reproducing two worktrees and running signature_diff.py needs a clean machine, not a chat transcript. If you do not want extra worktrees on a laptop, MonkeyCode's free model access and free server option can hold the checkout and run the commands above.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

Use the model only to draft the missing tests for each added default. Do not let it "fix" the freeze file. The reviewer still classifies each row in the decision table. Skip any hosted model if the tree is proprietary and policy forbids upload. The method does not depend on a vendor.

What to test when you keep a new default

If the team accepts a new optional parameter, the minimum suite is three cases, not one:

1. Omission. Call the old arity. Assert the old observable result, including timing bounds if the default is a timeout or batch size.
2. Explicit old value. Pass the previous implicit value. Assert equality with omission. If they diverge, the default is not the old behavior.
3. Explicit new value. Pass the agent's new path. Assert the new result, and assert it cannot be reached by omission.

For CLI flags, replace step 1 with the argv vector copied from a runbook. Do not invent a "typical" command line. Copy one from production logs or from the README operators already use.

Label any load or latency claim as unmeasured unless a bench already lives in the repo. A default limit change is a performance change even when CPU tests stay green.

Worked review comment (proposed text, not a recorded incident):

NACK default. `fetch_page(limit=1000)` changes every caller that omitted
`limit`. Old body used 100. Restore default=100. Keep `limit=` as opt-in.
Add tests: omit == 100, limit=100 == omit, limit=1000 is the new path only.

Limitations

• The AST walker ignores dynamically constructed signatures, functools.wraps mismatches, and protocols implemented in another language.
• __all__ lies. Prefer dir() on the installed package over reading __all__ by eye.
• Dataclass field defaults and Pydantic Field(default=...) are public contract. The sample script does not parse them. Extend it or review those files by hand.
• Internal-only apps with a single binary and no importers can skip the freeze file. They should not skip CLI default review.
• Generated protobuf or OpenAPI stubs are the schema, not the agent's Python wrapping. Review the schema file, then the wrapper.
• This method will not catch authorization gaps, oracle-contaminated tests, or hidden network calls. Those need other checklists.

Skip this workflow when the PR is a typo, a comment, or a pinned dependency with no signature change. Apply it when the agent "just added a parameter so the new branch compiles."

Teams with an existing API-freeze bot can keep the bot and still use the decision table. The table is the review standard. The script is only a finder.

Closing

An omitted argument is a caller. New defaults ship to that caller without a changelog line.

Diff the signatures. Revert convenience exports. Keep new behavior behind explicit arguments. Freeze one import list and one command line. That is the whole method.