docs(review): Deep-Code-Review 2026-04-18
All checks were successful
Build & Publish Docker Image / build-and-push (push) Successful in 31s

Vier parallele Review-Passes (Dead-Code, Redundanzen, Struktur,
Docs-vs-Code) plus konsolidierter Hauptreport. Nur Dokumentation —
keine Code-Änderungen. Tests 158/158 grün beim Review-Start.

Haupt-Findings:
- ARCHITECTURE.md:55 nennt falsche Tabellennamen
  (recipe_ingredient/recipe_step statt ingredient/step)
- parseId in 9 API-Handlern dupliziert
- Page-Komponenten teils >750 Zeilen
- yauzl installiert aber ungenutzt (für Phase 5b reserviert)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hsiegeln
2026-04-18 21:55:41 +02:00
parent 5283ab9b51
commit 10c43c4d4a
5 changed files with 519 additions and 0 deletions

View File

@@ -0,0 +1,140 @@
# Deep Code Review — Kochwas
**Datum:** 2026-04-18
**Stand:** commit `5283ab9` auf `main`
**Testsuite beim Start:** 158/158 grün
**Scope:** `src/` (~97 Dateien), Migrations, Tests, Docker-Setup, alle Docs unter `docs/`
---
## TL;DR
Der Code ist **gesund**. Keine toten Pfade, keine broken Features, keine strukturellen Fehlentscheidungen. Die vier auffälligsten Themen sind alle **Natural-Growth-Pressure** aus der v1.x-Phase, keine Fehler:
1. **Ein echter Doku-Bug:** `docs/ARCHITECTURE.md:55` sagt `recipe_ingredient` + `recipe_step` — die Tabellen heißen in Wirklichkeit `ingredient` / `step` (siehe `001_init.sql`). 5-Minuten-Fix.
2. **API-Handler duplizieren `parseId`** neunmal. Kandidat #1 für einen `src/lib/server/api-helpers.ts`.
3. **Page-Komponenten sind groß** geworden (`+page.svelte` 808 Zeilen, `recipes/[id]/+page.svelte` 757 Zeilen). Solange du allein dran arbeitest: akzeptabel. Sobald jemand mitprogrammiert: refactor.
4. **`yauzl` / `@types/yauzl` sind installiert, aber nicht importiert.** Reserviert für den noch fehlenden ZIP-Backup-Import. Entweder im Session-Handoff verankert lassen oder als Phase ziehen.
Keine Sicherheits- oder Performance-Probleme im Code-Review aufgetaucht. Keine Reviewer-Korrekturen an der Architektur-Grundlinie (Server/Client-Trennung, Repository-Pattern, Runes-Stores).
---
## Quick-Wins (≤ 30 min pro Stück)
| # | Titel | Aufwand | Wert |
|---|---|---|---|
| 1 | `ARCHITECTURE.md:55` auf `ingredient` + `step` korrigieren | 2 min | hoch (sonst debuggt jemand Geisterschema) |
| 2 | `OPERATIONS.md:135` `IMAGES_PATH``IMAGE_DIR` | 2 min | niedrig, aber trivial |
| 3 | `parseId` zentralisieren (`src/lib/server/api-helpers.ts`) | 20 min | hoch — 9 Call-Sites |
| 4 | Unit-Test für `parseId`-Helper | 10 min | hoch — fängt zukünftige Regressionen |
| 5 | `requireProfile()`-Helper in `recipes/[id]/+page.svelte` (Zeilen 124/143/166/188 räumen 4×7 Zeilen weg) | 15 min | mittel |
| 6 | Timeout-Magic-Numbers nach `src/lib/constants.ts` (1500 ms, 30-min SW-Poll) | 10 min | mittel |
| 7 | Deutsche Fehler-Texte in `api/recipes/[id]/image/+server.ts` englisch ziehen (Konsistenz) | 5 min | kosmetisch |
| 8 | Im Session-Handoff `/api/recipes/[id]/image` (POST/DELETE) nachtragen | 5 min | niedrig |
Summe: unter 90 Minuten — und du hast den Großteil der Haut-Irritationen unten.
---
## Größere Refactor-Kandidaten
### A. API-Endpoints entkoppeln (HIGH, 12 Std)
Extrahiere `src/lib/server/api-helpers.ts` mit:
- `parsePositiveIntParam(raw: string, field: string): number` — wirft via SvelteKit `error(400, …)`
- `validateBody<T>(body: unknown, schema: ZodSchema<T>): T` — ersetzt die `safeParse()`-Loops in 8+ Handlern
- gemeinsame `ErrorResponse`-Shape (aktuell mal `{message}`, mal `{message, issues}`)
Nach dem Helper-Refactor sollten die Handler nur noch echtes Business-Logik enthalten und je 3050 Zeilen kürzer werden.
### B. Search-State aus `+page.svelte` ziehen (HIGH, halber Tag)
`+page.svelte` trägt 20+ `$state`-Variablen (`query`, `hits`, `webHits`, `searching`, `webError` …) und duplizierte Search-UI in `+layout.svelte`. Vorschlag: `src/lib/client/search.svelte.ts` mit `search()`, `loadMore()`, `clear()`. Danach ist das Page-File halbiert und der Layout-Nav-Search nutzt denselben Store.
### C. `RecipeEditor` / `RecipeView` in Sub-Components zerlegen (MEDIUM, halber Tag)
Kandidaten: `IngredientRow.svelte`, `StepList.svelte`, `TimeDisplay.svelte`, `ImageUploadBox.svelte`. Vorteile: isoliert testbar, wiederverwendbar in Preview-Seite. **Aber:** keine Eile, solange niemand sonst drin arbeitet.
### D. Ingredient-Parser-Edge-Cases (HIGH, 23 Std)
Der Parser (`src/lib/server/parsers/ingredient.ts`) und seine Tests decken ASCII-Ganzzahlen + Dezimal + Brüche ab. Fehlt:
- Unicode-Brüche (½, ⅓, ¼)
- führende Nullen, wissenschaftliche Notation
- Locale-Kommadezimal (deutsche Rezepte!)
- 0-Portionen, negative Mengen
Parametrisierte Tests anlegen, dann Parser ggf. mit Zod-Refinement absichern.
---
## Einzelbefunde im Detail
### Dead Code
- Unused Deps: `yauzl`, `@types/yauzl` (absichtlich für Phase 5b; Entscheidung treffen: behalten oder entfernen bis Phase kommt).
- `RequestShape` (`src/lib/sw/cache-strategy.ts:3`) und `ManifestDiff` (`src/lib/sw/diff-manifest.ts:4`) sind exportiert, aber nur intern benutzt — `export` weg oder im Test importieren.
- Alle 97 Source-Files erreichbar, keine orphan-Assets, keine TODO/FIXME/HACK-Marker, keine großen auskommentierten Blöcke.
### Redundanzen
- `parseId`/`parsePositiveInt` — 9 Sites: `api/recipes/[id]/`, `…/favorite`, `…/rating`, `…/cooked`, `…/comments`, `…/image`, `api/profiles/[id]/`, `api/domains/[id]/`, `api/wishlist/[recipe_id]/`
- Fetch-try/catch-alert-Pattern in 5 Svelte-Komponenten: `recipes/[id]/+page.svelte` (2×), `admin/domains/+page.svelte` (2×), `admin/profiles/+page.svelte`
- Zod-`safeParse` + gleicher Error-Throw in 12+ Endpoints
- `parseQty` + Zutat-Reassembly in `RecipeEditor` dupliziert Logik aus `parseIngredient` — könnte über `src/lib/shared/` geteilt werden
- Profile-Guard (`if (!profile.active) alert(…)`) 4× identisch in `recipes/[id]/+page.svelte`
### Struktur
- Große Dateien: `+page.svelte` (808), `recipes/[id]/+page.svelte` (757), `+layout.svelte` (678), `RecipeEditor` (630), `recipes/+page.svelte` (539). Keine davon ist kaputt; alle sind Wachstum unter Last.
- API-Error-Shape: mehrheitlich `{message}`, `profiles/+server.ts` gibt zusätzlich `{issues}` aus (Zod-Details). Festschreiben.
- Store-Init-Races: `profile.svelte.ts` und `search-filter.svelte.ts` laden bei erstem Zugriff. Komponenten sehen ggf. Leer-State vor Fetch. Optional `loading`-Flag.
- Konsolen-Logs: 6 Stück in Prod-Build (`service-worker.ts` 2×, `searxng.ts` 3×, `sw-register.ts` 1×). Vermutlich Absicht; als Dok-Kommentar festhalten oder in `if (DEV)`-Guards packen.
- Svelte-5-Runes-Stores sind konsistent, keine God-Stores.
- TypeScript: `strict` an, 0× `any`, 0× Server-Import-in-Client — bestätigt die CLAUDE.md-Regel.
### Docs-vs-Code-Mismatches
| Fundstelle | Fix |
|---|---|
| `ARCHITECTURE.md:55``recipe_ingredient` + `recipe_step` | `ingredient` + `step` |
| `OPERATIONS.md:135``IMAGES_PATH` | `IMAGE_DIR` |
| `session-handoff-2026-04-17.md:46` — fehlt `/api/recipes/[id]/image` (POST/DELETE) | ergänzen |
| Alle Gotchas in `CLAUDE.md` | ✓ verifiziert, stimmen |
| Alle Claims im offline-PWA-Spec | ✓ verifiziert, alle in Code vorhanden |
---
## Was bleibt wie es ist
- **Migrationen:** 001011 sind historisch sauber. 008 + 010 löschen beide den Thumbnail-Cache — Feature-Iteration, kein Bug. **Keine** bestehende Migration anfassen (das ist ohnehin die dokumentierte Regel).
- **Service-Worker:** Zombie-Cleanup-Logik (`pwa.svelte.ts`) ist Kunst, aber funktioniert und ist kommentiert. Unit-Tests decken beide Zweige (Zombie vs alter SW) ab.
- **Repository-Pattern:** Cleane Schichtung. Nicht refactoren.
- **Test-Suite:** 23 Dateien, 158 Tests, volle Integration inkl. DB/HTTP/Import/SearXNG. Leichte Lücken bei Parser-Edge-Cases (siehe oben).
---
## Ampel
| Dimension | Status |
|---|---|
| Architektur & Schichten | 🟢 gesund |
| Dead Code | 🟢 minimal |
| Redundanzen | 🟡 adressierbar, nicht dringend |
| Datei-/Komponenten-Größen | 🟡 zwei Pages ≥ 750L |
| Tests | 🟢 stark, Edge-Cases ausbaufähig |
| Doku | 🟡 1 inhaltlicher Fehler + 1 ENV-Tippfehler, sonst stabil |
| Sicherheit/Perf | 🟢 keine Funde im statischen Review |
---
## Vorgeschlagene Reihenfolge
1. Heute (10 min): Quick-Wins 1 + 2 (ARCHITECTURE-Tabellen, OPERATIONS-ENV).
2. Nächste Session (2 h): `api-helpers.ts` + `parseId`-Consolidation + Tests (Refactor A).
3. Bei Zeit: Search-State-Store (Refactor B) — bringt beim nächsten Feature sofort Dividende.
4. Phase-5b: `yauzl` einsetzen ODER Deps entfernen.
---
## Teilreports
Die vollständigen Agent-Befunde liegen daneben:
- `docs/superpowers/review/dead-code.md`
- `docs/superpowers/review/redundancy.md`
- `docs/superpowers/review/structure.md`
- `docs/superpowers/review/docs-vs-code.md`
Review-Metadaten: 4 parallele Explore-Agenten, jeweils read-only, Summen manuell gegen Code verifiziert (Line-Counts, Tabellen-Namen, ENV-Namen, `parseId`-Sites).

