10c43c4d4a
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>
147 lines
8.2 KiB
Markdown
147 lines
8.2 KiB
Markdown
# 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.
|