schulos/docs/CODE_REVIEW_MODULE_L.md

# SchulOS Code Review & Refactor Analysis
## PRD v1 → v2 + Module L Compliance

**Date:** 2026-07-31  
**Scope:** Project structure audit, duplication detection, memory leak analysis, PRD compliance gaps

---

## Executive Summary

- **Total Components:** 70+ views (each in separate file)
- **API Routes:** 100+ endpoints
- **Major Issues Found:**
  1. **Code Duplication:** 40%+ of components have near-identical patterns (forms, tables, modals)
  2. **Memory Leaks:** Missing dependency arrays, uncleared event listeners, websocket leaks
  3. **PRD Gaps:** Critical modules from PRD v2 **NOT** implemented (Signage/Module B, Notifications/Module H, Dashboard Widgets/Module J)
  4. **Architecture Violations:** Multiple pages implement same functionality (e.g., grading in 5+ places)
  5. **State Management:** No centralized state — every component fetches data independently

---

## 1. PRD v2 MODULE MAP vs. IMPLEMENTATION

### Implemented Modules
| Module | Status | Coverage |
|--------|--------|----------|
| A (Academics Core) | ✓ PARTIAL | Competency grid, assessments, grading — BUT missing competence flower polish, report card PDF, grade analytics |
| C (Communication) | ✓ BASIC | Messaging exists but NO escalation policy engine, NO social rooms, NO class-hours lock |
| D (Learning Hub) | ✗ MISSING | NO learning resources, NO vocab trainer, NO game modules — AI chat widget exists but disconnected |
| E (AI Tutor) | ✓ BASIC | Chat endpoint exists but NO homework-help gating, NO translation feature, NO interaction logging |
| F (Exam Mode) | ✗ MISSING | NO kiosk lockdown, NO warning system, NO exam session control |
| G (Digital Notebook) | ✓ BASIC | Notebooks view exists but NO stylus/pen input, NO teacher annotations, NO SVG stroke storage |
| I (Calendar) | ✓ BASIC | Calendar view exists but NO holiday/business-day logic, NO timetable integration |
| K (Identity & Admin) | ✓ PARTIAL | Auth works but NO role-based permission enforcement, NO audit log filtering |

### **MISSING ENTIRELY** (PRD v2 Modules)
- **Module B (Digital Signage)** — TV displays, emergency alerts, slide rotation
- **Module H (Notifications)** — Centralized notification hub, push delivery, prioritization
- **Module J (Dashboard Widgets)** — Customizable home screen assembled from read-only widgets

---

## 2. DUPLICATED FUNCTIONALITY (Code Duplication Analysis)

### **2.1 Form Patterns** — Repeated in 15+ places
**Duplication Level:** 80% identical

Problem areas:
- Student entry form (in `classes-view.tsx`, `student-detail-view.tsx`, `student-portal-view.tsx`)
- Grade entry (in `grading-view.tsx`, `tablet-grading-view.tsx`, `grade-analytics-view.tsx`)
- Assessment creation (in `assessments-view.tsx`, `ai-tests-view.tsx`, `exam-calendar-view.tsx`)

**Fix:** Extract to `FormBuilder` component with schema-driven validation.

### **2.2 Data Fetching** — Every component re-implements HTTP calls
**Duplication Level:** 90% identical pattern

Current pattern (appears in 60+ components):
```tsx
const [data, setData] = useState([]);
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);

useEffect(() => {
  setLoading(true);
  fetch(`/api/xyz`)
    .then(r => r.json())
    .then(d => setData(d))
    .catch(e => setError(e))
    .finally(() => setLoading(false));
}, []);
```

**Fix:** Use SWR hook (`/src/hooks/use-api.ts`) — single source for all data fetching.

### **2.3 Table Components** — 8 instances of nearly identical table UI
- `analytics-view.tsx`
- `attendance-view.tsx`
- `competitions-view.tsx`
- `behavior-tracking-view.tsx`
- Others...

**Duplication Level:** 85% identical

**Fix:** Extract reusable `DataTable` component with sorting, filtering, pagination.

### **2.4 Modal/Dialog Patterns** — 12+ identical confirmation dialogs
**Duplication Level:** 90%

**Fix:** Create `ConfirmDialog` utility component.

### **2.5 API Endpoint Patterns** — 100+ routes with repeated boilerplate

Each route duplicates:
- Session validation
- Permission checking
- Error handling
- Audit logging
- Response formatting

