[Bug][Backend][Books] Chapter extraction is orphaned end-to-end — nothing ever populates Book.Chapters, and GET /books/{id}/chapters returns [] for every book, always #593

New Issue

2026-07-14T21:12:06Z

spikerj commented

2026-07-14 21:12:06 +00:00

Found while closing out #557 (whose PR #275 hardened BookAnalysisService against silent degradation). Verifying #557's remaining half — "route the model load through an honest gpu-coordinator lease" — turned up something that makes that half unwireable as written, and is a bigger finding than the ticket it came from.

Chapter extraction has no production caller. It has never had one.

The chain, verified on `master`

Nothing invokes the extractor. The only references to BookAnalysisService.ExtractChapters / AnalyzeBookWithLLM outside the class itself are the interface, the test mocks, and the integration-test fake. No host, no handler, no worker calls either method.
The book pipeline hardcodes empty. SpikerSoft.EventHandlers.BookManagement/Services/BookManagementService.cs:84 creates every book with:
```
Chapters = [],
```
and nothing writes to that field afterwards.
A live API endpoint serves the result. SpikerSoft.Api/Domain/Books/BookController.cs:325-352:
```
[Authorize]
[HttpGet("{id}/chapters")]
[ProducesResponseType(typeof(List<Chapter>), StatusCodes.Status200OK)]
public async Task<IActionResult> GetBookChapters(string id)
{
    ...
    var chapters = book.Chapters ?? new List<Chapter>();
    return Ok(chapters);   // always []
}
```
It returns 200 OK with an empty array for every book in the system, and always has. Not a 404, not an error — a confident, successful, empty answer. Same family as #568 (API book search is a permanent no-op) and #585 (embeddings are write-only): a feature that is present at every layer except the one that connects them.
No consumer. The Angular frontend never calls /chapters, so this is not a visible regression today — which is exactly why it has gone unnoticed. It is an entire feature wired to nothing.

Why this matters beyond the dead code

#557 was filed on a false premise, and this is the correction. That ticket says "books uploaded during GPU pressure get degraded chapter structure" and set out to route chapter extraction through a GPU lease. That cannot be happening — the code never runs. The service-level silent-degradation bug PR #275 fixed was real and worth fixing (ExtractChapters never called InitializeModelAsync; a blanket catch swallowed #553's CUDA-alloc failure and substituted an invented every-20-pages structure). But its stated impact never materialised, because no book has ever reached it.

There is also a latent trap here of the #573 kind: ExtractChapters is now hardened, model-loading, GPU-touching code sitting one DI registration away from live. Wiring it naively — without a gpu-coordinator lease — would reproduce #553 (a tenant taking VRAM it never declared) on the book-upload path.

The decision this needs (product call, not a code call)

Either:

