# Code Review Checklist — Property Match

> Use this checklist before every PR review and before considering any implementation "done".
> For architecture context see [ARCHITECTURE.md](./ARCHITECTURE.md).
> For coding rules see [CLAUDE.md](./CLAUDE.md).

---

## A. Component Size & Structure

- [ ] **Is the component too large?**
  - Page component: hard limit 300 lines, target 150–200
  - Feature component: hard limit 250 lines, target 100–150
  - Atom/shared component: hard limit 150 lines
  - If over limit: extract sub-components, move constants to `*Constants.ts`, move helpers to `*Utils.ts` (`.tsx` if JSX)

- [ ] **Is business logic inside JSX?**
  - `.filter()`, `.sort()`, `.reduce()`, conditional rendering based on computed values — these belong in `useMemo`, not inline in the return
  - The render body should read computed values, not compute them

- [ ] **Are hardcoded strings present?**
  - Status labels (`'ACTIVE'`, `'Aktiv'`, `'PROPERTY_MANAGER'`) must come from `enums.ts` or `constants.ts`
  - Route paths must come from `ROUTES` constants, not string literals
  - German UI labels for enums live in `constants.ts` (e.g., `ASSET_TYPE_LABELS`)

- [ ] **Are hardcoded colors or thresholds present?**
  - Colors like `'#1a7a4a'`, `'#dc2626'` directly in `sx` props → use `DS_COLORS` from `lib/ds.ts`
  - Score thresholds like `score >= 80` → use `SCORE_STRONG` from `constants.ts`
  - Confidence thresholds like `conf >= 0.75` → use `CONF_HIGH` from `constants.ts`

- [ ] **Are duplicate result type labels/colors defined locally?**
  - `RESULT_TYPE_META` in `lib/ds.ts` is the single source of truth
  - `RESULT_TYPE_LABEL` / `RESULT_TYPE_COLOR` inline objects anywhere else → delete them

---

## B. State & Data

- [ ] **Is server data stored in Zustand?**
  - Any `useEffect(() => setFoo(data), [data])` pattern that mirrors React Query data into local/Zustand state → remove it, read directly from the query
  - Server data lives exclusively in the React Query cache

- [ ] **Is derived state stored in state?**
  - `const [count, setCount] = useState(0)` + `useEffect` to keep it in sync → compute inline or use `useMemo`
  - Exception: expensive computations may use `useMemo`

- [ ] **Is the full Zustand store subscribed to without a selector?**
  - `const { a, b, c } = useMyStore()` — re-renders on every store mutation
  - Use `useMyStore(s => s.a)` for single fields, `useShallow` for multiple fields

- [ ] **Is there a side effect in the render body?**
  - `queryClient.invalidateQueries(...)` called during render → must be in `useEffect`
  - Any `console.log`, analytics call, or mutation triggered directly in render body → `useEffect`

- [ ] **Are all hooks called before conditional early returns?**
  - React rules of hooks: all `useState`, `useMemo`, `useCallback`, `useQuery` calls must appear before any `if (isLoading) return ...`

---

## C. Type Safety

- [ ] **Are there `any` casts?**
  - `as any`, `: any`, `// @ts-ignore` — fix the type or use `unknown` with a type guard
  - One exception: bridging untyped third-party code — must be isolated in a named adapter

- [ ] **Are domain types re-defined locally?**
  - Any interface or type that duplicates something in `src/domain/` → import from domain instead

- [ ] **Are response types used?**
  - Service functions should return `ListResponse<T>` or `ItemResponse<T>` from `services/types.ts`

- [ ] **Do `.tsx` files contain JSX and `.ts` files not?**
  - Any file with JSX (`<Component />`, `<Box>`, `<Typography>`) must have `.tsx` extension
  - Plain TypeScript utilities use `.ts`

---

## D. Layering & Architecture

- [ ] **Is business logic in a component?**
  - Scoring calculations, ranking, filtering rules, AI calls, data transformation → move to service or `features/` layer
  - Components only render and dispatch

- [ ] **Is a provider called directly from a component or hook?**
  - `MockupPropertyProvider.getAll()` called in a hook → must go through `propertyService`
  - All provider access is mediated by a service

- [ ] **Is an LLM API called directly?**
  - Any `fetch` to an AI API, or direct `OpenAI` / `openrouter` SDK calls outside `src/services/ai/` → move behind `IAIService`

- [ ] **Are there cross-workspace component imports?**
  - A supply component importing from `components/demand/` or vice versa → use `components/shared/` or `components/ui/` instead

- [ ] **Are the import direction rules respected?**
  - `services/` must not import React hooks
  - `domain/` must not import anything from `src/`
  - `lib/constants.ts` must not import from components or services
  - No circular imports

---

## E. Loading, Error & Empty States

- [ ] **Does every data-dependent view handle loading?**
  - Every `useQuery` that drives a list or table must show a skeleton or spinner while `isLoading`
  - Use the appropriate skeleton component (`FeedSkeleton`, `ReminderSkeleton`, etc.)