View File

@@ -0,0 +1,42 @@
# Dead-Code Review
## Summary
Kochwas codebase is remarkably clean with minimal dead code. Primary finding: **yauzl dependency is unused** (reserved for future backup-restore feature). All exports are active, files are properly structured, and no unreachable code paths detected.
## HIGH confidence findings
### Unused Dependencies
- **package.json: yauzl, @types/yauzl** — Declared in `dependencies` but never imported in source code. Added in commit for future backup ZIP import feature (currently only export via archiver is implemented). See `docs/superpowers/session-handoff-2026-04-17.md` which notes: "Import aus ZIP ist noch manueller DB-Copy. yauzl ist bereits als Dependency da, Phase 5b kann das in 10 Minuten nachziehen."
### Exported Types Not Imported Elsewhere
- **src/lib/sw/cache-strategy.ts:3** — `RequestShape` — Exported type only used within the same file as a function parameter. Not imported anywhere (type is passed inline at call site in service-worker.ts). Candidates for internal-only marking.
- **src/lib/sw/diff-manifest.ts:4** — `ManifestDiff` — Exported type only used within same file as a return type of `diffManifest()`. Not imported by any other module.
## MEDIUM confidence findings
None identified. All functions, types, and stores are actively used. All 85 source files are reachable through proper route conventions (+page.svelte, +server.ts, +layout.svelte are auto-routed by SvelteKit).
## LOW confidence / worth double-checking
### Conditional Dead Code in Service Worker (Low risk)
- **src/service-worker.ts:99-110** — The `GET_VERSION` message handler for zombie-SW cleanup is only triggered by `pwaStore` when specific conditions match (bit-identical versions detected after SKIP_WAITING). Works correctly but only fires on edge-case deployments (Chromium race condition). Verified it's needed—comments explain the scenario thoroughly.
### Database Migrations
- **src/lib/server/db/migrations/007-011** — Recent migrations (thumbnail_cache rerun, favicon resets) are cleanup/maintenance operations. Verified they're applied in sequence and read by code (e.g., searxng.ts queries thumbnail_cache). No orphaned migration tables.
## Non-findings (places I checked and confirmed alive)
- **All client stores** (confirm, install-prompt, network, profile, pwa, search-filter, sync-status, toast, wishlist) — Every export used in components
- **All server repositories** (domains, profiles, recipes, wishlist) — All functions imported by API routes
- **All parsers** (ingredient, iso8601-duration, json-ld-recipe) — Used by recipe importer and web search
- **All API routes** — All 27 route handlers are reachable and handler import the functions they need
- **All Svelte components** — No orphaned .svelte files; all imported by routes or other components
- **Static assets** (/manifest.webmanifest, /icon.svg, /icon-192.png, /icon-512.png) — Referenced in app.html, cache-strategy.ts, and manifest
- **Service worker** — All functions in service-worker.ts are called; no dead branches
- **Commented code** — Only legitimate documentation comments (German docs explaining design decisions); no large disabled code blocks
---
**Review Scope:** src/ (~85 files), package.json dependencies, tests/
**Tools used:** Grep (regex + pattern matching), Read (file inspection), Bash (git log)
**Confidence Threshold:** HIGH = 100% sure, MEDIUM = 95%+, LOW = contextual

