feat(frontend/recs): auto-refresh on page open when stale; drop freshness indicator + Refresh button (closes #284)#285
Conversation
…ness indicator + Refresh button (closes #284) - Add `STALE_THRESHOLD_MS = 24h` constant at module scope - Add `triggerAutoRefreshIfStale()` helper: checks freshness API and fires `refreshRecommendationsAPI()` as fire-and-forget when cache is null or older than 24h; shows in-flight / success / failure toasts - Wire the helper into `loadRecommendations()` replacing the old `renderFreshness('recommendations-freshness', loadRecommendations)` call - Remove `<section id="recommendations-freshness">` DOM node from index.html (dashboard-freshness section remains; freshness.ts is kept for it) - Import `getRecommendationsFreshness` and `refreshRecommendations` from `./api/recommendations` directly; drop the unused `renderFreshness` import - Add 5 new tests in `describe('auto-refresh on page open (#284)')`: cold cache, stale cache, fresh cache, cold+error, refresh failure - Mock `../toast` in the test file; reset freshness mock to "fresh" in beforeEach so existing tests are unaffected by the new toast calls
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAuto-refreshes recommendations on page open when cached data is >24h or absent, shows in-flight and result toasts (including collection errors), removes the freshness UI element, and adds tests and a test helper to control the auto-refresh in-flight guard. (≤50 words) ChangesAuto-Refresh on Page Open
Sequence DiagramsequenceDiagram
participant User
participant Page as Recommendations Page
participant Toast as Toast System
participant API as Recommendations API
User->>Page: open recommendations
Page->>API: getRecommendationsFreshness()
API-->>Page: last_collected_at, last_collection_error
alt cold or stale (>24h)
Page->>Toast: show "Refreshing recommendations…" (timeout: null)
Page->>API: refreshRecommendations() [async, deduped]
alt refresh succeeds
API-->>Page: refreshed results
Page->>Toast: show success summary
Page->>Page: reload recommendations list
else refresh fails
API-->>Page: rejection
Page->>Toast: show error (includes rejection message)
end
else fresh (<24h)
Note over Page: no refresh, no toast
end
alt last_collection_error present (cold/stale)
Page->>Toast: show error toast with collection error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
cristim
added
triaged
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/recommendations.ts`:
- Around line 149-153: The in-flight toast created by showToast (stored in
inFlight) should remain visible until refreshRecommendationsAPI() completes;
remove the fixed timeout (do not pass timeout: 4_000) or set timeout to
null/undefined so it doesn't auto-dismiss, and explicitly dismiss the toast
after the refresh settles by awaiting refreshRecommendationsAPI() and calling
inFlight.dismiss() (or the toast API's close method) in both success and error
paths; update the code around the inFlight variable and the
refreshRecommendationsAPI() call to ensure the toast is always cleaned up.
- Around line 125-177: triggerAutoRefreshIfStale can start multiple parallel
refreshes; add a module-scoped guard (e.g., let refreshInFlight: Promise<void> |
null) and use it inside triggerAutoRefreshIfStale to deduplicate: if
refreshInFlight exists reuse it instead of calling refreshRecommendationsAPI
again, otherwise set refreshInFlight = refreshRecommendationsAPI(), attach
.then/.catch/.finally to dismiss toasts and clear refreshInFlight when done;
update references to the in-flight toast (inFlight) so only one
info/success/error toast is shown per logical refresh and ensure refreshInFlight
is cleared in finally so future refreshes can proceed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8330a0bb-f154-405e-ba13-9fdf4efc9b99
📒 Files selected for processing (3)
frontend/src/__tests__/recommendations.test.tsfrontend/src/index.htmlfrontend/src/recommendations.ts
💤 Files with no reviewable changes (1)
- frontend/src/index.html
…dedup + persistent in-flight toast Two CR findings addressed: 1. Dedup concurrent stale-load refreshes (#285, major): add a module-level `autoRefreshInFlight: Promise<void> | null` guard. If a background refresh is already running when a second stale load fires, the second call skips the API call (and its toast) entirely. The guard is cleared in `.finally()` so the next page open after the refresh settles works normally. 2. Persistent in-flight toast (#285, minor): the "Refreshing recommendations…" toast previously had `timeout: 4_000` and disappeared after 4 s even when the refresh was still running. Changed to `timeout: null` — the `.then()`/`.catch()` paths already call `inFlight.dismiss()` so the toast stays visible until the refresh actually settles. Tests: two new cases in the `auto-refresh on page open (#284)` describe block — one asserting `refreshRecommendations` is called exactly once when two stale loads race, one asserting the in-flight toast has `timeout: null`.
CodeRabbit findings addressed — commit e1d924bFinding 1 (Major) — Dedup concurrent stale-load refreshesAdded a module-level Finding 2 (Minor) — Persistent in-flight toastChanged the in-flight "Refreshing recommendations…" toast from TestsTwo new tests in the
All 154 tests pass; @coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes.
Triggering a fresh review of the updated commits now. (◕‿◕✿) [review] |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/recommendations.ts`:
- Around line 149-152: The current isStale calculation can treat malformed
timestamps as non-stale because new Date(freshness.last_collected_at).getTime()
returns NaN; change the logic in the block that computes isStale (using
freshness.last_collected_at and STALE_THRESHOLD_MS) to first parse the timestamp
into a numeric value (e.g., const ts = new
Date(freshness.last_collected_at).getTime() or Date.parse(...)) and then treat
the entry as stale if freshness.last_collected_at is null OR Number.isNaN(ts) OR
Date.now() - ts > STALE_THRESHOLD_MS so malformed/invalid timestamps are
considered stale.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a61dc3d9-fd9a-40a7-bc8a-bd1c4bd20196
📒 Files selected for processing (2)
frontend/src/__tests__/recommendations.test.tsfrontend/src/recommendations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/tests/recommendations.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/recommendations.ts`:
- Around line 179-186: The success branch for autoRefreshInFlight currently only
calls inFlight.dismiss() and showToast after refreshRecommendationsAPI()
resolves, but it must also reload the UI data so users see fresh
recommendations: after refreshRecommendationsAPI() resolves, call the existing
client-side fetch that populates the recommendations list (e.g., invoke the
function used elsewhere to load recommendations such as fetchRecommendations or
loadRecommendationsForCurrentView) and await or chain it before dismissing the
inFlight state / showing the toast (or optionally poll the refresh status then
call that loader); update both occurrences (the block around
refreshRecommendationsAPI() at the shown lines and the similar block at 263-265)
to trigger the same reload call so the UI reflects the refreshed cache.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7c05afb-4ee4-4e5d-85fb-8b0502d30076
📒 Files selected for processing (1)
frontend/src/recommendations.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Closes #284.
The Recommendations page now auto-refreshes its cached data on every page open when the cache is cold (never collected) or older than 24 hours. The freshness timestamp badge and the explicit Refresh button are removed; a toast is the only user-visible feedback channel.
Behaviour change
Why drop the indicator + button
The freshness badge is UX overhead the user doesn't want to think about — the right behaviour is for the data to be fresh automatically. The auto-refresh with a toast ("Refreshing recommendations…") gives the user enough signal that numbers may move in a moment, without requiring any action.
The 24h threshold (
STALE_THRESHOLD_MS) is a named constant at module scope with a doc comment. Operators wanting to tune it can expose it via the/api/public-infoendpoint in a follow-up (noted as a TODO in the code).What's kept
refreshRecommendations()stays exported fromfrontend/src/api/recommendations.ts— the auto-refresh logic and any future programmatic callers still need it.freshness.tsandrenderFreshness()are untouched — the Dashboard page still uses them viadashboard-freshness.Test coverage
Five new tests in
describe('auto-refresh on page open (#284)'):last_collected_at: null→ refresh fires + in-flight toastPre-existing tests are unprotected against the new toast calls by resetting the freshness mock to "fresh (1h ago)" in
beforeEach, so no existing test needed changing.Test plan
npm run typecheck— cleannpm test -- --testPathPattern=recommendations— 152 tests pass (5 new)npm test(full suite, 40 suites / 1466 tests) — all passSummary by CodeRabbit
New Features
Removed
Tests