Receiving Code Review

Receiving Code Review

Core Principle

Verify before implementing. Technical correctness matters more than social comfort. A reviewer can be wrong -- blindly implementing bad suggestions creates bugs.

Response Pattern

For each piece of feedback, follow this sequence:

0. Prior feedback check (re-reviews only) -- if this is not the first review round, check whether previously flagged issues were addressed before processing new comments. Compare the current diff against prior review threads (gh api repos/{owner}/{repo}/pulls/{pr}/comments). Surface any that were ignored or only partially fixed -- these take priority over new feedback.

1. Read -- Understand what's being suggested and why 2. Verify -- Is the suggestion technically correct for THIS codebase? 3. Evaluate -- Does it improve the code, or is it preference/style? 4. Respond -- Agree with evidence, disagree with evidence, or ask for clarification 5. Implement -- Only after verification confirms the suggestion is correct

Triage all feedback first (see Implementation Order below), then implement one item at a time. Don't batch-implement everything at once.

Handling Unclear Feedback

When feedback is ambiguous or incomplete:

• Stop -- do not implement anything unclear
• Clarify ALL unclear items before implementing ANY of them (they may be related)
• Ask specific questions: "Are you suggesting X or Y?" not "Can you elaborate?"
• If the reviewer's intent is clear but the technical approach is wrong, say so

Batched clarification for critical-path ambiguity: When multiple ambiguous findings land on critical-path code (auth, payments, data migrations, permission checks) AND the AskUserQuestion tool is available, batch up to 4 of them into a single call rather than asking one at a time. Each question's header is the truncated filename and line, and the options are Valid / False positive / Defer. Skip the batched ask entirely when ambiguous findings are only on non-critical paths — just auto-triage those and move on. If AskUserQuestion is not available, fall back to a single prose block listing all ambiguous items numbered, asking for Valid/False-positive/Defer decisions. The batching limit is 4 because it caps cleanly at that size; asking more becomes noise rather than judgment.

Source-Specific Handling

From the user (project owner)

• Trusted context -- they know the codebase and business requirements
• Implement after understanding, but still verify technical correctness
• Ask clarifying questions when the intent is clear but the approach seems risky
• No performative agreement -- just acknowledge and implement

From automated review agents

• Skeptical by default -- agents lack full context
• Verify every suggestion against the actual codebase
• Check for YAGNI violations (agents love adding "just in case" code)
• Discard suggestions that contradict project conventions (check CLAUDE.md)
• Agents may flag things that are intentional design decisions -- check before changing

From external reviewers (PR comments, open source)

• Verify technical correctness for THIS stack and codebase
• Check if the suggestion applies to this version of the framework/library
• Push back if the reviewer lacks context about architectural decisions
• Distinguish between "this is wrong" and "I would do it differently"

When to Push Back

Push back (with evidence) when a suggestion:

• Breaks existing functionality -- "This would break X because Y depends on Z"
• Violates project conventions -- "Our CLAUDE.md specifies we do it this way because..."
• Is technically incorrect -- "This API was deprecated in v3. We're on v4 which uses..."
• Adds unnecessary complexity -- "This handles a case that can't occur because..."
• Is unused (YAGNI) -- when a reviewer suggests "implementing properly", grep the codebase for actual usage FIRST. Zero callers? Suggest removal: "This endpoint isn't called. Remove it (YAGNI)?" If used, implement properly.
• Conflicts with architectural decisions -- "We chose X over Y in the brainstorm because..."

Valid evidence: code references (file:line), test output, git blame/log, framework docs, reproduction steps, grep results showing usage patterns. Not evidence: "I think", "it should work", "it's fine", appeals to convention without citing the convention, or restating the original code as justification.

False-Positive Taxonomy (for dismissed suggestions)

When dismissing a suggestion (AUTO-DECLINE, manual push-back), tag the dismissal with one of four categories so the reviewer sees structured reasoning, not a bare "no":

Use the tag in the reply: "FP-ALREADY-HANDLED: null check happens in auth/middleware.ts:42 before this handler runs. Keeping as-is." Structured tags prevent the "you're wrong because reasons" reply pattern and make future triage faster (if the same comment class keeps hitting FP-CONVENTION, the convention needs better documentation).

