SuperAPI AI Directory
Partner中文AI newsFavorites
Skillsduck-review

duck-review

General & other

View repository

Updated 10-16 01:11

About this skill

Review 🦆. Keep terse, actionable format by default.

Purpose

Review changed code with risk-first, actionable findings in paste-ready format.

Philosophy Guardrails (skill-local)

Inherit shared guardrails from references/GUARDRAILS.md.

Comment and Documentation Integrity

  • Document durable constraints, invariants, contracts, and behavior.
  • Do not narrate development history in code comments or ordinary documentation.
  • Remove or flag rejected approaches, “this used to be” history, recent-change context, and references to reviews, tests, or sessions when they only explain how the change was produced.
  • Do not preserve session-only decisions, approval flags, or temporary markers in durable artifacts.
  • Keep historical context only in a durable artifact intended for history, such as an ADR or changelog.
  • Keep test or review references only when they state a durable constraint or verification requirement.

Skill-specific delta:

  • Provide findings and fix directions; user decides merge/approval outcomes.

Activation

Use when user asks to review diff/code/PR for issues and fix direction.

Method

1. Clarify context (if ambiguous)

  • ask 1-3 targeted clarifying questions when context is incomplete

  • state assumptions explicitly when evidence is missing

  • If review context is ambiguous, ask one targeted clarifying question first.

  • Anchor each finding in explicit diff/code evidence.

2. Apply Duck Ladder (complexity guard)

When proposing fix direction, stop at first rung:

  1. No change needed (YAGNI)
  2. Reuse existing local helper/pattern
  3. Replace with stdlib/native
  4. Use already-installed dependency
  5. Shrink to smallest safe diff
  6. Only then add new code/abstraction

3. Review workflow

  1. Confirm review input exists (diff, PR text, or pasted code chunk). If missing, ask for concrete review target.
  2. Scan in priority order: security -> correctness -> data integrity -> performance -> tests -> docs -> simplification.
  3. Emit only actionable findings. One line each: location, problem, fix direction.
  4. Use strongest matching prefix. If multiple apply, pick highest risk prefix.
  5. Enforce strict output shape: one-line prefixed comment template for every finding.
  6. For security or irreversible-risk findings, switch to full paragraph (Auto-Clarity), then resume terse comments.
  7. Huge refactor: report highest-impact findings first; avoid line-noise nits.
  8. Uncertain finding: ask one clarifying question instead of inventing certainty.
  9. Same line has multiple problems: split into separate comments when fixes differ.

When reviewing comments or documentation, flag development narration instead of a durable constraint as 📝 doc:.

If prefix choice unclear or reviewer needs wording examples, load references/review-comment-examples.md.

4. Output findings

One-line comment template:

<prefix> <path[:line]> — <problem>. Fix: <smallest safe change>.

Keep comments paste-ready for PR threads.

Rule (schema-first, prose-flexible):

  • Each finding line must start with approved prefix token
  • Each finding line must include location + problem + Fix: field
  • Only exception: Auto-Clarity for security/irreversible-risk comments; resume prefixed one-line format immediately after
  • Before final response, normalize any non-compliant finding to schema using strongest matching prefix (fallback ⚠️ bug:)

Final self-check before send:

  • If any finding line does not start with approved prefix token, rewrite before sending.
  • If any finding line is missing location or Fix:, rewrite before sending.
  • If reporting findings, use one format throughout (approved prefix tokens); do not mix formats.

Prefixes:

  • 🔒 sec: — security issue (injection, auth bypass, secrets, SSRF)
  • ⚠️ bug: — correctness/data-loss behavior risk
  • ⚡ perf: — performance concern (N+1, unnecessary alloc, bad complexity)
  • 🧪 test: — missing/outdated test coverage
  • 📝 doc: — missing/outdated docs or annotations
  • 🪶 yagni: — unnecessary abstraction/config/speculative flexibility
  • 📚 stdlib: — custom code replaceable by standard library
  • 🧱 native: — dependency/custom layer replaceable by platform feature
  • ✂️ shrink: — same behavior with materially fewer lines
  • 🗑️ delete: — dead/speculative code removable without replacement

Examples:

Good:

  • 🧪 test: src/auth/session.ts:88 — refresh-token expiry path untested. Fix: add test for expired refresh token returning 401.
  • 🔒 sec: db/userRepo.ts:44 — SQL built from raw user input enables injection. Fix: parameterize query placeholders and bind values.

Bad -> Good:

  • bad: - HIGH src/parseAge.ts:3 — invalid input becomes 0
  • good: ⚠️ bug: src/parseAge.ts:3 — invalid input collapses to 0 via falsy check. Fix: use Number.isNaN(n) and throw on invalid age.

Boundaries

  • Reviews only. Don't write patch, don't approve/request-changes, don't run linters/tests.
  • Severity precedence: if simplification and correctness/security both apply, emit higher-risk prefix first; simplification becomes separate comment only when non-duplicative.
  • Auto-Clarity: drop terse mode for security findings, architectural disagreements, onboarding contexts; resume terse after.