QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-05-07 12:54:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address all CodeRabbit PR review comments on insights feed

Browse Source 
- Add `Setting.insights_enabled` feature gate at top of GenerateInsightsJob#perform
- Fix race condition in upsert_insight with recursive retry on RecordNotUnique
- Use Provider::Openai.effective_model instead of hardcoded DEFAULT_MODEL
- Rescue Money::Currency::UnknownCurrency in currency_symbol fallback
- Fix budget dedup key to stable "budget_pacing:<date>" (prevents flip-flop between at_risk/on_track)
- Fix cash flow projection off-by-one: iterate (1..PROJECTION_DAYS) not (0..PROJECTION_DAYS)
- Eliminate N+1 in IdleCashGenerator by precomputing active_account_ids in one SQL query
- Rename all_time_high -> thirty_day_high (series is 30-day, not truly all-time)
- Update net_worth_milestone i18n: title_ath -> title_30d_high, "30-day high" language
- Anchor SpendingAnomalyGenerator baseline to current_period.start_date (not calendar months)
- Fix spending_anomaly dedup key to use current_period.start_date instead of Date.current
- Add .order(last_occurrence_date: :asc) to SubscriptionAuditGenerator for deterministic results
- Stub Setting.insights_enabled in job tests; add generated_at assertion to upsert test

https://claude.ai/code/session_014vY9xohpm3abSAxVxRF27a
This commit is contained in:
Claude
2026-04-12 14:17:38 +00:00
parent 9eb17bfd22
commit d25bb6e2a3
11 changed files with 83 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
app/jobs/generate_insights_job.rb
View File

@@ -4,6 +4,8 @@ class GenerateInsightsJob < ApplicationJob
# When called from cron (no family_id), iterates all families with insights enabled.
# Can also be triggered on-demand for a single family (e.g., from the UI "Refresh" button).
def perform(family_id: nil)
return unless Setting.insights_enabled
if family_id
family = Family.find_by(id: family_id)
generate_for_family(family) if family
@@ -25,38 +27,45 @@ class GenerateInsightsJob < ApplicationJob
def upsert_insights(family, generated_insights)
generated_insights.each do |gi|
existing = family.insights.find_by(dedup_key: gi.dedup_key)
if existing
# Reactivate only if the underlying numbers changed materially
numbers_changed = existing.metadata != gi.metadata
new_status = (numbers_changed && existing.dismissed?) ? "active" : existing.status
existing.update!(
title: gi.title,
body: gi.body,
metadata: gi.metadata,
priority: gi.priority,
status: new_status,
generated_at: Time.current,
period_start: gi.period_start,
period_end: gi.period_end
)
else
family.insights.create!(
insight_type: gi.insight_type,
priority: gi.priority,
status: "active",
title: gi.title,
body: gi.body,
metadata: gi.metadata,
currency: gi.currency || family.currency,
period_start: gi.period_start,
period_end: gi.period_end,
dedup_key: gi.dedup_key,
generated_at: Time.current
)
end
upsert_insight(family, gi)
end
end
def upsert_insight(family, gi, attempts: 0)
existing = family.insights.find_by(dedup_key: gi.dedup_key)
if existing
# Reactivate only if the underlying numbers changed materially
numbers_changed = existing.metadata != gi.metadata
new_status = (numbers_changed && existing.dismissed?) ? "active" : existing.status
existing.update!(
title: gi.title,
body: gi.body,
metadata: gi.metadata,
priority: gi.priority,
status: new_status,
generated_at: Time.current,
period_start: gi.period_start,
period_end: gi.period_end
)
else
family.insights.create!(
insight_type: gi.insight_type,
priority: gi.priority,
status: "active",
title: gi.title,
body: gi.body,
metadata: gi.metadata,
currency: gi.currency || family.currency,
period_start: gi.period_start,
period_end: gi.period_end,
dedup_key: gi.dedup_key,
generated_at: Time.current
)
end
rescue ActiveRecord::RecordNotUnique
raise if attempts >= 2
upsert_insight(family, gi, attempts: attempts + 1)
end
end

6
app/models/insight/generator.rb
View File

@@ -38,7 +38,7 @@ class Insight::Generator
response = llm.chat_response(
prompt,
model: Provider::Openai::DEFAULT_MODEL,
model: Provider::Openai.effective_model,
instructions: system_instructions
)
@@ -49,7 +49,7 @@ class Insight::Generator
end
def system_instructions
sym = Money::Currency.new(family.currency).symbol
sym = currency_symbol
<<~PROMPT
You are a concise financial insights writer for a personal finance app.
Write exactly 1-2 sentences in plain, conversational English.
@@ -60,5 +60,7 @@ class Insight::Generator
def currency_symbol
Money::Currency.new(family.currency).symbol
rescue Money::Currency::UnknownCurrency
family.currency
end
end

