docs(remaster): full-system audit + rebuild-vs-refactor decision
Deep, code-grounded audit of Job Tracker producing the mission deliverables under docs/remaster/: system audit, bug report, architecture/data-model/AI/UX reviews, remaster proposal, migration plan, competitor research, and the gated REBUILD_DECISION. Verdict: Incremental Refactor (no full rebuild). Evidence: no Critical defects; hardened cookie/CSRF auth (token never in JS storage), real SSRF defence, enforced multi-tenancy via global query filters, decoupled provider-swappable AI service, 135 backend tests. Debt is localised (god controllers/entity, missing hot-path indexes, prompt-injection hardening, CRA build debt) and reachable by in-place, test-guarded refactors. Also harden .gitignore: exclude agent tooling (.claude/, .bg-shell/, .agent.md) and restore/broaden the runtime-secrets block (**/keys/, **/backups/, exports, CV artifacts) so nested DataProtection keys can't be committed accidentally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cesnimda
@@ -0,0 +1,77 @@
|
||||
# Architecture Review — Job Tracker
|
||||
|
||||
**Companion to:** [SYSTEM_AUDIT_REPORT.md](SYSTEM_AUDIT_REPORT.md)
|
||||
|
||||
## 1. Current topology (as built, verified)
|
||||
|
||||
```
|
||||
React 19 / TS / MUI 7 (CRA) ──HTTP(cookie+CSRF)──▶ ASP.NET Core API (net9.0, EF Core 9)
|
||||
job-tracker-ui/ JobTrackerApi/ (+ JobTrackerBackend link-compile)
|
||||
│
|
||||
┌─────────────────────┼───────────────────────┐
|
||||
▼ ▼ ▼
|
||||
EF Core / SQLite|MySQL Hosted services HttpClient ──▶ FastAPI AI svc
|
||||
(global query filters) (reminders, rules, tools/summarizer/
|
||||
enrichment, export, Ollama | distilbart
|
||||
backup) (provider-swappable)
|
||||
──▶ Gmail API (OAuth), LibreTranslate
|
||||
```
|
||||
|
||||
## 2. What is genuinely good (keep)
|
||||
|
||||
- **AI service decoupling `[strength]`.** The .NET side (`SummarizerService`, `CvAiClassifier`,
|
||||
`CvAiNormalizer`) only speaks HTTP to the FastAPI service. Swapping Ollama→Gemini/Groq is a change in
|
||||
*one* Python file with *zero* .NET edits. This is textbook boundary placement.
|
||||
- **Multi-tenancy via global query filters** on `OwnerUserId` in `Data/JobTrackerContext.cs`. Centralised,
|
||||
hard to bypass accidentally, covered by `JobApplicationsAuthorizationTests`.
|
||||
- **SSRF-safe ingestion** (`JobImport/JobImportService.cs:133-210`): scheme allowlist, loopback/private/
|
||||
CGNAT/link-local/IPv6-ULA blocklist *after DNS resolution*, redirect-averse fetch, 4 MB cap.
|
||||
- **Deterministic domain services** — `JobCvMatchService`, `JobPipeline`, `StageAnalytics`,
|
||||
`EmailStatusClassifier` are small, pure, unit-testable. This is the model the controllers should follow.
|
||||
- **Provider-agnostic persistence** — SQLite default, Pomelo MySQL/MariaDB for prod.
|
||||
|
||||
## 3. Architectural weaknesses
|
||||
|
||||
### 3.1 God controllers `[Architectural weakness]` — highest impact
|
||||
`JobApplicationsController` = **3,271 lines**, `ProfileCvController` = **2,265**, `GmailController` =
|
||||
**1,179**. These are transaction scripts: they hold orchestration, validation, AI-context assembly,
|
||||
persistence, and DTO shaping inline. Consequences: untestable in isolation, merge-conflict magnets,
|
||||
duplicated `RulesEngine.GetSettings` calls, and read paths that load whole tables then filter in memory.
|
||||
**Fix:** extract cohesive services (`JobStatsService`, `AnalyticsService`, `CvContextBuilder`,
|
||||
`GmailImportService`, `GmailThreadRefresher`) + DTO files. The 135 integration tests make this safe.
|
||||
|
||||
### 3.2 Build-layout footgun `[Architectural weakness]`
|
||||
Controllers/services compile through a **separate `JobTrackerBackend` library** that globs
|
||||
`../JobTrackerApi/Controllers/**/*.cs` and `../Services/**/*.cs`, *not* through `JobTrackerApi.csproj`.
|
||||
New files "just compile" from the right folder — invisible magic that will confuse every new contributor.
|
||||
**Fix:** document loudly (done in CLAUDE/README) or collapse the split; not urgent.
|
||||
|
||||
### 3.3 Polling background services, no event bus `[Design flaw, low severity]`
|
||||
Reminders/rules/enrichment run on timers. Fine for a single node and a personal/low-tenant load; would
|
||||
need an outbox/queue if this becomes real multi-tenant SaaS. Not a problem *today*.
|
||||
|
||||
### 3.4 Frontend build platform `[Architectural weakness]`
|
||||
CRA / `react-scripts 5` is EOL-ish and carries transitive-vuln debt (`.gsd` D019 remediated only the
|
||||
direct `axios` finding and explicitly deferred the framework migration). **And** `.gsd/OVERRIDES.md`
|
||||
records an **active** directive *"use next.js"* (2026-04-10) that was **never executed**. So the shipped
|
||||
stack contradicts the last recorded frontend decision. Resolve intentionally (Vite for least churn, or
|
||||
Next.js per the override if SSR/SEO for a public product matters).
|
||||
|
||||
### 3.5 Scraper-plugin fragility `[Architectural weakness]`
|
||||
HTML-structure-coupled plugins against adversarial targets (LinkedIn/Indeed) will rot. No plugin-health
|
||||
metric, so failures are silent (fall back to universal parser or manual). **Fix:** health telemetry +
|
||||
lean on the already-solid manual fallback; treat scraping as best-effort, not a guarantee.
|
||||
|
||||
## 4. Service-boundary map (target)
|
||||
|
||||
| Concern | Today | Target owner |
|
||||
|---|---|---|
|
||||
| Job CRUD | `JobApplicationsController` | thin controller → `JobApplicationService` |
|
||||
| Stats/analytics | inline in controller (load-all) | `AnalyticsService` (server-side aggregation) |
|
||||
| CV context assembly | inline in `JobApplicationsController`/`ProfileCvController` | `CvContextBuilder` |
|
||||
| Gmail import/refresh | `GmailController` (N+1) | `GmailImportService` + `GmailThreadRefresher` |
|
||||
| Rule settings | repeated `RulesEngine.GetSettings` | cache in `IMemoryCache` (already registered) |
|
||||
|
||||
## 5. Verdict
|
||||
The **skeleton is correct**; the muscle is in the wrong place (controllers). This is the signature of a
|
||||
system that grew feature-first, not of one that is architecturally unsound. Refactor, do not rebuild.
|
||||
Reference in New Issue
Block a user