feat(stats): refresh stale dashboard charts on visit#1932
Conversation
|
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:
📝 WalkthroughWalkthroughAdds scope-aware chart refresh orchestration: new per-app/org timestamps, Supabase RPCs and DB migration to enqueue/track refreshes, backend trigger updates to coordinate completion, a dashboardRefresh service for polling/decision logic, UI wiring for scoped refresh requests and polling, and tests validating RPCs and cron behavior. Changes
Sequence DiagramsequenceDiagram
participant UI as Usage Component
participant Service as dashboardRefresh Service
participant Supabase as Supabase API
participant DB as DB Stored Procs
participant Cron as cron_stat_app Worker
UI->>UI: User clicks reload
UI->>Service: handleReloadClick(scope)
alt scope stale & should request refresh
Service->>Supabase: request_app_chart_refresh / request_org_chart_refresh
Supabase->>DB: RPC sets stats_refresh_requested_at, returns requested_at + queued ids
DB->>Cron: enqueue cron_stat_app jobs for queued apps
Service-->>UI: start polling (CHART_REFRESH_POLL_MS)
loop poll until settled or timeout
UI->>Service: fetch*ChartRefreshState()
Service->>Supabase: select stats_refresh_requested_at & stats_updated_at
Supabase-->>Service: refresh state
Service-->>UI: evaluate isChartRefreshInProgress / isOrgCacheReadyForRefresh
alt refresh complete and cache ready
UI->>UI: clear caches & reloadAllCharts()
UI->>Service: stop polling
end
end
else scope fresh or demo
UI->>UI: reloadAllCharts() immediately
end
UI->>UI: update button disabled/spinning state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: ece36195ad
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/dashboard-refresh.unit.test.ts (1)
7-35: Useit.concurrent()for these isolated helper specs.These cases only exercise pure functions and don't share mutable test state, so they can run concurrently. As per coding guidelines, "Use
it.concurrent()instead ofit()when possible to run tests in parallel within the same file, maximizing parallelism for faster CI/CD".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/dashboard-refresh.unit.test.ts` around lines 7 - 35, Replace the two synchronous test cases in the "dashboard refresh helpers" suite to run concurrently by changing the test declarations from it(...) to it.concurrent(...); update the first test that imports and asserts isChartDataStale, isChartRefreshInProgress, parseDashboardRefreshTimestamp, and shouldAutoRequestChartRefresh, and the second test that imports and asserts isOrgCacheReadyForRefresh to use it.concurrent so these pure-function specs run in parallel.tests/organization-api.test.ts (1)
1056-1058: Assertstats_updated_atin this RPC contract too.
src/components/dashboard/Usage.vuenow reads bothstats_refresh_requested_atandstats_updated_atto decide staleness and refresh completion. This test only guards one half of that contract, so a missingstats_updated_atfield fromget_orgs_v7would still slip through here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-api.test.ts` around lines 1056 - 1058, The test currently asserts presence of stats_refresh_requested_at but not stats_updated_at, leaving the RPC contract incomplete; update the test that inspects testOrg (result of get_orgs_v7) to also assert testOrg has the 'stats_updated_at' property and, if the fixture provides an rpcStatsUpdatedAt (or similarly named) value, assert testOrg.stats_updated_at equals that RPC value so Usage.vue’s staleness checks remain covered.
🤖 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/pages/app/`[app].vue:
- Around line 201-208: The three sibling chart components (BundleUploadsCard,
UpdateStatsCard, DeploymentStatsCard) can remain stale because Usage.vue handles
the stale-refresh polling and only triggers reloads for its children via its
internal reloadTrigger; on this page they are siblings so they never see that
trigger. Fix by exposing a shared reload signal from Usage (e.g., a reactive
property or emitted event on the usageComponent ref like emitReload or public
method triggerReload) or by moving the refresh orchestration up into this page:
when Usage completes refresh, call the exposed method or emit an event on
usageComponent and pass a shared prop (e.g., reloadSignal or refreshToken) down
into BundleUploadsCard, UpdateStatsCard, and DeploymentStatsCard so they watch
that prop and reload when it changes.
In `@src/services/dashboardRefresh.ts`:
- Around line 3-5: CHART_REFRESH_TIMEOUT_MS is shorter than the dedupe/throttle
window (CHART_REFRESH_STALE_MS), which can cause the UI to stop polling while
backend retries are still throttled; change the timeout so it is derived from or
at least as long as CHART_REFRESH_STALE_MS (e.g., set CHART_REFRESH_TIMEOUT_MS =
CHART_REFRESH_STALE_MS or CHART_REFRESH_STALE_MS + safety margin) and update any
references relying on CHART_REFRESH_TIMEOUT_MS accordingly to ensure the client
keeps polling through the backend throttle window.
In `@supabase/migrations/20260422104849_stale_chart_refresh_state.sql`:
- Around line 593-627: The org-level stats_refresh_requested_at is being set
before any apps are actually queued; move that update so it only happens when
v_queued_count > 0 (or restore the previous/null timestamp when nothing was
queued) to avoid marking an org as "refresh requested" if no app was enqueued.
Concretely, remove or delay the UPDATE public.orgs ... SET
stats_refresh_requested_at = v_request_started_at that occurs prior to the FOR
loop and instead perform that UPDATE after the loop only when v_queued_count > 0
(or, if v_queued_count = 0, ensure the RETURN QUERY returns the prior value or
NULL for the org's request timestamp). Ensure this change touches the code
around queue_cron_stat_app_for_app, v_queued_count, v_request_started_at and the
RETURN QUERY so the returned timestamp reflects whether any apps were actually
queued.
- Around line 391-435: The code currently acquires a session-level advisory lock
via pg_advisory_lock (v_lock_key) and explicitly calls pg_advisory_unlock before
the transaction commits, allowing another session to see inconsistent state;
replace the session lock with a transaction-scoped lock or hold the lock until
commit by using pg_advisory_xact_lock(v_lock_key) (or remove the
pg_advisory_unlock calls) so the lock is released only at transaction end,
ensuring the UPDATE to public.apps (stats_refresh_requested_at) and the
pgmq.send('cron_stat_app', ...) happen atomically while the lock is held.
---
Nitpick comments:
In `@tests/dashboard-refresh.unit.test.ts`:
- Around line 7-35: Replace the two synchronous test cases in the "dashboard
refresh helpers" suite to run concurrently by changing the test declarations
from it(...) to it.concurrent(...); update the first test that imports and
asserts isChartDataStale, isChartRefreshInProgress,
parseDashboardRefreshTimestamp, and shouldAutoRequestChartRefresh, and the
second test that imports and asserts isOrgCacheReadyForRefresh to use
it.concurrent so these pure-function specs run in parallel.
In `@tests/organization-api.test.ts`:
- Around line 1056-1058: The test currently asserts presence of
stats_refresh_requested_at but not stats_updated_at, leaving the RPC contract
incomplete; update the test that inspects testOrg (result of get_orgs_v7) to
also assert testOrg has the 'stats_updated_at' property and, if the fixture
provides an rpcStatsUpdatedAt (or similarly named) value, assert
testOrg.stats_updated_at equals that RPC value so Usage.vue’s staleness checks
remain covered.
🪄 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: b0fd3ec1-6349-4cf8-a0f1-9c6dd20ff603
📒 Files selected for processing (13)
src/components/dashboard/Usage.vuesrc/pages/app/[app].vuesrc/services/dashboardRefresh.tssrc/types/supabase.types.tssupabase/functions/_backend/triggers/cron_stat_app.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260422104849_stale_chart_refresh_state.sqltests/chart-refresh-rpc.test.tstests/cron_stat_app_followup.unit.test.tstests/cron_stat_refresh_completion.test.tstests/dashboard-refresh.unit.test.tstests/organization-api.test.tstests/organization-put-stripe-sync.unit.test.ts
…resh # Conflicts: # supabase/functions/_backend/triggers/cron_stat_app.ts # tests/cron_stat_app_followup.unit.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/chart-refresh-rpc.test.ts (1)
57-135: Prefer per-test seed data here.These cases all mutate the same org/apps/queue rows created in
beforeAll, so one leaked state change can cascade into later assertions and the suite can’t safely move toit.concurrent(). Creating fresh org/app ids per case would keep the new RPC coverage isolated.As per coding guidelines, "When creating tests that interact with shared resources, create dedicated seed data when your test modifies resources; reuse existing seed data only when you read without modification and create your own child resources".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/chart-refresh-rpc.test.ts` around lines 57 - 135, Tests currently share mutable org/app rows created in beforeAll (orgId, staleAppId, freshAppId) so state leakage prevents safe concurrent tests; change to create fresh seeded org and app IDs per test instead: move the org/apps seeding and calls to resetAndSeedAppDataStats/clearCronStatAppMessages into beforeEach (or create a helper used inside each it), generate unique orgId/staleAppId/freshAppId per test (eg via uuid), use createAuthClient signIn per-test or ensure test-scoped auth, and perform per-test cleanup in afterEach (calling resetAppDataStats, clearCronStatAppMessages and deleting the specific rows) rather than relying on global beforeAll/afterAll; update references to orgId, staleAppId, freshAppId, resetAndSeedAppDataStats, clearCronStatAppMessages, resetAppDataStats, createAuthClient and any DB inserts to use the per-test IDs.
🤖 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/dashboard/Usage.vue`:
- Around line 121-143: The staleness clock (refreshStateClock) only updates
during polling or prop/store changes so the computed isCurrentScopeStale never
becomes true when the page is idle; add a lightweight periodic tick that
advances refreshStateClock (e.g., setInterval updating refreshStateClock.value =
Date.now()) so computed stale/refresh logic (isCurrentScopeStale,
isCurrentScopeRefreshing, hasRefreshableScope) re-evaluates even when idle,
store the interval handle (reuse or separate from refreshPollTimer) and ensure
you start it in the component setup/onMounted and clear it on beforeUnmount to
avoid leaks; tie the interval frequency to the staleness window (e.g., 30–60s)
and ensure it respects autoRefreshScopeKey/refreshPollStartedAt semantics if
needed.
In `@supabase/functions/_backend/triggers/cron_stat_app.ts`:
- Around line 159-170: syncAppStatsRefresh currently updates
apps.stats_updated_at directly via
supabase.from(...).update(...).throwOnError(), which can fail transiently;
change it to use the existing retry wrapper runSupabaseResultWithRetry (the same
helper used for the org follow-up write) so the update is retried on
5xx/temporary errors. Replace the direct supabase.update call inside
syncAppStatsRefresh (and the analogous update at the other occurrence) with a
call that constructs the same update query and passes it to
runSupabaseResultWithRetry, preserving the app_id filter and refreshCompletedAt
payload so that stats_updated_at reliably flips even on transient failures.
---
Nitpick comments:
In `@tests/chart-refresh-rpc.test.ts`:
- Around line 57-135: Tests currently share mutable org/app rows created in
beforeAll (orgId, staleAppId, freshAppId) so state leakage prevents safe
concurrent tests; change to create fresh seeded org and app IDs per test
instead: move the org/apps seeding and calls to
resetAndSeedAppDataStats/clearCronStatAppMessages into beforeEach (or create a
helper used inside each it), generate unique orgId/staleAppId/freshAppId per
test (eg via uuid), use createAuthClient signIn per-test or ensure test-scoped
auth, and perform per-test cleanup in afterEach (calling resetAppDataStats,
clearCronStatAppMessages and deleting the specific rows) rather than relying on
global beforeAll/afterAll; update references to orgId, staleAppId, freshAppId,
resetAndSeedAppDataStats, clearCronStatAppMessages, resetAppDataStats,
createAuthClient and any DB inserts to use the per-test IDs.
🪄 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: 656c0591-a20c-4d65-9b6e-157f57d2fd1a
📒 Files selected for processing (9)
src/components/dashboard/Usage.vuesrc/pages/app/[app].vuesrc/services/dashboardRefresh.tssupabase/functions/_backend/triggers/cron_stat_app.tssupabase/migrations/20260422104849_stale_chart_refresh_state.sqltests/chart-refresh-rpc.test.tstests/cron_stat_app_followup.unit.test.tstests/dashboard-refresh.unit.test.tstests/organization-api.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/dashboard-refresh.unit.test.ts
- supabase/migrations/20260422104849_stale_chart_refresh_state.sql
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32552725d7
ℹ️ 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".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/dashboard/Usage.vue (1)
244-253: Consider using store actions instead of direct mutation.
syncLocalOrgRefreshStatedirectly mutateseffectiveOrganization.value.stats_updated_atandstats_refresh_requested_at. SinceeffectiveOrganizationis derived fromorganizationStore, this bypasses the store's update mechanism.While this works for keeping the UI in sync during polling, consider whether the organization store should expose a mutation action for these fields to maintain consistency with the store pattern used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/Usage.vue` around lines 244 - 253, syncLocalOrgRefreshState currently mutates effectiveOrganization.value.stats_updated_at and stats_refresh_requested_at directly; instead add or use an organizationStore action (e.g., updateOrgStatsTimestamps or setStatsTimestamps) to apply those two fields and call that action from syncLocalOrgRefreshState after updating localOrgStatsUpdatedAt/localOrgStatsRefreshRequestedAt; update the code to stop writing to effectiveOrganization.value directly and invoke organizationStore's action (create the action if missing) so all org updates go through the store API.
🤖 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/dashboard/Usage.vue`:
- Around line 244-253: syncLocalOrgRefreshState currently mutates
effectiveOrganization.value.stats_updated_at and stats_refresh_requested_at
directly; instead add or use an organizationStore action (e.g.,
updateOrgStatsTimestamps or setStatsTimestamps) to apply those two fields and
call that action from syncLocalOrgRefreshState after updating
localOrgStatsUpdatedAt/localOrgStatsRefreshRequestedAt; update the code to stop
writing to effectiveOrganization.value directly and invoke organizationStore's
action (create the action if missing) so all org updates go through the store
API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdb04d72-b262-4986-936c-d975785e60d9
📒 Files selected for processing (4)
src/components/dashboard/Usage.vuesupabase/functions/_backend/triggers/cron_stat_app.tssupabase/migrations/20260422104849_stale_chart_refresh_state.sqltests/cron_stat_app_followup.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/cron_stat_app_followup.unit.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74336ba37b
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/triggers/cron_stat_app.ts`:
- Around line 190-200: The read in hasPendingAppStatsRefresh can fail
transiently and leave pendingAppRefreshes true; wrap the Supabase query in
hasPendingAppStatsRefresh in a small retry-with-backoff loop (e.g., 3 attempts,
short delay, retry on 5xx/network errors) and only throw after retries are
exhausted so callers can make a correct decision; also update the caller's catch
(where pendingAppRefreshes is set) to set pendingAppRefreshes = true only if
hasPendingAppStatsRefresh ultimately failed after retries (or if the error is
non-transient), referencing hasPendingAppStatsRefresh and the
pendingAppRefreshes variable so the orgs.stats_updated_at advancement is not
blocked by transient PostgREST 5xxs.
🪄 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: 67bb5a41-09e9-4d20-b0a1-d55614db643a
📒 Files selected for processing (5)
src/types/supabase.types.tssupabase/functions/_backend/triggers/cron_stat_app.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260422104849_stale_chart_refresh_state.sqltests/cron_stat_app_followup.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/cron_stat_app_followup.unit.test.ts
- src/types/supabase.types.ts
- supabase/migrations/20260422104849_stale_chart_refresh_state.sql
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3224f2c87
ℹ️ 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".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?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: 7b9249e62a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67e26534bd
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43f4724dd3
ℹ️ 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".
|
1 participant
Summary (AI generated)
Motivation (AI generated)
Dashboard charts were always showing stale data until the next scheduled stats cycle. This change lets the app request fresher data on demand while keeping the existing cron generation path and rate limits intact.
Business Impact (AI generated)
This reduces confusion on the dashboard, improves trust in usage reporting, and avoids turning page reloads into an unbounded background load problem. It gives users fresher metrics without changing the hot-path stats API contract.
Test Plan (AI generated)
bunx eslint src/components/dashboard/Usage.vue 'src/pages/app/[app].vue' src/services/dashboardRefresh.ts tests/dashboard-refresh.unit.test.ts tests/chart-refresh-rpc.test.ts tests/cron_stat_refresh_completion.test.ts tests/cron_stat_app_followup.unit.test.ts tests/organization-api.test.ts tests/organization-put-stripe-sync.unit.test.ts supabase/functions/_backend/triggers/cron_stat_app.tsbun typecheckbunx vitest run tests/dashboard-refresh.unit.test.ts tests/cron_stat_app_followup.unit.test.ts tests/organization-put-stripe-sync.unit.test.tsbun run supabase:startand DB-backed tests (tests/chart-refresh-rpc.test.ts,tests/cron_stat_refresh_completion.test.ts) once Docker is available locallyGenerated with AI
Summary by CodeRabbit
New Features
Behavior
Bug Fixes
Chores
Tests