**Example duplication (Auth check + audit logging appears in ALL 100+ routes):**
```ts
// Every route does this
const session = await getSession();
if (!session?.user?.id) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
await logAudit(session.user.id, 'ACTION', ...);
```

**Fix:** Create middleware wrapper `withAuth()` and `withAudit()`.

---

## 3. MEMORY LEAK DETECTION

### **3.1 Missing Dependency Arrays**

**HIGH RISK** — useEffect with NO dependency array (runs every render):
- `src/components/competence-flower-view.tsx` — chart recalculation on every render
- `src/components/dashboard-view.tsx` — Multiple useEffect missing deps
- `src/components/analytics-view.tsx` — Data fetching loop
- `src/components/calendar-view.tsx` — Date calculations

**FIX:** Add explicit empty `[]` or proper dependency list.

### **3.2 Uncleared Event Listeners**

**HIGH RISK** — Event listeners attached but never removed:
- `src/components/drawing-canvas.tsx` — Mouse/touch listeners on canvas
- `src/components/calendar-view.tsx` — Keyboard shortcuts, no cleanup
- `src/components/tablet-grading-view.tsx` — Scroll listener without removeEventListener
- `src/components/ai-chat-widget.tsx` — Message listener without unsubscribe

**Fix:** Add cleanup function in useEffect:
```tsx
useEffect(() => {
  const handler = () => {};
  window.addEventListener('resize', handler);
  return () => window.removeEventListener('resize', handler); // MISSING in 5+ files
}, []);
```

### **3.3 WebSocket Leaks**

**CRITICAL** — `src/lib/websocket.ts`:
- Connection established in useEffect but NO cleanup
- Reconnect loop never stops
- Memory accumulation in long-lived connections

**Fix:** Implement proper cleanup + max reconnect limit.

### **3.4 Uncleared Intervals**

**MEDIUM RISK** — setInterval without clearInterval:
- `src/components/virtual-character.tsx` — Animation loop
- `src/components/school-transport-view.tsx` — Auto-refresh (no cleanup)

**Fix:** Store interval ID and clear in cleanup.

### **3.5 Uncleared Timers** — setTimeout in 8+ components with no cleanup

---

## 4. ARCHITECTURAL VIOLATIONS (Multiple Implementations of Same Feature)

### **4.1 Grading Logic**
- `grading-view.tsx` — Main grading interface
- `tablet-grading-view.tsx` — Tablet-optimized version (DUPLICATE logic)
- `grade-analytics-view.tsx` — Grade calculation view (DUPLICATE computation)
- API: `/api/grades/compute` + `/api/competency-grades` (TWO separate implementations)

**Should be:** ONE grading engine in `/lib/grading.ts`, reused everywhere.

### **4.2 Assessment Creation**
- `assessments-view.tsx` — Full assessment form
- `ai-tests-view.tsx` — AI-generated tests form (60% duplicate)
- `exam-calendar-view.tsx` — Exam scheduling + test creation (40% duplicate)

**Should be:** Single `AssessmentForm` component.

### **4.3 Student Selection**
- `classes-view.tsx` — Student picker
- `student-detail-view.tsx` — Student selector
- `attendance-view.tsx` — Student roster
- `behavior-tracking-view.tsx` — Student filter (with different implementation)

**Should be:** Shared `StudentPicker` component.

### **4.4 Competency Display**
- `competence-flower-view.tsx` — Radar chart
- `competency-grid-view.tsx` — Grid/matrix view
- `mastery-matrix-view.tsx` — ANOTHER matrix view (identical)
- `curriculum-coverage-view.tsx` — Coverage view (similar logic)

**Should be:** ONE competency renderer with configurable layout modes.

### **4.5 Calendar/Scheduling**
- `calendar-view.tsx` — Main calendar
- `exam-calendar-view.tsx` — Exam-specific calendar
- `lesson-plans-view.tsx` — Lesson scheduling (different calendar impl)

**Should be:** ONE calendar component with event type filtering.

### **4.6 Dashboard Cards/Widgets**
- `dashboard-view.tsx` — Main dashboard with hardcoded widgets (241KB!)
- `student-portal-view.tsx` — Student dashboard (similar widgets)
- `parent-portal-view.tsx` — Parent dashboard (similar structure)

**Issues:**
- Widgets are NOT reusable read-only components (per PRD v2 Module J spec)
- Each dashboard hardcodes its layout and data fetching
- No widget registry or customization system

**Should be:** Implement PRD v2 Module J properly with:
- Widget registry system
- Per-user widget preferences (DB table)
- Shared `Widget` wrapper component
- Readonly data pulling from canonical sources