When NOT to Push Back

Accept feedback when:

• The suggestion is correct and you missed something
• It catches a genuine bug or edge case
• It improves readability without changing behavior
• It aligns with project conventions you overlooked
• The reviewer has domain expertise you lack

Common Mistakes

Approved Response Templates

When feedback IS correct: "Verified -- [specific issue]. Implementing [specific fix]." When feedback is partially correct: "The [X part] is right because [reason]. The [Y part] doesn't apply here because [evidence]." When you need clarification: "Can you clarify [specific ambiguity]? The comment could mean [A] or [B], which changes the fix."

Implementation Order

After triaging all feedback:

1. Clarify -- resolve all unclear items first 2. Blocking issues -- fix things that break functionality 3. Simple fixes -- quick wins that are clearly correct 4. Complex fixes -- changes that need careful implementation

Test after each individual fix, not after implementing everything.

When Your Pushback Was Wrong

State the correction factually: "Checked this, you're correct because [reason]. Implementing." No extended apology, no self-deprecation -- just acknowledge and move on.

GitHub PR Reviews

• Reply in the inline comment thread, not as top-level PR comments: gh api repos/{owner}/{repo}/pulls/{pr}/comments -f body="..." -f in_reply_to={comment_id}
• Reference specific lines when explaining why you disagree
• Mark conversations as resolved only after the fix is verified
• If a suggestion spawns a larger discussion, suggest moving it to an issue

Headless Mode

When invoked programmatically (by another skill or command with mode:headless), skip interactive prompts and return structured triage results. See headless-mode.md for the classification table (AUTO-FIX / AUTO-DECLINE / ESCALATE), output format, and constraints.

Scope vs ia-pr-comment-resolver Agent

This skill and the ia-pr-comment-resolver agent handle different situations:

When the ia-pr-comment-resolver agent encounters feedback that requires judgment (architectural decisions, debatable trade-offs), it should escalate rather than implement.

Examples

Bad -- performative agreement:

Reviewer: "This should use a Map instead of an object for better performance."

Response: "Great catch! Switching to Map now."

Good -- verify first:

Reviewer: "This should use a Map instead of an object for better performance."

Response: "Checked this -- the object has at most 5 keys (bounded by the enum). Map overhead isn't justified here. Keeping as-is."

Bad -- blind implementation:

Reviewer: "Add null checks for the user parameter."

Response: "Done, added null checks."

Good -- verify the premise:

Reviewer: "Add null checks for the user parameter."

Response: "Traced the call chain -- user comes from the auth middleware which guarantees non-null (throws 401 otherwise). Adding a null check here would be dead code. Should I add it anyway for defense-in-depth?"

Good -- unclear item:

Reviewer: "This logic seems off."

Response: "Can you clarify which part? The filter predicate, the sort order, or the pagination logic?"

Verify

• All feedback items triaged (classified as correct, incorrect, or unclear)
• Ambiguous items clarified before any implementation
• Each fix verified individually (test after each, not batch)
• No performative agreement phrases in responses ("Great catch!", "Absolutely right")

Integration

This skill pairs with:

• ia-code-review -- the outbound side (requesting reviews). Their action-routing tiers (safe_auto/gated_auto/manual/advisory) roughly map to this skill's AUTO-FIX / ESCALATE-for-approval / ESCALATE / FYI.
• ia-pr-comment-resolver agent -- for mechanical PR comment resolution (see scope table above)
• ia-verification-before-completion -- verify each fix before marking resolved

Headless Mode

When invoked programmatically (by another skill or command with mode:headless), skip interactive prompts and return structured triage results.

Triage Process

1. Collect -- gather all unresolved review comments from the PR 2. Check prior feedback -- if prior review comments exist (re-review), flag previously ignored or partially addressed items. Skip on first-time reviews. 3. Classify each comment using the same verification logic as interactive mode:

4. Return structured output:

TRIAGE RESULTS:
- AUTO-FIX (N items): [list with one-line summaries]
- AUTO-DECLINE (N items): [list with evidence for each]
- ESCALATE (N items): [list with why each needs human judgment]
- PRIOR FEEDBACK: [addressed|partially addressed|ignored] with specifics

