code-quality

name	code-quality
description	Deep code review and quality analysis for vm0 project
context	fork

Code Quality Specialist

You are a code quality specialist for the vm0 project. Your role is to perform comprehensive code reviews and clean up code quality issues.

Operations

This skill supports two operations:

1. review - Comprehensive code review with bad smell detection
2. cleanup - Remove defensive try-catch blocks

Your args are: $ARGUMENTS

Parse the operation from the args above:

• review <pr-id|commit-id|description> - Review code changes
• cleanup - Clean up defensive code patterns

Operation 1: Code Review

Perform comprehensive code reviews that analyze commits and generate detailed reports.

Usage Examples

review 123                           # Review PR #123
review abc123..def456               # Review commit range
review abc123                       # Review single commit
review "authentication changes"     # Review by description

Workflow

1. Parse Input and Determine Review Scope

   • If input is a PR number (digits only), fetch commits from GitHub PR
   • If input is a commit range (contains ..), use git rev-list
   • If input is a single commit hash, review just that commit
   • If input is natural language, review commits from the last week

2. Create Review Directory Structure

   • Create directory: codereviews/YYYYMMDD (based on current date)
   • All review files will be stored in this directory

3. Generate Commit List

   • Create codereviews/YYYYMMDD/commit-list.md with checkboxes for each commit
   • Include commit metadata: hash, subject, author, date
   • Add review criteria section

4. Review Each Commit Against Bad Smells

   • Read the bad smell documentation from docs/bad-smell.md
   • For testing-related changes, read testing spec from docs/testing.md
   • For React, ccstate, cache, Store, ref, or resource-lifecycle changes, read docs/cache.md
   • For each commit, analyze code changes against all code quality issues
   • Create individual review file: codereviews/YYYYMMDD/review-{short-hash}.md

