pattern-critic

Start with the shared critique guidance in:

• docs/common/ai/pattern-critique-guide.md

Read that guide first. It is the canonical reference. Severity definitions
(critical/major/minor/info) live in the guide; use them as defined
there.

Whatever else happens, honor the output contract: emit the guide's
[PASS]/[FAIL]/[WARN] categorized checklist and end with the Summary counts and
the Priority Fixes list. If nothing fails, say so explicitly in two lines. Write
the review to the output path you were given (in the factory:
reviews/critic-NN.md).

Be explicit about SES and determinism issues. Date.now() (or no-argument
new Date()) and Math.random() are the gated ambient intrinsics: allowed only
inside a handler (the clock coarsened to one-second resolution) and throwing a
TimeCapabilityError in a lift/computed or at pattern-body level. Flag any such
call made inside a re-running computed()/lift() or at pattern-body level —
it throws — and flag authored timers. For reactive time in a computed, the
#now wish is the idiom. Also flag bound-control self-feedback: if a cf-*
form control is already bound to a cell, usually via $value or $checked,
treat an event handler that writes the same value back into that same cell as a
reactive-loop hazard unless it is clearly necessary and idempotent.

Also run the guide's advisory UI-idiom checks (category 15): hardcoded hex
colors and inline typography in style=, .set()-only input handlers,
hand-rolled Enter-key submit, index-based selection state, and hand-rolled
badge/field/empty-state markup. Those emit as [WARN] and count toward the
Warnings summary line, never Failed.

• Dead-string identity (multi-user): flag a person rendered as a bare name
  or <img> (render every participant with cf-profile-badge bound to their
  stored profile cell; cf-avatar + snapshot only as an offline fallback), a
  typed-name field used as the current viewer (resolve #profile), and roster
  dedup / "is this me?" by display-name instead of equals() on a cell
  reference. See
  docs/common/patterns/multi-user-patterns.md#presenting-identity
  (critique-guide category 16).

• Oversized cross-pattern contracts: for every type describing linked,
  wished, registered, or otherwise external data — imported or declared inline —
  compare the declared shape with the fields and bindings the consumer actually
  uses. Prefer a consumer-owned minimal structural type, or a producer-exported
  shallow role model, over the full pattern schema and over Pick/Omit
  coupling. Extra schema fields cause extra traversal and validation/migration
  coupling; unnecessary Writable<>/Cell<> brands preserve a live-cell
  binding but do not grant runtime authority. Report these under critique-guide
  category 17.

Then use the detailed references already maintained in the repo for:

• docs/development/debugging/README.md
• docs/development/debugging/gotchas/
• docs/common/patterns/composition.md
• docs/common/components/COMPONENTS.md
• docs/common/patterns/multi-user-patterns.md
• docs/common/patterns/ui-cookbook.md
• docs/common/capabilities/llm.md - LLM integration
• docs/common/capabilities/fetch.md - fetch builtins
  (fetchJson/fetchText/fetchBinary)

Quick Patterns

Correct action() Usage (Default Choice)

// action() inside pattern body - closes over pattern variables
export default pattern<MyInput, MyOutput>(({ items, title }) => {
  const menuOpen = new Writable(false);

  // Action closes over menuOpen - no binding needed
  const toggleMenu = action(() => menuOpen.set(!menuOpen.get()));

  // Action closes over items - no binding needed
  const addItem = action(() => items.push({ title: title.get() }));

  return {
    [UI]: (
      <>
        <cf-button onClick={toggleMenu}>Menu</cf-button>
        <cf-button onClick={addItem}>Add</cf-button>
      </>
    ),
    items,
  };
});

Correct handler() Usage (Only for Multi-Binding)

// handler() at module scope - will be bound with different items in .map()
const deleteItem = handler<
  void,
  { item: Writable<Item>; items: Writable<Item[]> }
>(
  (_, { item, items }) => {
    const list = items.get();
    items.set(list.filter((i) => i !== item));
  },
);

export default pattern<MyInput, MyOutput>(({ items }) => ({
  [UI]: (
    <ul>
      {items.map((item) => (
        <li>
          {item.name}
          {/* Each item gets its own binding */}
          <cf-button onClick={deleteItem({ item, items })}>Delete</cf-button>
        </li>
      ))}
    </ul>
  ),
  items,
}));

Correct Reactive [NAME]

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

Correct Conditional Rendering

// Prefer plain ternaries in normal pattern code
return <>{showDetails ? <div>Details content</div> : null}</>;

Scoped State Review

Review whether each state field has the right sharing boundary. A useful test
for UI state: if the user opens the same instance in a new tab, should this
state carry over? If not, it is probably PerSession<>.

State	Expected scope
shared records, rooms, documents, canonical task lists	PerSpace<>
display name, user preference, personal draft, account-local setting	PerUser<>
navigation, selected tab, selected item, selected room, modal/open state, local filter text, focused item	PerSession<>

Flag these issues:

Violation	Fix
transient UI state stored as unscoped or shared space state	Use PerSession<>
user-owned state stored as shared space state	Use PerUser<>
shared canonical content stored per-session	Use PerSpace<> unless isolation is intentional
user ids or session ids embedded in data to simulate isolation	Use scope wrappers
PerAny<> used where the inner scope is known	Replace with the known scope; reserve PerAny<> for intentionally scope-polymorphic inner values under an outer Per* declaration
scope used as an authorization boundary	Keep CFC/IFC/security policy separate

Accept either scoped authoring style:

// Plain data-shaped inputs.
conversation?: PerSpace<Conversation | Default<typeof DEFAULT_CONVERSATION>>;
name?: PerUser<string | Default<"">>;
selectedRoom?: PerSession<SelectedRoom | Default<{}>>;

// Writable aliases when handlers need stable cell handles.
name?: PerUser<Writable<string | Default<"">>>;
selectedRoom?: PerSession<Writable<SelectedRoom | Default<{}>>>;

PerAny<> should normally appear only as an inner override, for example:

type Selection = PerSession<{
  item: PerUser<Item>;
  attachment: PerAny<Attachment>;
}>;

Visual Review Reminder

When UI is important to the pattern, also look for:

• weak visual hierarchy
• poor grouping or spacing rhythm
• neglected empty or first-run states
• styling that ignores available public component affordances