OpenCut/.cursor/commands/review.md

Code Review Checklist

Review every point below carefully to ensure files follow consistent code style and best practices.

---

Function Signatures & Parameters

• Every function accepts a single object parameter with destructuring in the signature (for readability and future extensibility)

  • Exception: tiny one-liner callbacks (e.g. array.find(x => ...), map, filter, sort) do not need destructuring if it hurts readability

  // ❌ wrong
  function formatTime(seconds: number, fps: number) { ... }
  
  // ✅ correct
  function formatTime({ seconds, fps }: { seconds: number; fps: number }) { ... }
  

TypeScript & Type Safety

• No any references
• General interfaces are in the types folder, not scattered in components
  • Example: TimelineTrack interface belongs in src/types/timeline.ts, not src/components/timeline/index.tsx

JSX & Components

• JSX is clean — no comments explaining what each part does
• Complex/reusable JSX is extracted into sub-components (placed below the main component)
• Components shared across multiple files are in separate files
• File order: constants specific to file (top) -> utils specific to file -> main component → sub-components (bottom)
• Components render UI only — domain logic lives in hooks, utilities, or managers
  • Simple interaction logic (gestures, modifier keys) can stay if not complex

Code Organization & File Structure

• Each file has one single purpose/responsibility
  • Example: timeline/index.tsx should not define validateElementTrackCompatibility — that belongs in a lib file
  • Example: lib/timeline-utils.ts should not declare TRACK_COLORS — that belongs in constants/
• File name accurately reflects what the file contains — a misleading name is a bug waiting to happen
• Business logic lives in either src/lib, src/core or src/services folder

Comments

• No AI comments — only human comments that explain why, not what
  • Bad: changelog-style comments, explaining readable code, using more words than necessary
• All comments are lowercase

Naming Conventions

• Readability over brevity — use element not el, event not e
• Booleans are named isSomething, hasSomething, or shouldSomething — not something
• No title case for multi-word text/UI — use Hello world not Hello World

Tailwind & Styling

• Always use cn() for className — never string interpolation with ${} or ternaries inline

  // ❌ wrong
  className={`base-class ${isActive && "active"} ${someHelper()}`}
  
  // ✅ correct
  className={cn("base-class", isActive && "active", someHelper())}
  

• Use gap-* instead of mb-* or mt-* for consistent spacing
• Use size-* instead of h-* w-* when width and height are the same
• When using size-* on icons inside <Button>, use ! modifier to override default size-4

  <Button>
    <PlusIcon className="!size-6" /> {/* ✅ correct */}
    <PlusIcon className="size-6" /> {/* ❌ wrong */}
    <PlusIcon className="!size-4" /> {/* ❌ unnecessary, size-4 is default */}
    <PlusIcon className="size-4" />{" "}
    {/* ❌ completely wrong, 1) doesn't override and 2) size-4 is default */}
  </Button>
  

State Management (Zustand)

• React components never use someStore.getState() — use the useSomeStore hook instead

• High-frequency stores (timeline, playback, selections) use selectors — useStore((s) => s.value) not const { value } = useStore()

• Store/manager methods are not passed as props — sub-components access them directly

  // ❌ wrong
  function Parent() {
    const { selectedElements } = useTimelineStore();
    return <Child selectedElements={selectedElements} />;
  }
  
  // ✅ correct
  function Parent() {
    return <Child />;
  }
  function Child() {
    const { selectedElements } = useTimelineStore();
  }
  

• Components and hooks should use the useEditor hook. Only use EditorCore.getInstance() if you are outside of a react component/hook. Eg: in a utility function, event handler.

Code Quality

• Code is scannable — use variables and helper functions to make intent clear at a glance

• Complex logic is extracted into well-named variables or helpers

• No magic numbers or magic values — extract inline literals into named constants

  • Applies to colors, durations, thresholds, sizes, config values, etc.
  • If it's domain-specific to one file, a const at the top of that file is fine
  • If it's generic enough, it belongs in constants/

• No redundant single/plural function variants — if a function can operate on multiple items, it should accept an array and handle both cases. Don't create doThing() + doThings().

  // ❌ wrong — redundant variants
  function updateElement({ element }: { element: Element }) { ... }
  function updateElements({ elements }: { elements: Element[] }) { ... }
  
  // ✅ correct — one function, accepts array
  function updateElements({ elements }: { elements: Element[] }) { ... }
  

---

Function Keywords

Context	Keyword
Next.js page components	export default function
Main react component	export function
Sub-components	function
Utility functions	export function
Functions inside react components	const

---

Review Methodology

Do NOT review by reading the file top-to-bottom and noting what jumps out. Instead:

1. Go through each checklist section one at a time
2. For each section, scan the entire file for violations of that specific rule
3. Only move to the next section after you've exhausted the current one
4. After all sections are checked, do a final pass: re-read every checklist item and confirm you didn't skip it

Before outputting the review, list each checklist section and confirm you checked it: Signatures ✓ | TypeScript ✓ | JSX ✓ | Organization ✓ | File Names ✓ | Comments ✓ | Naming ✓ | Tailwind ✓ | State ✓ | Quality ✓ | Keywords ✓

---

IMPORTANT: Review Rules

• ONLY flag issues that are explicitly covered by a checklist item above.
• Do NOT invent your own rules, suggestions, or "nice to haves" as primary issues.
• The review output must be a list of issues, each one mapping to a specific checklist item.
• If something looks off but isn't covered by the checklist, you can mention it as a brief side note at the end — but keep it clearly separate from the actual review. Always default to fixing the issues covered by the checklist above, unless the user says otherwise.

You WILL miss things if you try to review the whole file in one pass. Iterate rule by rule.

---

Think Bigger

After the checklist review, step back and ask the hard questions. The biggest architectural problems get solved by the biggest questions.

• Does this abstraction actually need to exist? Could it be deleted entirely?
• Is this the right layer for this logic? (wrong layer = future pain)
• Is this solving a real problem, or a problem we invented?
• Would a simpler data model make this whole file unnecessary?
• Are we adding complexity to work around a bad decision made earlier?
• Could this field be derived from other existing fields? Redundant data in a model is a source of bugs.

Don't be shy about flagging these. A "why does this exist?" question is often worth more than 10 style fixes.