Skip to content

Speed up signed-in dashboard with rollup-backed today and goals data#1169

Open
skyfallwastaken wants to merge 6 commits intomainfrom
rollup-today-goals-pr
Open

Speed up signed-in dashboard with rollup-backed today and goals data#1169
skyfallwastaken wants to merge 6 commits intomainfrom
rollup-today-goals-pr

Conversation

@skyfallwastaken
Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings April 15, 2026 16:00
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 15, 2026

Greptile Summary

This PR introduces rollup-backed today stats (new today_context, today_total_duration, today_language_count, today_editor_count dimensions) and rollup-backed goals progress (goals_period_total/project/language dimensions) to the signed-in dashboard, replacing per-request DB queries with cached pre-aggregated data. The three concerns raised in the previous review — stale rollup guard, language count inflation, and concurrent RecordNotUnique — have all been addressed.

Confidence Score: 5/5

Safe to merge — all three previous P0/P1 findings are resolved, and no new blocking issues were found.

The stale rollup guard, language count inflation fix (simple GROUP BY COUNT), and pg_advisory_xact_lock for concurrent today-upsert are all correctly implemented. Test coverage addresses the key paths including stale-context re-compute, goals-dimension-absent fallback, and AND-type goal DB fallback. Only minor P2 observations remain, none of which block merge.

No files require special attention.

Important Files Changed

Filename Overview
app/controllers/concerns/dashboard_data.rb Adds today_stats_data (reads today rollup context; on staleness, re-computes, upserts with advisory lock, clears memo) and dashboard_rollup_today_stats (validates timezone+date context before serving cached rows). Logic is correct and well-structured.
app/services/dashboard_rollup_refresh_service.rb Adds goals_rollup_data, goals_period_scope, and project_grouped_durations_for to compute goals-period totals/project/language breakdowns; adds today_rollup_records and upsert_today_rollup! (with pg advisory lock). Language counts use simple GROUP BY + COUNT — no window-function inflation.
app/services/programming_goals_progress_service.rb Adds rollup-backed tracked_seconds_from_rollup with a guard checking GOALS_PERIOD_TOTAL presence before trusting rollup data; AND-type goals (both project + language) correctly fall through to DB queries.
app/models/dashboard_rollup.rb Adds 6 new dimension strings to the DIMENSIONS constant and the corresponding inclusion validation — no issues.
app/controllers/static_pages_controller.rb Wires today_stats and programming_goals_progress into dashboard_stats_payload; passes dashboard_rollup_rows to ProgrammingGoalsProgressService to avoid redundant DB queries.
test/services/programming_goals_progress_service_test.rb Covers rollup-backed unfiltered/project/language goals, fallback for AND-type goals, and fallback when goals dimensions are absent from a stale rollup — good coverage of key paths.
test/controllers/concerns/dashboard_data_test.rb Adds test for stale today context triggering re-compute and upsert; validates both the returned stats and the refreshed rollup row — comprehensive.
test/services/dashboard_rollup_refresh_service_test.rb Verifies goals period totals are correctly stored; covers today context/total/language/editor dimensions as well.
test/models/user_test.rb Adds test verifying that timezone changes clear today_stats cache keys for both old and new timezones and schedule an immediate rollup refresh.
app/models/user.rb Extends handle_timezone_change to also delete the today_stats cache keys for both previous and current timezone and schedule a wait: 0.seconds rollup refresh — correct.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Controller as StaticPagesController
    participant DD as DashboardData concern
    participant Cache as Rails.cache
    participant Rollup as DashboardRollup (DB)
    participant DRRS as DashboardRollupRefreshService
    participant PGPS as ProgrammingGoalsProgressService

    Browser->>Controller: GET / (signed in)
    Controller->>DD: dashboard_stats_payload (deferred)

    Note over DD: today_stats_data
    DD->>Cache: fetch("user/:id/today_stats/:tz/:date", 1.min)
    alt Cache hit
        Cache-->>DD: cached today stats
    else Cache miss
        DD->>Rollup: dashboard_rollup_grouped_rows
        alt Context matches (tz + date)
            Rollup-->>DD: today context/total/language/editor rows
            DD->>DD: build_today_stats_from_rollup_data
        else Context stale or missing
            DD->>DRRS: today_rollup_data_for(user)
            DRRS->>Rollup: heartbeats.today (queries)
            DRRS-->>DD: today_data hash
            DD->>DRRS: upsert_today_rollup! (advisory lock)
            DRRS->>Rollup: DELETE today dims + INSERT new rows
            DD->>DD: clear rollup rows memo
            DD->>DD: build_today_stats_from_rollup_data
        end
        DD->>Cache: store result
    end

    Note over DD: programming_goals_progress_data
    DD->>Rollup: dashboard_rollup_rows (re-fetch if memo cleared)
    DD->>PGPS: new(user, goals, rollup_rows)
    loop each goal
        alt Has GOALS_PERIOD_TOTAL rows AND not AND-type goal
            PGPS->>PGPS: tracked_seconds_from_rollup
        else Missing goals dims OR project+language AND goal
            PGPS->>Rollup: heartbeats.where(time: period) (fallback)
        end
    end
    PGPS-->>DD: goals progress array
    DD-->>Controller: dashboard_stats_payload
    Controller-->>Browser: Inertia props