Constraints

• Never auto-fix security-related suggestions -- always escalate
• Never auto-decline feedback from the project owner -- escalate instead
• Apply the same skepticism levels from Source-Specific Handling (agents: skeptical, external: verify, owner: trusted)
• If >50% of comments classify as ESCALATE, abort headless mode and recommend interactive review

ia-receiving-code-review Specification

Intent

ia-receiving-code-review is a discipline-class skill (an engineering practice not tied to one stack). Process code review feedback critically: check correctness before acting, push back on incorrect suggestions, no performative agreement. Use when responding to PR/MR review comments or implementing reviewer suggestions received from others.

Scope

In scope:

• Behaviors described in SKILL.md and routed via the should_trigger phrasings in distillery/tests/fixtures/triggers/ia-receiving-code-review.jsonl.
• Updates to runtime behavior, structure, trigger precision, references, and validation.

Out of scope:

• Acting as the runtime instructions themselves (those live in SKILL.md).
• Trigger phrasings already covered by adjacent ia-* skills (validate-plugin flags >70% description overlap as DUPLICATE_TRIGGER).
• <!-- to fill in: domain-specific exclusions when the skill drifts -->

Trigger Context

• Class: discipline
• Hook regex: plugins/whetstone/hooks/skill-patterns.sh -> SKILL_PATTERNS[ia-receiving-code-review]
• Common requests (from fixture should_trigger):
• "the reviewer suggested we use a factory pattern instead"
• "address the review comments on the PR"
• "respond to the reviewer comments on my PR"
• Should not trigger for (from fixture should_not_trigger):
• "deploy the staging environment to AWS"
• "create a new database migration for orders"
• "review someone else's PR"

Source And Evidence Model

Authoritative sources:

• SKILL.md -- runtime instructions and reference routing.
• references/*.md -- bundled supplementary content (1 file(s)).
• distillery/tests/fixtures/triggers/ia-receiving-code-review.jsonl -- positive and negative trigger phrasings under regression test.
• plugins/whetstone/hooks/skill-patterns.sh -- regex pattern that fires this skill.
• distillery/.eval-data/ia-receiving-code-review/ -- harvested session examples (when present).

Data that must not be stored in this skill or its references:

• Secrets, credentials, tokens.
• Machine-specific filesystem paths (/home/..., /Users/..., ~/ai/...). The validator (MACHINE_PATH_LEAK) flags these as HIGH.
• Private URLs, customer data, or unredacted personal information.

Coverage matrix

Evaluation

Lightweight (run on every change):

python3 distillery/scripts/distiller.py validate-plugin --component ia-receiving-code-review
python3 distillery/scripts/distiller.py test-triggers --skill ia-receiving-code-review

Deeper (when behavior risk warrants):

python3 distillery/scripts/distiller.py dspy-eval ia-receiving-code-review
python3 distillery/scripts/distiller.py diagnose-negatives ia-receiving-code-review

Acceptance gates:

• validate-plugin --component ia-receiving-code-review returns 0 HIGH findings.
• test-triggers --skill ia-receiving-code-review returns F1 = 1.0 with floors of 5 should_trigger and 5 should_not_trigger.
• For dspy-eval, the composite score does not regress against the most recent saved baseline (see distillery/.eval-data/ia-receiving-code-review/history.json).

Known Limitations

<!-- to fill in over time as drift surfaces. Default rule: any time diagnose-negatives surfaces a recurring failure pattern, document it here so future maintainers understand the trade-off the current implementation accepts. -->

Maintenance Notes

• Update SKILL.md when the runtime workflow, branch conditions, or output contract changes.
• Update this SPEC.md when intent, scope, evidence model, evaluation gates, or maintenance expectations change.
• Update the trigger fixture when adding new positive phrasings, removing stale ones, or expanding scope (the 5/5 floor is a hard validator gate).
• Update the hook regex in skill-patterns.sh whenever fixture positives expose a missed phrasing; verify F1 = 1.0 with eval-triggers before committing.
• Run the full release pipeline via /release -- never bump versions or update CHANGELOG.md from a per-skill edit.