Finding
File: src/lib/haven-catalog.ts (entire module)
Reviewer persona: QA Lead
Severity: P2
The new buildCatalogSummary and buildWorkDetail functions contain non-trivial merging logic — they overlay live DB data onto curated seed metadata with fallback behavior (live title/author/description override seed values; seed values are used when DB is empty or unavailable). None of this logic has tests.
The repo has an established test pattern (src/lib/__tests__/format.test.ts, src/lib/pipeline/__tests__/claude.test.ts, etc.) using vitest describe/it blocks. The new module should follow the same convention.
Untested paths
- Seed-only catalog (DB empty/unavailable → all works
coming_soon, zero episodes)
- Live data overlay (DB has a book with episodes → status flips to
ready, counts/durations computed)
- Partial live data (book exists but no ready episodes →
coming_soon with zero counts)
- Unknown slug →
buildWorkDetail returns null → 404
- Episode ordering (sorted by
episodeNumber)
totalDurationSeconds sums durationSeconds ?? 0 correctly
Potential Impact
The seed/live merge is the core contract logic. A regression here (e.g., live data silently not overlaying, wrong status, wrong counts) would break the Haven app's display with no test to catch it.
Linked PR
#9
Root Cause Analysis
DevOps perspective
No CI gate prevents untested code from merging. The existing test files run in CI, but new modules without test files simply don't get coverage — there's no enforcement.
Engineering perspective
The shaping logic (buildCatalogSummary, buildWorkDetail) is pure data transformation once loadLiveCollection is mocked. The DB access is already isolated in one function, making the shaping functions easily testable with a mock. The lack of tests is an oversight, not a structural barrier.
Architecture perspective
The module is well-structured for testing — loadLiveCollection is the only async/DB boundary, and the shaping functions are deterministic given its output. The correct pattern is to mock loadLiveCollection (or inject it) and test the shaping functions against seed-only, live-overlay, and edge-case inputs. Follow the existing src/lib/__tests__/ convention.
Recommended Resolution
Create src/lib/__tests__/haven-catalog.test.ts with vitest:
- Mock
loadLiveCollection (or the db module) to return controlled data.
- Test
buildCatalogSummary: seed-only fallback, live overlay, status flip, count/duration computation.
- Test
buildWorkDetail: known slug with episodes, known slug without episodes, unknown slug → null.
- Test episode ordering and
totalDurationSeconds null handling.
Finding
File:
src/lib/haven-catalog.ts(entire module)Reviewer persona: QA Lead
Severity: P2
The new
buildCatalogSummaryandbuildWorkDetailfunctions contain non-trivial merging logic — they overlay live DB data onto curated seed metadata with fallback behavior (live title/author/description override seed values; seed values are used when DB is empty or unavailable). None of this logic has tests.The repo has an established test pattern (
src/lib/__tests__/format.test.ts,src/lib/pipeline/__tests__/claude.test.ts, etc.) using vitestdescribe/itblocks. The new module should follow the same convention.Untested paths
coming_soon, zero episodes)ready, counts/durations computed)coming_soonwith zero counts)buildWorkDetailreturnsnull→ 404episodeNumber)totalDurationSecondssumsdurationSeconds ?? 0correctlyPotential Impact
The seed/live merge is the core contract logic. A regression here (e.g., live data silently not overlaying, wrong status, wrong counts) would break the Haven app's display with no test to catch it.
Linked PR
#9
Root Cause Analysis
DevOps perspective
No CI gate prevents untested code from merging. The existing test files run in CI, but new modules without test files simply don't get coverage — there's no enforcement.
Engineering perspective
The shaping logic (
buildCatalogSummary,buildWorkDetail) is pure data transformation onceloadLiveCollectionis mocked. The DB access is already isolated in one function, making the shaping functions easily testable with a mock. The lack of tests is an oversight, not a structural barrier.Architecture perspective
The module is well-structured for testing —
loadLiveCollectionis the only async/DB boundary, and the shaping functions are deterministic given its output. The correct pattern is to mockloadLiveCollection(or inject it) and test the shaping functions against seed-only, live-overlay, and edge-case inputs. Follow the existingsrc/lib/__tests__/convention.Recommended Resolution
Create
src/lib/__tests__/haven-catalog.test.tswith vitest:loadLiveCollection(or thedbmodule) to return controlled data.buildCatalogSummary: seed-only fallback, live overlay, status flip, count/duration computation.buildWorkDetail: known slug with episodes, known slug without episodes, unknown slug → null.totalDurationSecondsnull handling.