mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
fix(resilience): address PR #2943 review (hoist formatter call, calendar validity)
Addresses two P2 comments from Greptile on PR #2943: 1. **Double call to formatResilienceDataVersion in the widget footer render.** The original spread expression called the formatter twice, once in the conditional guard and once inside the child text node. Because the function is pure the output is unchanged, but the regex and (now) Date parse execute twice per render. Rewrote as an IIFE that stores the result in a local `dataVersionLabel` and references it in both places. Matches the pattern used by the existing scoreInterval / baselineScore blocks in the same file. 2. **Regex validates shape but not calendar validity.** The previous `/^\d{4}-\d{2}-\d{2}$/` accepted strings like `9999-99-99`, `2024-13-45`, `2024-02-30`, and similar. A stale or corrupted Redis key could emit one, and the widget would render it without complaint. Added a defensive round-trip check: parse the string with `new Date(...)`, reject if `getTime()` is NaN, and reject if `toISOString().slice(0, 10)` does not equal the input. This catches both impossible dates (`9999-99-99`) and silent normalization (`2024-02-30` rolling to `2024-03-01`). New widget-format test added covering the calendar validity matrix: - 9999-99-99 rejected - 2024-13-45 rejected - 2024-00-15 rejected - 2024-02-30 rejected (silent roll guard) - 2024-02-31 rejected - 2024-02-29 accepted (real leap day) - 2023-02-28 accepted No runtime behavior change for well-formed inputs; under the current server-side path in `_shared.ts` the field is produced by `Date.toISOString().slice(0, 10)` which always round-trips. This is a pure defensive hardening of the consumer boundary. Testing: - npx tsx --test tests/resilience-widget.test.mts: 10/10 pass (9 existing + 1 new calendar-validity test) - npm run typecheck: clean Generated with Claude Opus 4.6 (1M context) via Claude Code + Compound Engineering v2.49.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -299,16 +299,23 @@ export class ResilienceWidget {
|
||||
formatResilienceConfidence(data),
|
||||
),
|
||||
h('span', { className: 'resilience-widget__delta' }, formatResilienceChange30d(data.change30d)),
|
||||
...(formatResilienceDataVersion(data.dataVersion)
|
||||
? [h(
|
||||
'span',
|
||||
{
|
||||
className: 'resilience-widget__data-version',
|
||||
title: 'Date the underlying source data was last refreshed by the Railway static-seed job.',
|
||||
},
|
||||
formatResilienceDataVersion(data.dataVersion),
|
||||
)]
|
||||
: []),
|
||||
...(() => {
|
||||
// Hoisted so the formatter (which runs a regex + Date parse) is
|
||||
// only invoked once per render instead of twice (guard + child).
|
||||
// Raised in review of PR #2943 for consistency with the existing
|
||||
// scoreInterval / baselineScore blocks in this file.
|
||||
const dataVersionLabel = formatResilienceDataVersion(data.dataVersion);
|
||||
return dataVersionLabel
|
||||
? [h(
|
||||
'span',
|
||||
{
|
||||
className: 'resilience-widget__data-version',
|
||||
title: 'Date the underlying source data was last refreshed by the Railway static-seed job.',
|
||||
},
|
||||
dataVersionLabel,
|
||||
)]
|
||||
: [];
|
||||
})(),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -61,10 +61,20 @@ export function formatBaselineStress(baseline: number, stress: number): string {
|
||||
|
||||
// Formats the dataVersion field (ISO date YYYY-MM-DD, sourced from the
|
||||
// seed-meta key) for display in the widget footer. Returns an empty string
|
||||
// when dataVersion is missing or malformed so the caller can skip rendering.
|
||||
// Format is stable and regex-tested by resilience-widget.test.mts.
|
||||
// when dataVersion is missing, malformed, or not a real calendar date so
|
||||
// the caller can skip rendering. Format is stable and regex + calendar
|
||||
// tested by resilience-widget.test.mts.
|
||||
const ISO_DATE_PATTERN = /^\d{4}-\d{2}-\d{2}$/;
|
||||
export function formatResilienceDataVersion(dataVersion: string | null | undefined): string {
|
||||
if (typeof dataVersion !== 'string' || !ISO_DATE_PATTERN.test(dataVersion)) return '';
|
||||
// Regex-shape is not enough: `/^\d{4}-\d{2}-\d{2}$/` accepts values like
|
||||
// `9999-99-99` or `2024-13-45`. A stale or corrupted Redis key could emit
|
||||
// one, and the widget would render it without complaint. Defensively
|
||||
// verify the string parses to a real calendar date AND round-trips back
|
||||
// to the same YYYY-MM-DD slice (so e.g. `2024-02-30` does not silently
|
||||
// become `2024-03-01`). Raised in review of PR #2943.
|
||||
const parsed = new Date(dataVersion);
|
||||
if (Number.isNaN(parsed.getTime())) return '';
|
||||
if (parsed.toISOString().slice(0, 10) !== dataVersion) return '';
|
||||
return `Data ${dataVersion}`;
|
||||
}
|
||||
|
||||
@@ -100,6 +100,23 @@ test('formatResilienceDataVersion returns empty for missing or malformed dataVer
|
||||
assert.equal(formatResilienceDataVersion('not-a-date'), '');
|
||||
});
|
||||
|
||||
test('formatResilienceDataVersion rejects regex-valid but calendar-invalid dates (PR #2943 review)', () => {
|
||||
// Regex `/^\d{4}-\d{2}-\d{2}$/` accepts these strings but they are not
|
||||
// real calendar dates. A stale or corrupted Redis key could emit one,
|
||||
// and without the round-trip check the widget would render it unchecked.
|
||||
assert.equal(formatResilienceDataVersion('9999-99-99'), '');
|
||||
assert.equal(formatResilienceDataVersion('2024-13-45'), '');
|
||||
assert.equal(formatResilienceDataVersion('2024-00-15'), '');
|
||||
// February 30th parses as a real Date in JS but not the same string
|
||||
// when round-tripped through toISOString; the round-trip check catches
|
||||
// this slip, so `2024-02-30` silently rolling to `2024-03-01` is rejected.
|
||||
assert.equal(formatResilienceDataVersion('2024-02-30'), '');
|
||||
assert.equal(formatResilienceDataVersion('2024-02-31'), '');
|
||||
// Legitimate calendar dates still pass.
|
||||
assert.equal(formatResilienceDataVersion('2024-02-29'), 'Data 2024-02-29'); // leap year
|
||||
assert.equal(formatResilienceDataVersion('2023-02-28'), 'Data 2023-02-28');
|
||||
});
|
||||
|
||||
test('baseResponse includes dataVersion (regression for T1.4 wiring)', () => {
|
||||
// Guards against a future change that accidentally drops the dataVersion
|
||||
// field from the service response shape. The scorer writes it from the
|
||||
|
||||
Reference in New Issue
Block a user