Repository navigation
fix(frontend/recommendations): stop runaway refresh loop hammering the collector - #1123
Conversation
…e collector triggerAutoRefreshIfStale fired a background re-collect whenever the recommendations cache looked stale, and startRecommendationsRefresh's success path called onReload() (loadRecommendations on Opportunities, loadDashboard on Home), which re-entered triggerAutoRefreshIfStale. When the backend collection never advances last_collected_at (e.g. it fails at a DB error, so freshness stays stale forever) the success path re-fired the collect on every reload with no terminal condition. Observed in production at ~4-10 POST /recommendations/refresh per second, sustained (~18k calls/hour). The calls currently fail fast at a DB error before reaching Cost Explorer, but once the imminent DB fix lands every iteration becomes a paid Cost Explorer / pricing call (~$0.01 each), continuously. The existing autoRefreshInFlight guard only dedups *concurrent* refreshes and is cleared per cycle, so it does not stop this sequential re-trigger. Add a once-per-page-session latch (autoRefreshAttempted) so the stale-on-open auto-refresh fires at most once regardless of how many times loadRecommendations / loadDashboard re-run (filter changes, tab re-entry, the post-refresh reload). The latch is checked before the freshness GET so neither the GET nor the collect repeats. Explicit user actions (lookback change, manual refresh) call startRecommendationsRefresh directly and are never gated by the latch. A hard page reload resets module state, re-arming the once-per-load convenience. Add a regression test that reproduces the real scenario (collect POST resolves, freshness stays stale) and asserts the collect endpoint is hit exactly once across repeated loads and the post-refresh reload. Fails pre-fix (2 calls), passes post-fix (1 call). Reset the latch in the recommendations test beforeEach so the latch cannot leak across tests.
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 7 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Stops a runaway auto-refresh loop on the Recommendations (Opportunities) and Home (Dashboard) pages that fires the backend recommendation collector continuously. This is a cost risk: each collector run is a paid Cost Explorer / pricing API call (~$0.01) once it reaches AWS.
Repro
triggerAutoRefreshIfStalesees stale ->startRecommendationsRefresh-> POST/api/recommendations/refresh(the collector)..thenpath callsonReload()(loadRecommendationson Opportunities,loadDashboardon Home).onReloadre-runstriggerAutoRefreshIfStale. Because the collector never advanceslast_collected_at(currently it fails fast at a DB error, so freshness stays stale forever), the page is still stale -> fire the collect again -> reload -> stale -> ... with no terminal condition.Evidence
Production logs show the backend collector being hit ~4-10 times per second, sustained (~18k calls/hour), accompanied by a repeating "Recommendations refreshed" toast. Today those calls fail fast at a DB error before reaching Cost Explorer, but a DB fix is imminent: once it lands, every loop iteration becomes a continuous paid Cost Explorer / pricing call.
Root cause
frontend/src/recommendations.ts:triggerAutoRefreshIfStale(the stale-on-open auto-refresh) has no once-per-session guard.startRecommendationsRefresh's success path unconditionally callsonReload(), which re-enterstriggerAutoRefreshIfStale.autoRefreshInFlightguard only dedups concurrent refreshes and is cleared in.finally()each cycle, so it cannot stop this sequential re-trigger.This is a reactive/effect loop: the refresh's success reload re-evaluates the stale condition that triggered it, and the condition never clears.
Fix
Add a once-per-page-session latch (
autoRefreshAttempted) checked at the top oftriggerAutoRefreshIfStale, before the freshness GET. The stale-on-open auto-refresh now fires at most once per page session, regardless of how many timesloadRecommendations/loadDashboardre-run (filter changes, tab re-entry, the post-refresh reload). One change fixes both the Opportunities and Home entry points, since both reload paths funnel throughtriggerAutoRefreshIfStale.Explicit user actions (lookback change, manual refresh) call
startRecommendationsRefreshdirectly and are intentionally not gated by the latch. A hard page reload resets module state, re-arming the once-per-load convenience. The fix does not just suppress the toast; it stops the redundant backend calls themselves.Test
Added a regression test that reproduces the real scenario (collect POST resolves, freshness stays stale) and asserts the collect endpoint is hit exactly once across repeated loads and the post-refresh reload.
The latch is reset in the recommendations test
beforeEachso it cannot leak across tests.Verification
npx tsc --noEmit: clean.npx jestfor the touched suites (recommendations, recommendations-permissions, recommendations-enabled-providers, recommendations-lookback, dashboard, navigation, dashboard-ownership-950, settings-accounts): 658 passed, 0 failed.