feat(freshness): surface seed health in risk panel#3493
Conversation
|
@lspassos1 is attempting to deploy a commit to the World Monitor Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR surfaces backend seed-health metadata from Confidence Score: 4/5Safe to merge; only P2 findings present, both isolated to StrategicRiskPanel.refreshHealthFreshness(). All findings are P2. The dead src/components/StrategicRiskPanel.ts — refreshHealthFreshness throttle stamping and dead catch handler. Important Files Changed
Sequence DiagramsequenceDiagram
participant Panel as StrategicRiskPanel
participant HF as health-freshness.ts
participant API as /api/health
participant DF as dataFreshness (singleton)
participant UI as render()
Panel->>Panel: refresh()
Panel-->>HF: void refreshHealthFreshness() [non-blocking]
Panel->>DF: getSummary()
Panel->>UI: render() → renderFreshnessSurface()
UI->>DF: getAllSources()
DF-->>UI: sources (status recalculated)
Note over HF,API: runs concurrently
HF->>API: GET /api/health
API-->>HF: { checkedAt, checks: { ... } }
HF->>HF: map check names → DataSourceId[]
HF->>HF: deduplicate (worst-status wins per source)
HF->>DF: recordSeedHealth(updates)
DF->>DF: calculateStatus() with maxStaleMin cadence
DF-->>Panel: notifyListeners() → triggers next refresh
Reviews (1): Last reviewed commit: "fix(freshness): decouple health polling ..." | Re-trigger Greptile |
|
|
||
| public async refresh(): Promise<boolean> { | ||
| void this.refreshHealthFreshness().catch((error) => { |
There was a problem hiding this comment.
Dead
.catch() handler — never fires
refreshHealthFreshness() has its own try/catch that swallows every exception without rethrowing, so the promise it returns can never reject. The .catch() registered here is unreachable dead code and will mislead future readers into thinking errors can surface from this call.
| public async refresh(): Promise<boolean> { | |
| void this.refreshHealthFreshness().catch((error) => { | |
| void this.refreshHealthFreshness(); |
| if (now - this.lastHealthFreshnessRefreshAt < 60_000) return; | ||
| this.lastHealthFreshnessRefreshAt = now; |
There was a problem hiding this comment.
Timestamp stamped before fetch — failed requests lock out retries for 60 s
this.lastHealthFreshnessRefreshAt = now is written before await refreshDataFreshnessFromHealth(...). If the request fails (network error, non-200, abort), the timestamp is still recorded and the next 60 seconds of refreshHealthFreshness() calls return early, silently skipping the retry window. Setting the timestamp only after a successful call would allow faster recovery when /api/health is momentarily unavailable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44d6ff7399
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const payload = await resp.json() as HealthResponse; | ||
| const checkedAtMs = payload.checkedAt ? Date.parse(payload.checkedAt) : Date.now(); | ||
| const updatesBySource = new Map<DataSourceId, SeedHealthUpdate>(); | ||
|
|
||
| for (const [checkName, check] of Object.entries(payload.checks ?? {})) { |
There was a problem hiding this comment.
Propagate top-level REDIS_DOWN health response to sources
refreshDataFreshnessFromHealth() only consumes payload.checks, but /api/health returns a top-level { status: 'REDIS_DOWN' } with no checks when Redis is unavailable. In that path this code applies zero updates and preserves prior freshness state, so the Strategic Risk panel can continue to display previously fresh sources during a full backend outage instead of surfacing an error/degraded condition.
Useful? React with 👍 / 👎.
| source.lastError = this.healthStatusIsError(update.status) ? update.status : null; | ||
| source.lastUpdate = this.healthStatusHasNoData(update.status) |
There was a problem hiding this comment.
Degrade COVERAGE_PARTIAL instead of reporting fresh status
Seed-health ingestion only maps SEED_ERROR/REDIS_* to lastError and EMPTY* to no-data; statuses like COVERAGE_PARTIAL keep lastUpdate and are then scored purely by age. When a check is coverage-partial but recent (for example, low-record chokepoint data), this path classifies it as fresh, so the new freshness UI can show a green source even though /api/health explicitly flagged degraded coverage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88b32fc05f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const freshThreshold = source.maxStaleMin ? source.maxStaleMin * 60_000 : FRESH_THRESHOLD; | ||
| const staleThreshold = source.maxStaleMin ? source.maxStaleMin * 2 * 60_000 : STALE_THRESHOLD; | ||
| const veryStaleThreshold = source.maxStaleMin ? source.maxStaleMin * 3 * 60_000 : VERY_STALE_THRESHOLD; | ||
| if (age <= freshThreshold) return source.healthStatus === 'COVERAGE_PARTIAL' ? 'stale' : 'fresh'; |
There was a problem hiding this comment.
Treat STALE_SEED checks as degraded freshness
calculateStatus() only downgrades recent data when healthStatus === 'COVERAGE_PARTIAL', so STALE_SEED is ignored when age is within the fresh window. /api/health can emit STALE_SEED without seedAgeMin (e.g. missing seed-meta), and recordSeedHealth() then preserves the prior lastUpdate; that combination leaves the source marked fresh even though backend health explicitly says the seed is stale. This causes false-green freshness in exactly the stale-seed failure mode this feature is meant to surface.
Useful? React with 👍 / 👎.
| await refreshDataFreshnessFromHealth({ signal: this.signal }); | ||
| this.lastHealthFreshnessRefreshAt = Date.now(); | ||
| } catch (error) { |
There was a problem hiding this comment.
Throttle health refresh attempts on failure
lastHealthFreshnessRefreshAt is only advanced after a successful health fetch. If /api/health is failing, each subsequent refresh() call retries immediately because the timestamp never moves, and this panel refreshes often via the dataFreshness subscription. That can create repeated concurrent requests and noisy logs during outages, adding avoidable load while the backend is already unhealthy.
Useful? React with 👍 / 👎.
|
Follow-up pushed in Changes:
Validation:
|
Summary
Surfaces seed-health freshness in the Strategic Risk panel by ingesting
/api/healthcadence metadata into the existingdataFreshnesstracker. Analysts get a compact data freshness signal without blocking the main risk refresh path.cc @koala73
Refs #3296
Type of change
Affected areas
/api/*)Root cause
The frontend freshness tracker was mostly session-local. The backend already knows seed age, cadence, and health state, but that signal was not visible in the Strategic Risk panel, so stale or missing seeded data could be hard to distinguish from genuinely quiet conditions.
Changes
refreshDataFreshnessFromHealth()to fetch/api/health, map health checks to frontend data sources, and record cadence-aware seed health.dataFreshnesswith seed-health metadata, per-sourcemaxStaleMin, and error/no-data handling for health statuses.StrategicRiskPanelwith source state and last-update timing./api/health.Validation
npx tsx --test tests/data-freshness-health.test.mtsnpm run typechecknpx biome lint src/components/StrategicRiskPanel.ts src/locales/en.json tests/data-freshness-health.test.mtsgit diff --checkRisk
Low to moderate. The health fetch is additive and non-blocking; if it fails, existing session-local freshness remains in use. The mapping guard test should catch future drift between frontend source IDs and
/api/healthcheck names.