(a) Wire it. Chapter extraction becomes a real step in the book pipeline. Per the heavy-dependency-isolation rule it cannot run in BookManagement — it needs an IRemoteCommand → Workers.* → EventHandlers.* hop into an AI worker, taking an honest gpu-coordinator lease (the #553 contract), with the failure semantics PR #275 already built (GpuResourceUnavailableException retryable, LlmOutputUnparsableException not). Then GET /books/{id}/chapters starts telling the truth and the frontend can use it.

(b) Retire it. Delete ExtractChapters / AnalyzeBookWithLLM / ChapterResponseParser / CreateChaptersFromPageRanges, drop the Chapters field, and remove the endpoint — rather than leave a documented API that lies. (ExtractSubjectsFromPage on the same class is live in the quiz worker and must stay.)

I have deliberately not guessed. (a) is a real feature build with a GPU-lease design; (b) deletes a capability someone may be counting on. Both are cheap to do once the direction is known — happy to take either.

Refs: #557 (the ticket this came out of; its silent-degradation half is fixed and merged in PR #275, its lease-routing half is blocked on this decision), #553 (the GPU-lease contract any wiring must honour), #573 (dead-but-armed code, deleted), #568 / #585 (same no-op-feature family).

Found while closing out **#557** (whose PR #275 hardened `BookAnalysisService` against silent degradation). Verifying #557's *remaining* half — "route the model load through an honest gpu-coordinator lease" — turned up something that makes that half unwireable as written, and is a bigger finding than the ticket it came from. **Chapter extraction has no production caller. It has never had one.** ## The chain, verified on `master` 1. **Nothing invokes the extractor.** The only references to `BookAnalysisService.ExtractChapters` / `AnalyzeBookWithLLM` outside the class itself are the interface, the test mocks, and the integration-test fake. No host, no handler, no worker calls either method. 2. **The book pipeline hardcodes empty.** `SpikerSoft.EventHandlers.BookManagement/Services/BookManagementService.cs:84` creates every book with: ```csharp Chapters = [], ``` and nothing writes to that field afterwards. 3. **A live API endpoint serves the result.** `SpikerSoft.Api/Domain/Books/BookController.cs:325-352`: ```csharp [Authorize] [HttpGet("{id}/chapters")] [ProducesResponseType(typeof(List<Chapter>), StatusCodes.Status200OK)] public async Task<IActionResult> GetBookChapters(string id) { ... var chapters = book.Chapters ?? new List<Chapter>(); return Ok(chapters); // always [] } ``` It returns **200 OK with an empty array for every book in the system**, and always has. Not a 404, not an error — a confident, successful, empty answer. Same family as **#568** (API book search is a permanent no-op) and **#585** (embeddings are write-only): a feature that is present at every layer except the one that connects them. 4. **No consumer.** The Angular frontend never calls `/chapters`, so this is not a visible regression today — which is exactly why it has gone unnoticed. It is an entire feature wired to nothing. ## Why this matters beyond the dead code **#557 was filed on a false premise, and this is the correction.** That ticket says *"books uploaded during GPU pressure get degraded chapter structure"* and set out to route chapter extraction through a GPU lease. That cannot be happening — the code never runs. The service-level silent-degradation bug PR #275 fixed was real and worth fixing (`ExtractChapters` never called `InitializeModelAsync`; a blanket `catch` swallowed #553's CUDA-alloc failure and substituted an invented every-20-pages structure). But its *stated impact* never materialised, because no book has ever reached it. There is also a latent trap here of the **#573** kind: `ExtractChapters` is now hardened, model-loading, GPU-touching code sitting one DI registration away from live. Wiring it naively — without a gpu-coordinator lease — would reproduce #553 (a tenant taking VRAM it never declared) on the book-upload path. ## The decision this needs (product call, not a code call) Either: **(a) Wire it.** Chapter extraction becomes a real step in the book pipeline. Per the heavy-dependency-isolation rule it cannot run in `BookManagement` — it needs an `IRemoteCommand` → `Workers.*` → `EventHandlers.*` hop into an AI worker, **taking an honest gpu-coordinator lease** (the #553 contract), with the failure semantics PR #275 already built (`GpuResourceUnavailableException` retryable, `LlmOutputUnparsableException` not). Then `GET /books/{id}/chapters` starts telling the truth and the frontend can use it. **(b) Retire it.** Delete `ExtractChapters` / `AnalyzeBookWithLLM` / `ChapterResponseParser` / `CreateChaptersFromPageRanges`, drop the `Chapters` field, and remove the endpoint — rather than leave a documented API that lies. (`ExtractSubjectsFromPage` on the same class **is** live in the quiz worker and must stay.) I have deliberately **not** guessed. (a) is a real feature build with a GPU-lease design; (b) deletes a capability someone may be counting on. Both are cheap to do once the direction is known — happy to take either. **Refs:** #557 (the ticket this came out of; its silent-degradation half is fixed and merged in PR #275, its lease-routing half is blocked on this decision), #553 (the GPU-lease contract any wiring must honour), #573 (dead-but-armed code, deleted), #568 / #585 (same no-op-feature family).

spikerj referenced this issue

2026-07-14 21:12:22 +00:00

[Bug][Backend] BookAnalysisService (ExtractChapters / AnalyzeBookWithLLM) silently ships degraded book data on GPU/model-load failure — same class as quiz-gen #553, different pipeline #557

spikerj commented

2026-07-22 03:57:56 +00:00

Board-sweep status (2026-07-22): the claim still holds on current master — nothing anywhere assigns Book.Chapters (verified: zero write sites), and GET /books/{id}/chapters (BookController.cs:400) still returns book.Chapters ?? [], i.e. always empty. But the resolution has changed shape since this was filed: the Reader/Kavita-parity epic shipped GET /book/{bookId}/epub-chapters (live EPUB TOC extraction, GetEpubChaptersQuery), and the Angular reader consumes ONLY that (reader.service.ts → epub-chapters; nothing in the app calls the legacy /chapters route). So the orphaned mechanism no longer needs implementing — it needs retiring: drop the dead /chapters endpoint, the Book.Chapters field, and the Chapter model. Leaving open until that removal lands; suggest retitling toward cleanup.

Board-sweep status (2026-07-22): the claim still holds on current master — nothing anywhere assigns `Book.Chapters` (verified: zero write sites), and `GET /books/{id}/chapters` (`BookController.cs:400`) still returns `book.Chapters ?? []`, i.e. always empty. **But the resolution has changed shape since this was filed:** the Reader/Kavita-parity epic shipped `GET /book/{bookId}/epub-chapters` (live EPUB TOC extraction, `GetEpubChaptersQuery`), and the Angular reader consumes ONLY that (`reader.service.ts` → `epub-chapters`; nothing in the app calls the legacy `/chapters` route). So the orphaned mechanism no longer needs implementing — it needs **retiring**: drop the dead `/chapters` endpoint, the `Book.Chapters` field, and the `Chapter` model. Leaving open until that removal lands; suggest retitling toward cleanup.

spikerj referenced this issue

2026-07-29 15:29:08 +00:00

exposeAgePublicly parental permission + Age on SponsorFamilyMember (consent-boundary design) #873

spikerj referenced this issue

2026-07-29 15:30:46 +00:00

EPIC: Sponsor family card redesign — image-backed destination tiles, achievements strip, parent-gated age #868

spikerj commented

2026-07-29 16:10:21 +00:00

Re-verified against origin/master — NOT DONE. Still orphaned end to end, exactly as filed.

Nothing writes Book.Chapters. git grep -nE "Chapters\s*=\s*[^=]" origin/master -- '*.cs' excluding tests returns zero hits. There is no assignment to that property anywhere in production code.

The extractor exists and is fully built — SpikerSoft.Business.Ai.Workers/Services/BookAnalysisService.cs:161 (ExtractChapters(Stream, …)), declared on the interface at IBookAnalysisService.cs:22, with a real LLM implementation at :470 (ExtractChaptersWithLLM) and OTel instrumentation at :169. It has even been maintained: the comment at :182 records a #557 fix ("the model was NEVER loaded on this path"), and :474 notes a branch is "unreachable now that ExtractChapters initializes the model".

So someone debugged and fixed a bug inside a method that nothing calls. git grep -n "ExtractChapters" origin/master -- '*.cs' excluding tests returns only the definition sites and the interface declaration — no callers.

And the read side confirms the consequence: BookController.cs:437 exposes [HttpGet("{id}/chapters")], and :461 reads var chapters = book.Chapters ?? new List<Chapter>(); before returning at :463. With no writer, that endpoint returns [] for every book, always — which is precisely what this ticket says.

This is the sharpest instance of a pattern that has recurred throughout this audit: code that shipped, is tested, is maintained, and has no caller. Others found this week — #698's consumer-liveness registry that nothing registers with, #847's GenerationParams:Enabled flag no code path reads, #756's DLQ config that NotificationsRpcConsumerHostedService never binds, #663's /quiz/{id}/review endpoint with no Angular consumer, and #785's SCSS class no template binds.

Remaining: call ExtractChapters from the book-processing pipeline and persist the result to Book.Chapters. The extractor is ready; only the wiring and the write are missing.

Re-verified against `origin/master` — **NOT DONE. Still orphaned end to end, exactly as filed.** **Nothing writes `Book.Chapters`.** `git grep -nE "Chapters\s*=\s*[^=]" origin/master -- '*.cs'` excluding tests returns **zero** hits. There is no assignment to that property anywhere in production code. **The extractor exists and is fully built** — `SpikerSoft.Business.Ai.Workers/Services/BookAnalysisService.cs:161` (`ExtractChapters(Stream, …)`), declared on the interface at `IBookAnalysisService.cs:22`, with a real LLM implementation at `:470` (`ExtractChaptersWithLLM`) and OTel instrumentation at `:169`. It has even been *maintained*: the comment at `:182` records a #557 fix ("the model was NEVER loaded on this path"), and `:474` notes a branch is "unreachable now that ExtractChapters initializes the model". So someone debugged and fixed a bug inside a method that **nothing calls**. `git grep -n "ExtractChapters" origin/master -- '*.cs'` excluding tests returns only the definition sites and the interface declaration — **no callers**. **And the read side confirms the consequence:** `BookController.cs:437` exposes `[HttpGet("{id}/chapters")]`, and `:461` reads `var chapters = book.Chapters ?? new List<Chapter>();` before returning at `:463`. With no writer, that endpoint returns `[]` for every book, always — which is precisely what this ticket says. This is the sharpest instance of a pattern that has recurred throughout this audit: **code that shipped, is tested, is maintained, and has no caller**. Others found this week — #698's consumer-liveness registry that nothing registers with, #847's `GenerationParams:Enabled` flag no code path reads, #756's DLQ config that `NotificationsRpcConsumerHostedService` never binds, #663's `/quiz/{id}/review` endpoint with no Angular consumer, and #785's SCSS class no template binds. **Remaining:** call `ExtractChapters` from the book-processing pipeline and persist the result to `Book.Chapters`. The extractor is ready; only the wiring and the write are missing.

spikerj referenced this issue

2026-07-29 17:03:21 +00:00

[Epic][Blog][UX] Blog management & UX overhaul — findings from the 2026-07-18 full review #705

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#593

[Bug][Backend][Books] Chapter extraction is orphaned end-to-end — nothing ever populates Book.Chapters, and GET /books/{id}/chapters returns [] for every book, always #593

The chain, verified on master

Why this matters beyond the dead code

The decision this needs (product call, not a code call)

The chain, verified on `master`