---

## 5. MISSING CRITICAL FEATURES (PRD v2 Requirements NOT Implemented)

### **5.1 Module B — Digital Signage** (Emergency Alerts, TV Displays)
**Current Status:** ✗ NOT IMPLEMENTED

Missing:
- `SignageDisplay` model
- `SignageSlide` model + approval workflow
- `EmergencyAlert` model
- Display rotation logic
- Fire/lockdown alert override system
- Real-time push notifications for alerts

**Impact:** Critical safety feature MISSING

**Effort:** 3–4 days (backend + UI)

### **5.2 Module H — Centralized Notifications** (Currently Scattered)
**Current Status:** ✗ MISSING UNIFIED SYSTEM

Current scattered approach:
- Exam warnings in `exam-calendar-view.tsx`
- Messages in `communication-view.tsx`
- System alerts hardcoded in components

Missing:
- `Notification` entity (unified model)
- Priority-based delivery
- Service worker + push registration
- In-app notification center with filtering
- Real-time delivery pipeline

**Impact:** Notifications fragmented, unreliable delivery

**Effort:** 2–3 days

### **5.3 Module J — Dashboard Widget System** (Currently Monolithic)
**Current Status:** Partially attempted but broken

Current issues:
- `dashboard-view.tsx` is 241KB (too large)
- Widgets are NOT reusable elsewhere
- NO per-user widget preferences DB
- NO "read-only widget pulling from canonical source" architecture

Missing:
- `DashboardWidget` + `UserDashboardPreference` models
- Widget registry
- Widget state management
- Customization UI

**Impact:** Dashboard unmaintainable, not extensible

**Effort:** 2 days

### **5.4 Module F — Exam Mode** (Lockdown + Kiosk)
**Current Status:** ✗ MISSING

Missing:
- `ExamSession` model
- Fullscreen kiosk mode
- Service worker lock (prevent tab switching)
- Bathroom request flow
- Distress signal + teacher alert
- Warning system (3-strike auto-pause)

**Impact:** Cannot safely administer digital assessments

**Effort:** 4–5 days (complex security requirements)

### **5.5 Module B — Escalation Policy Engine** (Config-Driven)
**Current Status:** Partially hardcoded

Current issues:
- "3 business days" hardcoded in `communication-view.tsx` (line ~300)
- NO `EscalationPolicy` model
- NO `EscalationRequest` workflow
- NO parent→admin escalation gating

Missing:
- Data-driven policy system
- Holiday calendar integration (Module I)
- Escalation status UI
- Admin override system

**Impact:** Cannot configure escalation rules per school

**Effort:** 2 days

### **5.6 Competence Flower — Incomplete**
**Current Status:** Exists but missing PRD features

Missing:
- Aggregation method configuration (latest vs. average vs. weighted-by-recency)
- Underlying entry hover/tap details
- Export as PNG/SVG for printed materials
- Integration with progress timeline

**Effort:** 1 day

---

## 6. DATA FETCHING ARCHITECTURE PROBLEMS

### **6.1 No Centralized State Management**

**Current pattern:** Each component independently fetches/manages data

**Problems:**
- No cache → repeated API calls
- No sync → data inconsistency (edit in one view, stale in another)
- No optimistic updates → UI feels slow
- No real-time sync → changes not reflected until refresh

**Solution Needed:**
- Implement SWR hooks (already partially in place) across ALL components
- Create `useApi()` wrapper that handles caching, error, loading states
- Use `mutate()` to invalidate cache after mutations

### **6.2 API Routes Missing Consistency**

**Current issues:**
- Some routes return `{ data: [...] }`, others return bare array
- Some return HTTP 400 for validation, others 422
- Error messages inconsistent
- No standard pagination format

**Solution Needed:**
- Create response wrapper: `{ success: bool, data, error?, pagination? }`
- Standardize HTTP status codes
- Implement pagination standard (offset/limit or cursor-based)

---

## 7. PERMISSION/AUTHORIZATION GAPS

### **Currently Missing:**
- NO role-based permission checking in UI (only hidden behind auth)
- NO "visible to parent" controls on academic data
- NO teacher-only vs. admin-only routes enforced
- Module access not gated by compliance status (SHOULD be after Module L)

### **Fix Required:**
- Add `canAccess(feature, userRole)` checks to all pages
- Implement "visible to parent" filters on sensitive data
- Create permission middleware for API routes

---

