feat/api-dx2

Confusing pre-increment side effect in ternary — correct but fragile

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

The while-loop uses ++n inside both branches of the ternary expression. It is correct because only one branch executes per iteration, but the style is fragile: any refactor that splits the ternary into an if/else risks double-incrementing. Recommend extracting n++ as a standalone statement before the ternary and using n in the string interpolation.

toCuratedErrorCode default silently maps unknown codes to collection_failed

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

The default branch is the right sentinel for preventing information disclosure, but it also silently swallows future error codes added to the pipeline. New codes (e.g. RATE_LIMIT_EXCEEDED) will appear to consumers as collection_failed without any observability. The switch should be kept in sync with the CfeErrorCode enum, ideally with the compile-time assertNever guard mentioned in the tests lens.

periodByRpu may use stale period if bills query returns multiple rows per RPU

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

The loop inserts into periodByRpu on every bill with a non-null rpu, and later writes overwrite earlier ones. If listLatestPaymentStatusByContractPublicIds returns multiple rows per RPU, the last writer wins — which may not be the most recent bill. Verify the query guarantees at most one row per RPU (i.e. is already deduplicated at the DB level).

contractPublicIds security invariant is doc-only, not type-enforced

domains/utility/src/bill/bill.queries.ts:1016

The new JSDoc correctly documents the tenancy precondition but relies on caller discipline. A branded type AccessibleContractPublicIds returned by getAccessibleContractPublicIds and accepted by listLatestPaymentStatusByContractPublicIds would make the invariant compiler-enforced.

getLatestPerPipelineByRpus — verify strict (orgId, rpu) join, not rpu-only filter

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

loadMonitoringHealthByRpu receives both orgId and rpus as caller-supplied parameters. The security guarantee depends on getLatestPerPipelineByRpus filtering jobs by (orgId AND rpu IN rpus), not just rpu IN rpus. If the query uses only the RPU list as the filter, a caller who supplies RPUs belonging to another org could retrieve those org's job status. Auditors should verify the query performs a strict (org_id, rpu) join.

contractPublicIds tenancy gate is caller-convention, not structurally enforced

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

listLatestPaymentStatusByContractPublicIds performs no org re-check — the org-scoping is caller's responsibility (documented in the new bill.queries.ts comment). Now that the function is extracted to a reusable utility, future call sites may omit the getAccessibleContractPublicIds pre-filter and silently return cross-org bill data. Consider a branded type (AccessibleContractPublicIds) or encapsulating the gate inside loadMonitoringHealthByRpu.

Verify job_id is ULID-based public ID, not internal UUID

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

collect.publicId is exposed as job_id in the health payload. If it follows the project's {3-char prefix}_{ULID} convention this is safe. If it is an internal auto-increment or raw UUID, sequential enumeration becomes trivial. Confirm CfeJob uses a ULID-based public ID.

Verify no raw error object leaks alongside curated error_code

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

toCuratedErrorCode correctly redacts internal error codes, but confirm the MonitoringHealth shape returned to the consumer only exposes error_code (the curated string) and not any raw error field from the collect job object. A response mapper that spreads the collect object could inadvertently include the raw code.

File-header JSDoc describes WHAT, not WHY

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

The multi-line file header restates what the code does (mirrors file-read, keeps handler thin, does a join). The only non-obvious constraint is the RLS caveat. Per project convention, comments should explain WHY (hidden constraint), not WHAT. Remove the first two paragraphs; the module name and the file-read analogy are derivable from reading the code.

toCuratedErrorCode JSDoc describes WHAT rather than WHY

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

The comment says 'Map a raw internal CfeErrorCode to a curated public code' — which the function name already conveys. The genuinely non-obvious WHY would be: why these specific codes are collapsed together (e.g. why four session codes become one authentication_failed), and why catch-all collection_failed is acceptable rather than forwarding unknown codes. Replace with that rationale.

