--- name: review-patterns description: Review code changes for Effect-TS pattern compliance in the Reactive Agents framework. Use when reviewing PRs, completed packages, or any code changes to enforce project conventions. argument-hint: --- # Review Code: $ARGUMENTS ## Review Checklist Ultrathink through each of these categories systematically. ### Category 1: Type Definitions Search for type definitions in the target files: **PASS if:** - All data shapes use `Schema.Struct` from Effect - IDs use `Schema.brand()` - Enums/unions use `Schema.Literal()` - Optional fields use `Schema.optional()` - Types are derived with `typeof XxxSchema.Type` **FAIL if:** - Uses `interface` for data shapes (should be `Schema.Struct`) - Uses `type X = { ... }` for complex objects (should be `Schema.Struct`) - Uses TypeScript `enum` (should be `Schema.Literal`) - Uses `?:` for optional fields (should be `Schema.optional()`) ### Category 2: Error Handling **PASS if:** - All error classes extend `Data.TaggedError("TagName")<{ ... }>` - Tag string matches class name - All properties are `readonly` - Package defines a union type for all errors - No `throw new Error()` or `throw new XxxError()` anywhere **FAIL if:** - Uses `throw` anywhere - Uses `new Error()` instead of `Data.TaggedError` - Error properties are not `readonly` - Tag string doesn't match class name ### Category 3: Service Definitions **PASS if:** - Services extend `Context.Tag("Name")()` - Tag string matches the class name - All methods are `readonly` - All methods return `Effect.Effect` - Live implementation uses `Layer.effect(Tag, Effect.gen(...))` - Dependencies resolved via `yield* OtherService` - Scoped resources use `Layer.scoped` + `Effect.acquireRelease` **FAIL if:** - Uses OOP class with constructor for service - Uses dependency injection via constructor params - Methods return `Promise` instead of `Effect.Effect` - Uses `new Service()` anywhere ### Category 4: State Management **PASS if:** - All mutable state uses `Ref` from Effect - State initialization with `Ref.make()` - State reads with `Ref.get()` - State updates with `Ref.update()` or `Ref.modify()` - Atomic operations use `Ref.modify()` **FAIL if:** - Uses `let` for mutable state - Uses class properties for state - Uses global mutable variables - State mutations without Ref ### Category 5: Async Operations **PASS if:** - Sync operations (bun:sqlite) wrapped in `Effect.sync()` - Async operations (fetch, file I/O) use `Effect.tryPromise()` - `Effect.tryPromise` has a `catch` that returns `Data.TaggedError` - LLMService calls are NOT wrapped in `Effect.tryPromise` (they already return Effect) **FAIL if:** - Uses raw `await` - Uses `Promise.then()` / `Promise.catch()` - Wraps Effect-returning functions in `Effect.tryPromise` - Missing error handler in `Effect.tryPromise` ### Category 6: Layer Composition **PASS if:** - Package exports `createXxxLayer()` factory function - Uses `Layer.mergeAll()` for independent services - Uses `Layer.provide()` for dependent services - Layer factory takes configuration arguments when needed **FAIL if:** - No layer factory exported - Dependencies not wired with `Layer.provide()` - Missing services in the merged layer ### Category 7: Imports and Exports **PASS if:** - Relative imports use `.js` extension (Bun ESM requirement) - `index.ts` exports all public types, errors, services, and layer factory - Internal imports use correct package names (`@reactive-agents/xxx`) - No circular imports between packages **FAIL if:** - Missing `.js` extension on relative imports - `index.ts` missing public exports - Internal implementation details exported ### Category 8: LLMService Usage (if applicable) **PASS if:** - Calls `llm.complete({ messages: [...], ... })` with `messages` array - Reads `response.content` for text output - Reads `response.usage.estimatedCost` for cost - Reads `response.usage.totalTokens` for token count - Does NOT wrap in `Effect.tryPromise` **FAIL if:** - Uses `prompt:` field (doesn't exist) - Reads `.text` instead of `.content` - Reads `.usage.cost` instead of `.usage.estimatedCost` - Reads `.usage.confidence` (doesn't exist) - Wraps in `Effect.tryPromise` ### Category 9: Kernel Extension Compliance (if applicable) Only applies if changes touch `packages/reasoning/src/strategies/kernel/`. **PASS if:** - New phases have the exact signature: `(state: KernelState, context: KernelContext) => Effect.Effect` - New guards return `GuardOutcome`: either `{ allow: true }` or `{ block: true; reason: string }` — nothing else - New MetaTools are registered in `metaToolRegistry` in `act.ts` — not handled inline in kernel-runner - `kernel-runner.ts` main loop was not modified to add per-turn logic - Dead code areas (`buildDynamicContext`, `buildStaticContext`, dead context-engine sections) were not touched **FAIL if:** - Phase function has extra parameters beyond `(state, context)` - Guard returns a boolean, void, or throws instead of `GuardOutcome` - Phase or guard logic was added directly inside `kernel-runner.ts` (bypassing the phase pipeline) - `buildDynamicContext` or `buildStaticContext` were re-enabled or modified **Check commands:** ```bash # Phase signatures grep -n "^export const.*= (" packages/reasoning/src/strategies/kernel/phases/ -r # Guard return types — must see GuardOutcome grep -n "GuardOutcome" packages/reasoning/src/strategies/kernel/phases/guard.ts # Ensure kernel-runner was not touched for per-turn logic git diff HEAD packages/reasoning/src/strategies/kernel/kernel-runner.ts | head -50 ``` ## Output Format For each category, report: - **PASS** / **FAIL** / **N/A** - If FAIL: list the specific files and lines with the violation - Suggested fix for each violation Summary: X/9 categories passed. [List critical issues if any.]