You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
101 lines
4.7 KiB
Markdown
101 lines
4.7 KiB
Markdown
# Code Audit v2 — Consolidated Summary
|
|
|
|
**Date**: 2026-03-27
|
|
**Auditors**: Software Architect, Frontend Expert, Rust Expert, Tech Lead, QA Engineer
|
|
|
|
## Overall Assessment
|
|
|
|
The codebase has strong fundamentals — zero `unsafe`, zero `unwrap()` in production, excellent security (SSRF, encryption, CSRF), clean error handling, and 689 total tests. The project has grown significantly with multi-theme, scheduling, preferred sources, and Brave Search. The main issues are accumulated complexity in `synthesis.rs` and `ThemeManager.tsx`.
|
|
|
|
---
|
|
|
|
## Top Issues (agreed by 3+ auditors)
|
|
|
|
### 1. `synthesis.rs` God Module — 2010 lines, ~800-line main function
|
|
|
|
**Confirmed by**: Architect, Tech Lead, Rust Expert
|
|
|
|
The scrape+classify batch loop is duplicated across Phase 1 and Brave Search paths (~300 lines). The function manages 6+ tracking HashMaps and mixes job store, pipeline orchestration, URL utilities, and provider resolution.
|
|
|
|
**Recommended refactoring:**
|
|
- Extract `scrape_and_classify_batch()` — shared helper (~300 lines saved)
|
|
- Extract `GenerationContext` struct for tracking state (filled_counts, source_counts, etc.)
|
|
- Move `JobStore` to its own module (`services/job_store.rs`)
|
|
- Split into `synthesis/pipeline.rs`, `synthesis/job_store.rs`, `synthesis/helpers.rs`
|
|
|
|
### 2. `ThemeManager.tsx` — 935 lines with ~30 signals
|
|
|
|
**Confirmed by**: Frontend Expert, Tech Lead
|
|
|
|
Mixes theme content editing, source management, bulk import, CSV import/export, preferred toggles, and schedule config in one component.
|
|
|
|
**Recommended refactoring:**
|
|
- Extract `SourceManager` component (reusable, shared with Sources.tsx)
|
|
- Extract `ThemeContentForm` component (content settings)
|
|
- Already have `SettingsSchedule` and `SettingsBraveSearch` as sub-components
|
|
|
|
### 3. Scheduler has zero tests
|
|
|
|
**Confirmed by**: QA Engineer, Rust Expert
|
|
|
|
`run_scheduled_jobs()` is an autonomous background process that triggers generation and sends emails. No unit, integration, or E2E tests.
|
|
|
|
### 4. Hardcoded French strings bypass i18n
|
|
|
|
**Confirmed by**: Frontend Expert, Tech Lead
|
|
|
|
"Lire la suite", "Reduire", day labels in SynthesisDetail.tsx and other pages use hardcoded French instead of the i18n system.
|
|
|
|
---
|
|
|
|
## High Priority Issues
|
|
|
|
| # | Issue | Source | Impact |
|
|
|---|-------|--------|--------|
|
|
| 5 | No concurrency limit on JoinSet spawns | Rust Expert | Under load, hundreds of parallel HTTP requests |
|
|
| 6 | Silent `.ok()` on DB trace writes | Rust Expert | Lost traceability data without logging |
|
|
| 7 | Duplicated source management logic (~80%) between Sources.tsx and ThemeManager.tsx | Tech Lead | Maintenance burden |
|
|
| 8 | `createEffect` with fire-and-forget async in ThemeManager | Frontend Expert | Race conditions on rapid theme switching |
|
|
| 9 | SSE progress stream untested at integration level | QA Engineer | Only tested via flaky E2E |
|
|
| 10 | Date extraction/max_age filtering untested | QA Engineer | Pipeline filtering logic uncovered |
|
|
|
|
---
|
|
|
|
## Medium Priority Issues
|
|
|
|
| # | Issue | Source |
|
|
|---|-------|--------|
|
|
| 11 | `serde_json::Value` for categories/days/emails — should be `Vec<String>` | Rust Expert |
|
|
| 12 | Inconsistent feedback patterns (inline banners vs Toast) | Frontend Expert |
|
|
| 13 | Duplicated delete confirmation logic (3 different patterns) | Frontend Expert |
|
|
| 14 | Raw SQL in `resolve_model()` breaking db-module abstraction | Tech Lead |
|
|
| 15 | `UpdateThemeRequest` has no validation | Rust Expert |
|
|
| 16 | Email validation too permissive (just checks `@`) | Rust Expert |
|
|
| 17 | Scheduler runs jobs sequentially — one blocks all | Rust Expert |
|
|
| 18 | 10 failing frontend unit tests | QA Engineer |
|
|
| 19 | `normalizeUrl`/`isValidUrl` utilities live in page files | Frontend Expert |
|
|
|
|
---
|
|
|
|
## Positive Highlights
|
|
|
|
All auditors noted these strengths:
|
|
- **Zero unsafe code**, zero unwrap in production paths
|
|
- **Strong security**: AES-256-GCM, SSRF with 74 tests, CSRF, anti-enumeration
|
|
- **Clean `AppError` enum** — consistent JSON responses, internal errors never leaked
|
|
- **Well-designed `LlmProvider` trait** — Strategy + Factory patterns
|
|
- **689 tests** — good coverage on scraper, providers, encryption, CRUD
|
|
- **SSE utility** is exemplary SolidJS reactive code
|
|
- **Clean API client layer** — type-safe, consistent patterns
|
|
- **Excellent module documentation** — `//!` doc comments on every module
|
|
|
|
---
|
|
|
|
## Detailed Reports
|
|
|
|
- [Architect Report](v2-architect-report.md) — SOLID principles, design patterns, architecture
|
|
- [Frontend Report](v2-frontend-report.md) — SolidJS patterns, component architecture
|
|
- [Rust Expert Report](v2-rust-expert-report.md) — Idiomatic Rust, async, safety
|
|
- [Tech Lead Report](v2-tech-lead-report.md) — Complexity, duplication, refactoring plan
|
|
- [QA Report](v2-qa-report.md) — Test coverage gaps (15 gaps found)
|