Loading

Reviews (2): Last reviewed commit: "Use bigint-safe advisory lock key for to..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to speed up the signed-in dashboard by serving “today stats”, activity graph daily durations, and (some) programming goals progress from DashboardRollup rows instead of running per-request heartbeat queries.

Changes:

  • Extend DashboardRollupRefreshService to persist rollups for daily durations, today-context/today-stats, and goals-period aggregates.
  • Update dashboard controllers/concerns to read today stats + activity graph data from rollups (with cache + staleness handling) and to pass rollup rows into goals progress calculation.
  • Add/extend tests covering rollup generation, rollup-backed goals progress behavior, today-context staleness refresh, and timezone-change cache invalidation + rollup refresh scheduling.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/services/dashboard_rollup_refresh_service.rb Generates new rollup dimensions for daily durations, today stats, and goal-period aggregates; adds targeted today upsert.
app/controllers/concerns/dashboard_data.rb Uses rollup-backed daily durations and today stats with cache/staleness logic; centralizes rollup row memoization.
app/controllers/static_pages_controller.rb Switches goals progress payload to use ProgrammingGoalsProgressService with rollup rows (removes prior cache wrapper).
app/services/programming_goals_progress_service.rb Adds optional rollup-backed tracked-seconds computation for unfiltered / project-only / language-only goals.
app/models/dashboard_rollup.rb Adds new allowed dimension names for the new rollup records.
app/models/user.rb Adjusts timezone-change handling to clear timezone-specific cache keys and schedule immediate rollup refresh.
test/services/dashboard_rollup_refresh_service_test.rb Adds assertions for new rollup dimensions/records (daily durations, today stats, goals totals).
test/services/programming_goals_progress_service_test.rb Adds coverage for rollup-backed goals progress and query fallback for combined filters.
test/controllers/concerns/dashboard_data_test.rb Verifies today stats refreshes rollup-backed data when rollup context is stale.
test/models/user_test.rb Verifies timezone change clears related caches and enqueues immediate rollup refresh.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/models/user.rb
Comment thread app/models/user.rb Outdated
Comment thread app/models/user.rb Outdated
Comment thread app/services/dashboard_rollup_refresh_service.rb Outdated
Comment thread app/controllers/static_pages_controller.rb
Comment thread app/services/programming_goals_progress_service.rb
Comment thread app/services/dashboard_rollup_refresh_service.rb
Comment thread app/controllers/concerns/dashboard_data.rb
@skyfallwastaken
Copy link
Copy Markdown
Member Author

@copilot @greptileai Can I get a review?

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

@copilot @greptileai Can I get a review?

Reviewed. The fallback-goals assertion issue was addressed in 45cb1ef, and CI is now passing on that commit.

@skyfallwastaken
Copy link
Copy Markdown
Member Author

@greptileai can I have review ty

DashboardRollup.transaction do
lock_key = today_rollup_lock_key(user.id)
DashboardRollup.connection.execute(
"SELECT pg_advisory_xact_lock(#{lock_key})"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants