feat(freshness): surface seed health in risk panel#31
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces health-driven data-freshness ingestion and UI surfacing: a new health-freshness service fetches health checks, maps them to frontend data sources, records cadence-aware seed health into data-freshness, and StrategicRiskPanel displays a compact freshness surface and enhanced badge info; includes locale additions and tests. Changes
Sequence DiagramsequenceDiagram
participant UI as StrategicRiskPanel
participant HealthSvc as HealthFreshnessSvc
participant HTTP as Health Endpoint
participant Core as DataFreshnessCore
participant Listeners as UIListeners
UI->>HealthSvc: trigger refresh (rate-limited, non-blocking)
HealthSvc->>HTTP: GET /api/health (or resolved URL)
HTTP-->>HealthSvc: JSON payload (checkedAt, checks...)
HealthSvc->>HealthSvc: map checks → SeedHealthUpdate candidates
HealthSvc->>HealthSvc: dedupe by statusRank, stalenessRatio
HealthSvc->>Core: recordSeedHealth(updates)
Core->>Core: update DataSourceState (status, itemCount, lastUpdate, maxStaleMin, healthStatus)
Core->>Listeners: emit updates
Listeners->>UI: re-render freshness surface & badges
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/data-freshness-health.test.mts (1)
15-61: Good focused test, but consider test isolation.The test effectively covers the core contract: status mapping, field population, and
lastUpdatecalculation. However,dataFreshnessis a singleton, so test state may leak between runs in larger test suites.Consider resetting or isolating state if more tests are added:
🧪 Consider adding setup/teardown for isolation
+import { beforeEach } from 'node:test'; + describe('health freshness ingestion', () => { + // If dataFreshness exposed a reset method, call it here + // beforeEach(() => dataFreshness.reset()); + it('hydrates dataFreshness from /api/health cadence metadata', async () => {If this becomes part of a larger test suite, consider whether
dataFreshnessshould expose a reset method for testing, or if the test should verify relative state changes rather than absolute values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/data-freshness-health.test.mts` around lines 15 - 61, The test mutates the singleton dataFreshness via refreshDataFreshnessFromHealth which can leak state across tests; modify the test to isolate state by resetting or recreating the singleton before/after the case—e.g., call a reset/clear method on dataFreshness (or reinstantiate it) and/or add setup/teardown hooks around the test that clear dataFreshness so refreshDataFreshnessFromHealth and dataFreshness.getSource('gdelt'|'weather'|'cyber_threats') operate on a known clean state.src/components/StrategicRiskPanel.ts (2)
359-384: LGTM with a minor observation.The filtering, sorting by severity, and XSS protection with
escapeHtmlare well-implemented. The status ordering map correctly prioritizes error states.One small thing: on line 375, if
source.healthStatusis undefined, the title will show "undefined". Consider using a fallback:title="${escapeHtml(source.healthStatus ?? source.status)}"This is already partially handled since
source.statusis always defined, but using it as a fallback would be cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/StrategicRiskPanel.ts` around lines 359 - 384, In renderFreshnessSurface(), change the title attribute construction to use a fallback when source.healthStatus is undefined (use source.healthStatus ?? source.status) so the title never becomes the string "undefined"; update the template expression where title="${escapeHtml(source.healthStatus || source.status)}" (or currently title="${escapeHtml(source.healthStatus)}") to use the nullish coalescing form and keep escapeHtml around the result to preserve XSS protection.
164-173: Consider logging swallowed errors for diagnostics.The silent catch is intentional per the PR design (health is additive, local freshness remains useful), but completely discarding errors makes troubleshooting harder. A debug-level log would preserve observability without alarming users.
🔧 Suggested improvement
try { await refreshDataFreshnessFromHealth({ signal: this.signal }); } catch { // Health is additive; local session freshness remains useful if it fails. + console.debug('[StrategicRiskPanel] Health freshness fetch failed (non-fatal)'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/StrategicRiskPanel.ts` around lines 164 - 173, The catch in refreshHealthFreshness silently swallows errors from refreshDataFreshnessFromHealth which hinders diagnostics; update refreshHealthFreshness to catch the error into a variable and emit a debug-level log including the error and context (e.g., function name and this.lastHealthFreshnessRefreshAt) using your component's existing logger if available (or console.debug if not) while preserving the current behavior of not surfacing the error to users; keep the rest of the logic (throttling by lastHealthFreshnessRefreshAt and using this.signal) unchanged.src/services/data-freshness.ts (1)
308-314: Extract health status strings to constants to reduce duplication and ensure consistency.The status strings in these helper methods correctly match the backend API values defined in
api/health.js, but they appear as magic strings in bothdata-freshness.tsandhealth-freshness.ts. Creating a shared constant file (e.g.,src/constants/health-status.ts) would centralize these values and make future API changes easier to track across the frontend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/data-freshness.ts` around lines 308 - 314, Extract the hard-coded health status strings into a shared constants module (e.g., create and export constants from src/constants/health-status.ts) and replace the magic strings used in DataFreshnessService.private methods healthStatusIsError and healthStatusHasNoData with references to those constants; also update the corresponding usages in health-freshness.ts to import and use the same constants so both files reference a single source of truth (export named constants like SEED_ERROR, REDIS_PARTIAL, EMPTY, EMPTY_DATA, EMPTY_ON_DEMAND and import them where needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/StrategicRiskPanel.ts`:
- Around line 359-384: In renderFreshnessSurface(), change the title attribute
construction to use a fallback when source.healthStatus is undefined (use
source.healthStatus ?? source.status) so the title never becomes the string
"undefined"; update the template expression where
title="${escapeHtml(source.healthStatus || source.status)}" (or currently
title="${escapeHtml(source.healthStatus)}") to use the nullish coalescing form
and keep escapeHtml around the result to preserve XSS protection.
- Around line 164-173: The catch in refreshHealthFreshness silently swallows
errors from refreshDataFreshnessFromHealth which hinders diagnostics; update
refreshHealthFreshness to catch the error into a variable and emit a debug-level
log including the error and context (e.g., function name and
this.lastHealthFreshnessRefreshAt) using your component's existing logger if
available (or console.debug if not) while preserving the current behavior of not
surfacing the error to users; keep the rest of the logic (throttling by
lastHealthFreshnessRefreshAt and using this.signal) unchanged.
In `@src/services/data-freshness.ts`:
- Around line 308-314: Extract the hard-coded health status strings into a
shared constants module (e.g., create and export constants from
src/constants/health-status.ts) and replace the magic strings used in
DataFreshnessService.private methods healthStatusIsError and
healthStatusHasNoData with references to those constants; also update the
corresponding usages in health-freshness.ts to import and use the same constants
so both files reference a single source of truth (export named constants like
SEED_ERROR, REDIS_PARTIAL, EMPTY, EMPTY_DATA, EMPTY_ON_DEMAND and import them
where needed).
In `@tests/data-freshness-health.test.mts`:
- Around line 15-61: The test mutates the singleton dataFreshness via
refreshDataFreshnessFromHealth which can leak state across tests; modify the
test to isolate state by resetting or recreating the singleton before/after the
case—e.g., call a reset/clear method on dataFreshness (or reinstantiate it)
and/or add setup/teardown hooks around the test that clear dataFreshness so
refreshDataFreshnessFromHealth and
dataFreshness.getSource('gdelt'|'weather'|'cyber_threats') operate on a known
clean state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 966a2bae-73d6-4e8f-bfc2-19f9d9a9310a
📒 Files selected for processing (5)
src/components/StrategicRiskPanel.tssrc/locales/en.jsonsrc/services/data-freshness.tssrc/services/health-freshness.tstests/data-freshness-health.test.mts
|
Added the requested mapping drift guard in Additional coverage:
Validation re-run:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/health-freshness.ts`:
- Around line 76-79: The stalenessRatio function currently uses falsy checks and
treats 0 the same as null/undefined; change the guards to explicit
null/undefined checks for update.seedAgeMin and update.maxStaleMin (e.g., check
=== null or === undefined) so valid zero values are preserved; then handle the
zero-divide case: if maxStaleMin === 0 return 0 when seedAgeMin === 0 otherwise
return Infinity (or another distinct large value) so ties are ranked correctly;
keep these changes inside the stalenessRatio function.
- Around line 56-73: statusRank currently omits REDIS_DOWN and OK so REDIS_DOWN
is treated as least severe; update the statusRank function to add an explicit
case for 'REDIS_DOWN' returning the highest/severe rank (same level as
'SEED_ERROR'/'REDIS_PARTIAL', e.g., 5) and add an explicit case for 'OK'
returning 0; keep the default as a safe fallback (0) for unknown statuses but
ensure 'statusRank' now includes the new 'REDIS_DOWN' and 'OK' cases to avoid
masking critical errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b7c77093-36b7-47bf-84f0-d8824da5a9a2
📒 Files selected for processing (2)
src/services/health-freshness.tstests/data-freshness-health.test.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/data-freshness-health.test.mts
|
Addressed the bot review items on health freshness selection:
Validation run locally:
|
|
Correction to the previous validation note: the bot review items are addressed in a21ac24. Changes:
Validation run locally:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/StrategicRiskPanel.ts`:
- Line 87: The panel refresh should not be blocked by the non-fatal additive
health fetch: remove the awaiting of refreshHealthFreshness and invoke it as a
background task instead (e.g., call this.refreshHealthFreshness() without await
and attach a .catch to swallow/log errors) at the site where you currently have
await this.refreshHealthFreshness(); and at the similar invocation block around
lines 164-173; ensure you do not leave an unhandled promise by catching/logging
errors so the main refresh path proceeds immediately.
- Around line 144-146: The badgeDetail string in StrategicRiskPanel is hardcoded
and not localized; update the construction of badgeDetail (where badgeDetail is
set using this.freshnessSummary) to use the app's i18n/translation utility
(e.g., t or translate) to generate the pluralized/localized "sources" label and
interpolate this.freshnessSummary.activeSources and totalSources into that
localized string so the UI uses the proper locale-aware text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ac97117-2a07-4457-a093-789020cffb8b
📒 Files selected for processing (4)
src/components/StrategicRiskPanel.tssrc/services/data-freshness.tssrc/services/health-freshness.tstests/data-freshness-health.test.mts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/data-freshness-health.test.mts
- src/services/data-freshness.ts
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a21ac24f6a
ℹ️ 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".
| private lastRiskFingerprint = ''; | ||
|
|
||
| public async refresh(): Promise<boolean> { | ||
| await this.refreshHealthFreshness(); |
There was a problem hiding this comment.
refresh() now blocks on await this.refreshHealthFreshness(), so every strategic-risk refresh path (initial render, manual refresh, and freshness-listener refreshes) is gated by a network call to /api/health even though that health sync is intended to be additive/non-fatal. When /api/health is slow (e.g., Redis pipeline timeout path), the panel can appear frozen for seconds before showing risk data, which is a user-visible regression in responsiveness. Run this fetch in the background (or enforce a short timeout) so core risk rendering is not serialized behind health polling.
Useful? React with 👍 / 👎.
|
Addressed the latest bot findings in 44d6ff7. Changes:
Validation run locally:
|
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".
| if (now - this.lastHealthFreshnessRefreshAt < 60_000) return; | ||
| try { | ||
| await refreshDataFreshnessFromHealth({ signal: this.signal }); | ||
| this.lastHealthFreshnessRefreshAt = Date.now(); |
There was a problem hiding this comment.
The 60-second gate is only keyed off lastHealthFreshnessRefreshAt, but that value is written only after await refreshDataFreshnessFromHealth(...) succeeds. When /api/health is slow or returning errors, every subsequent refresh() call (including ones triggered by the freshness listener) can start another fetch, so requests overlap and retry far more often than once per minute right when the dependency is unhealthy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/StrategicRiskPanel.ts`:
- Line 37: The throttle is bypassed because lastHealthFreshnessRefreshAt is only
updated after the awaited health fetch in StrategicRiskPanel.refresh(), allowing
concurrent calls to proceed; fix by marking the refresh as started before the
await (e.g., set lastHealthFreshnessRefreshAt = Date.now() or flip an
isRefreshing flag at the top of refresh()) and only reset/adjust after the
awaited operation completes or fails, or add a short mutex/guard (isRefreshing)
in refresh() to return early when true; update any error/failure paths to ensure
the guard/timestamp is correctly maintained so retries remain throttled.
In `@src/services/data-freshness.ts`:
- Line 7: The service file imports getCSSColor from the UI utils which violates
layering; remove the import from src/services/data-freshness.ts and eliminate
direct dependency on '@/utils/theme-colors' by either (A) reading the needed
color values from an allowed config/type module (move the color mapping into a
shared config exported from the config/types layer) and import that instead, or
(B) change the service API (e.g., the function(s) that currently call
getCSSColor) to accept the color(s) as parameters from the caller; update all
usages of getCSSColor in functions within data-freshness.ts accordingly and add
any new config exports in the allowed layer so the service only imports from
types/config.
- Around line 153-155: The current logic in data-freshness.ts drops a deliberate
zero cadence because it checks update.maxStaleMin > 0 and later treats the value
as falsy; change the checks to preserve 0 by testing for a numeric value (e.g.,
typeof update.maxStaleMin === 'number') when assigning maxStaleMin and in the
later usage (lines around where maxStaleMin is evaluated at 299-301), so zero is
accepted and not replaced by legacy thresholds; update any conditional branches
that rely on truthiness to explicitly check for typeof ... === 'number' (refer
to the update.maxStaleMin variable and the maxStaleMin local) to ensure
deterministic zero-staleness handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bf13ec3-f04a-4c78-88e0-2cad3e6dbd53
📒 Files selected for processing (4)
src/components/StrategicRiskPanel.tssrc/services/data-freshness.tssrc/services/health-freshness.tstests/data-freshness-health.test.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/health-freshness.ts
| private breakingAlerts: Map<string, { threatLevel: 'critical' | 'high'; timestamp: number }> = new Map(); | ||
| private boundOnBreaking: ((e: Event) => void) | null = null; | ||
| private breakingExpiryTimer: ReturnType<typeof setTimeout> | null = null; | ||
| private lastHealthFreshnessRefreshAt = 0; |
There was a problem hiding this comment.
The 60s throttle is bypassable under overlap/failure.
Line 169 checks a timestamp that is only updated after a successful await on Line 172. Multiple refresh() calls can launch concurrent health requests, and repeated failures are effectively unthrottled.
Proposed fix
private lastHealthFreshnessRefreshAt = 0;
+ private healthFreshnessInFlight: Promise<void> | null = null;
@@
private async refreshHealthFreshness(): Promise<void> {
const now = Date.now();
- if (now - this.lastHealthFreshnessRefreshAt < 60_000) return;
+ if (this.healthFreshnessInFlight) return this.healthFreshnessInFlight;
+ if (now - this.lastHealthFreshnessRefreshAt < 60_000) return;
+ this.lastHealthFreshnessRefreshAt = now;
- try {
- await refreshDataFreshnessFromHealth({ signal: this.signal });
- this.lastHealthFreshnessRefreshAt = Date.now();
- } catch (error) {
- // Health is additive; local session freshness remains useful if it fails.
- console.debug('[StrategicRiskPanel] Health freshness fetch failed (non-fatal)', error);
- }
+ this.healthFreshnessInFlight = (async () => {
+ try {
+ await refreshDataFreshnessFromHealth({ signal: this.signal });
+ } catch (error) {
+ // Health is additive; local session freshness remains useful if it fails.
+ console.debug('[StrategicRiskPanel] Health freshness fetch failed (non-fatal)', error);
+ } finally {
+ this.healthFreshnessInFlight = null;
+ }
+ })();
+ await this.healthFreshnessInFlight;
}Also applies to: 167-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/StrategicRiskPanel.ts` at line 37, The throttle is bypassed
because lastHealthFreshnessRefreshAt is only updated after the awaited health
fetch in StrategicRiskPanel.refresh(), allowing concurrent calls to proceed; fix
by marking the refresh as started before the await (e.g., set
lastHealthFreshnessRefreshAt = Date.now() or flip an isRefreshing flag at the
top of refresh()) and only reset/adjust after the awaited operation completes or
fails, or add a short mutex/guard (isRefreshing) in refresh() to return early
when true; update any error/failure paths to ensure the guard/timestamp is
correctly maintained so retries remain throttled.
| */ | ||
|
|
||
| import { getCSSColor } from '@/utils'; | ||
| import { getCSSColor } from '@/utils/theme-colors'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep service-layer imports within the allowed dependency direction.
This import pulls from utils in a service module, which breaks the configured layering rule for src/**/*.{ts,tsx} files.
As per coding guidelines, src/**/*.{ts,tsx} must follow dependency direction where services import from types and config.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/data-freshness.ts` at line 7, The service file imports
getCSSColor from the UI utils which violates layering; remove the import from
src/services/data-freshness.ts and eliminate direct dependency on
'@/utils/theme-colors' by either (A) reading the needed color values from an
allowed config/type module (move the color mapping into a shared config exported
from the config/types layer) and import that instead, or (B) change the service
API (e.g., the function(s) that currently call getCSSColor) to accept the
color(s) as parameters from the caller; update all usages of getCSSColor in
functions within data-freshness.ts accordingly and add any new config exports in
the allowed layer so the service only imports from types/config.
| const maxStaleMin = typeof update.maxStaleMin === 'number' && update.maxStaleMin > 0 | ||
| ? update.maxStaleMin | ||
| : undefined; |
There was a problem hiding this comment.
Preserve maxStaleMin = 0 instead of falling back to legacy thresholds.
Line 153 currently discards 0, and Lines 299-301 treat 0 as falsy. That makes a zero cadence behave like default 15m/2h/6h thresholds, not deterministic zero-staleness handling.
Proposed fix
- const maxStaleMin = typeof update.maxStaleMin === 'number' && update.maxStaleMin > 0
- ? update.maxStaleMin
- : undefined;
+ const maxStaleMin = typeof update.maxStaleMin === 'number' && Number.isFinite(update.maxStaleMin) && update.maxStaleMin >= 0
+ ? update.maxStaleMin
+ : source.maxStaleMin;
@@
- 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;
+ const hasCadence = source.maxStaleMin !== undefined;
+ const freshThreshold = hasCadence ? source.maxStaleMin * 60_000 : FRESH_THRESHOLD;
+ const staleThreshold = hasCadence ? source.maxStaleMin * 2 * 60_000 : STALE_THRESHOLD;
+ const veryStaleThreshold = hasCadence ? source.maxStaleMin * 3 * 60_000 : VERY_STALE_THRESHOLD;Also applies to: 299-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/data-freshness.ts` around lines 153 - 155, The current logic in
data-freshness.ts drops a deliberate zero cadence because it checks
update.maxStaleMin > 0 and later treats the value as falsy; change the checks to
preserve 0 by testing for a numeric value (e.g., typeof update.maxStaleMin ===
'number') when assigning maxStaleMin and in the later usage (lines around where
maxStaleMin is evaluated at 299-301), so zero is accepted and not replaced by
legacy thresholds; update any conditional branches that rely on truthiness to
explicitly check for typeof ... === 'number' (refer to the update.maxStaleMin
variable and the maxStaleMin local) to ensure deterministic zero-staleness
handling.
Greptile SummaryThis PR surfaces backend seed freshness from
Confidence Score: 5/5Safe to merge. The health fetch is additive, throttled, and wrapped in try/catch so failures fall back gracefully to existing session-local freshness. The change is additive — no existing data paths are removed or gated behind the health fetch. Failure is caught and logged without affecting the panel render. The worst-case merging logic in health-freshness is correct and well-tested. The only notable gap is that recordUpdate does not clear a health-stamped healthStatus, which can transiently show a live-data source as stale for up to 60 seconds after a backend recovery, but it self-corrects on the next health poll. No files require special attention. src/services/data-freshness.ts has a minor recordUpdate/healthStatus interaction worth watching in production, but it is bounded and non-breaking. Important Files Changed
Sequence DiagramsequenceDiagram
participant Panel as StrategicRiskPanel
participant HF as health-freshness
participant API as /api/health
participant DF as DataFreshnessTracker
participant UI as render()
Panel->>Panel: refresh() called
Panel-->>HF: void refreshHealthFreshness() [fire-and-forget]
Panel->>DF: getSummary()
Panel->>UI: render() with current freshnessSummary
Note over Panel,HF: async, throttled 60s
HF->>HF: check lastHealthFreshnessRefreshAt
HF->>API: GET /api/health
API-->>HF: "{ checkedAt, checks, status? }"
alt checks present
HF->>HF: statusRank + stalenessRatio merge per sourceId
HF->>DF: recordSeedHealth(updates)
else top-level REDIS_DOWN/PARTIAL, no checks
HF->>DF: recordSeedHealth(all mapped sources to error)
end
DF->>DF: calculateStatus() for each updated source
DF-->>Panel: notifyListeners() debounced refresh()
Panel->>UI: re-render with updated freshnessSummary
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/components/StrategicRiskPanel.ts:1
**IMPORTANT: `Closes #26` should be `Refs #26`**
The PR description uses `Closes #26`, which automatically closes issue #26 on merge. Per repo rules, auto-closing keywords should only be used when explicitly requested. The PR description says "Refs koala73/worldmonitor#3296" for the cross-repo reference — the same pattern should apply here. Consider updating the description to `Refs #26` to leave issue triage to the team.
### Issue 2 of 2
src/services/data-freshness.ts:120-126
`recordUpdate` clears `lastError` but leaves `healthStatus` intact. If a health check previously stamped `healthStatus = 'STALE_SEED'` on a source, any subsequent local `recordUpdate` call (e.g. a live polling response) will still have `calculateStatus` return `'stale'` — even for freshly-received data — until the next 60-second health refresh clears it. Resetting `healthStatus` here keeps the local signal authoritative when the frontend directly observes new data.
```suggestion
recordUpdate(sourceId: DataSourceId, itemCount: number = 1): void {
const source = this.sources.get(sourceId);
if (source) {
source.lastUpdate = new Date();
source.itemCount += itemCount;
source.lastError = null;
source.healthStatus = undefined;
source.status = this.calculateStatus(source);
```
Reviews (2): Last reviewed commit: "fix(freshness): debounce failed health r..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
2 participants
Summary
Surfaces existing
/api/healthseed metadata in the Strategic Risk panel so freshness reflects Redis seed age and cadence thresholds instead of only session-local updates. Closes #26. Refs koala73#3296.Root cause
The frontend
DataFreshnessTrackertracked local data updates with fixed thresholds. It did not consume the canonical seed freshness metadata already exposed by/api/health, so panel badges could understate stale or missing backend feeds.Changes
dataFreshnessusingseedAgeMinandmaxStaleMin.health-freshnessservice that maps selected/api/healthchecks to existing frontend data source IDs and keeps the worst status per source.StrategicRiskPanelto refresh health freshness at most once per minute, show active source counts in its badge, and render a compact freshness surface./api/healthfreshness ingestion.Validation
npx tsx --test tests/data-freshness-health.test.mtsnpm run typechecknpx biome lint src/services/data-freshness.ts src/services/health-freshness.ts src/components/StrategicRiskPanel.ts tests/data-freshness-health.test.mtsgit diff --checknpm run build:fullRisk
Low to medium. The health fetch is additive and throttled; if it fails, the panel falls back to existing local freshness. The main residual risk is mapping drift if new health keys are added without extending
HEALTH_CHECK_SOURCE_MAP.