Code Review Workflow

Overview

This skill defines the canonical code review standards for Clypra. In a desktop video editor, subtle concurrency glitches, unreleased resources, and violated timeline invariants can lead to permanent render freezes, corrupted projects, or AV desynchronization. Reviews must be technical, evidence-based, and prioritized by risk.

When to Use This Skill

• Reviewing git diffs or pull requests before merging.
• Evaluating changes generated by another AI agent.
• Pre-commit verification of local working-tree modifications.

When NOT to Use This Skill

• Authoring new features or bug fixes directly (use feature-implementation or bugfix-regression).
• Measuring performance benchmarks (use benchmark tools).

Review Priorities (Strict Hierarchy)

Reviews must assess changes in this exact order:

1. Functional Defects & Invariants Violation

   • Does the change break the single time authority (PlaybackClock)?
   • Does any render path bypass evaluateTimelineSceneCached() or EvaluatedScene?
   • Are timeline mutations bypassing historyStore.dispatch(command)?
   • Is asset.path accessed before hydration/probe completes?
   • In NativeProgramPreview.tsx, is renderInFlight guaranteed to reset across all exit paths?

2. Asynchronous Races & Lifecycle Errors

   • Can out-of-order IPC responses or promise resolutions overwrite fresher state?
   • Are Web Workers or native resources leaking on unmount/dispose?
   • Is cancellation handled when user scrubs or switches projects rapidly?

3. Error Handling & Resource Leakage

   • Are errors silently caught without fallback or logging?
   • Are GPU textures, buffers, or audio nodes disposed cleanly?

4. Security & Data Integrity

   • Does Tauri IPC path handling validate incoming file paths against traversal attacks?
   • Is project JSON deserialization robust against corrupted or legacy schemas?

5. Test Coverage & Verification

   • Does a bug fix include a corresponding automated regression test?
   • Are assertions meaningful rather than trivial existence checks?
   • Do all modified files compile clean under npx tsc --noEmit?

6. Performance & Compute Boundaries

   • Are heavy computations (>2ms) kept off the main UI thread?
   • Are IPC calls batched or debounced rather than called per animation frame?
   • Is frame batching used for export rendering?

7. Maintainability & Documentation

   • Were existing comments and docstrings preserved?
   • Are public types well-documented and backward-compatible?

Review Procedure

1. Diff Inspection

• Run git diff against base branch (master or target commit).
• Check modified files, deleted lines, and new dependencies.
• Ensure no accidental edits, leftover debug statements, or secrets were committed.

2. Static Analysis & Compilation Check

Verify compiler diagnostics:

npx tsc --noEmit
npm run docs:check

3. Verification of Tests

Verify that test assertions directly exercise the changed behavior and that existing suites still pass:

npx vitest run src/components/editor/preview/__tests__/ProgramPreview.renderLoop.test.ts

4. Structure the Review Report

Format findings clearly:

• Summary: Concise overview of the change and overall recommendation (Approve, Request Changes, Discuss).
• Critical Issues (Blockers): Confirmed bugs, invariant violations, data-loss risks, or broken tests. Include file and line numbers.
• Risks & Edge Cases: Potential race conditions or lifecycle concerns requiring verification.
• Improvements (Non-blocking): Suggestions for clarity, maintainability, or performance.

Important Rule: The default review mode must never modify code or push commits unless explicitly authorized by the user.