property-match/CODE_REVIEW_CHECKLIST.md

Code Review Checklist — Property Match

Use this checklist before every PR review and before considering any implementation "done". For architecture context see ARCHITECTURE.md. For coding rules see 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