review-code

Reviewing a summonpot change

Check the diff against the defects this project has actually shipped, in the order they have cost the most.

1. Does a doc claim something the code does not do?

The most repeated defect here. Shipped examples: stream=True documented as working while nothing read the flag; method= accepted and discarded; "capabilities themselves remain deterministic" claiming a guarantee the framework cannot give.

A claim is a defect. Check README.md, ROADMAP.md, docs/, docstrings, and src/summonpot/templates/skills/summonpot.md.

grep -rn "stream\|method=" README.md docs/ src/summonpot/templates/skills/

2. Does a new guard reject valid code?

Every registration guard needs a false-positive test, not only a true-positive one. Two guards shipped rejecting valid input:

• an unbound-method check that rejected any function whose first parameter was self;
• an annotation check that rejected request: "Request" where Request existed.

For any new rule, write the case that should still be accepted and run it.

3. Is the failure raised at the right time?

Registration beats request time; request time beats silence. A wrong declaration should never reach traffic. Summon.__call__() in src/summonpot/summon.py is the registration entry point. Place each rule in the responsible validation layer — including capability normalization in src/summonpot/tools.py and contract checks in src/summonpot/_validation.py — and ensure the entry point invokes it before serving or execution.

4. Does the test fail without the fix?

Assert it in an isolated worktree so verification cannot modify the reviewer's working tree or any user-owned recovery state. Apply only the test diff to the base revision, then run the focused regression there:

base=${BASE_REF:-origin/main}
red_tree=$(mktemp -d)
git worktree add --detach "$red_tree" "$base"
git diff "$base" -- tests/ | git -C "$red_tree" apply -
(cd "$red_tree" && uv sync --all-extras --locked && uv run pytest tests/ -q)
git worktree remove --force "$red_tree"