# Project Review Summary

**Project:** Theseus
**Date:** 2026-06-06
**Stack:** Rust / axum 0.7 / SQLite / Cytoscape.js

## Scores

| Angle | Score | Key Finding |
|-------|-------|-------------|
| Architecture | **B+** | Clean layered design, good dependency hygiene. Main risks: hardcoded column indices in `db.rs`, dual DeepSeek clients, `lib.rs` god-file (901 lines, 6 responsibilities) |
| Code Quality | **B** | Consistent conventions, zero TODO/FIXME. Concerns: `handle_project()` ~800 lines, 9× test boilerplate duplication, ~20 `unwrap()` in row deserializers |
| Error Handling | **B+** | Excellent `AppError` with bilingual messages. Gaps: agent errors collapse to generic 500, some routes use raw `ApiResponse::err()` bypassing structured errors |
| Testing | **C** | 26 Rust tests + 10 Python E2E files, but 52% module coverage. **Zero CI**. `main.rs` (2657 lines) has zero tests. Entire `agent/` module untested |
| API Design | **B** | Consistent response envelope, good input validation. Issues: no API versioning, no OpenAPI spec, `secret` leaked in credential responses, unused response types |
| Infrastructure | **B-** | Excellent README, locked deps, clean config. **Zero CI pipeline** is the critical gap. No changelog, no release automation, no `rust-toolchain.toml` |
| Runtime Health | **C+** | Solid logging and config handling. **No timeouts** on any I/O path. **Zero retry logic**. **No graceful shutdown**. Single `Mutex<Connection>` bottleneck |
| Network Resilience | **D+** | Works on reliable local network. **Zero timeouts** on HTTP clients. No WebSocket ping/pong. No exponential backoff. `reqwest::Client::new()` per-call (no connection pooling) |

## Overall Grade: **C+**

Solid engineering for a domain tool with excellent error handling and clean architecture. The codebase is well-structured and conventions are followed consistently. However, **zero CI**, **zero timeouts**, **zero retry logic**, and **zero graceful shutdown** represent significant gaps for anything beyond local single-user usage. These are all fixable without architectural changes.

## Top 3 Strengths

1. **Error handling infrastructure** — `AppError` with bilingual messages, structured error codes (`db.not_found`, `auth.invalid_api_key`), operation/entity/field/hint tracing. Best-in-class for a CLI+server tool.
2. **Clean architecture** — Strict dependency direction (`main` → `lib` → `routes` → `db` → `types`), no cycles. CLI has zero direct DB access. Frontend fully decoupled via REST + WebSocket.
3. **Agent subsystem design** — Self-contained module with clean boundaries: `llm.rs` (client), `loop.rs` (orchestration), `memory.rs` (persistence), `prompts.rs` (engineering), `tools.rs` (13 tools). Use as template for future modules.

## Top 3 Areas to Improve

1. **No CI pipeline** — Zero automation for lint, test, or build. Adding `.github/workflows/ci.yml` with `cargo fmt --check && cargo clippy && cargo test` would catch regressions immediately. (`docs/review/infra.md`, `docs/review/testing.md`)
2. **No timeouts or retry on any network call** — `reqwest::Client::new()` with default (infinite) timeouts. DeepSeek API hangs block request handlers indefinitely. Add `.timeout(Duration::from_secs(60))` and retry with backoff for 429/502/503. (`docs/review/runtime-health.md`, `docs/review/network-resilience.md`)
3. **No graceful shutdown** — `axum::serve(listener, app).await.unwrap()` with no signal handling. In-flight requests, WebSocket connections, and agent runs are dropped on SIGTERM. Use `with_graceful_shutdown(shutdown_signal())`. (`docs/review/runtime-health.md`)

## Quick Wins (high impact, low effort)

1. **Add HTTP client timeouts** (~5 lines) — `.timeout(Duration::from_secs(60)).connect_timeout(Duration::from_secs(10))` on all `reqwest::Client` instances. Prevents indefinite hangs.
2. **Add graceful shutdown** (~15 lines) — `axum::serve().with_graceful_shutdown(ctrl_c + sigterm)`. Prevents data loss on restart.
3. **Add `PRAGMA busy_timeout = 5000`** (1 line) — Prevents `SQLITE_BUSY` errors under concurrent access.
4. **Share `reqwest::Client` in `AppState`** (~20 lines) — Move from per-call `Client::new()` to shared instance. Enables TCP reuse and centralized timeout config.
5. **Add GitHub Actions CI** (~30 lines YAML) — `cargo fmt --check`, `cargo clippy`, `cargo test`. Catches regressions on every push.
6. **Replace `unwrap()` in row deserializers** (~10 lines) — `Uuid::parse_str(...).unwrap()` → `.map_err(...)`. Prevents panics on corrupt DB data.

## Systemic Issues (patterns, not one-offs)

1. **Per-request `reqwest::Client` allocation** — `deepseek_chat()` at `lib.rs:563` and 9 route handlers create `Client::new()` per call. Each allocates a new connection pool + TLS session. Should be one shared client in `AppState`.
2. **Hardcoded column indices in `db.rs`** — `row.get(0)`, `row.get(2)`, etc. across 5 deserializer functions. Any SELECT reordering = silent data corruption. Use named access (`row.get("id")`) instead.
3. **Analysis types in wrong module** — `AttackPath`, `CheckResult`, `AttackAnalysis` etc. defined in `db.rs:724-799` instead of `types.rs`. Breaks the "single source of truth" pattern. 9 types, ~80 lines.
4. **`lib.rs` god-file** — 901 lines handling `AppError`, `AppState`, `Config`, middleware, `deepseek_chat()`, and 3 `resolve_*_id()` helpers. Extract into `error.rs`, `config.rs`, `resolve.rs`.
5. **No retry/backoff anywhere** — DeepSeek 429 rate limits, network blips, 502/503 gateway errors are all treated as permanent failures. The agent subsystem (multi-turn conversation) is especially vulnerable — one failed turn kills the entire run.