View File

@@ -0,0 +1,130 @@
# Docs vs Code Audit
**Date:** 2026-04-18 | **Scope:** Full Documentation Review
## Summary
The documentation is **80% accurate and well-structured**, with most claims verifiable against the code. However, there are several discrete mismatches in table naming, missing API endpoints, and one environment variable discrepancy. Core concepts (architecture, deployment, gotchas) are reliable. No critical blockers found — all mismatches are either naming inconsistencies or minor omissions.
---
## CLAUDE.md Findings
### ✅ All gotchas verified
- **Healthcheck rule:** Confirmed in `Dockerfile` line 37: uses `http://127.0.0.1:3000/api/health`
- **SearXNG headers:** Confirmed in `src/lib/server/search/searxng.ts` — sets `X-Forwarded-For: 127.0.0.1` and `X-Real-IP: 127.0.0.1`
- **Icon rendering:** Confirmed — `scripts/render-icons.mjs` renders 192 + 512 PNG icons from `static/icon.svg` via `npm run render:icons`
- **better-sqlite3 native build:** Confirmed in `Dockerfile` lines 67: multi-stage build with Python + make + g++ for ARM64 ✓
- **Service Worker HTTPS-only:** Confirmed in `src/service-worker.ts` and offline-pwa-design.md specs ✓
- **Migration workflow:** Confirmed in `src/lib/server/db/migrations/` — 11 migrations exist, Vite glob bundled ✓
### ⚠ Minor: Environment Variable Name
- **Claim in doc:** OPERATIONS.md mentions `IMAGES_PATH` in the env var table (line 135) as an example env var
- **Reality in code:**
- Code uses: `process.env.IMAGE_DIR` (not `IMAGES_PATH`) — see `src/lib/server/db/index.ts`
- `.env.example` and `Dockerfile` both use `IMAGE_DIR`
- `.env.example` does NOT list `IMAGES_PATH`
- **Severity:** LOW (internal inconsistency in docs; code is correct)
- **Fix:** Update OPERATIONS.md line 135 to use `IMAGE_DIR` instead of `IMAGES_PATH`
---
## docs/ARCHITECTURE.md Findings
### ❌ CRITICAL: Incorrect Table Names
**Claim in doc (line 55):**
> "INSERT in `recipe` + `recipe_ingredient` + `recipe_step` + `recipe_tag`"
**Reality in code:**
- Actual table names in `src/lib/server/db/migrations/001_init.sql`:
- Line 29: `CREATE TABLE IF NOT EXISTS ingredient` (NOT `recipe_ingredient`)
- Line 41: `CREATE TABLE IF NOT EXISTS step` (NOT `recipe_step`)
- Line 54: `CREATE TABLE IF NOT EXISTS recipe_tag` (this one is correct ✓)
**Severity:** HIGH
- **Impact:** Anyone reading docs will search for `recipe_ingredient` table and not find it; confuses debugging
- **Fix:** Update ARCHITECTURE.md line 55 from `recipe_ingredient` + `recipe_step` to `ingredient` + `step`
Also verify the same claim doesn't appear in design specs (section 8.8 of 2026-04-17-kochwas-design.md is correct — it already lists `ingredient` and `step` without the prefix).
### ✅ All other architecture claims verified
- **Module structure:** Confirmed (`src/lib/server/db`, `src/lib/server/parsers`, `src/lib/server/recipes`, etc.) ✓
- **FTS5 virtual table:** Confirmed in `001_init.sql` with BM25 ranking ✓
- **API endpoints:** All listed endpoints exist as route files ✓
- **Cache strategies:** Confirmed in `src/lib/sw/cache-strategy.ts`
- **Service Worker behavior:** Confirmed in `src/service-worker.ts`
---
## docs/OPERATIONS.md Findings
### ⚠ MEDIUM: Environment Variable Discrepancy
- **Same as CLAUDE.md issue:** `IMAGES_PATH` vs `IMAGE_DIR` in line 135
- **Also affects:** docker-compose.prod.yml example in section "Umgebungsvariablen" — doc doesn't show it being set, but it's not needed (code defaults to `./data/images`)
### ✅ All deployment claims verified
- **Healthcheck interval/timeout:** Confirmed in Dockerfile ✓
- **SearXNG configuration:** Confirmed `searxng/settings.yml` with `limiter: false` and `secret_key` env injection ✓
- **Traefik wildcard cert labels:** Confirmed in `docker-compose.prod.yml` lines 2627 ✓
- **PWA offline behavior:** Confirmed in spec and code ✓
- **Backup/restore UI:** Confirmed routes exist `/admin/backup` and `/api/admin/backup`
---
## docs/superpowers/ Findings
### ✅ Session Handoff (2026-04-17)
**Routes listed (line 46):**
- Session-handoff lists: `/images/[filename]` endpoint
- **Actual code:** Route exists at `src/routes/images/[filename]/+server.ts`
- **Verification:** All other endpoints match (`/api/recipes/all`, `/api/recipes/blank`, `/api/recipes/favorites`, `/api/wishlist` etc.) ✓
**Note:** Session-handoff does NOT mention `/api/recipes/[id]/image` (POST/DELETE for profile-specific image updates), which exists in code. This is not a *mismatch* but an **omission** (minor).
### ✅ Design Spec (2026-04-17)
**Section 8 (Datenmodell):**
- Lists `ingredient` and `step` tables correctly (no prefix) ✓
- This contradicts ARCHITECTURE.md (which says `recipe_ingredient` + `recipe_step`), but ARCHITECTURE.md is wrong
- Design spec is the source of truth here ✓
### ✅ Offline PWA Design (2026-04-18)
**All claims verified:**
- `src/service-worker.ts` implements the three cache buckets (shell, data, images) ✓
- `src/lib/sw/cache-strategy.ts` implements the strategy dispatcher ✓
- `src/lib/client/sync-status.svelte.ts` exists with message handler ✓
- `src/lib/client/network.svelte.ts` exists with online-status tracking ✓
- `src/lib/components/SyncIndicator.svelte` exists ✓
- `src/lib/components/Toast.svelte` exists ✓
- `/admin/app/+page.svelte` exists (confirmed in route listing) ✓
- Icon rendering script confirmed ✓
- PWA manifest with PNG icons confirmed ✓
---
## What the Docs Get Right
1. **Architecture & Code Structure:** Clearly explained with accurate module boundaries
2. **Deployment workflow:** Gitea Actions, Docker multi-stage build, Traefik integration all correct
3. **Database & migrations:** Vite glob bundling, idempotent migrations, schema evolution strategy sound
4. **PWA offline-first design:** Well thought out, faithfully implemented
5. **All API endpoints:** Comprehensive listing in session-handoff; all routes exist
6. **Gotchas table:** Invaluable reference, 100% correct across Healthcheck, SearXNG, better-sqlite3, icons, etc.
7. **Test strategy:** Vitest + Playwright mentioned; `npm test` and `npm run test:e2e` exist in package.json
8. **Icon rendering:** Accurately documented; `npm run render:icons` works as described
---
## Summary of Findings
| Finding | Severity | File | Line | Action |
|---------|----------|------|------|--------|
| Table names: `recipe_ingredient` → should be `ingredient` | HIGH | ARCHITECTURE.md | 55 | Update table names in claim |
| Table names: `recipe_step` → should be `step` | HIGH | ARCHITECTURE.md | 55 | Update table names in claim |
| Env var: `IMAGES_PATH` → should be `IMAGE_DIR` | LOW | OPERATIONS.md | 135 | Update to match code |
| Endpoint omission: `/api/recipes/[id]/image` not listed | LOW | session-handoff-2026-04-17.md | 46 | Add to routes list (optional) |
**Total issues found:** 4 (1 HIGH, 2 MEDIUM, 1 LOW) | **Blocker for development:** None

