Back to skills
extension
Category: Development & EngineeringNo API key required

pattern-critic

Critic agent that reviews pattern code for violations of documented rules, gotchas, and anti-patterns. Produces categorized checklist output with [PASS]/[FAIL] for each rule.

personAuthor: jakexiaohubgithub

Pattern Critic

Systematically review pattern code for violations of Common Tools documentation rules and gotchas.

Workflow

  1. Read the pattern file to review
  2. Check each category below against the code
  3. Output results in the checklist format (see Output Format)
  4. For any [FAIL], include the line number and fix

Violation Categories

1. Module Scope Violations

Check that these are NOT inside the pattern body:

| Violation | Fix | |-----------|-----| | handler() defined inside pattern | Move to module scope, or use action() instead | | lift() immediately invoked (lift(...)(args)) | Use computed() or define lift at module scope | | Helper functions defined inside pattern | Move to module scope |

Allowed inside patterns: computed(), action(), .map() callbacks, JSX event handlers.

2. Reactivity Violations

| Violation | Fix | |-----------|-----| | [NAME]: someProp (reactive value) | [NAME]: computed(() => someProp) | | [NAME]: \text ${someProp}`|[NAME]: computed(() => `text ${someProp}`)| |Writable.of(reactiveValue)| Initialize empty, set in handler/action | |.get()on computed/lift result | Access directly (only Writable has .get()) | |items.filter(...)inline in JSX | Wrap incomputed()outside JSX | |items.sort(...)inline in JSX | Wrap incomputed()outside JSX | | Nested computed with outer scope vars | Pre-compute with lift or outer computed | | lift() closing over reactive deps | Pass deps as explicit params | | Cells from composed patterns in ifElse | Wrap in localcomputed()` |

3. Conditional Rendering

| Violation | Fix | |-----------|-----| | {show ? <Element /> : null} | {ifElse(show, <Element />, null)} | | onClick inside computed() | Move button outside, use disabled attr |

Note: Ternaries ARE fine for attributes: style={done ? {...} : {}}

4. Type System

| Violation | Fix | |-----------|-----| | Array without Default<T[], []> | Add default to prevent undefined | | Missing Writable<> for .set()/.push() | Add Writable<T> to input type | | Map or Set in cell data | Use plain objects/arrays (serialization) | | Custom id property for identity | Use equals() function instead |

5. Binding

| Violation | Fix | |-----------|-----| | checked={item.done} | $checked={item.done} (add $ prefix) | | value={title} | $value={title} (add $ prefix) | | $checked={item} (whole item) | $checked={item.done} (bind property) | | Wrong event name | Use onct-send, onct-input, onct-change |

6. Style Syntax

| Element Type | Required Syntax | |--------------|-----------------| | HTML (div, span) | Object: style={{ backgroundColor: "#fff" }} | | Custom (ct-*) | String: style="background-color: #fff;" |

| Violation | Fix | |-----------|-----| | String style on HTML | Convert to object syntax | | Object style on ct-* | Convert to string syntax | | kebab-case props on ct-* | Use camelCase: allowCustom not allow-custom |

7. Handler Binding

| Violation | Fix | |-----------|-----| | onClick={addItem({ title: "x", items })} | Event data comes at runtime, bind state only | | Creating handlers inside .map() | Create handler once at module/pattern scope |

8. Stream/Async

| Violation | Fix | |-----------|-----| | Stream.of() | Doesn't exist. Bound handler IS the stream | | .subscribe() on stream | Doesn't exist. Return stream from pattern | | async/await in handlers | Use fetchData() (blocks UI otherwise) | | await generateText(...) | Reactive, not a promise. Use .result | | await generateObject(...) | Reactive, not a promise. Use .result |

9. LLM Integration

| Violation | Fix | |-----------|-----| | Array as root schema for generateObject | Wrap in object: { items: T[] } | | Missing /// <cts-enable /> directive | Add at top of file | | Prompt derived from agent-written cells | Causes infinite loop. Use separate cells | | Invalid model name format | Use vendor:model (e.g., anthropic:claude-sonnet-4-5) |

10. Performance

| Violation | Fix | |-----------|-----| | Handler created per-item in .map() | Create handler once, bind with item | | Expensive computation inside loop | Pre-compute outside, reference result |

11. Design Review

Check the domain model quality:

| Check | What to look for | |-------|------------------| | Clear entity boundaries | Each pattern represents one concept (Card, Column, Board) | | Actions match user intent | Handler names reflect what user wants (addCard, moveCard, removeCard) | | Unidirectional data flow | Parent owns state, children receive props | | Normalized state | No duplicate data, single source of truth | | Self-documenting types | Type names and fields are clear without comments | | Appropriate granularity | Not too fine (trivial patterns) or too coarse (god patterns) |

12. Regression Check (for updates only)

When reviewing changes to existing code:

| Check | What to verify | |-------|----------------| | Tests still pass | Run existing tests after changes | | Type signatures preserved | Or intentionally changed with migration path | | Handlers still work | Existing functionality not broken | | No unintended side effects | Changes scoped to intended area |

Output Format

## Pattern Review: [filename]

### 1. Module Scope
- [PASS] No handler() inside pattern
- [FAIL] lift() immediately invoked (line 23)
  Fix: Use computed() or move lift to module scope

### 2. Reactivity
- [PASS] [NAME] properly wrapped
- [FAIL] Writable.of(deck.name) uses reactive value (line 15)
  Fix: Initialize empty, set in action()

### 3. Conditional Rendering
- [PASS] Using ifElse() correctly
- [N/A] No conditional rendering

[...continue for all categories...]

### 11. Design Review
- [PASS] Clear entity boundaries
- [WARN] Handler names could be clearer (moveCard vs reorderCard)
- [PASS] Unidirectional data flow

### 12. Regression Check (if updating)
- [PASS] Existing tests pass
- [N/A] No type signature changes

## Summary
- Passed: 22
- Failed: 3
- Warnings: 1
- N/A: 2

## Priority Fixes
1. [Line 15] Writable.of() with reactive value
2. [Line 23] lift() inside pattern
3. [Line 45] Missing $ prefix on binding

Documentation References

  • docs/development/debugging/README.md - Error reference table
  • docs/development/debugging/gotchas/ - Individual gotcha files
  • docs/common/components/COMPONENTS.md - UI components and binding
  • docs/common/capabilities/llm.md - LLM integration

Quick Patterns

Correct Module-Scope Handler

const addItem = handler<{ title: string }, { items: Writable<Item[]> }>(
  ({ title }, { items }) => items.push({ title })
);

export default pattern<Input, Input>(({ items }) => ({
  [UI]: <ct-button onClick={addItem({ items })}>Add</ct-button>,
  items,
}));

Correct Reactive [NAME]

export default pattern<Input>(({ deck }) => ({
  [NAME]: computed(() => `Study: ${deck.name}`),
  // ...
}));

Correct Conditional Rendering

{ifElse(showDetails,
  <div>Details content</div>,
  null
)}

Correct Style Syntax

<div style={{ display: "flex", gap: "1rem" }}>
  <ct-vstack style="flex: 1; padding: 1rem;">
    Content
  </ct-vstack>
</div>