- [ ] **Does every data-dependent view handle error?**
  - `isError` from `useQuery` must show `<ErrorState message={error.message} />`
  - Never leave an error state blank or silently swallowed

- [ ] **Does every list handle the empty state?**
  - Empty results after filtering → `<FeedEmptyState filtered={true} />` with a reset action
  - Empty because no data exists → `<FeedEmptyState filtered={false} />`

---

## F. Performance

- [ ] **Are list items wrapped in `React.memo`?**
  - Components rendered inside `.map()` in feeds, tables, or grids → should be `memo()`
  - Without memo, every parent state change (e.g., filter toggle, `selectedId`) re-renders the entire list

- [ ] **Are callbacks to memoized children wrapped in `useCallback`?**
  - Passing `() => setSelectedId(id)` inline as a prop to a `memo()`-wrapped child → the memo is broken
  - Wrap with `useCallback` and explicit dependency array

- [ ] **Are multiple aggregations over the same list collapsed into one pass?**
  - Three separate `useMemo` calls that each `.filter()` the same array → collapse into one `useMemo` with a `for` loop

- [ ] **Is `useMemo` used for expensive filters and sorts?**
  - `.filter()` + `.sort()` over 25+ items in the render body → `useMemo`
  - Decision aggregates (matchReady count, criticalGaps count, etc.) in `Properties.tsx` → already in `useMemo`

- [ ] **Are routes lazy-loaded?**
  - All page imports in `App.tsx` must use `React.lazy()` — never static imports for route components

---

## G. Explainability & Trust

- [ ] **Does every score shown have a breakdown path?**
  - A match score badge with no way to see *why* it's that score → add `ScoreBreakdownPanel` or link to MatchDetail

- [ ] **Are confidence levels shown proactively?**
  - Data with `confidenceScore < CONF_MEDIUM` or `freshness === 'STALE'` → must be visually flagged
  - `ConfidenceBadge`, `FreshnessIndicator`, `DataQualityBar` exist for this purpose

- [ ] **Are Future Availability signals clearly labeled as probabilistic?**
  - `FUTURE_AVAILABILITY` results must always show `probabilityScore` and `signalBasis`
  - They must never look identical to confirmed availability listings

- [ ] **Does AI output show its confidence?**
  - AI-extracted need criteria → shown with confidence indicator
  - AI match suggestions → shown with `MAISON_WORK` badge and probability

---

## H. Security

- [ ] **Is the React Query cache cleared on logout?**
  - Logout flow must call `queryClient.clear()` — otherwise stale data from previous user persists

- [ ] **Are role checks using `permissions.ts` helpers?**
  - `user.role === 'PROPERTY_MANAGER'` inline → use `canViewMatchCenter(user)` from `lib/permissions.ts`

- [ ] **Are workspace route guards in place?**
  - Every supply/demand/ops route wrapped in `<ProtectedRoute workspace={...} />`
  - Never guard with `if (user.workspace !== ...)` inside a component body

- [ ] **Is sensitive data stored in Zustand?**
  - PII, tokens, and sensitive fields beyond current-user identity should not persist in client state

---

## I. Backend-Readiness

- [ ] **Is the provider interface complete?**
  - Any new entity that creates a mock shortcut (array mutation inline, no interface) → add `I[Entity]Provider` and `Mockup[Entity]Provider`

- [ ] **Are async patterns consistent?**
  - All provider methods are `async` — if a new provider method is synchronous, fix it before it gets called from a service

- [ ] **Are query keys stable and specific?**
  - Query key `['properties']` is fine for list; `['properties', id]` for single item
  - Keys that include derived or computed objects (not primitives) will break cache lookup

- [ ] **Are stale times centralized?**
  - No `staleTime: 60_000` or `staleTime: 5 * 60 * 1000` inline in hook files
  - Use `STALE_PROPERTIES`, `STALE_MATCHES`, etc. from `src/lib/constants.ts`

---

## J. AI & Model Governance

- [ ] **Is the AI call going through `IAIService`?**
  - Direct fetch to AI API anywhere outside `src/services/ai/` → move it

- [ ] **Is the AI response validated with Zod before use?**
  - Raw LLM string parsed inline with `JSON.parse()` without a schema → add Zod validation

- [ ] **Is the AI output logged via `aiMonitoringService`?**
  - Any AI call that influences a business decision (need parsing, match recommendation, market signal) → log it

- [ ] **Does the AI flow have a fallback?**
  - If `aiService` throws or returns `confidence < threshold`, is there a manual entry path?
  - AI failures must never block the user workflow

---

## Quick Checklist — Pre-Commit

Before every commit, verify:

```
[ ] npx tsc --noEmit  →  zero errors
[ ] No new hardcoded colors, routes, or role strings
[ ] No business logic added to a JSX return block
[ ] No React Query data mirrored into Zustand or useState
[ ] All hooks called before early returns
[ ] New list-item components wrapped in React.memo
[ ] New provider methods are async and match the interface
[ ] New feature goes through Provider → Service → Hook → Component
[ ] Loading, error, and empty states handled for any new data-fetching view
[ ] File with JSX has .tsx extension
```