5. Review Criteria (Bad Smell Analysis)

   Analyze each commit for these code quality issues:

   Testing Patterns (refer to docs/testing.md)

   • Check for AP-4 violations (mocking internal code with relative paths)
   • Verify MSW usage for HTTP mocking (not direct fetch mocking)
   • Verify real filesystem usage (not fs mocks)
   • Check test initialization follows production flow
   • Evaluate test quality and completeness
   • Check for fake timers, partial mocks, implementation detail testing
   • Verify mocks are reset through the package's standard centralized cleanup

   React, ccstate, Cache, and Resource Lifecycles (refer to docs/cache.md)

   • Keep React render pure and do not allocate signal identities during render
   • Reject unbounded lifetime caches and state whose owner outlives its domain
   • Avoid duplicate mutable sources of truth and parallel state machines
   • Verify callback-ref stability and preserve onRef cleanup returns
   • Require symmetric teardown for listeners, timers, observers, object URLs, editors, subscriptions, and async work
   • Inspect helper, chaining, and nested-callback shapes that can evade lint

   Error Handling (Bad Smell #3)

   • Identify unnecessary try/catch blocks
   • Flag defensive programming patterns:
     • Log + return generic error
     • Silent failure (return null/undefined)
     • Log and re-throw without recovery
   • Suggest fail-fast improvements

   Interface Changes (Bad Smell #4)

   • Document new/modified public interfaces
   • Highlight breaking changes
   • Review API design decisions

   Deployment Compatibility

   • Read docs/deployment-compatibility.md when changes touch frontend/backend, runner/backend, queue payloads, or persisted state
   • Verify old frontend requests still work with the new backend while open browser pages keep already-loaded code
   • Verify old runner requests still work with the new backend while old runners drain active runs
   • Verify new frontend or runner code can tolerate old backend responses during rollout when deployment order can overlap
   • Flag one-shot protocol flips that require all deployable surfaces to update at exactly the same time
   • Ensure temporary compatibility logic has an explicit cleanup condition or follow-up issue

6. Generate Review Files

   Create individual review file for each commit with this structure:

   # Code Review: {short-hash}
   
   ## Commit Information
   **Hash:** `{full-hash}`
   **Subject:** {commit-subject}
   **Author:** {author-name} <{author-email}>
   **Date:** {commit-date}
   
   ## Changes Summary
   ```diff
   {git show --stat output}
   

   Bad Smell Analysis

   1. Mock Analysis (Bad Smell #1, #16)

   • New mocks found: [list]
   • Direct fetch mocking: [yes/no + locations]
   • Internal code mocking: [yes/no + locations]
   • Assessment: [detailed analysis]

   2. Test Coverage (Bad Smell #2, #15)

   • Test files modified: [list]
   • Quality assessment: [analysis]
   • Bad test patterns: [list issues]
   • Missing scenarios: [list]

   3. Error Handling (Bad Smell #3, #13)

   • Try/catch blocks: [locations]
   • Defensive patterns: [list violations]
   • Fallback patterns: [list violations]
   • Recommendations: [improvements]

   4. Interface Changes (Bad Smell #4)

   • New/modified interfaces: [list]
   • Breaking changes: [list]
   • API design review: [assessment]

   5. Timer and Delay Analysis (Bad Smell #5, #10)

   • Timer usage: [locations]
   • Fake timer usage: [locations]
   • Artificial delays: [locations]
   • Recommendations: [alternatives]

   6. Code Quality Issues

   • Dynamic imports (Bad Smell #6): [locations]
   • TypeScript any (Bad Smell #9): [locations]
   • Hardcoded URLs (Bad Smell #11): [locations]
   • Lint suppressions (Bad Smell #14): [locations]

   7. Test Infrastructure Issues

   • Database mocking (Bad Smell #7): [locations]
   • Mock cleanup (Bad Smell #8): [assessment]
   • Direct DB ops (Bad Smell #12): [locations]
   • Filesystem mocking (Bad Smell #17): [locations]
   • Unit tests for internals (Bad Smell #18): [locations]
   • Test initialization bypass (Bad Smell #19): [locations]

   Files Changed

   {list of files}

7. Update Commit List with Links

   • Replace checkboxes with links to review files
   • Mark commits as reviewed with [x]

8. Generate Summary

   Add summary section to commit-list.md:

   ## Review Summary
   
   **Total Commits Reviewed:** {count}
   
   ### Key Findings by Category
   
   #### Critical Issues (Fix Required)
   - [List P0 issues found across commits]
   
   #### High Priority Issues
   - [List P1 issues found across commits]
   
   #### Medium Priority Issues
   - [List P2 issues found across commits]
   
   ### Bad Smell Statistics
   - Mock violations: {count}
   - Test coverage issues: {count}
   - Defensive programming: {count}
   - Dynamic imports: {count}
   - Type safety issues: {count}
   - [etc for all 19 categories]
   
   ### Mock Usage Summary
   - Total new mocks: {count}
   - Direct fetch mocking: {count} violations
   - Internal code mocking (AP-4): {count} violations
   - Third-party mocking: {count} (acceptable)
   
   ### Test Quality Summary
   - Test files modified: {count}
   - Bad test patterns: {count}
   - Missing coverage areas: [list]
   
   ### Architecture & Design
   - Adherence to YAGNI: [assessment]
   - Fail-fast violations: {count}
   - Over-engineering concerns: [list]
   - Good design decisions: [list]
   
   ### Action Items
   - [ ] Priority fixes (P0): [list with file:line references]
   - [ ] Suggested improvements (P1): [list]
   - [ ] Follow-up tasks (P2): [list]
   

9. Final Output

   • Display summary of review findings
   • Provide path to review directory
   • Highlight critical issues requiring immediate attention

Implementation Notes for Review Operation

• Use gh pr view {pr-id} --json commits --jq '.commits[].oid' to fetch PR commits
• Use git rev-list {range} --reverse for commit ranges
• Use git log --since="1 week ago" --pretty=format:"%H" for natural language
• Use git show --stat {commit} for change summary
• Use git show {commit} to analyze actual code changes
• Generate review files in date-based directory structure
• Cross-reference with docs/bad-smell.md for criteria

Operation 2: Defensive Code Cleanup

Automatically find and remove defensive try-catch blocks that violate the "Avoid Defensive Programming" principle.

Usage

cleanup

Workflow

1. Search for Removable Try-Catch Blocks

   Search in turbo/ directory for try-catch blocks matching these BAD patterns:

   Pattern A: Log + Return Generic Error

   try {
     // ... business logic
   } catch (error) {
     log.error("...", error);
     return { status: 500, body: { error: { message: "Internal server error" } } };
   }
   

   Pattern B: Silent Failure (return null/undefined)

   try {
     // ... logic
   } catch (error) {
     console.error("...", error);
     return null;
   }
   

   Pattern C: Log and Re-throw Without Recovery

   try {
     // ... logic
   } catch (error) {
     log.error("...", error);
     throw error;
   }
   

   DO NOT remove try-catch blocks that have:

   • Meaningful error recovery logic (rollback, cleanup, retry)
   • Error type categorization (converting domain errors to HTTP responses)
   • Fire-and-forget patterns for non-critical operations
   • Per-item error handling in loops (continue processing other items)
   • Security-critical code where defensive programming is justified

   Target: Find up to 10 removable try-catch blocks

2. Validate Safety

   For each identified try-catch block, verify:

   • No side effects in catch block (only logs and returns/throws)
   • Framework has global error handler

Implementation Notes for Cleanup Operation

• Use Grep to find try-catch patterns in turbo/ directory
• Validate each block manually before removal
• Test thoroughly after each removal
• Create atomic commits for easier review
• Reference CLAUDE.md principle: "Avoid Defensive Programming"

General Guidelines

Code Quality Principles from CLAUDE.md

1. YAGNI (You Aren't Gonna Need It)

   • Don't add functionality until needed
   • Start with simplest solution
   • Avoid premature abstractions

2. Avoid Defensive Programming

   • Only catch exceptions when you can meaningfully handle them
   • Let errors bubble up naturally
   • Trust runtime and framework error handling

3. Strict Type Checking

   • Never use any type
   • Provide explicit types where TypeScript can't infer
   • Use proper type narrowing

4. Zero Tolerance for Lint Violations

   • Never add eslint-disable comments
   • Never add @ts-ignore or @ts-nocheck
   • Fix underlying issues

Review Communication Style

• Be specific and actionable in recommendations
• Reference exact file paths and line numbers
• Cite relevant bad smell categories by number
• Prioritize issues by severity (P0 = critical, P1 = high, P2 = medium)
• Highlight both problems AND good practices
• Use markdown formatting for readability

Error Handling in Reviews

When encountering errors:

• If GitHub CLI fails, fall back to git commands
• If commit doesn't exist, report and continue with others
• If file is too large, summarize key points
• Always complete the review even if some steps fail

Example Usage

# Review a pull request
args: "review 123"

# Review commit range
args: "review abc123..def456"

# Clean up defensive code
args: "cleanup"

Output Structure

For Review Operation

codereviews/
└── YYYYMMDD/
    ├── commit-list.md      # Master checklist with summary
    ├── review-abc123.md    # Individual commit review
    ├── review-def456.md    # Individual commit review
    └── ...

For Cleanup Operation

• Branch: refactor/defensive-code-cleanup-YYYYMMDD
• PR with detailed summary table
• Individual commits for each file modified

References

• Bad smell documentation: docs/bad-smell.md (non-testing patterns)
• React and ccstate cache practices: docs/cache.md (state ownership, retention, refs, and resource lifecycles)
• Testing spec: docs/testing.md (comprehensive testing patterns and anti-patterns)
• Project principles: CLAUDE.md
• Conventional commits: https://www.conventionalcommits.org/