feat/api-dx2

periodByRpu last-write-wins when an RPU spans multiple contracts

apps/platform/src/api/utils/public-v1-monitoring-read.ts:79

A single RPU can span multiple contracts (terminated + replacement). The loop `for (const b of latestBills) { periodByRpu.set(b.rpu, b.periodEnd) }` overwrites on the second contract; the winning `latestPeriodEnd` depends on DB-alphabetical order of `contractPublicId`, not on which contract has the most recent bill. Pre-existing behavior that survived the extraction, but now less visible. Low-impact for most RPUs; can silently surface the wrong period on reassigned RPUs.

Zip-slip: DB-sourced filenames used as zip entry names without path sanitization

apps/platform/src/api/utils/public-v1-file-read.ts:60

Filenames come from `bill_files.filename` stored verbatim from CFE's Content-Disposition header. The parsing regex admits `/` and `..`. A compromised or MITM'd CFE response could persist a path-traversal filename that survives into a zip entry name; naive unzip tools on the customer's machine may traverse directories. Risk is low (CFE is first-party, S3 keys are separately sanitized, harm is customer-side), but `path.basename(f.filename)` in `dedupeZipEntryNames` would mitigate completely.

last_collection.status exposes internal CfeJobStatus enum verbatim — inconsistent with curated error_code

apps/platform/src/api/utils/public-v1-monitoring-read.ts:89

Unlike `error_code` which goes through `toCuratedErrorCode`, `status` is forwarded as-is (`collect.status`). Values like `partial_success` and `cancelled` reveal internal pipeline internals. Any future internal-only status added to `CfeJobStatus` would immediately leak to public callers. Consider a `toCuratedJobStatus` allow-list (schema already restricts this to `z.string()`, open-ended).

PUBLIC_CODES set duplicated in test rather than derived from production

apps/platform/src/api/utils/__tests__/public-v1-monitoring-read.test.ts:10

The `PUBLIC_CODES` Set on line 10 is hand-written in the test. If a new public code is added to `toCuratedErrorCode` but not to this local Set, the invariant test fails — desired. Conversely, if the Set is accidentally widened without production changes the test passes vacuously. Importing or deriving `PUBLIC_CODES` from the production module would close this drift vector. Low priority — the current Set exactly matches the switch.

loadMonitoringHealthByRpu returns Promise<Map<…>> not Result<T,E> — ADR-016 violation

apps/platform/src/api/utils/public-v1-monitoring-read.ts:64

This is an exported, async, fallible function (two parallel DB queries via Promise.all). ADR-016 requires all fallible public operations to return `Result<T, E>`. It currently throws on DB failure, forcing the handler to rely on its implicit try/catch. The file-read counterparts (`listPublicFiles`, `getPublicFile`, `generatePublicFilesZip`) all correctly return `Promise<Result<…, PublicFileError>>`. Consistency requires the same wrapping here.

Inline admin-required comments repeat the file-level JSDoc verbatim

apps/platform/src/api/utils/public-v1-monitoring-read.ts:71

Lines 71–74 annotate each `Promise.all` branch with `// admin-required: service-role read…` restating what the file-level JSDoc already explains. Project convention: only comment when the WHY is non-obvious at that exact callsite. The file header covers it; the per-line repetition adds noise.

loadMonitoringHealthByRpu has zero test coverage

apps/platform/src/api/utils/__tests__/public-v1-monitoring-read.test.ts

The new test file covers only `toCuratedErrorCode`. `loadMonitoringHealthByRpu` — the exported coordinator that drives `last_collection` and `latest_period_end` on every monitoring row — is entirely untested. Key untested paths: RPU with no jobs → `lastCollection: null`; RPU with no bills → `latestPeriodEnd: null`; the `periodByRpu` join when bill.rpu is null; empty `rpus []` early-exit. An integration test seeding real `cfe_jobs` + `bills` rows closes the gap; at minimum, the in-memory mapping logic should be unit-tested.

toCuratedErrorCode test has no compile-time binding to CfeErrorCode

apps/platform/src/api/utils/__tests__/public-v1-monitoring-read.test.ts:26

The test uses raw string literals (`'INVALID_CREDENTIALS'`, etc.) instead of importing `CfeErrorCode` from the error types package. If a code is renamed upstream the switch falls to `default → collection_failed` silently — the tests still pass because test string literals stay unchanged. Using `import type { CfeErrorCode }` would make the contract load-bearing at compile time.

Multi-RPU same-period test asserts structure but not exact filenames

apps/platform/src/api/utils/__tests__/public-v1-file-read.test.ts:30

The three-RPU same-period test checks `length` and `allDistinct` but not the actual filenames emitted. A bug that wrongly renames `456_2026-05.pdf` to `456_2026-05_2.pdf` would still pass. The adjacent tests use exact `toEqual(…)` — add the same here to make it load-bearing.

storagePath passthrough test uses non-null assertion on indexed access

apps/platform/src/api/utils/__tests__/public-v1-file-read.test.ts:94

Lines 94–96 use `out[0]!` and `out[1]!`. If a regression shrinks the output array, the assertions silently dereference `undefined`. Add `expect(out).toHaveLength(2)` before the property access to make the failure message informative.

No test for dedupeZipEntryNames with multi-dot filenames

apps/platform/src/api/utils/__tests__/public-v1-file-read.test.ts

The `lastIndexOf('.')` invariant picks the last dot. A filename like `bill.2026-05.pdf` produces suffix insertion before the final dot: `bill.2026-05_2.pdf`. This is the correct convention but not explicitly tested. A single case would document the contract as a regression guard if the logic is revisited.

Redundant rpus filter when latest_only already scopes bill IDs by RPU

apps/platform/src/api/utils/public-v1-file-read.ts:129

When `q.latest_only` is true, `latestBillIds` is already pre-filtered by `rpuFilter`. Passing `rpus: q.rpu` to `listEnrichedFiles` alongside `billPublicIds: latestBillIds` evaluates a redundant SQL predicate. The clause is subsumed — omitting `rpus` when `latestBillIds` is populated avoids unnecessary DB work: `rpus: latestBillIds ? undefined : q.rpu`.

toCuratedErrorCode accepts empty-string but type says string | null — undocumented contract

apps/platform/src/api/utils/public-v1-monitoring-read.ts:31

The `if (!code) return null` guard coerces `''` to null. The test pins this (line 71), but the type signature `string | null` doesn't document the empty-string case. The caller at line 90 uses `collect.error?.code ?? null` which can't produce `''`, so there's no live bug — but making the guard explicit (`if (code === null || code === '')`) or adding a comment closes comprehension drift.

dot > 0 inline comment understates the leading-dot edge case

apps/platform/src/api/utils/public-v1-file-read.ts:61

The comment reads 'invariant over the suffix loop; > 0 only when there's a real extension'. A leading-dot file like `.gitignore` gives `lastIndexOf('.') === 0`, which falls to the extensionless branch — counterintuitive. Clarifying to 'leading-dot names (dot===0) are treated as extensionless' saves the next reader a trip to the test file.