fix/next-check-ui
needs attentionviewing older commita52a19d · fullPR #313reviewed 2026-07-15 23:33 UTC2H · 5M · 3L- Purpose
- The Contratos pipeline strip was showing the raw `nextExpectedBillDate` as 'Próxima', which could sit days in the past once a CFE bill ran late, making the healthy monitoring engine look broken.
- Goal
- Show when the daily cron will actually fire next for each RPU row — a value that can never be in the past — by mirroring the backend due-gate logic in a route-local pure helper.
- Sub-goals
- Pure helper module next-check.ts mirroring collect and payment due gates
- PipelineCell wired to use computed cron-fire time instead of raw expected-bill date
- Cadence label derived from real billing cycle instead of hardcoded 'Mensual'
- TWIN cross-reference comments added at both backend constant sites and CDK cron rules
- What
- New `_lib/next-check.ts` with 3 pure functions + 16 tests; `PipelineCell.tsx` wired to use them; comment-only twin cross-refs in backend queries and CDK lambda stack.
- Why
- The raw `nextExpectedBillDate` field stays fixed when a bill is late; the real next-check time is always the next daily cron tick, which is always in the future. Frontend-side prediction needs no API/schema change since `nextExpectedBillDate` is already live-streamed via CDC.
- Areas
- apps/platform/.../bills/_lib/next-check.ts+108−0apps/platform/.../bills/_lib/__tests__/next-check.test.ts+128−0apps/platform/.../contratos/_components/PipelineCell.tsx+22−3domains/utility (comment-only twin refs)+8−0infra/cdk/...lambda.stack.ts (comment-only twin refs)+4−0
- Blast
- 6 files, +270/-3 lines. UI-only change to one component; backend and CDK are comment-only. Zero schema/API/migration impact.
Findings · 10
correctness1
`computeNextPaymentCheckAt` return type `string | null` but never returns null
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/next-check.ts:83
Both branches return a string (`dueTick.toISOString()` or `nextDailyTick(...).toISOString()`). The spurious `| null` propagates to `PipelineCell` where `nextRun` is `string | null`, forcing a `!` non-null assertion at the render site. Narrowing to `string` removes the assertion and aligns with `computeNextCollectCheckAt`'s return type.
conventions1
Multi-paragraph per-function JSDoc blocks violate one-short-line max rule
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/next-check.ts:47
CLAUDE.md prohibits multi-paragraph docstrings; the module-level TWINS block (justified: documents hidden constraints) is fine, but each of the three exported functions has a 6-12 line JSDoc that narrates the branching logic in prose already evident from the code. Collapse each to a single terse line (or remove) — the TWINS block already covers the non-obvious WHY.
tests5
Payment 08:00 UTC exact-tick boundary not tested
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/__tests__/next-check.test.ts
The collect suite has a dedicated test for `now = T14:00:00Z` (boundary rolls to next day via `<=`). The payment suite has no mirror: `now = T08:00:00Z` with a paid+past-gate case never exercises the exact-equality rollover. An off-by-one reintroduced in `nextDailyTick`'s comparison would pass the payment tests undetected.
Invalid date string (null return from tickOnDueDate) path never exercised
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/__tests__/next-check.test.ts
`tickOnDueDate` returns `null` for strings that fail the regex or produce NaN. Both callers guard with `dueTick &&` before using the result, falling back to `nextDailyTick`. No test passes a garbage string like `'not-a-date'`; deleting the guard would cause a throw rather than a graceful fallback, and no test would catch it.
`deriveCollectCadenceLabel` 45-day boundary not tested
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/__tests__/next-check.test.ts
Existing tests use 31d (Mensual) and 62d (Bimestral) — well inside each bucket. No test covers `cycleDays = 45` (should be Bimestral) or `cycleDays = 44` (should be Mensual). An accidental change from `>= 45` to `> 45` would silently misclassify valid 45-day cycles.
`computeNextPaymentCheckAt` never-returns-null invariant not locked in
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/__tests__/next-check.test.ts
The declared return type is `string | null` but the function always returns a string. No test explicitly asserts non-null across all input combinations. If a future refactor introduced a real null return path to match the declared type, callers piping the result into `formatDateTime(nextRun!)` would throw at runtime.
Reversed date order to `deriveCollectCadenceLabel` is untested
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/__tests__/next-check.test.ts
If `lastBillDate` is after `nextExpectedBillDate` (data corruption or swapped args), `cycleDays` is negative and `Math.round(negative)` falls below 45 → returns `'Mensual'` silently. No test documents whether this silent-default behavior is intentional.
improvement3
Single `GRACE_DAYS` constant should be split to mirror backend's two independent constants
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/next-check.ts:24
The backend uses `COLLECT_GRACE_DAYS = 3` (monitoring-subscription.queries.ts) and `PS_GRACE_DAYS = 3` (payment-monitoring-subscription.queries.ts) as separately named constants. The frontend merges them into one `GRACE_DAYS`. If either backend value drifts independently the frontend silently computes wrong times for one pipeline; naming them `COLLECT_GRACE_DAYS`/`PAYMENT_GRACE_DAYS` makes future drift visible.
`cadence: string` prop type in StatusReadout should be the literal union
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/_components/PipelineCell.tsx:941
The value is always `'Mensual' | 'Bimestral' | 'Diaria'`. Tightening to that union (or exporting `CollectCadenceLabel` from `next-check.ts`) would catch future mismatches at the call site.
`nextRun!` non-null assertion is a downstream consequence of the wrong return type
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/_components/PipelineCell.tsx:1077
The `formatRelativeShort(nextRun!)` assertion only exists because of the `string | null` return type on `computeNextPaymentCheckAt`. Fixing that return type removes the need for this assertion — the `showNext && nextRun` guard already ensures truthiness before the JSX branch renders.