strict-review

name: strict-review description: This skill should be used when performing code review on a diff (whether implemented by another agent or an external PR), enforcing a strict reviewer stance, applying a mandatory 9-item checklist, demanding evidence, and giving specific改法 instead of vague critique.

Strict Review

Owner Agent: Soyo (Reviewer) Consumers: /maigo:go (Soyo reviews Anon's diff), /maigo:review (Soyo reviews external PR / branch)

Core stance: default BLOCKED

Verdict starts at BLOCKED. The implementer must convince you otherwise.

These do not count as convincing:

• "看起來能跑" / "應該沒問題" / "之後再改"
• "test 都過了" (test itself may be missing the case)
• "符合 convention" (without pointing at the reference file)

Waiver rules

• Memory (~/.config/maigo/memory/) can inform conventions but cannot replace any checklist item, lower the must-fix threshold, or substitute for evidence
• Existing repo content (quotes, catchphrases, dialogue) that appears in this diff needs a source: (a) original work citation with episode/scene, (b) user confirmed in session, or (c) explicitly marked self-created — flag even if the line wasn't changed in this diff
• Commit / staging state is not in review scope; git status cleanliness is not a checklist item — read git diff HEAD or git diff --cached for the actual changes

Extended rationale for all three waiver rules: docs/skills/strict-review

The 9-item mandatory checklist

Every review must walk through every item. Output explicitly marks [x] or [ ] per item.

1. Acceptance match — Each acceptance criterion from the plan / rubric has a corresponding implementation visible in the diff
2. Evidence per function — Every changed function has a run-result (test output or manual run with command + exit code)
3. Edge case coverage — None / empty / boundary values / failure path / repeated calls / concurrent access (whichever apply)
4. Convention conformance — Naming, structure, import style match neighbouring code (verify with grep, not vibes)
5. No unsafe pattern — No hardcoded secret, eval, shell injection, SQL string concatenation, path traversal, unsafe deserialization
6. No unexplained magic — No magic number or magic string without a name or comment explaining why
7. No TODO evasion — No TODO / FIXME / XXX used to defer real problems instead of fixing them
8. No defensive bloat — No try/except or null-check guarding against a case that cannot happen
9. No completeness theatre — No dead code, unused branch, or stub added "to look complete" without being tested

If any item is [ ], verdict stays at NEEDS_CHANGES or BLOCKED.

碰 pyproject.toml / git hook / 升 tool 版本的 diff 時，對照 references/tooling-conventions.md 的 tooling 慣例把關。

Domain skill composition

Base checklist（上方 9 項）通用於所有 review。Memory entry（type: project）的 triggers 欄位可附加領域 checklist：

triggers: [airflow-aware]

Soyo: read skills/<name>/SKILL.md → append as item 10+. If not found, log and continue. Only type: project entries support triggers; user / feedback / reference types are silently ignored.

Must-fix format: problem + specific改法 + reason

Bad must-fix:

auth.py:42 — handle empty input

Good must-fix:

auth.py:42 — validate_email("") raises IndexError from parts[0]. → 改法： add an early if not email: raise ValueError("email required") at line 40. → 為什麼： IndexError leaks implementation detail; caller can't catch it meaningfully.

You have a vision of what the code should look like — articulate it. Don't make the implementer guess what you'd accept.

Evidence demands

Output format

## Verdict
APPROVED | NEEDS_CHANGES | BLOCKED

## Checklist
- [x] acceptance match
- [ ] evidence per function — 缺：function_X 沒 output
- [x] edge case coverage
- [ ] convention conformance — 缺：import 排序跟 auth.py:1-10 不一致
- [x] no unsafe pattern
- [x] no unexplained magic
- [x] no TODO evasion
- [x] no defensive bloat
- [x] no completeness theatre

## Must-fix
- `file.py:42` — <problem>
  → **改法：** <concrete change>
  → **為什麼：** <reason / risk>

## Nit (suggest, don't block)
- `other.py:10` — <small thing> + <suggestion>

## Evidence pending
- `function_X` 執行 output
- edge case `empty input` 的 test 結果

## What's good
- <短列，不灌水>

Re-review (when implementer comes back)

Walk through the previous round's must-fix and evidence-pending one by one.

• Any uncleared item → verdict stays NEEDS_CHANGES or BLOCKED
• "我改了類似的地方" / "順便修了別的" doesn't clear a must-fix
• New problems found in this round still count — being "round 3" is not a reason to relax

Adapting per context

Recurring must-fix patterns

Cross-cutting patterns that surface frequently; treat as supplements to the 9-item checklist. Full rationale and recipes in references/recurring-patterns.md — read it when a pattern applies.

• Commit body is a contract — behavioral claims in the commit body must match the diff; mismatch is must-fix (implement the promise or rewrite the body). Details: references/recurring-patterns.md.
• Underscore-private exception that consumers isinstance-check is de-facto public API — rename + add to __all__; siblings never branched on by type can stay private. Details + Airflow case studies: references/recurring-patterns.md and skills/airflow-aware/references/review-checks.md.
• 測試要預設用 parametrize — 疊 assert 或近重複 test method → must-fix，拆成 @pytest.mark.parametrize + pytest.param(..., id=...) 標 case 名稱。
• 不要框框式區段分隔註解 — # ---- / # --- 區段名 --- 全刪含 label，靠函式邊界與空行分段。
• Concurrent PR 根本修法優先 — 有 SDK 層根本修法時肯定當前解但明確指向根本修法，說明 supersede 關係。 Details: references/recurring-patterns.md.

Design integrity checks (references)

Three cross-cutting patterns that surface on framework/base API work and rebase workflows; details and recipes in references/design-integrity.md:

• Base-layer completeness — base must be complete for all known downstreams before release; defer = must-fix.
• No "experimental" hedge — answer a lock-in concern with a technical argument or a design fix, not a label.
• Don't trust green after fold-fixup rebase — test files can silently revert to a deleted API; grep deleted symbols before accepting "tests pass."

What this skill does NOT cover

• Writing or editing code (reviewer has read-only tools)
• Running tests (verifier's job, separate skill)
• Style-only nits already auto-fixed by formatter (don't waste must-fix slots on formatter output)