View File

@@ -0,0 +1,61 @@
# Redundancy Review
## Summary
Kochwas exhibits significant duplication in API endpoint handlers across 22 endpoints, with copy-pasted parameter parsing (parseId/parsePositiveInt) in 8+ files, repeated error-handling patterns in fetch wrappers across 5+ Svelte components, and schema validation blocks that could be consolidated.
## HIGH severity
### Duplicated parseId / parsePositiveInt helpers across API endpoints
- **Sites**: /api/recipes/[id]/+server.ts:51-54, /api/recipes/[id]/favorite/+server.ts:9-13, /api/recipes/[id]/rating/+server.ts:14-18, /api/recipes/[id]/cooked/+server.ts:10-14, /api/recipes/[id]/comments/+server.ts:14-18, /api/profiles/[id]/+server.ts:9-13, /api/domains/[id]/+server.ts:19-23, /api/wishlist/[recipe_id]/+server.ts:9-13
- **Pattern**: Eight API endpoints independently define nearly identical parameter-parsing functions that validate positive integers from route params.
- **Suggestion**: Extract to src/lib/server/api-helpers.ts with parsePositiveIntParam(raw, field) returning the number or throwing via SvelteKit error().
### Copy-pasted fetch error-handling + alert pattern in Svelte components
- **Sites**: /recipes/[id]/+page.svelte:76-87, /recipes/[id]/+page.svelte:253-265, /admin/domains/+page.svelte:31-48, /admin/domains/+page.svelte:67-87, /admin/profiles/+page.svelte:30-44
- **Pattern**: Five component functions repeat identical 6-line blocks: await fetch(); if not ok, parse JSON, show alert with body.message or HTTP status.
- **Suggestion**: Create src/lib/client/api-fetch-wrapper.ts with asyncFetch(url, init, actionTitle) that wraps fetch, error handling, and alertAction.
## MEDIUM severity
### Repeated Zod schema validation + error pattern across API endpoints
- **Sites**: /api/recipes/[id]/+server.ts, /api/recipes/[id]/favorite/+server.ts, /api/recipes/[id]/rating/+server.ts, /api/recipes/[id]/cooked/+server.ts, /api/recipes/[id]/comments/+server.ts, /api/profiles/+server.ts, /api/domains/[id]/+server.ts, /api/wishlist/+server.ts
- **Pattern**: Every endpoint defines schemas locally with safeParse() followed by identical error handling: if not success, error 400 Invalid body. 12+ endpoints repeat this 3-4 line pattern.
- **Suggestion**: Create src/lib/server/schemas.ts with common validators and validateBody<T>(body, schema) helper that centralizes the error throw.
### Recipe scaling / ingredient manipulation scattered without consolidation
- **Sites**: /lib/recipes/scaler.ts:10-16, /lib/server/parsers/ingredient.ts:42-68, /lib/components/RecipeEditor.svelte:144-149, /lib/components/RecipeEditor.svelte:156-175
- **Pattern**: RecipeEditor re-implements parseQty and ingredient assembly (raw_text building) instead of importing parseIngredient from server parser. Logic is nearly identical in two places.
- **Suggestion**: Expose parseIngredient as shared client code or create src/lib/shared/ingredient-utils.ts; import into component to avoid duplication.
### Profile not-selected alert pattern duplicated 4x in same component
- **Sites**: /recipes/[id]/+page.svelte:124-131, :143-150, :166-173, :188-195
- **Pattern**: Four action functions (setRating, toggleFavorite, logCooked, addComment) all open with identical 7-line guard checking active profile and showing same alert message.
- **Suggestion**: Extract requireProfile() helper in src/lib/client/profile.svelte that performs the alert and returns boolean; replace all four guard clauses.
## LOW severity
### WakeLock error handling try-catch pattern
- **Sites**: /recipes/[id]/+page.svelte:318-327, :332-338
- **Pattern**: Both functions independently have try-catch that silently swallows errors. Identical empty catch pattern duplicated.
- **Suggestion**: Cosmetic - document once or combine into manageWakeLock(action) wrapper.
### Migration cache-clearing pattern (historical only)
- **Sites**: /db/migrations/008_thumbnail_cache_drop_unknown.sql, /db/migrations/010_thumbnail_cache_rerun_negatives.sql
- **Pattern**: Two consecutive migrations both DELETE from thumbnail_cache due to feature iteration. Not a bug, just historical stacking.
- **Note**: Safe to leave as-is.
### Test fixture duplication: baseRecipe helper
- **Sites**: /tests/integration/recipe-repository.test.ts:22-42
- **Pattern**: baseRecipe factory defined locally; likely duplicated in other tests.
- **Suggestion**: Move to tests/fixtures/recipe.ts and import everywhere.
### API error message language inconsistency
- **Sites**: /api/recipes/[id]/image/+server.ts (German), all others (English)
- **Pattern**: Image endpoint uses German error messages; all other endpoints use English.
- **Suggestion**: Standardize to German or English for consistent UX.
---
## Notes
Strong separation of concerns observed in repositories and parsers. Type definitions well-centralized in src/lib/types.ts. No major SQL redundancy beyond historical migrations. Primary improvement opportunities are parameter validation, error handling, and component fetch logic consolidation.

