128 lines
5.6 KiB
Markdown
128 lines
5.6 KiB
Markdown
# 🔍 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** |
|