Files
triqura-ecd/docs/reports/code-review-build-failures-20251123.md
2025-11-23 11:42:40 +01:00

512 lines
14 KiB
Markdown

# Code Review Rapport: Build Failures en Architectuur Analyse
**Datum:** 2025-11-23
**Reviewer:** Lead Developer
**Project:** Mini EPD Prototype (v0.1.0)
**Branch:** intake
**Status:** 🔴 CRITICAL - Build gefaald
---
## Executive Summary
De `pnpm build` commando faalt doordat gearchiveerde TypeScript bestanden in `/app/epd/_archive/` worden meegecompileerd door de TypeScript compiler, ondanks dat deze code niet meer actief gebruikt wordt. De root cause is een ontbrekende configuratie in `tsconfig.json` om de archive directory uit te sluiten van compilatie.
**Impact:** Build failures blokkeren deployment naar productie en CI/CD pipelines.
**Prioriteit:** P0 - Kritisch
**Geschatte Fix Tijd:** 5 minuten
**Complexiteit:** Laag
---
## 1. Probleem Analyse
### 1.1 Symptomen
```bash
Failed to compile.
./app/epd/_archive/clients_backup_20251122/[id]/intakes/actions.ts:3:10
Type error: Module '"@/lib/supabase/server"' declares 'createClient' locally,
but it is not exported.
3 | import { createClient } from '@/lib/supabase/server';
| ^
```
**Aantal Beïnvloede Bestanden:** 28 TypeScript bestanden in archive
**Fout Type:** Type error (TS2305 - Export niet gevonden)
**Build Tool:** Next.js 15 + TypeScript 5.x
### 1.2 Root Cause Analyse
#### Primaire Oorzaak
De `tsconfig.json` exclude list bevat alleen `node_modules`, maar **niet** de `app/epd/_archive/` directory:
```json
// tsconfig.json:39-41
"exclude": [
"node_modules"
]
```
Dit betekent dat alle `.ts` en `.tsx` bestanden in de archive **nog steeds worden gecompileerd** tijdens `pnpm build`.
#### Secundaire Oorzaken
1. **API Breaking Change in `lib/supabase/server.ts`**
- **Huidige Export:** `supabaseAdmin` (service role client)
- **Oude Code Verwacht:** `createClient` functie
- **Locatie:** `lib/supabase/server.ts:20-25`
```typescript
// lib/supabase/server.ts - Huidige export
export const supabaseAdmin = createClient(supabaseUrl, supabaseServiceRoleKey, {
auth: {
autoRefreshToken: false,
persistSession: false,
},
})
// ❌ Geen export van 'createClient'
```
2. **Incomplete Migratie Cleanup**
- Archive aangemaakt (✅): `app/epd/_archive/clients_backup_20251122/`
- Build configuratie niet aangepast (❌): `tsconfig.json` exclude list
- Documentatie wel bijgewerkt (✅): `docs/migratie-clients-naar-patients.md`
---
## 2. Architectuur Review
### 2.1 Migratie Status
Volgens `docs/migratie-clients-naar-patients.md` (v1.0, datum: 2025-11-22) is er een volledige migratie uitgevoerd van `/clients/` naar `/patients/` route met de volgende kenmerken:
#### Gekozen Architectuur: Custom API (Optie B)
**Oude Aanpak (gearchiveerd):**
```typescript
// Direct Supabase access in server actions
import { createClient } from '@/lib/supabase/server';
export async function getIntakesByClientId(clientId: string) {
const supabase = await createClient();
const { data } = await supabase.from('intakes').select('*')
// ...
}
```
**Nieuwe Aanpak (actief):**
```typescript
// API-based approach via Custom API endpoints
export async function getIntakesByPatientId(patientId: string): Promise<Intake[]> {
const response = await fetch(`/api/intakes?patientId=${patientId}`)
const data: IntakeListResponse = await response.json()
return data.intakes
}
```
### 2.2 Impact Scope
#### Beïnvloede Modules
| Module | Gearchiveerd | Actief | Status |
|--------|-------------|---------|--------|
| Patient List | `/clients/` | `/patients/` | ✅ Gemigreerd |
| Patient CRUD | `/clients/actions.ts` | `/patients/actions.ts` | ✅ Gemigreerd |
| Intake Module | `/clients/[id]/intakes/` | `/patients/[id]/intakes/` | ✅ Gemigreerd |
| Intake API | N/A | `/api/intakes/` | ✅ Nieuw gebouwd |
#### Statistieken
- **Gearchiveerde LOC:** ~538 lijnen (Intake module) + ~400 lijnen (Client module) = **~938 LOC**
- **Aantal gearchiveerde bestanden:** 28 TypeScript bestanden
- **Actieve imports naar `createClient`:** 0 (correct)
- **Gearchiveerde imports naar `createClient`:** 2 bestanden (foutief)
### 2.3 Git Status Review
**Modified Files (uncommitted):**
```
M app/api/intakes/[intakeId]/route.ts
M app/api/intakes/route.ts
M app/api/screenings/[screeningId]/documents/[documentId]/route.ts
M app/api/screenings/[screeningId]/documents/route.ts
M app/epd/_archive/clients_backup_20251122/[id]/intakes/[intakeId]/layout.tsx
M components/ui/hero-section-2.tsx
M components/ui/sign-in.tsx
M next.config.mjs
```
**Observaties:**
- ✅ API routes zijn recent aangepast (intake + screening modules)
- ⚠️ Archive bestand is modified (`layout.tsx`) - waarschijnlijk post-archive edit
- ✅ UI componenten zijn aangepast (onafhankelijk van deze issue)
- ✅ Next.js config aangepast (performance optimizations)
---
## 3. Code Quality Assessment
### 3.1 Positieve Aspecten ✅
1. **Goede Architectuur Keuze**
- Custom API benadering is pragmatisch en past bij MVP fase
- Scheiding tussen FHIR (Patient/Practitioner) en Custom (Intakes) is logisch
- Future-proof: FHIR Encounter migratie is nog mogelijk
2. **Uitstekende Documentatie**
- `docs/migratie-clients-naar-patients.md` is zeer gedetailleerd (656 regels)
- Story point breakdown en risk analysis aanwezig
- API documentatie in `docs/api/intakes-api.md`
3. **Type Safety**
- Strikte TypeScript configuratie (`strict: true`)
- Zod validatie in nieuwe API endpoints
- Dedicated type definitions in `lib/types/intake.ts`
4. **Error Handling**
- Auth redirect detection in API calls
- Proper HTTP status codes (200, 201, 404, 400)
- Validation error messages in Dutch (UX friendly)
### 3.2 Verbeterpunten ⚠️
1. **Build Configuratie (CRITICAL)**
- Archive directory niet uitgesloten in `tsconfig.json`
- Geen `.eslintignore` of `.prettierignore` voor archive
- Geen build-time verificatie dat archive wordt genegeerd
2. **Git Hygiene**
- 8 uncommitted modified files op feature branch
- Archive bestand is modified na archivering
- Geen `.gitattributes` voor linguist om archive te negeren
3. **Testing Gap**
- Volgens migratieplan: Tests nog niet uitgevoerd (checkboxes unchecked)
- Geen geautomatiseerde tests voor API endpoints
- Geen regression tests gedraaid
4. **Incomplete Cleanup**
- Archive bevat nog modificaties (zie git status)
- Documentatie verwijst naar nog uit te voeren taken
- Fase 5 (Testing & Validatie) nog niet afgerond
---
## 4. Beveiligingsanalyse
### 4.1 Potentiële Risico's
1. **Service Role Key Exposure (LOW RISK - Gemitigeerd)**
```typescript
// lib/supabase/server.ts:4
const supabaseServiceRoleKey = process.env.SUPABASE_SERVICE_ROLE_KEY!
```
- ✅ Correct: Alleen server-side gebruik
- ✅ Bypass RLS is gedocumenteerd en intentioneel
- ✅ Warning in comments: "NEVER expose to client!"
2. **Authentication Handling (GOOD)**
```typescript
// Cookies worden correct doorgegeven in nieuwe API calls
const cookieHeader = await getCookieHeader();
const response = await fetch(url, {
headers: { Cookie: cookieHeader }
});
```
- ✅ Auth cookies worden forwarded naar API
- ✅ HTML redirect detection voor unauthorized access
3. **Input Validatie (GOOD)**
- ✅ Zod schemas in nieuwe API endpoints
- ✅ UUID validatie voor patient/intake IDs
- ✅ SQL injection risico laag (prepared statements via Supabase)
### 4.2 Recommendations
- Voeg `SUPABASE_SERVICE_ROLE_KEY` toe aan `.env.example` met duidelijke warning
- Implementeer rate limiting op `/api/intakes` endpoints
- Overweeg CSRF protection voor POST/PUT/DELETE operations
---
## 5. Performance Review
### 5.1 Current Implementation
**Next.js Config Optimizations:**
```javascript
// next.config.mjs
experimental: {
optimizePackageImports: ['lucide-react', '@react-three/fiber', '@react-three/drei'],
},
webpack: {
splitChunks: {
three: { // Heavy 3D library isolated
name: 'three',
test: /[\\/]node_modules[\\/](three|@react-three)[\\/]/,
priority: 30,
},
},
}
```
**Assessment:**
- ✅ Gzip compression enabled
- ✅ Image optimization configured (AVIF/WebP)
- ✅ Code splitting voor heavy libraries
- ⚠️ Geen caching strategie voor API calls
- ⚠️ Geen lazy loading in intake components
### 5.2 API Performance Concerns
**Oude Aanpak (Direct Supabase):**
- 1 round trip naar database
- Latency: ~10-50ms (lokaal), ~50-150ms (remote)
**Nieuwe Aanpak (Custom API):**
- 2 round trips: Server Action → API Route → Supabase
- Latency: ~20-100ms (lokaal), ~100-300ms (remote)
- **Overhead:** ~10-150ms extra
**Mitigatie Opties:**
1. Implementeer response caching (SWR of React Query)
2. Gebruik `cache: 'force-cache'` voor static data
3. Optimistic UI updates voor mutations
---
## 6. Aanbevelingen
### 6.1 Immediate Actions (P0 - Kritisch)
#### ✅ Fix #1: Update tsconfig.json
**Priority:** P0
**Effort:** 5 min
**Impact:** HIGH - Blokkeert alle builds
```json
// tsconfig.json
{
"exclude": [
"node_modules",
"app/epd/_archive/**/*"
]
}
```
**Rationale:** TypeScript compiler moet archive volledig negeren.
#### ✅ Fix #2: Add .eslintignore
**Priority:** P0
**Effort:** 2 min
**Impact:** MEDIUM - Voorkomt linting errors
```
# .eslintignore
app/epd/_archive/
```
#### ✅ Fix #3: Verify Build
**Priority:** P0
**Effort:** 5 min
**Impact:** HIGH - Valideer fix werkt
```bash
pnpm build
# Should complete without errors
```
### 6.2 Short-term Actions (P1 - Hoog)
#### 📋 Action #1: Clean Git Status
**Priority:** P1
**Effort:** 15 min
```bash
# Commit meaningful changes
git add app/api/intakes/
git add app/api/screenings/
git commit -m "feat: update intake and screening API endpoints"
# Review and commit UI changes
git add components/ui/
git commit -m "chore: update UI components"
# Commit config changes
git add next.config.mjs
git commit -m "chore: add performance optimizations"
# Reset archive modifications (if accidental)
git checkout -- app/epd/_archive/
```
#### 📋 Action #2: Complete Migration Testing
**Priority:** P1
**Effort:** 2-4 hours
Volgens `docs/migratie-clients-naar-patients.md` Fase 5:
- [ ] Run functional tests (5.1)
- [ ] Run API tests (5.2)
- [ ] Run regression tests (5.3)
- [ ] Document results
#### 📋 Action #3: Add .gitattributes
**Priority:** P1
**Effort:** 5 min
```
# .gitattributes
app/epd/_archive/** linguist-vendored
```
**Rationale:** GitHub telt archive niet mee in repository statistics.
### 6.3 Medium-term Actions (P2 - Medium)
#### 🔧 Enhancement #1: API Response Caching
**Priority:** P2
**Effort:** 4-8 hours
Implementeer SWR of TanStack Query voor intake data:
```typescript
// Example with SWR
import useSWR from 'swr'
export function useIntakes(patientId: string) {
const { data, error, mutate } = useSWR(
`/api/intakes?patientId=${patientId}`,
fetcher,
{ revalidateOnFocus: false }
)
return { intakes: data?.intakes, error, refresh: mutate }
}
```
#### 🔧 Enhancement #2: Automated Testing
**Priority:** P2
**Effort:** 1-2 days
- Unit tests voor API routes (Jest/Vitest)
- Integration tests voor server actions
- E2E tests voor kritische flows (Playwright)
#### 🔧 Enhancement #3: CI/CD Pipeline
**Priority:** P2
**Effort:** 4-8 hours
```yaml
# .github/workflows/ci.yml
name: CI
on: [push, pull_request]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: pnpm install
- run: pnpm build
- run: pnpm test # When tests exist
```
---
## 7. Conclusies
### 7.1 Overall Assessment
**Code Kwaliteit:** ⭐⭐⭐⭐☆ (4/5)
- Goede architectuur keuzes
- Uitstekende documentatie
- Type-safe implementatie
- Kleine configuratie issues
**Migratie Uitvoering:** ⭐⭐⭐☆☆ (3/5)
- Feature implementatie volledig
- Testing fase nog niet afgerond
- Cleanup incomplete (archive in build)
- Git hygiene kan beter
**Beveiligingspositie:** ⭐⭐⭐⭐☆ (4/5)
- Goede auth handling
- Input validatie aanwezig
- Service role key correct gebruikt
- Kleine verbeterpunten (rate limiting, CSRF)
### 7.2 Blockers voor Productie
1. ❌ **Build failures** - Moet gefixed voor deployment
2. ⚠️ **Ontbrekende tests** - Verhoogt risico op regressies
3. ⚠️ **Uncommitted changes** - Moeilijk om release te taggen
### 7.3 Aanbevolen Roadmap
**Vandaag (2-3 uur):**
1. Fix tsconfig.json + eslintignore
2. Verify build succeeds
3. Clean git status + commit changes
4. Tag release candidate: `v0.2.0-rc.1`
**Deze Week (1-2 dagen):**
1. Complete Fase 5 testing (volgens migratieplan)
2. Document test results
3. Deploy to staging environment
4. Tag stable release: `v0.2.0`
**Volgende Sprint:**
1. Implement API caching (SWR/React Query)
2. Add automated tests
3. Setup CI/CD pipeline
4. Plan FHIR Encounter migration (indien gewenst)
---
## 8. Appendix
### 8.1 Betrokken Bestanden
**Configuratie:**
- `tsconfig.json` (REQUIRES CHANGE)
- `.eslintignore` (TO BE CREATED)
- `.gitattributes` (TO BE CREATED)
- `next.config.mjs` (OK)
**Archive (28 bestanden):**
- `app/epd/_archive/clients_backup_20251122/**/*`
**Actieve Implementatie:**
- `app/epd/patients/[id]/intakes/` (OK)
- `app/api/intakes/` (OK)
- `lib/types/intake.ts` (OK)
- `lib/supabase/server.ts` (OK - nieuwe export)
### 8.2 Referenties
- Migratie Documentatie: `docs/migratie-clients-naar-patients.md`
- API Documentatie: `docs/api/intakes-api.md`
- Bouwplan v1.0: `docs/specs/UI/bouwplan-mini-epd-v1.0.md`
- CHANGELOG: `CHANGELOG.md`
### 8.3 Metrics
**Code Coverage:**
- Archive LOC: ~938 lijnen (deprecated)
- Active LOC: ~1200+ lijnen (geschat)
- API Routes: 2 endpoints (GET/POST/PUT/DELETE)
- Test Coverage: 0% (geen tests)
**Build Metrics:**
- Build tijd: N/A (fails currently)
- Bundle size: N/A (geen successful build)
- Type errors: 1 (createClient import in archive)
---
**Rapport Versie:** 1.0
**Gegenereerd op:** 2025-11-23
**Review Status:** ✅ Compleet
**Actie Vereist:** Ja - Zie Sectie 6.1 (Immediate Actions)