feat(library): enforce manifest-backed chapter availability - #170
Conversation
Context: Metadata ranges could project chapters that had no exact published identity. Changes: Make validated manifests authoritative for catalog projection, package completeness, and generated publication output. Impact: Declared invalid manifests fail closed; legacy packages retain prior behavior until migrated. Tests: 6 focused files, 56 tests; TypeScript; diff check. Docs: CORE-015 amendment, library guides, WORKLOG. ADR: CORE-015
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5e9af0599
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const manifestContextFor = (novel: NovelEntry, version: NovelVersion) => ({ | ||
| novelId: novel.id, | ||
| versionId: version.versionId, | ||
| sessionUrl: version.sessionJsonUrl, |
There was a problem hiding this comment.
For publications built with the default GitHub base URL, the builder records a raw.githubusercontent.com/.../session.json URL in both metadata and the manifest, but RegistryService rewrites only the metadata session URL to media.githubusercontent.com/media/... before this context is constructed. The exact comparison in validateChapterManifest therefore rejects the matching builder-generated manifest, causing both catalog projection and reader import to fail for the default hosted output. Normalize both URLs consistently or compare their canonical artifact identities.
Useful? React with 👍 / 👎.
| const expectedPublication = await resolveExpectedChapterPublication(novel, requestedVersionId); | ||
| const expectedChapterCount = expectedPublication.count; | ||
| const expectedChapterNumbers = expectedPublication.numbers; |
There was a problem hiding this comment.
When a publisher corrects a session without changing its version ID, an older scoped cache can contain every manifest chapter number while holding different stable IDs/content. Reducing the manifest to numbers here lets that cache satisfy the completeness check, so the session is not replayed and the corrected manifest-backed identities are never acquired. Cache completeness should compare the manifest's stable identity tuple for each chapter, not only membership of its number.
Useful? React with 👍 / 👎.
| cache.set(key, []); | ||
| return []; |
There was a problem hiding this comment.
If the manifest request fails transiently—for example while the client is briefly offline—this stores an empty catalog under the version key. Production has no caller of clearCatalogCache, so subsequent hook reruns continue returning the cached empty array even after connectivity recovers, leaving the dropdown limited to already imported rows for the rest of the page session. Fail closed for the current request, but avoid caching transport failures so a later load can retry.
Useful? React with 👍 / 👎.
|
Independent source review completed for this stack, including this PR's source at Final verdict: APPROVE for #169/#170/#171/#172, with no remaining LFS-contract blockers. Review Node 24.19 focused tests, types, production build and client/integrity checks passed on the integrated source. Production Chromium verifies the three late-navigation scope cases and the frozen-graph offline round trip. Real chapter-2 acquisition fetched one 5.34 MB published artifact in 2.61 seconds with zero full-session requests; cached offline navigation took 97 ms (single measurements, not semantic scan latency). Remaining acceptance limits: actual package main publication, complete novel/index, real semantic scan and physical mobile/Safari. Ordinary backup reopen without a graph loses active selection (CONS-07); readable chapters remain and its assertion is still failing. No full-feature acceptance is claimed. |
1 participant
Summary
Uses the validated chapter manifest for catalog rows and cache completeness. Missing or invalid declared manifests fail closed; packages without a manifest retain the existing legacy range behavior. The manifest foundation is merged in #169, and this PR now targets main.
Changes
Validation
The combined chapter stack passes 137 focused tests on Node 24.19.0, types, production build and client security checks. Main refresh
07e2123has the identical Git tree as reviewed heade5e9af0; current-head CI is linked below.Independent Grok 4.6 source review approved the stack and publisher follow-up (
gen-1788706940-9sQzYYjBNNTha8hDheXr). All supplied file bytes were matched to anonymous public GitHub downloads. Codex review was requested but returned no verdict.Per-chapter output and acquisition are reviewed separately in #171/#172. Companion publication remains separate; no complete novel, semantic scan or physical-device acceptance is claimed.