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):

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):

// 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:

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

- useEffect(() => {
+ useEffect(() => {
    connect();
-   // missing cleanup!
+   return () => disconnect(); // ADD THIS
  }, []);

File: src/components/drawing-canvas.tsx

  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

  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)