Perf/dashboard endpoint optimization (#1897)

* optimize net_category_totals() by using memoized cache

* fix issue - net_category_totals cache is never populated - suggested by coderabbitAI

* fix 422 error for service-worker

* remove warning of [assigned but unused variables] - income_statement.rb

* remove warnings of [assigned but unused] from Prism - income_statement_test.rb

* add some measurements to improve docstring coverage, follow CodeRabbit recommendation

* attach Skylight monitoring for dev env as well - use my own Skylight auth token

* integrate Skylight with my own account auth token for local benchmark

* fix PR review suggestion - Move fallback release-note copy to i18n keys

* follow PR review - Fix changelog GitHub fetch timeout bounding

* FIX - Variable shadowing; Prefer stubbing the specific instance over any_instance.expects

* fix CodeRabbit feedback - Reusing the same stub for both classifications hides a contract mismatch

* fix CodeRabbit FEEDBACK - Reconsider enabling Skylight by default in development

* fix CodeRabbitAI FEEDBACK - reconsider unconditionally enabling Skylight in development

* fix Security scan FEEDBACK before PR merge

* fix jjmata feedback

.env.example

@@ -188,3 +188,8 @@ POSTHOG_HOST=
 # GCS_BUCKET=
 # GCS_KEYFILE_JSON= <- JSON content of service account key (preferred)
 # GCS_KEYFILE= <- path to service account JSON key file
+
+# Skylight
+# ========
+SKYLIGHT_AUTHENTICATION=
+SKYLIGHT_ENABLED=

.env.local.example

@@ -107,3 +107,8 @@ AI_DEBUG_MODE =
 # GCS_BUCKET=
 # GCS_KEYFILE_JSON=
 # GCS_KEYFILE=
+
+# Skylight
+# ========
+SKYLIGHT_AUTHENTICATION=
+SKYLIGHT_ENABLED=

Gemfile

@@ -44,7 +44,11 @@ gem "sentry-rails"
 gem "sentry-sidekiq"
 gem "posthog-ruby"
 gem "logtail-rails"
-gem "skylight", groups: [ :production ]
+if ENV["SKYLIGHT_ENABLED"] == "true"
+  gem "skylight", group: :development, require: false
+else
+  gem "skylight", group: :production
+end
 
 # Active Storage
 gem "aws-sdk-s3", "~> 1.208.0", require: false

app/controllers/pages_controller.rb

@@ -49,9 +49,9 @@ class PagesController < ApplicationController
       @release_notes = {
         avatar: "https://github.com/we-promise.png",
         username: "we-promise",
-        name: "Release notes unavailable",
+        name: t("pages.release_notes_unavailable.name"),
         published_at: Date.current,
-        body: "<p>Unable to fetch the latest release notes at this time. Please check back later or visit our <a href='https://github.com/we-promise/sure/releases' target='_blank'>GitHub releases page</a> directly.</p>"
+        body: t("pages.release_notes_unavailable.body_html")
       }
     end
 

app/models/income_statement.rb

@@ -31,21 +31,23 @@ class IncomeStatement
 
   def expense_totals(period: Period.current_month)
     # Memoized per instance so callers that also invoke `net_category_totals`
+    key = period_cache_key(period)
     @expense_totals_by_period ||= {}
-    @expense_totals_by_period[period_cache_key(period)] ||=
-      build_period_total(classification: "expense", period: period)
+    return @expense_totals_by_period[key] if @expense_totals_by_period.key?(key)
+    @expense_totals_by_period[key] = build_period_total(classification: "expense", period: period)
   end
 
   def income_totals(period: Period.current_month)
+    key = period_cache_key(period)
     @income_totals_by_period ||= {}
-    @income_totals_by_period[period_cache_key(period)] ||=
-      build_period_total(classification: "income", period: period)
+    return @income_totals_by_period[key] if @income_totals_by_period.key?(key)
+    @income_totals_by_period[key] = build_period_total(classification: "income", period: period)
   end
 
   def net_category_totals(period: Period.current_month)
+    key = period_cache_key(period)
     @net_category_totals_by_period ||= {}
-    cached = @net_category_totals_by_period[period_cache_key(period)]
-    return cached if cached
+    return @net_category_totals_by_period[key] if @net_category_totals_by_period.key?(key)
 
     expense = expense_totals(period: period)
     income = income_totals(period: period)
@@ -68,9 +70,9 @@ class IncomeStatement
     raw_expense_categories = []
     raw_income_categories = []
 
-    all_keys.each do |key|
-      exp_ct = expense_by_cat[key]
-      inc_ct = income_by_cat[key]
+    all_keys.each do |cat|
+      exp_ct = expense_by_cat[cat]
+      inc_ct = income_by_cat[cat]
       exp_total = exp_ct&.total || 0
       inc_total = inc_ct&.total || 0
       net = exp_total - inc_total
@@ -96,7 +98,7 @@ class IncomeStatement
       CategoryTotal.new(category: r[:category], total: r[:total], currency: family.currency, weight: weight)
     end
 
-    @net_category_totals_by_period[period_cache_key(period)] = NetCategoryTotals.new(
+    @net_category_totals_by_period[key] = NetCategoryTotals.new(
       net_expense_categories: net_expense_categories,
       net_income_categories: net_income_categories,
       total_net_expense: total_net_expense,
@@ -148,8 +150,6 @@ class IncomeStatement
       other_investments_category = family.categories.other_investments
 
       category_totals = [ *categories, uncategorized_category, other_investments_category ].map do |category|
-        subcategory = categories.find { |c| c.id == category.parent_id }
-
         parent_category_total = if category.uncategorized?
           # Regular uncategorized: NULL category_id and NOT uncategorized investment
           totals.select { |t| t.category_id.nil? && !t.is_uncategorized_investment }&.sum(&:total) || 0

config/application.rb

@@ -40,7 +40,13 @@ module Sure
     }
 
     # Enable Skylight instrumentation for ActiveJob (background workers)
-    config.skylight.probes << "active_job" if defined?(Skylight)
+    # Developers can opt-in to Skylight locally by setting SKYLIGHT_ENABLED=true
+    if defined?(Skylight) && config.respond_to?(:skylight)
+      config.skylight.probes << "active_job"
+      if ENV["SKYLIGHT_ENABLED"] == "true"
+        config.skylight.environments += [ "development" ]
+      end
+    end
 
     # Enable Rack::Attack middleware for API rate limiting
     config.middleware.use Rack::Attack

config/initializers/mime_types.rb

@@ -8,3 +8,6 @@ Mime::Type.register "application/vnd.openxmlformats-officedocument.spreadsheetml
 
 # Register .mjs so Propshaft serves ES modules with the correct Content-Type.
 Mime::Type.register "text/javascript", :mjs unless Mime::Type.lookup_by_extension(:mjs)
+
+# Service Worker
+Mime::Type.register "application/javascript", :serviceworker

config/locales/views/pages/en.yml

@@ -26,6 +26,9 @@ en:
       refresh_page: Refresh Page
     changelog:
       title: What's new
+    release_notes_unavailable:
+      name: Release notes unavailable
+      body_html: "<p>Unable to fetch the latest release notes at this time. Please check back later or visit our <a href='https://github.com/we-promise/sure/releases' target='_blank' rel='noopener noreferrer'>GitHub releases page</a> directly.</p>"
     privacy:
       title: Privacy Policy
       heading: Privacy Policy

config/skylight.yml

@@ -0,0 +1 @@
+authentication: <%= ENV["SKYLIGHT_AUTHENTICATION"] %>

test/controllers/pages_controller_test.rb

@@ -14,6 +14,39 @@ class PagesControllerTest < ActionDispatch::IntegrationTest
     assert_response :ok
   end
 
+  test "dashboard memoizes income statement period totals while rendering" do
+    income_statement = IncomeStatement.new(@family)
+    IncomeStatement.stubs(:new).returns(income_statement)
+
+    fake_expense_period_total = IncomeStatement::PeriodTotal.new(
+      classification: "expense",
+      total: 0,
+      currency: @family.currency,
+      category_totals: []
+    )
+
+    fake_income_period_total = IncomeStatement::PeriodTotal.new(
+      classification: "income",
+      total: 0,
+      currency: @family.currency,
+      category_totals: []
+    )
+
+    income_statement.expects(:build_period_total)
+      .with(classification: "expense", period: kind_of(Period))
+      .once
+      .returns(fake_expense_period_total)
+
+    income_statement.expects(:build_period_total)
+      .with(classification: "income", period: kind_of(Period))
+      .once
+      .returns(fake_income_period_total)
+
+    get root_path
+
+    assert_response :ok
+  end
+
   test "intro page requires guest role" do
     get intro_path
 

test/models/income_statement_test.rb

@@ -39,6 +39,22 @@ class IncomeStatementTest < ActiveSupport::TestCase
     assert_equal expected_total_expense, expense_totals.category_totals.find { |ct| ct.category.id == @food_category.id }.total
   end
 
+  test "memoizes expense and income period totals across repeated calculations" do
+    income_statement = IncomeStatement.new(@family)
+    period = Period.last_30_days
+
+    expense_period_total = IncomeStatement::PeriodTotal.new("expense", 900, @family.currency, [])
+    income_period_total = IncomeStatement::PeriodTotal.new("income", 1000, @family.currency, [])
+
+    income_statement.expects(:build_period_total).with(classification: "expense", period: period).once.returns(expense_period_total)
+    income_statement.expects(:build_period_total).with(classification: "income", period: period).once.returns(income_period_total)
+
+    income_statement.net_category_totals(period: period)
+    income_statement.expense_totals(period: period)
+    income_statement.income_totals(period: period)
+    income_statement.net_category_totals(period: period)
+  end
+
   test "calculates income for a period" do
     income_statement = IncomeStatement.new(@family)
     income_totals = income_statement.income_totals(period: Period.last_30_days)
@@ -154,8 +170,8 @@ class IncomeStatementTest < ActiveSupport::TestCase
   # NOTE: These tests now pass because kind filtering is working after the refactoring!
   test "excludes regular transfers from income statement calculations" do
     # Create a regular transfer between accounts
-    outflow_transaction = create_transaction(account: @checking_account, amount: 500, kind: "funds_movement")
-    inflow_transaction = create_transaction(account: @credit_card_account, amount: -500, kind: "funds_movement")
+    _outflow_transaction = create_transaction(account: @checking_account, amount: 500, kind: "funds_movement")
+    _inflow_transaction = create_transaction(account: @credit_card_account, amount: -500, kind: "funds_movement")
 
     income_statement = IncomeStatement.new(@family)
     totals = income_statement.totals(date_range: Period.last_30_days.date_range)
@@ -168,7 +184,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
 
   test "includes loan payments as expenses in income statement" do
     # Create a loan payment transaction
-    loan_payment = create_transaction(account: @checking_account, amount: 1000, category: nil, kind: "loan_payment")
+    _loan_payment = create_transaction(account: @checking_account, amount: 1000, category: nil, kind: "loan_payment")
 
     income_statement = IncomeStatement.new(@family)
     totals = income_statement.totals(date_range: Period.last_30_days.date_range)
@@ -181,7 +197,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
 
   test "excludes one-time transactions from income statement calculations" do
     # Create a one-time transaction
-    one_time_transaction = create_transaction(account: @checking_account, amount: 250, category: @groceries_category, kind: "one_time")
+    _one_time_transaction = create_transaction(account: @checking_account, amount: 250, category: @groceries_category, kind: "one_time")
 
     income_statement = IncomeStatement.new(@family)
     totals = income_statement.totals(date_range: Period.last_30_days.date_range)
@@ -194,7 +210,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
 
   test "excludes payment transactions from income statement calculations" do
     # Create a payment transaction (credit card payment)
-    payment_transaction = create_transaction(account: @checking_account, amount: 300, category: nil, kind: "cc_payment")
+    _payment_transaction = create_transaction(account: @checking_account, amount: 300, category: nil, kind: "cc_payment")
 
     income_statement = IncomeStatement.new(@family)
     totals = income_statement.totals(date_range: Period.last_30_days.date_range)
@@ -288,7 +304,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
 
   test "includes investment_contribution transactions as expenses in income statement" do
     # Create a transfer to investment account (marked as investment_contribution)
-    investment_contribution = create_transaction(
+    _investment_contribution = create_transaction(
       account: @checking_account,
       amount: 1000,
       category: nil,
@@ -318,7 +334,7 @@ class IncomeStatementTest < ActiveSupport::TestCase
 
     # Provider-imported contribution shows as inflow (negative amount) to the investment account
     # kind is investment_contribution, which should be treated as expense regardless of sign
-    provider_contribution = create_transaction(
+    _provider_contribution = create_transaction(
       account: investment_account,
       amount: -500, # Negative = inflow to account
       category: nil,