fix/cfe-inputs
needs attentionviewing older commite367b60 · incrementalPR #287reviewed 2026-07-09 18:25 UTC0H · 2M · 2L · 2I- Purpose
- Fix silent discard of caller-supplied service_name and latest_total in CFE collect pipeline; confirmed live on Tiendas Neto (BAT-268)
- Goal
- Honor caller inputs end-to-end: service_name persisted via contract update, latest_total threaded through cacheCheckShell; confidence-gate the stored-total fallback; add LATEST_TOTAL_REQUIRED user-fixable error code; public /v1/jobs API parity
- Sub-goals
- SG-1: Plumb callerLatestTotal through decideCacheStatus + cacheCheckShell
- SG-2: Confidence gate — stale stored total → null instead of wrong number
- SG-3: LATEST_TOTAL_REQUIRED error code + UI prompt + public API surface
- SG-4: service_name applied to existing contracts via updateServiceNameByRpuShell before collect
- SG-5 (this commit): Refactor — extract isBillTotalTrustworthy, named UpdateServiceNameByRpuResult, service_name length guard, test edge cases
- What
- Third commit (loop-review follow-up #2): extracts isBillTotalTrustworthy as a named pure function, exports UpdateServiceNameByRpuResult interface, adds MAX_SERVICE_NAME_LENGTH=200 guard in public-v1-validation.ts, adds boundary tests for latest_total cap and service_name length, fixes a test to use TOTAL_MISMATCH directly, adds REGISTRATION_ prefix-stripping test
- Why
- Address second loop-review pass findings: named extraction over inline computation, service_name guard before DB write, improved test clarity
- Areas
- apps/platform+45−3domains/utility+45−22services/utility+15−3
- Blast
- 6 files changed across 3 packages; pure refactor + boundary tests — no behavior change to the happy path, guard added at the API edge only
Findings · 6
conventions1
UpdateServiceNameByRpuResult missing from barrel index
domains/utility/src/utility-contract/index.ts
Defined and exported from utility-contract.shells.ts but not re-exported in index.ts. All other *Result types (CreateContractResult, UpdateContractResult, DeleteContractResult, RestoreContractResult, EnsureContractForTariffResult, EnsureContractFromBillsResult) are listed. Add `type UpdateServiceNameByRpuResult` to the existing shells block in index.ts.
tests4
TOTAL_MISMATCH + hadTrustedTotal=true no longer tests real-world prefixed input
services/utility/bills/cfe/__tests__/unit/handlers/collector-lambda.test.ts:86
Changed from `REGISTRATION_TOTAL_MISMATCH` to `TOTAL_MISMATCH`. Production callers always pass the `REGISTRATION_` prefixed code (documented at collector.lambda.ts:937). The prefix-stripping test added covers SERVICE_NAME_MISMATCH but not TOTAL_MISMATCH + hadTrustedTotal=true. Consider changing back to the prefixed form, or add a parallel test with `REGISTRATION_TOTAL_MISMATCH + hadTrustedTotal=true`.
isBillTotalTrustworthy has no direct unit test
domains/utility/src/cfe-job/__tests__/cfe-job.decisions.test.ts
Extracted as a named pure function with its own JSDoc contract; covered only indirectly through decideCacheStatus tests. A direct test would pin its contract independently and make regressions immediately locatable.
latest_total: missing one-above-cap rejection test
apps/platform/src/api/utils/__tests__/public-v1-validation.test.ts
New test confirms 100_000_000 is accepted but does not assert 100_000_001 is rejected. A boundary configured with lte vs lt could pass the at-cap test but allow over-cap values through.
service_name: no test for empty string or whitespace-only values
apps/platform/src/api/utils/__tests__/public-v1-validation.test.ts
ContractServiceNameSchema uses min(1) + trim(); these edge cases are not covered by the new tests.
improvement1
MAX_SERVICE_NAME_LENGTH duplication is intentional and well-documented
apps/platform/src/api/utils/public-v1-validation.ts
The comment correctly acknowledges the source (ContractServiceNameSchema.max(200)) and the isolation rationale. No action needed.