2
app/models/insight/generators/budget_insight_generator.rb
View File

@@ -62,7 +62,7 @@ class Insight::Generators::BudgetInsightGenerator < Insight::Generator
currency: family.currency,
period_start: current_period.start_date,
period_end: current_period.end_date,
dedup_key: "#{insight_type}:#{current_period.start_date.strftime("%Y-%m")}"
dedup_key: "budget_pacing:#{current_period.start_date.strftime("%Y-%m-%d")}"
)
]
end

2
app/models/insight/generators/cash_flow_warning_generator.rb
View File

@@ -59,7 +59,7 @@ class Insight::Generators::CashFlowWarningGenerator < Insight::Generator
balance = current_cash
balance_by_date = {}
(0..PROJECTION_DAYS).each do |day_offset|
(1..PROJECTION_DAYS).each do |day_offset|
date = Date.current + day_offset
events.each do |event|

16
app/models/insight/generators/idle_cash_generator.rb
View File

@@ -7,17 +7,17 @@ class Insight::Generators::IdleCashGenerator < Insight::Generator
def generate
depository_accounts = family.accounts.visible.assets.where(accountable_type: "Depository")
depository_accounts.filter_map do |account|
active_account_ids = Entry
.where(account_id: depository_accounts.select(:id))
.where("date >= ?", IDLE_THRESHOLD_DAYS.days.ago.to_date)
.where(entryable_type: "Transaction")
.distinct
.pluck(:account_id)
depository_accounts.where.not(id: active_account_ids).filter_map do |account|
balance = account.balance.to_f
next unless balance >= IDLE_AMOUNT_THRESHOLD
recent_activity = account.entries
.where("date >= ?", IDLE_THRESHOLD_DAYS.days.ago.to_date)
.where(entryable_type: "Transaction")
.exists?
next if recent_activity
metadata = {
"account_id" => account.id,
"account_name" => account.name,

33
app/models/insight/generators/net_worth_milestone_generator.rb
View File

@@ -1,5 +1,4 @@
# Surfaces an insight when net worth crosses a round-number milestone or hits an all-time high
# over the past 30 days.
# Surfaces an insight when net worth crosses a round-number milestone or hits a 30-day high.
class Insight::Generators::NetWorthMilestoneGenerator < Insight::Generator
ROUND_MILESTONES = [
1_000, 5_000, 10_000, 25_000, 50_000,
@@ -21,32 +20,32 @@ class Insight::Generators::NetWorthMilestoneGenerator < Insight::Generator
prior_values = extract_prior_values(prior_series)
return [] if prior_values.empty?
prior_nw = prior_values.last.to_f
series_high = prior_values.max.to_f
all_time_high = current_nw >= series_high && current_nw > prior_nw
prior_nw = prior_values.last.to_f
series_high = prior_values.max.to_f
thirty_day_high = current_nw >= series_high && current_nw > prior_nw
crossed = ROUND_MILESTONES.find { |m| current_nw >= m && prior_nw < m }
return [] unless crossed || all_time_high
return [] unless crossed || thirty_day_high
metadata = {
"milestone" => crossed,
"current_net_worth" => current_nw.round(2),
"previous_net_worth" => prior_nw.round(2),
"all_time_high" => all_time_high
"thirty_day_high" => thirty_day_high
}
title = if all_time_high && crossed
title = if thirty_day_high && crossed
I18n.t("insights.net_worth_milestone.title_both",
milestone: format_amount(crossed))
elsif all_time_high
I18n.t("insights.net_worth_milestone.title_ath")
elsif thirty_day_high
I18n.t("insights.net_worth_milestone.title_30d_high")
else
I18n.t("insights.net_worth_milestone.title_milestone",
milestone: format_amount(crossed))
end
body = generate_body(build_prompt(crossed, all_time_high, current_nw, prior_nw))
body = generate_body(build_prompt(crossed, thirty_day_high, current_nw, prior_nw))
[
GeneratedInsight.new(
@@ -58,7 +57,7 @@ class Insight::Generators::NetWorthMilestoneGenerator < Insight::Generator
currency: family.currency,
period_start: prior_period.start_date,
period_end: Date.current,
dedup_key: "net_worth_milestone:#{crossed || "ath"}:#{Date.current.strftime("%Y-%m")}"
dedup_key: "net_worth_milestone:#{crossed || "30d_high"}:#{Date.current.strftime("%Y-%m")}"
)
]
end
@@ -73,15 +72,15 @@ class Insight::Generators::NetWorthMilestoneGenerator < Insight::Generator
end
end
def build_prompt(crossed, all_time_high, current_nw, prior_nw)
def build_prompt(crossed, thirty_day_high, current_nw, prior_nw)
sym = currency_symbol
current_fmt = "#{sym}#{current_nw.round(0).to_s(:delimited)}"
prior_fmt = "#{sym}#{prior_nw.round(0).to_s(:delimited)}"
if all_time_high && crossed
"Net worth crossed #{sym}#{crossed.to_s(:delimited)} and hit an all-time high of #{current_fmt}."
elsif all_time_high
"Net worth hit an all-time high of #{current_fmt}, up from #{prior_fmt} 30 days ago."
if thirty_day_high && crossed
"Net worth crossed #{sym}#{crossed.to_s(:delimited)} and hit a 30-day high of #{current_fmt}."
elsif thirty_day_high
"Net worth hit a 30-day high of #{current_fmt}, up from #{prior_fmt} 30 days ago."
else
"Net worth crossed the #{sym}#{crossed.to_s(:delimited)} milestone, now at #{current_fmt}."
end

10
app/models/insight/generators/spending_anomaly_generator.rb
View File

@@ -5,11 +5,11 @@ class Insight::Generators::SpendingAnomalyGenerator < Insight::Generator
MIN_BASELINE_SPEND = 50 # Ignore tiny categories (noise reduction)
def generate
baseline_period = Period.custom(
start_date: 3.months.ago.to_date.beginning_of_month,
end_date: 1.month.ago.to_date.end_of_month
)
current_period = Period.current_month_for(family)
baseline_period = Period.custom(
start_date: current_period.start_date - 3.months,
end_date: current_period.start_date - 1.day
)
income_stmt = family.income_statement
@@ -79,7 +79,7 @@ class Insight::Generators::SpendingAnomalyGenerator < Insight::Generator
currency: family.currency,
period_start: current_period.start_date,
period_end: current_period.end_date,
dedup_key: "spending_anomaly:#{cat_id}:#{Date.current.strftime("%Y-%m")}"
dedup_key: "spending_anomaly:#{cat_id}:#{current_period.start_date.strftime("%Y-%m-%d")}"
)
end

1
app/models/insight/generators/subscription_audit_generator.rb
View File

@@ -10,6 +10,7 @@ class Insight::Generators::SubscriptionAuditGenerator < Insight::Generator
.where("last_occurrence_date < ?", OVERDUE_DAYS.days.ago.to_date)
.where("next_expected_date < ?", Date.current)
.includes(:merchant)
.order(last_occurrence_date: :asc)
.limit(5)
return [] if stale.empty?

4
config/locales/views/insights/en.yml
View File

@@ -22,9 +22,9 @@ en:
cash_flow_warning:
title: Low cash projected around %{date}
net_worth_milestone:
title_ath: New all-time high net worth
title_30d_high: New 30-day high net worth
title_milestone: Net worth crossed %{milestone}
title_both: Net worth crossed %{milestone} — new all-time high
title_both: Net worth crossed %{milestone} — new 30-day high
subscription_audit:
title: Is %{name} still active?
savings_rate_change:

2
test/fixtures/insights.yml vendored
View File

@@ -48,7 +48,7 @@ net_worth_milestone:
milestone: 100000
current_net_worth: 101342.00
previous_net_worth: 98800.00
all_time_high: true
thirty_day_high: true
currency: USD
period_start: <%= 30.days.ago.to_date %>
period_end: <%= Date.current %>

4
test/jobs/generate_insights_job_test.rb
View File

@@ -3,6 +3,7 @@ require "test_helper"
class GenerateInsightsJobTest < ActiveJob::TestCase
setup do
@family = families(:dylan_family)
Setting.stubs(:insights_enabled).returns(true)
end
test "performs without error for a family with no accounts" do
@@ -46,7 +47,7 @@ class GenerateInsightsJobTest < ActiveJob::TestCase
test "updates existing insight body and generated_at on repeated runs" do
dedup = "net_worth_milestone:update_test:#{Date.current.strftime("%Y-%m")}"
@family.insights.create!(
old_insight = @family.insights.create!(
insight_type: "net_worth_milestone",
priority: "high",
status: "active",
@@ -78,5 +79,6 @@ class GenerateInsightsJobTest < ActiveJob::TestCase
refreshed = @family.insights.find_by(dedup_key: dedup)
assert_equal "New title", refreshed.title
assert_equal "New body", refreshed.body
assert refreshed.generated_at > old_insight.generated_at, "generated_at should be updated on upsert"
end
end