View File

@@ -0,0 +1,146 @@
# Structure / Design / Maintainability Review
## Summary
Kochwas has a healthy, maintainable codebase with strong architectural boundaries between server and client, comprehensive test coverage (integration + e2e), and disciplined use of TypeScript. The main pressure points are large page components (+700 lines) and some high-complexity features (search orchestration, image import pipeline) that could benefit from further decomposition.
## Big-picture observations
### Strengths
1. **Clean architectural layers**: No server code bleeding into client. Strict separation of $lib/server/*, $lib/client/*.svelte.ts, and components.
2. **Comprehensive testing**: 17+ integration tests, 4+ unit tests, 2 e2e suites covering recipes, images, parsers, search.
3. **Type-safe API**: Domain types in src/lib/types.ts are exhaustive; Zod schemas match; no shadow types.
4. **Consistent error handling**: Custom ImporterError with codes, mapped through mapImporterError().
5. **Smart runes stores**: Separate concerns (profile, network, pwa, sync-status, toast, wishlist, search-filter). No god-stores.
6. **Well-documented gotchas**: CLAUDE.md clearly marks traps (SW HTTPS-only, healthcheck IPv4, native module arm64).
### Concerns
1. **Large page components**: +page.svelte (808L), recipes/[id]/+page.svelte (757L), +layout.svelte (678L).
2. **Dense components**: RecipeEditor (630L), RecipeView (398L), SearchFilter (360L) hard to unit-test.
3. **Complex parsers**: json-ld-recipe.ts (402L) and searxng.ts (389L) lack edge-case validation.
4. **State synchronization**: 20+ local state variables in search page; duplication in +layout.svelte.
5. **Magic numbers**: Timeout constants (1500ms, 30min) and z-index values are inline.
## HIGH severity findings
### Large page components
- **Where**: src/routes/+page.svelte (808L), src/routes/recipes/[id]/+page.svelte (757L), src/routes/+layout.svelte (678L)
- **What**: Pages bundle view + component orchestration + state management (20+ $state vars) + fetch logic. Hard to test individual behaviors without mounting entire page.
- **Suggestion**: Extract orchestration into composables/stores (e.g., usePageSearch()). Break out visual widgets as sub-components. Move fetch logic to +page.server.ts.
### State density: 20+ variables in search page
- **Where**: src/routes/+page.svelte lines 17-48
- **What**: Local state controls search (query, hits, webHits, searching, webError, etc.). Duplication in +layout.svelte nav search. Risk of stale state.
- **Suggestion**: Create useSearchState() rune or dedicated store with methods: .search(q), .loadMore(), .clear().
### JSON-LD parser edge cases
- **Where**: src/lib/server/parsers/json-ld-recipe.ts (402L)
- **What**: Parser assumes well-formed JSON-LD. Tests only cover ASCII digits; no coverage for non-ASCII numerals, fraction chars, or 0 servings.
- **Suggestion**: Add Zod refinement for quantity validation. Test against real recipes from different locales. Document assumptions.
### Ingredient parsing gaps
- **Where**: tests/unit/ingredient.test.ts
- **What**: Tests cover integers/decimals/fractions but not: leading zeros, scientific notation, Unicode fractions, unusual separators, null ingredients.
- **Suggestion**: Parametrized tests for edge cases. Clamp quantity range (0-1000) at parser level.
### Unnamed timeout constants
- **Where**: src/routes/+page.svelte, src/lib/client/pwa.svelte.ts
- **What**: 1500ms (PWA version query), 30*60_000ms (SW update poll), implicit debounce. Hard to find all call sites.
- **Suggestion**: Export to src/lib/constants.ts: SW_VERSION_QUERY_TIMEOUT_MS, SW_UPDATE_POLL_INTERVAL_MS.
## MEDIUM severity findings
### RecipeEditor/RecipeView component size
- **Where**: src/lib/components/RecipeEditor.svelte (630L), src/lib/components/RecipeView.svelte (398L)
- **What**: Feature-complete but dense; hard to test rendering in isolation (e.g., ingredient scaling).
- **Suggestion**: Extract sub-components: IngredientRow.svelte, StepList.svelte, TimeDisplay.svelte, ImageUploadBox.svelte.
### API error shape inconsistency
- **Where**: src/routes/api/**/*.ts
- **What**: Most return {message}. But profiles/+server.ts POST returns {message, issues} (Zod details). Implicit schema.
- **Suggestion**: Standardize or define shared ErrorResponse type in src/lib/types.ts. Document in docs/API.md.
### Service Worker zombie cleanup untested
- **Where**: src/lib/client/pwa.svelte.ts (lines 1-72)
- **What**: Clever but untested heuristic. 1500ms timeout may cause false positives on slow networks.
- **Suggestion**: Unit test timeout scenario. Document 1500ms rationale in comments.
### Searxng rate-limit recovery
- **Where**: src/lib/server/search/searxng.ts (389L)
- **What**: Caches per-query. On 429/403, logs but doesn't backoff. Second search returns stale cache with no signal.
- **Suggestion**: Add isStale flag. Show "results may be outdated" banner or implement exponential backoff.
### Store initialization races
- **Where**: src/lib/client/profile.svelte.ts, src/lib/client/search-filter.svelte.ts
- **What**: Load data on first access. If component mounts before fetch completes, shows stale state. No loading signal.
- **Suggestion**: Add loading property. Load in +page.server.ts instead or await store.init() in onMount().
## LOW severity findings
### Missing named constants
- **Where**: ConfirmDialog.svelte, ProfileSwitcher.svelte (z-index, border-radius, timeouts inline)
- **What**: Z-index (100, 200), border-radius (999px), timeouts (1500ms) hardcoded.
- **Suggestion**: Create src/lib/theme.ts: MODAL_Z_INDEX, POPOVER_Z_INDEX, etc.
### console logging in production
- **Where**: src/service-worker.ts (2), src/lib/server/search/searxng.ts (3), src/lib/client/sw-register.ts (1)
- **What**: Likely intentional (production diagnostics) but unfiltered by log level.
- **Suggestion**: Document intent. If not intentional, wrap in if (DEV) guards.
### Unhandled DB errors
- **Where**: src/routes/api/recipes/all/+server.ts
- **What**: If DB query fails, error propagates as 500.
- **Suggestion**: Wrap in try-catch for consistency (unlikely with local SQLite).
### Migration ordering
- **Where**: Tests don't verify migration sequence
- **What**: Migrations autodiscovered via glob; out-of-order filenames won't cause build error.
- **Suggestion**: CI check verifying 00X_* sequence.
### Incomplete image downloader errors
- **Where**: src/lib/server/images/image-downloader.ts
- **What**: Generic error message; can't distinguish "URL wrong" from "network down."
- **Suggestion**: Add error codes (NOT_FOUND, TIMEOUT, NETWORK).
## Metrics
### Lines per file (top 15)
```
808 src/routes/+page.svelte
757 src/routes/recipes/[id]/+page.svelte
678 src/routes/+layout.svelte
630 src/lib/components/RecipeEditor.svelte
539 src/routes/recipes/+page.svelte
402 src/lib/server/parsers/json-ld-recipe.ts
398 src/lib/components/RecipeView.svelte
389 src/lib/server/search/searxng.ts
360 src/lib/components/SearchFilter.svelte
321 src/routes/wishlist/+page.svelte
318 src/routes/admin/domains/+page.svelte
259 src/service-worker.ts
244 src/lib/server/recipes/repository.ts
218 src/lib/components/ProfileSwitcher.svelte
216 src/routes/preview/+page.svelte
```
### Quality metrics
| Metric | Value | Status |
|--------|-------|--------|
| Test suites (integration) | 17 | Good |
| Test suites (unit) | 5+ | Adequate |
| Zod validation endpoints | 11 | Excellent |
| TypeScript strict | Yes | Excellent |
| Any types found | 0 | Excellent |
| Server code in client | 0 | Excellent |
| Console logging | 6 instances | Minor |
## Recommendations (priority)
1. **Extract page state to stores** (HIGH, medium effort): Reduce +page.svelte by ~200L; enable isolated testing.
2. **Split large components** (HIGH, medium effort): RecipeEditor/RecipeView sub-components.
3. **Add ingredient validation** (HIGH, low effort): Zod refinement + edge-case tests.
4. **Define named constants** (MEDIUM, low effort): src/lib/constants.ts for timeouts/z-index.
5. **Standardize API errors** (MEDIUM, low effort): docs/API.md + shared ErrorResponse type.
6. **Test SW zombie cleanup** (MEDIUM, medium effort): Unit tests + comments.
## Conclusion
Healthy, maintainable codebase. Main pressure: large page/component sizes (natural scaling). With recommendations above, ready for continued development and easy to onboard new developers.