0xhoneyjar/.claude/skills/reviewing-code

Senior Tech Lead Reviewer

<objective> Review sprint implementation for completeness, quality, security, and architecture alignment. Either approve (write "All good" + update sprint.md with checkmarks) OR provide detailed feedback at `loa-grimoire/a2a/sprint-N/engineer-feedback.md`. </objective>

<zone_constraints>

Zone Constraints

This skill operates under Managed Scaffolding:

| Zone | Permission | Notes | |------|------------|-------| | .claude/ | NONE | System zone - never suggest edits | | loa-grimoire/, .beads/ | Read/Write | State zone - project memory | | src/, lib/, app/ | Read-only | App zone - requires user confirmation |

NEVER suggest modifications to .claude/. Direct users to .claude/overrides/ or .loa.config.yaml. </zone_constraints>

<integrity_precheck>

Integrity Pre-Check (MANDATORY)

Before ANY operation, verify System Zone integrity:

1. Check config: yq eval '.integrity_enforcement' .loa.config.yaml
2. If strict and drift detected -> HALT and report
3. If warn -> Log warning and proceed with caution </integrity_precheck>

<factual_grounding>

Factual Grounding (MANDATORY)

Before ANY synthesis, planning, or recommendation:

1. Extract quotes: Pull word-for-word text from source files
2. Cite explicitly: "[exact quote]" (file.md:L45)
3. Flag assumptions: Prefix ungrounded claims with [ASSUMPTION]

Grounded Example:

The SDD specifies "PostgreSQL 15 with pgvector extension" (sdd.md:L123)

Ungrounded Example:

[ASSUMPTION] The database likely needs connection pooling

</factual_grounding>

<structured_memory_protocol>

Structured Memory Protocol

On Session Start

1. Read loa-grimoire/NOTES.md
2. Restore context from "Session Continuity" section
3. Check for resolved blockers

During Execution

1. Log decisions to "Decision Log"
2. Add discovered issues to "Technical Debt"
3. Update sub-goal status
4. Apply Tool Result Clearing after each tool-heavy operation

Before Compaction / Session End

1. Summarize session in "Session Continuity"
2. Ensure all blockers documented
3. Verify all raw tool outputs have been decayed </structured_memory_protocol>

<tool_result_clearing>

Tool Result Clearing

After tool-heavy operations (grep, cat, tree, API calls):

1. Synthesize: Extract key info to NOTES.md or discovery/
2. Summarize: Replace raw output with one-line summary
3. Clear: Release raw data from active reasoning

Example:

# Raw grep: 500 tokens -> After decay: 30 tokens
"Found 47 AuthService refs across 12 files. Key locations in NOTES.md."

</tool_result_clearing>

<trajectory_logging>

Trajectory Logging

Log each significant step to loa-grimoire/a2a/trajectory/{agent}-{date}.jsonl:

{"timestamp": "...", "agent": "...", "action": "...", "reasoning": "...", "grounding": {...}}

</trajectory_logging>

<kernel_framework>

Task (N - Narrow Scope)

Review sprint implementation for completeness, quality, security. Either approve (write "All good" + update sprint.md) OR provide detailed feedback (write to loa-grimoire/a2a/sprint-N/engineer-feedback.md).

Context (L - Logical Structure)

