feat: Ingestion Pipeline Stabilization and WASM Service Proxies #42

Merged
mjasin merged 3 commits from refactor/skill-aligned-code-review into develop 2026-05-13 18:24:24 +00:00
Collaborator

This PR stabilizes the Nexus Ingestion Engine by implementing functional service proxies for the Blazor WASM client and refining the backend infrastructure for real-time progress tracking and database compatibility.

Key Changes

  • Infrastructure Stabilization:
    • Implemented production-grade EbookRepository with PostgreSQL EF.Functions.ILike support.
    • Enforced IsReadyForReading = false state for newly added ebooks (resolves #35).
    • Updated SignalRSyncBroadcaster to support targeted user messaging and ingestion-specific progress updates (resolves #37).
  • WASM Client Functional Proxies:
    • Replaced "Throwing" dummy services with WasmEbookRepository, WasmSyncBroadcaster, WasmBookStorageService, and WasmEmbeddingGenerator.
    • These services proxy requests to the backend via a new set of Minimal API endpoints in NexusReader.Web.
  • Domain Refinement:
    • Added IsReadyForReading flag to the Ebook entity to manage background AI processing states.

Related Issues

This PR stabilizes the Nexus Ingestion Engine by implementing functional service proxies for the Blazor WASM client and refining the backend infrastructure for real-time progress tracking and database compatibility. ### Key Changes - **Infrastructure Stabilization**: - Implemented production-grade `EbookRepository` with PostgreSQL `EF.Functions.ILike` support. - Enforced `IsReadyForReading = false` state for newly added ebooks (resolves #35). - Updated `SignalRSyncBroadcaster` to support targeted user messaging and ingestion-specific progress updates (resolves #37). - **WASM Client Functional Proxies**: - Replaced "Throwing" dummy services with `WasmEbookRepository`, `WasmSyncBroadcaster`, `WasmBookStorageService`, and `WasmEmbeddingGenerator`. - These services proxy requests to the backend via a new set of Minimal API endpoints in `NexusReader.Web`. - **Domain Refinement**: - Added `IsReadyForReading` flag to the `Ebook` entity to manage background AI processing states. ### Related Issues - Fixes #35 - Fixes #36 - Fixes #37
Antigravity added 2 commits 2026-05-13 18:05:25 +00:00
Critical fixes (review findings #1, #2, #3):
- Create IEbookRepository abstraction in Application layer
- Remove illegal EF Core dependency from IngestEbookCommandHandler
- Create EbookRepository implementation in Infrastructure/Persistence
- Create ISyncBroadcaster in Application/Abstractions/Messaging
- Create SignalRSyncBroadcaster in Infrastructure/RealTime
- Move UpdateReadingProgressCommandHandler from Infrastructure → Application
- Add EbookId to GetReaderPageQuery and IEpubReader signature
- Rewrite EpubReaderService: DB-resolved file path, remove auto-provisioning
- Split EpubService.cs into EpubReaderService.cs + EpubMetadataExtractor.cs
- Add CurrentEbookId to IReaderNavigationService and ReaderNavigationService
- Update WasmEpubReader and /api/epub endpoint for new signature

High severity fixes (#4, #6, #7, #8, #16):
- Change BookStorageService registration from Singleton → Scoped
- Fix empty catch{} in ReaderCanvas JS interop init — now logs warnings
- Replace all Console.WriteLine with ILogger in KnowledgeService + ReaderCanvas
- Cache JsonSerializerOptions as static field in KnowledgeService
- Wrap SyncService Task.Run body in comprehensive try/catch with ILogger

Medium/Low fixes (#11, #13, #14, #15, #18, #20):
- BookIngestionModal.DisposeAsync now nullifies _epubBytes (50MB array)
- KnowledgeCoordinator.OnGraphUpdated: Action<T> → Func<T, Task>
- BookStorageService: Path.Combine → forward-slash string interpolation
- SignalR CancellationToken passed as named parameter (not payload arg)
Antigravity reviewed 2026-05-13 18:12:17 +00:00
Antigravity left a comment
Author
Collaborator

This PR introduces severe architectural violations regarding CQRS and Client/Server boundaries in a Blazor Hybrid application. Instead of dispatching commands to the server, this PR implements functional WASM proxies for backend Infrastructure interfaces (IEbookRepository, ISyncBroadcaster, etc.) allowing MediatR handlers to be resolved locally. This approach destroys transactional integrity across Minimal API boundaries. Furthermore, changes to the Domain layer are missing corresponding EF Core migrations. Please address the inline comments and revert the WASM repository patterns.

This PR introduces severe architectural violations regarding CQRS and Client/Server boundaries in a Blazor Hybrid application. Instead of dispatching commands to the server, this PR implements functional WASM proxies for backend Infrastructure interfaces (`IEbookRepository`, `ISyncBroadcaster`, etc.) allowing MediatR handlers to be resolved locally. This approach destroys transactional integrity across Minimal API boundaries. Furthermore, changes to the Domain layer are missing corresponding EF Core migrations. Please address the inline comments and revert the WASM repository patterns.
@@ -43,0 +45,4 @@
/// Gets or sets a value indicating whether the ebook has been processed by the AI ingestion engine
/// and is ready for reading (Knowledge Units generated).
/// </summary>
public bool IsReadyForReading { get; set; } = false;
Author
Collaborator

Missing EF Core Migration: A new property IsReadyForReading was added to the Ebook domain entity, but no corresponding EF Core migration was generated. According to project standards, every change to a Domain entity must be accompanied by a new migration.

Missing EF Core Migration: A new property `IsReadyForReading` was added to the `Ebook` domain entity, but no corresponding EF Core migration was generated. According to project standards, every change to a Domain entity must be accompanied by a new migration.
mjasin marked this conversation as resolved
@@ -48,1 +50,3 @@
builder.Services.AddSingleton<IBookStorageService>(new ThrowingBookStorageService());
builder.Services.AddScoped<IEmbeddingGenerator<string, Embedding<float>>, WasmEmbeddingGenerator>();
builder.Services.AddScoped<IBookStorageService, WasmBookStorageService>();
builder.Services.AddScoped<IEbookRepository, WasmEbookRepository>();
Author
Collaborator

Architectural Violation (Clean Architecture / CQRS): Building 'Wasm' proxies for Infrastructure interfaces (IEbookRepository, ISyncBroadcaster, etc.) in order to satisfy DI for MediatR handlers executing locally on the WASM client is an anti-pattern. MediatR handlers that depend on server-side infrastructure must not be executed directly from client environments. The client should send an HTTP API request (or SignalR message), and the MediatR handler should be executed exclusively on the server.

Architectural Violation (Clean Architecture / CQRS): Building 'Wasm' proxies for Infrastructure interfaces (`IEbookRepository`, `ISyncBroadcaster`, etc.) in order to satisfy DI for MediatR handlers executing locally on the WASM client is an anti-pattern. MediatR handlers that depend on server-side infrastructure must not be executed directly from client environments. The client should send an HTTP API request (or SignalR message), and the MediatR handler should be executed exclusively on the server.
mjasin marked this conversation as resolved
@@ -0,0 +42,4 @@
private readonly List<Author> _stagedAuthors = new();
private readonly List<Ebook> _stagedEbooks = new();
public async Task<int> SaveChangesAsync(CancellationToken cancellationToken = default)
Author
Collaborator

Transactional Integrity Flaw: Staging entities locally and then saving them across multiple Minimal API endpoints (/api/repository/author/add, /api/repository/ebook/add, and /api/repository/save) completely destroys Unit of Work and transaction scope. Each API call runs in a separate DI scope, meaning SaveChangesAsync on the server will have no tracked entities and will do nothing. The entire operation should be encapsulated in a single backend MediatR command.

Transactional Integrity Flaw: Staging entities locally and then saving them across multiple Minimal API endpoints (`/api/repository/author/add`, `/api/repository/ebook/add`, and `/api/repository/save`) completely destroys Unit of Work and transaction scope. Each API call runs in a separate DI scope, meaning `SaveChangesAsync` on the server will have no tracked entities and will do nothing. The entire operation should be encapsulated in a single backend MediatR command.
mjasin marked this conversation as resolved
@@ -0,0 +12,4 @@
_httpClient = httpClient;
}
public async Task BroadcastProgressAsync(
Author
Collaborator

Redundant Implementation: The WASM client does not and should not broadcast sync progress via HTTP. SyncService.cs already correctly uses HubConnection.SendAsync to communicate with the SyncHub on the server, which then dispatches the UpdateReadingProgressCommand. This proxy serves no architectural purpose and is misleading.

Redundant Implementation: The WASM client does not and should not broadcast sync progress via HTTP. `SyncService.cs` already correctly uses `HubConnection.SendAsync` to communicate with the `SyncHub` on the server, which then dispatches the `UpdateReadingProgressCommand`. This proxy serves no architectural purpose and is misleading.
mjasin marked this conversation as resolved
@@ -296,0 +318,4 @@
return Results.Ok();
});
repoApi.MapPost("/ebook/add", (Ebook ebook, IEbookRepository repo) =>
Author
Collaborator

Domain Leakage: Exposing raw Domain Entities (Ebook, Author) directly as parameters in REST APIs is unsafe and prone to JSON serialization failures (e.g., navigation properties, circular references). Furthermore, these endpoints expose repository CRUD operations instead of adhering to the CQRS pattern via Application Commands.

Domain Leakage: Exposing raw Domain Entities (`Ebook`, `Author`) directly as parameters in REST APIs is unsafe and prone to JSON serialization failures (e.g., navigation properties, circular references). Furthermore, these endpoints expose repository CRUD operations instead of adhering to the CQRS pattern via Application Commands.
mjasin marked this conversation as resolved
mjasin added 1 commit 2026-05-13 18:18:36 +00:00
Author
Collaborator

Architectural Remediation Applied

Thank you for the rigorous review. I have addressed all architectural violations and stabilized the PR:

  1. Reverted WASM Proxies: Deleted WasmEbookRepository, WasmSyncBroadcaster, WasmBookStorageService, and WasmEmbeddingGenerator. These were an anti-pattern that violated CQRS and transactional integrity.
  2. Restored DI Integrity: Restored Throwing services in the WASM client to satisfy DI validation without allowing illegal client-side execution of infrastructure-dependent handlers.
  3. CQRS Enforcement: Removed all CRUD-like repository proxy endpoints from the server. All data operations now flow through the IngestEbookCommand on the server.
  4. EF Core Migration: Generated and added the missing migration AddEbookReadyFlag for the Ebook entity changes.
  5. Removed Redundancy: Deleted the unnecessary broadcaster proxy as SyncService already correctly handles SignalR communication.

The solution now adheres to Clean Architecture and project standards. Ready for re-review.

### Architectural Remediation Applied Thank you for the rigorous review. I have addressed all architectural violations and stabilized the PR: 1. **Reverted WASM Proxies**: Deleted `WasmEbookRepository`, `WasmSyncBroadcaster`, `WasmBookStorageService`, and `WasmEmbeddingGenerator`. These were an anti-pattern that violated CQRS and transactional integrity. 2. **Restored DI Integrity**: Restored `Throwing` services in the WASM client to satisfy DI validation without allowing illegal client-side execution of infrastructure-dependent handlers. 3. **CQRS Enforcement**: Removed all CRUD-like repository proxy endpoints from the server. All data operations now flow through the `IngestEbookCommand` on the server. 4. **EF Core Migration**: Generated and added the missing migration `AddEbookReadyFlag` for the `Ebook` entity changes. 5. **Removed Redundancy**: Deleted the unnecessary broadcaster proxy as `SyncService` already correctly handles SignalR communication. The solution now adheres to Clean Architecture and project standards. Ready for re-review.
Antigravity reviewed 2026-05-13 18:22:01 +00:00
Antigravity left a comment
Author
Collaborator

Excellent work! The architectural boundaries and strict CQRS constraints have been fully restored.

I have verified the following fixes:

  • 🟢 WASM Proxies (WasmEbookRepository, WasmSyncBroadcaster, etc.) were removed.
  • 🟢 Minimal API CRUD endpoints were removed, ensuring state mutations flow through IngestEbookCommand.
  • 🟢 Throwing services in NexusReader.Web.Client are properly implemented to satisfy DI while preventing accidental local execution of infrastructure-bound handlers.
  • 🟢 AddEbookReadyFlag EF Core migration was correctly generated and added to NexusReader.Data.
  • 🟢 The solution builds cleanly (dotnet build reports 0 errors).

The pull request is now fully compliant with the Clean Architecture and Blazor Hybrid guidelines defined in our project. Approving the changes (via comment).

Excellent work! The architectural boundaries and strict CQRS constraints have been fully restored. I have verified the following fixes: - 🟢 WASM Proxies (`WasmEbookRepository`, `WasmSyncBroadcaster`, etc.) were removed. - 🟢 Minimal API CRUD endpoints were removed, ensuring state mutations flow through `IngestEbookCommand`. - 🟢 `Throwing` services in `NexusReader.Web.Client` are properly implemented to satisfy DI while preventing accidental local execution of infrastructure-bound handlers. - 🟢 `AddEbookReadyFlag` EF Core migration was correctly generated and added to `NexusReader.Data`. - 🟢 The solution builds cleanly (`dotnet build` reports 0 errors). The pull request is now fully compliant with the Clean Architecture and Blazor Hybrid guidelines defined in our project. Approving the changes (via comment).
mjasin merged commit 5a2223a4c8 into develop 2026-05-13 18:24:24 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mjasin/Nexus.Reader#42