switch cases formatted as multi-case same-line chains

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

Collapsing multiple case labels onto a single line (e.g. case 'INVALID_CREDENTIALS': case 'LOGIN_FAILED': ...) is non-standard and produces illegible diffs. Use one case per line with fall-through, or replace with a lookup table (const CODE_MAP: Record<string, PublicErrorCode> = {...}) which also makes coverage reviews and additions easier.

MonitoringHealth type could live with the API types package

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

MonitoringHealth is an intermediate API-tier type. If it's referenced in tests or mappers in the future, its natural home is @batu/api/types/public/monitoring.public-api-types. Low urgency while it's only used in this util.

toCuratedErrorCode() has no unit tests

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

This pure mapping function has a closed output set and a default/fallback branch — it's precisely the kind of function that benefits most from exhaustive unit tests. Without them: a new CfeErrorCode added upstream silently falls through to the wrong public label, and the mapping of e.g. SESSION_EXPIRED → authentication_failed is untestably assumed. Needed: one test per output bucket, one test for an unrecognized code → collection_failed, and an exhaustiveness guard against future CfeErrorCode additions.

Zip dedup: file whose natural name ends in _2 not covered by a test

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

The stated motivation for rewriting from Map to Set was that the old approach would collide when a file's natural name ends in _2. No test asserts this exact scenario, so a regression to the old approach would go undetected. Add a test with files ['rpu1_jan_2.pdf', 'rpu1_jan.pdf'] where the second duplicate would resolve to rpu1_jan_2.pdf and assert both are present as distinct entries.

Zip dedup: no-extension filenames are an untested edge case

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

The while-loop has a branch for dot <= 0 (no extension) that appends _${n} to the whole name. No test exercises duplicate files with no extension. Hidden files like '.gitignore' (where lastIndexOf('.') === 0) would also hit the wrong branch. Add tests for both the no-extension case and the leading-dot case.

Zip dedup: 3+ duplicates of the same filename not tested

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

The while-loop handles arbitrarily many duplicates but no test exercises three or more files with the same base name. Add a test with 4 identical filenames to confirm the loop terminates correctly and produces _2, _3, _4 suffixes without gaps or collisions.

Zip dedup: all-same-period multi-RPU batch not tested

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

When all files in a batch share the same period (e.g. 2 RPUs × 2026-05.pdf), every entry after the first requires dedup. A test with N identical-period entries from different RPUs should assert that the output set has N distinct names.

loadMonitoringHealthByRpu() join logic has no integration test

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

The parallel fetch + in-memory join by RPU is new coordinator logic. Edge cases that need integration coverage: RPU in jobs but absent from bills (null latestPeriodEnd), RPU in bills but absent from jobs (null lastCollection), and both present — happy path. Without DB-level tests these nullability mismatches surface as runtime errors in production.

toCuratedErrorCode: no compile-time exhaustiveness guard

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

If CfeErrorCode is a TypeScript union type, add an assertNever(code) call in the default branch of the test file's switch. This turns any new CfeErrorCode addition into a compile-time test failure, making the mapping self-maintaining.

periodByRpu construction uses imperative for-loop; declarative Map() is cleaner

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

The for loop building periodByRpu can be replaced with: new Map(latestBills.filter(b => b.rpu).map(b => [b.rpu!, b.periodEnd])). This eliminates the mutable intermediate and makes the data flow declarative.

Deduplication loop recomputes lastIndexOf on every collision — hoist it

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

base never changes inside the while loop, so lastIndexOf('.') returns the same value every iteration. Hoist const dot = base.lastIndexOf('.') above the loop to avoid redundant work and clarify intent.

toCuratedErrorCode signature accepts string | null but callers already null-coalesce

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

The only call site passes collect.error?.code ?? null, so null is guaranteed by the caller. Narrowing the signature to (code: string): string makes the function a pure string→string mapping, easier to test and reason about in isolation. Move the null-coalesce to the call site.