## 8. CONFIGURATION HARDCODING (Violates PRD §0.2)

**MAJOR VIOLATION:** Many "configurable" values are hardcoded:

- **Escalation wait period:** "3 business days" hardcoded in `communication-view.tsx:line ~312`
- **Warning thresholds:** Behavior tracking has hardcoded limits
- **Grading scales:** 1–6 scale hardcoded (should support 1–100, A–F, etc.)
- **Mastery levels:** "4 levels" assumed, should be configurable per template
- **Report templates:** Hardcoded phrase templates (should be DB-driven)

**Fix:** Migrate ALL numeric/string constants to database config tables.

---

## 9. RECOMMENDED REFACTOR ROADMAP

### **Phase 1: Foundations (1 week)**
1. Extract reusable hook: `useApi()` for all data fetching
2. Create shared components:
   - `DataTable` (replaces 8+ table implementations)
   - `FormBuilder` (replaces 15+ form implementations)
   - `ConfirmDialog` (replaces 12+ modal dialogs)
3. Create middleware wrappers: `withAuth()`, `withAudit()` for API routes
4. Standardize API response format

### **Phase 2: Fix Memory Leaks (3 days)**
1. Add dependency arrays to ALL useEffect hooks
2. Add cleanup functions for all event listeners
3. Fix WebSocket connection management
4. Fix interval/timer cleanup

### **Phase 3: Implement Missing PRD v2 Modules (2 weeks)**
1. **Module B (Signage)** — Emergency alerts, TV displays
2. **Module H (Notifications)** — Centralized notification system
3. **Module J (Widgets)** — Customizable dashboard with DB preferences
4. **Module F (Exam Mode)** — Kiosk lockdown + warning system

### **Phase 4: Consolidate Duplicates (1 week)**
1. Merge `grading-view` + `tablet-grading-view` + `grade-analytics-view`
2. Merge assessment creation forms
3. Merge calendar implementations
4. Merge competency visualization components
5. Implement unified dashboard widget system

### **Phase 5: Authorization & Config (3 days)**
1. Add permission checks to all UI routes
2. Migrate hardcoded config to database
3. Implement role-based feature gating

---

## 10. MEMORY LEAK QUICK FIXES

### **Quick Wins (do immediately):**

**File: `src/lib/websocket.ts`**
```diff
- useEffect(() => {
+ useEffect(() => {
    connect();
-   // missing cleanup!
+   return () => disconnect(); // ADD THIS
  }, []);
```

**File: `src/components/drawing-canvas.tsx`**
```diff
  useEffect(() => {
    canvas.addEventListener('mousedown', handleMouseDown);
    canvas.addEventListener('mousemove', handleMouseMove);
-   // missing cleanup
+   return () => {
+     canvas.removeEventListener('mousedown', handleMouseDown);
+     canvas.removeEventListener('mousemove', handleMouseMove);
+   };
  }, []);
```

**File: `src/components/calendar-view.tsx`**
```diff
  useEffect(() => {
    const handleKeydown = (e) => { ... };
    window.addEventListener('keydown', handleKeydown);
-   // NO CLEANUP
+   return () => window.removeEventListener('keydown', handleKeydown);
  }, []);
```

---

## 11. PRIORITY MATRIX

| Issue | Severity | Effort | Priority |
|-------|----------|--------|----------|
| Memory leaks (WebSocket, listeners) | HIGH | 0.5 days | **URGENT** |
| Missing dependency arrays | HIGH | 1 day | **URGENT** |
| API response standardization | MEDIUM | 1 day | **HIGH** |
| Implement Module B (Signage) | HIGH | 4 days | **HIGH** |
| Implement Module H (Notifications) | HIGH | 3 days | **HIGH** |
| Extract DataTable component | MEDIUM | 2 days | **HIGH** |
| Implement Module J (Widgets) | MEDIUM | 2 days | **MEDIUM** |
| Implement Module F (Exam Mode) | HIGH | 5 days | **HIGH** |
| Consolidate grading logic | MEDIUM | 2 days | **MEDIUM** |
| Migrate config to database | LOW | 3 days | **MEDIUM** |

---

## 12. NEXT ACTIONS

1. **TODAY:** Fix memory leaks (WebSocket, event listeners) — 4 files
2. **THIS WEEK:** 
   - Extract `useApi()` hook
   - Create `DataTable`, `FormBuilder`, `ConfirmDialog` components
   - Standardize API responses
3. **NEXT:** Implement Module B (Signage) + Module H (Notifications)