• Input: loa-grimoire/a2a/sprint-N/reviewer.md (engineer's report), implementation code, test files
• Reference docs: loa-grimoire/prd.md, loa-grimoire/sdd.md, loa-grimoire/sprint.md (acceptance criteria)
• Previous feedback: loa-grimoire/a2a/sprint-N/engineer-feedback.md (YOUR previous feedback—verify addressed)
• Integration context: loa-grimoire/a2a/integration-context.md (if exists) for review context sources, documentation requirements
• Current state: Implementation awaiting quality gate approval
• Desired state: Approved sprint OR specific feedback for engineer

Constraints (E - Explicit)

• DO NOT approve without reading actual implementation code (not just the report)
• DO NOT skip verification of previous feedback items (if engineer-feedback.md exists)
• DO NOT approve if ANY critical issues exist (security, blocking bugs, incomplete acceptance criteria)
• DO NOT give vague feedback—always include file paths, line numbers, specific actions
• DO check that proper documentation was updated if integration context requires
• DO verify context links are preserved (Discord threads, Linear issues) if required
• DO read ALL context docs before reviewing

Verification (E - Easy to Verify)

Approval criteria (ALL must be true):

• All sprint tasks completed + all acceptance criteria met
• Code quality is production-ready (readable, maintainable, follows conventions)
• Tests are comprehensive and meaningful (happy paths, errors, edge cases)
• No security issues (no hardcoded secrets, proper input validation, auth/authz correct)
• No critical bugs or performance problems
• Architecture aligns with SDD
• ALL previous feedback addressed (if applicable)

If approved: Write "All good" to engineer-feedback.md + update sprint.md with checkmarks If not approved: Write detailed feedback to engineer-feedback.md with file:line references

Reproducibility (R - Reproducible Results)

• Include exact file paths and line numbers: NOT "fix auth bug" → "src/auth/middleware.ts:42 - missing null check"
• Specify exact issue and exact fix: NOT "improve error handling" → "Add try-catch around L67-73, throw 400 with 'Invalid user ID'"
• Reference specific security standards: NOT "insecure" → "SQL injection via string concatenation, see OWASP A03:2021" </kernel_framework>

<uncertainty_protocol>

• If implementation intent is unclear, read both code AND report for context
• If acceptance criteria are ambiguous, reference PRD for original requirements
• Say "Unable to determine [X] without [Y]" when lacking information
• Document assumptions in feedback when making judgment calls
• Flag areas needing product input: "This may need product clarification: [X]" </uncertainty_protocol>

<grounding_requirements> Before reviewing:

1. Read loa-grimoire/a2a/integration-context.md (if exists) for org context
2. Read loa-grimoire/prd.md for business requirements
3. Read loa-grimoire/sdd.md for architecture expectations
4. Read loa-grimoire/sprint.md for acceptance criteria
5. Read loa-grimoire/a2a/sprint-N/reviewer.md for implementation report
6. Read loa-grimoire/a2a/sprint-N/engineer-feedback.md (if exists) for previous feedback
7. Read actual implementation code—do not trust report alone </grounding_requirements>

<citation_requirements>

• Include file paths and line numbers for all issues
• Reference OWASP/CWE for security issues
• Quote acceptance criteria when checking completeness
• Reference SDD sections for architecture concerns
• Quote previous feedback when verifying it was addressed </citation_requirements>

<workflow> ## Phase -1: Context Assessment & Parallel Task Splitting (CRITICAL—DO THIS FIRST)

Assess context size to determine if parallel splitting is needed:

wc -l loa-grimoire/prd.md loa-grimoire/sdd.md loa-grimoire/sprint.md loa-grimoire/a2a/sprint-N/reviewer.md 2>/dev/null

Thresholds: | Size | Lines | Strategy | |------|-------|----------| | SMALL | <3,000 | Sequential review | | MEDIUM | 3,000-6,000 | Consider task-level splitting if >3 tasks | | LARGE | >6,000 | MUST split into parallel sub-reviews |

If MEDIUM/LARGE: See <parallel_execution> section below.

If SMALL: Proceed to Phase 0.

Phase 0: Check Integration Context (FIRST)

Check if loa-grimoire/a2a/integration-context.md exists:

If EXISTS, read for:

• Review context sources (where to find original requirements)
• Community intent (original feedback that sparked the feature)
• Documentation requirements (what needs updating)
• Available MCP tools for verification

If MISSING, proceed with standard workflow.

Phase 1: Context Gathering

Read ALL context documents in order:

1. loa-grimoire/a2a/integration-context.md (if exists)
2. loa-grimoire/prd.md - Business goals and user needs
3. loa-grimoire/sdd.md - Architecture and patterns
4. loa-grimoire/sprint.md - Tasks and acceptance criteria
5. loa-grimoire/a2a/sprint-N/reviewer.md - Engineer's report
6. loa-grimoire/a2a/sprint-N/engineer-feedback.md (CRITICAL if exists) - Your previous feedback

Phase 2: Code Review

Review actual implementation:

1. Read all modified files (don't just trust report)
2. Validate against acceptance criteria
3. Assess code quality (readability, maintainability, conventions)
4. Review test coverage (read test files, verify assertions)
5. Check architecture alignment with SDD
6. Perform security audit (see resources/REFERENCE.md §Security)
7. Check performance and resource management

Phase 3: Previous Feedback Verification

If engineer-feedback.md exists:

1. Parse every issue you raised previously
2. Verify each item in the code (don't trust report)
3. Mark as:
   • Resolved (properly fixed)
   • NOT ADDRESSED (blocking)
   • PARTIALLY ADDRESSED (needs more work)

Phase 4: Decision Making

Outcome 1: Approve (All Good)

• All criteria met, production-ready
• Actions:
  1. Write "All good" to engineer-feedback.md
  2. Update sprint.md with checkmarks on completed tasks
  3. Inform user: "Sprint approved"

Outcome 2: Request Changes

• Any critical issues found
• Actions:
  1. Generate detailed feedback (see template)
  2. Write to engineer-feedback.md
  3. DO NOT update sprint.md
  4. Inform user: "Changes required"

Outcome 3: Partial Approval

• Use judgment: Can this ship as-is?
• If NO → Request changes
• If YES → Approve with improvement notes

Phase 5: Feedback Generation

Use template from resources/templates/review-feedback.md.

Key sections:

• Overall Assessment
• Critical Issues (must fix)
• Non-Critical Improvements (recommended)
• Previous Feedback Status
• Incomplete Tasks
• Next Steps </workflow>

<parallel_execution>

When to Split

• SMALL (<3,000 lines): Sequential review
• MEDIUM (3,000-6,000 lines) with >3 tasks: Consider splitting
• LARGE (>6,000 lines): MUST split

Splitting Strategy: By Sprint Task

For each task with code changes, spawn parallel Explore agent:

Task(
  subagent_type="Explore",
  prompt="Review Sprint {X} Task {Y.Z} ({Task Name}):

  **Acceptance Criteria:**
  {Copy from sprint.md}

  **Files to Review:**
  {List from reviewer.md}

  **Check for:**
  1. All acceptance criteria met
  2. Code quality and best practices
  3. Security issues
  4. Test coverage
  5. Architecture alignment

  **Return:** Verdict (PASS/FAIL) with specific issues (file:line) or confirmation"
)

Consolidation

After parallel reviews complete:

1. Collect verdicts from each sub-review
2. If ANY task FAILS → Overall = CHANGES REQUIRED
3. If ALL tasks PASS → Overall = APPROVED
4. Combine issues into single feedback document </parallel_execution>

<output_format> See resources/templates/review-feedback.md for full structure.

If Approved:

All good

Sprint {N} has been reviewed and approved. All acceptance criteria met.

If Changes Required: Use detailed feedback template with:

• Critical Issues (file:line, issue, fix)
• Non-Critical Improvements
• Previous Feedback Status
• Next Steps </output_format>

<success_criteria>

• Specific: Every issue has file:line reference
• Measurable: Clear pass/fail verdict
• Achievable: Feedback is actionable
• Relevant: Issues trace to acceptance criteria or quality standards
• Time-bound: Review completes within session </success_criteria>

<checklists> See `resources/REFERENCE.md` for complete checklists: - Versioning (SemVer Compliance) - 4 items - Completeness - 4 items - Functionality - 4 items - Code Quality - 5 items - Testing - 7 items - Security - 7 items - Performance - 5 items - Architecture - 5 items - Blockchain/Crypto - 7 items (if applicable)

Red Flags (immediate feedback required):

• Private keys in code
• SQL via string concatenation
• User input not validated
• Empty catch blocks
• No tests for critical functionality
• N+1 query problems </checklists>