# 🔍 NexusReader Code Review Backlog

## 🔴 CRITICAL — Fix Before Next Release

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Removed `AddMediatR` from `AddApplication()` and `AddInfrastructure()`. Unified registration in Host (`Program.cs`, `MauiProgram.cs`). Added `IInfrastructureMarker` and a startup validation check in `Web.New` that throws `InvalidOperationException` if `AddInfrastructure` is missing.
- **DoD:** Application fails fast with a clear error if `AddInfrastructure()` is omitted.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Added `VerifyGroundednessAsync` to `IKnowledgeService` and implemented it in `KnowledgeService` (Infrastructure). Updated `VerifyGroundednessCommandHandler` in Application to inject `IKnowledgeService` instead of `IChatClient`.
- **DoD:** No `IChatClient` or `IEmbeddingGenerator` references remain in `NexusReader.Application`.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Threaded `tenantId` through all `IKnowledgeService` methods and `ProcessKnowledgeUnitsAsync`. Scoped `SemanticKnowledgeCache` and `KnowledgeUnit` lookups/writes to the provided `tenantId`. Updated API endpoints in `Program.cs` and `WasmKnowledgeService` to pass the authenticated user's `TenantId`.
- **DoD:** No hardcoded `"global"` TenantId in write paths. Extracted units are always scoped to the caller's tenant.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Changed `NexusUser.TenantId` from `Guid` to `string`. All entities now use `string` for `TenantId`, allowing the use of `"global"` as a sentinel value.
- **DoD:** All entities use the same `TenantId` type. All query filters are consistent.

---

## 🟠 MAJOR — High Priority Fixes

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Added `File.Exists` check and granular `try-catch` around `EpubReader.ReadBookAsync` to prevent unhandled exceptions and provide descriptive error messages.
- **DoD:** Corrupted or missing files return `Result.Fail` instead of crashing.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Verified `IDbContextFactory<AppDbContext>` is correctly registered via `AddDbContextFactory` in `Infrastructure/DependencyInjection.cs`.
- **DoD:** Webhook and profile endpoints successfully resolve the factory.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Implemented `Coordinator.Clear()` (which calls `KnowledgeGraphService.Clear()`) in `ReaderCanvas.razor`'s `OnInitialized`.
- **DoD:** Stale graph data is cleared upon component initialization.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Created `UserProfileDto` to exclude sensitive internal IDs like `TenantId` and DB GUIDs. Updated `/identity/profile` endpoint to project into this DTO using `.Select()`.
- **DoD:** Internal IDs are no longer exposed in the profile API.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Added `HasIndex(x => x.TenantId)` to `NexusUser`, `Ebook`, and `QuizResult` in `AppDbContext`. `KnowledgeUnit` and `SemanticKnowledgeCache` already had them.
- **DoD:** Tenant-scoped queries are optimized via DB indexes.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Refactored `KnowledgeService.ProcessKnowledgeUnitsAsync` to pre-fetch all existing unit IDs in a single batch query, eliminating the N+1 `FindAsync` and `AnyAsync` calls.
- **DoD:** Batch processing performance is significantly improved.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Added `TenantId` property to `Ebook` entity with mandatory validation and index. Updated `AppDbContext` configuration.
- **DoD:** Ebooks are now isolated at the database level.

---

- **Status:** ✅ Resolved (2026-05-03)
- **Implementation:** Added `TenantId` property to `QuizResult` entity with mandatory validation and index.
- **DoD:** Quiz results are now isolated at the database level.

---

## 🟡 MINOR — Technical Debt & UX

### [MN-01] Missing Logging in `KnowledgeCoordinator`
- **Action:** Add `ILogger<KnowledgeCoordinator>` and log successful/failed extraction steps.

### [MN-02] Hardcoded "Gemini-1.5-Flash" in Domain
- **File:** `Domain/Entities/SemanticKnowledgeCache.cs:20`
- **Action:** Move the default model ID to a constant in `AiSettings`.

### [MN-03] UI: Shimmer Effect Lack Animation
- **File:** `UI.Shared/Components/Molecules/GroundednessBadge.razor`
- **Action:** Add `@keyframes` for the shimmer effect in CSS.

### [MN-04] Identity: Google Callback Lack Error Handling
- **File:** `Web.New/Program.cs:340`
- **Action:** Better UI feedback when `ExternalLoginInfo` is null.

### [MN-05] Tokenizer Initialization is Expensive
- **File:** `Infrastructure/Services/KnowledgeService.cs:43`
- **Action:** Make `_tokenizer` static or Singleton to avoid recreating it per request.

### [MN-06] Mapster: Global Configuration Check
- **Action:** Ensure `TypeAdapterConfig.GlobalSettings.Scan(...)` is only called once.

### [MN-07] SignalR: Missing Reconnection Logic
- **Action:** Implement `hubConnection.OnReconnected` in `SyncService.cs`.

### [MN-10] Performance: Large EPUB Parsing
- **Action:** Implement streaming extraction for EPUBs over 10MB.

---

## 🧪 TESTING — Coverage Gaps

### [TEST-01] Integration Tests for KM-RAG Retrieval
- **Action:** Create `tests/NexusReader.IntegrationTests`.
- **Scenario:** Ingest a document, then verify that `GetRelevantContext` returns the correct snippets with tenant isolation active.

---

## 📊 Summary Table

| Severity | Count | Status |
|---|---|---|
| 🔴 Critical | 4 | 4 resolved |
| 🟠 Major | 8 | 8 resolved |
| 🟡 Minor | 8 | Unresolved |
| 🧪 Tests | 1 | Unresolved |
| **Total** | **21** | **12 resolved** |