Recurring fixes #454
Recurring fixes #454
Conversation
and add toggle for recurring feature
WalkthroughDebounces recurring-transaction identification into a background job with advisory-lock concurrency, adds a family-level DB flag and UI toggle to enable/disable recurring processing, preserves a synchronous bang API for immediate identification, and adds persisted collapsible state for the transactions UI. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as RecurringTransactionsController
participant Job as IdentifyRecurringTransactionsJob
participant Cache as Cache
participant DB as PostgreSQL
participant Identifier as RecurringTransaction::Identifier
Controller->>Job: IdentifyRecurringTransactionsJob.schedule_for(family) (sets cache timestamp, enqueue after DEBOUNCE_DELAY)
Note right of Job: Job.perform(family_id, scheduled_at)
Job->>DB: load Family by id
alt family missing or disabled
Job-->>Job: return early
else
Job->>Cache: read cache_key(family_id) (staleness check)
alt stale schedule (newer exists)
Job-->>Job: return
else
Job->>DB: check incomplete syncs (plaid_items, simplefin_items, lunchflow_items, enable_banking_items, accounts)
alt incomplete syncs present
Job-->>Job: return
else
Job->>DB: pg_try_advisory_lock(advisory_key)
alt lock acquired
Job->>Identifier: Identifier.identify_recurring_patterns(family)
Identifier->>DB: find_or_initialize/create -> save!
alt RecordNotUnique on create
Identifier->>DB: re-fetch existing record -> update!
end
Job->>DB: release advisory lock
else
Job-->>Job: skip (another worker running)
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rb (1)
1-5: Consider renaming for clarity.The migration correctly enforces constraints at the database level and uses the proper Rails version. However, the double-negative naming (
recurring_transactions_disabled) could lead to confusion. Considerrecurring_transactions_enabledwithdefault: truefor clearer semantics in conditional logic throughout the codebase.app/models/recurring_transaction/identifier.rb (1)
94-103: Redundant attribute assignment on new records.Lines 96-100 set
merchant_idornameon new records, butfind_or_initialize_by(find_conditions)already initializes these attributes fromfind_conditions(lines 76-82). This code is harmless but redundant.Consider removing the redundant assignment:
- # Set the name or merchant_id on new records - if recurring_transaction.new_record? - if pattern[:merchant_id].present? - recurring_transaction.merchant_id = pattern[:merchant_id] - else - recurring_transaction.name = pattern[:name] - end - # New auto-detected recurring transactions are not manual - recurring_transaction.manual = false - end + # New auto-detected recurring transactions are not manual + recurring_transaction.manual = false if recurring_transaction.new_record?app/jobs/identify_recurring_transactions_job.rb (2)
26-37: Cache key duplication violates DRY.Line 30 manually constructs
"recurring_transaction_identify:#{family.id}"but the privatecache_keymethod (line 42) does the same thing. Sinceschedule_foris a class method, it can't call the instance method directly.Extract to a class method that both can use:
+ def self.cache_key(family_id) + "recurring_transaction_identify:#{family_id}" + end + def self.schedule_for(family) return if family.recurring_transactions_disabled? scheduled_at = Time.current.to_f - cache_key = "recurring_transaction_identify:#{family.id}" + key = cache_key(family.id) # Store the latest scheduled time - Rails.cache.write(cache_key, scheduled_at, expires_in: DEBOUNCE_DELAY + 10.seconds) + Rails.cache.write(key, scheduled_at, expires_in: DEBOUNCE_DELAY + 10.seconds) # Schedule the job with delay set(wait: DEBOUNCE_DELAY).perform_later(family.id, scheduled_at) end private - def cache_key(family_id) - "recurring_transaction_identify:#{family_id}" + def cache_key(family_id) + self.class.cache_key(family_id) end
61-76: Silent skip when lock unavailable may hinder debugging.When
pg_try_advisory_lockfails to acquire the lock (another job is running), the method silently returns without executing the block. This is intentional for the debounce safety pattern, but the lack of any logging could make it difficult to debug production issues.Consider adding a debug log when the lock is unavailable:
acquired = ActiveRecord::Base.connection.select_value( ActiveRecord::Base.sanitize_sql_array([ "SELECT pg_try_advisory_lock(?)", lock_key ]) ) - return unless acquired + unless acquired + Rails.logger.debug { "IdentifyRecurringTransactionsJob: Lock unavailable for family #{family_id}, skipping" } + return + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/controllers/recurring_transactions_controller.rb(1 hunks)app/jobs/identify_recurring_transactions_job.rb(1 hunks)app/models/family/sync_complete_event.rb(1 hunks)app/models/recurring_transaction.rb(1 hunks)app/models/recurring_transaction/identifier.rb(1 hunks)app/views/recurring_transactions/index.html.erb(3 hunks)config/locales/views/recurring_transactions/en.yml(1 hunks)config/routes.rb(1 hunks)db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rb(1 hunks)db/schema.rb(2 hunks)test/models/recurring_transaction_test.rb(8 hunks)
🧰 Additional context used
📓 Path-based instructions (29)
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbdb/schema.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/family/sync_complete_event.rbapp/models/recurring_transaction/identifier.rbapp/models/recurring_transaction.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbapp/views/recurring_transactions/index.html.erbdb/schema.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbapp/views/recurring_transactions/index.html.erbdb/schema.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/family/sync_complete_event.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbapp/views/recurring_transactions/index.html.erbdb/schema.rb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbapp/views/recurring_transactions/index.html.erbdb/schema.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/models/family/sync_complete_event.rbtest/models/recurring_transaction_test.rbapp/jobs/identify_recurring_transactions_job.rbconfig/routes.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbdb/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbapp/models/recurring_transaction.rbapp/views/recurring_transactions/index.html.erbdb/schema.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/models/family/sync_complete_event.rbapp/jobs/identify_recurring_transactions_job.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbapp/models/recurring_transaction.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/models/family/sync_complete_event.rbapp/jobs/identify_recurring_transactions_job.rbapp/controllers/recurring_transactions_controller.rbapp/models/recurring_transaction/identifier.rbapp/models/recurring_transaction.rb
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/views/recurring_transactions/en.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/views/recurring_transactions/en.ymlconfig/routes.rb
test/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
test/**/*.rb: Use Minitest with fixtures (never RSpec or FactoryBot) for testing
Use mocha gem for mocks and VCR for external API tests
Use fixtures for base cases in tests (2-3 per model) and create edge cases on-the-fly within test context
Test critical code paths only: test Commands for correct params, Queries for output
test/**/*.rb: Use Rails' built-in Minitest framework with fixtures for test data. Avoid RSpec and FactoryBot.
Use VCR for external API testing.
Usemochagem for stubs and mocks. PreferOpenStructfor mock instances. Only mock what's necessary.Write tests using Minitest and fixtures as the testing framework
test/**/*.rb: Always use Minitest and fixtures for testing, NEVER rspec or factories
For tests that require a large number of fixture records, use Rails helpers to help create the records needed for the test, then inline the creation instead of defining them in fixtures
Distinguish between commands and query methods. Test output of query methods; test that commands were called with the correct params.
Never test the implementation details of one class in another class's test suite
Use themochagem for stubs and mocks
Always preferOpenStructwhen creating mock instances, or in complex cases, a mock class
Only mock what's necessary. If you're not testing return values, don't mock a return value.
Files:
test/models/recurring_transaction_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test files should use*_test.rbnaming convention and mirrorapp/structure (e.g.,test/models/*_test.rb)
Use Minitest framework for tests with names*_test.rb, run viabin/rails test, and usetest/fixturesand VCR cassettes for HTTP testing
Files:
test/models/recurring_transaction_test.rb
app/jobs/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/jobs/**/*.rb: Use Sidekiq for background jobs (e.g., SyncJob, ImportJob, AssistantResponseJob)
Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)
Use background jobs (Sidekiq) for heavy operations instead of synchronous processing
Files:
app/jobs/identify_recurring_transactions_job.rb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/recurring_transactions_controller.rb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use simple validations (null checks, unique indexes) in database layer
Files:
db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rb
db/migrate/*.rb
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
ActiveRecord migrations must inherit from
ActiveRecord::Migration[7.2]. Do NOT use version 8.0 yetEnforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Files:
db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rb
db/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Data migrations should be placed in
db/directory with test fixtures intest/fixtures/
Files:
db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbdb/schema.rb
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/recurring_transactions/index.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-sideViews should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by
erb-lintper.erb_lint.yml)
Files:
app/views/recurring_transactions/index.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/recurring_transactions/index.html.erb
**/*.{erb,html,vue,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined inapp/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g.,text-primarynottext-white).
Prefer semantic HTML elements over JavaScript components (e.g., use<dialog>for modals,<details><summary>for disclosures).
Files:
app/views/recurring_transactions/index.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb}: Useiconhelper for icons, never uselucide_icondirectly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript.
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/recurring_transactions/index.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep domain logic out of view templates. Logic belongs in component files, not template files.
Files:
app/views/recurring_transactions/index.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bundle exec erb_lint ./app/**/*.erb -afor ERB linting with auto-correct before pull requests.
Files:
app/views/recurring_transactions/index.html.erb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/recurring_transactions/index.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/recurring_transactions/index.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/recurring_transactions/index.html.erb
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Account sync must perform transfer auto-matching, calculate daily balance records from start_date to Date.current, calculate holdings, and optionally enrich transaction data
Applied to files:
app/models/family/sync_complete_event.rb
📚 Learning: 2025-11-24T16:56:30.669Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.669Z
Learning: Syncable models (Account, PlaidItem, Family) must implement sync operations tracked via Sync records that log status, errors, and maintain an audit trail
Applied to files:
app/models/family/sync_complete_event.rbdb/schema.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
config/locales/views/recurring_transactions/en.ymlapp/views/recurring_transactions/index.html.erb
📚 Learning: 2025-09-13T11:27:55.271Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/controllers/enable_banking_items_controller.rb:48-62
Timestamp: 2025-09-13T11:27:55.271Z
Learning: The `create_enable_banking_item!` method in `Family::EnableBankingConnectable` uses `find_or_create_by(id: enable_banking_id)` to either find an existing EnableBankingItem or create a new one, so it automatically handles updates to existing items without creating duplicates.
Applied to files:
test/models/recurring_transaction_test.rbapp/models/recurring_transaction/identifier.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)
Applied to files:
app/jobs/identify_recurring_transactions_job.rbapp/models/recurring_transaction.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Use Sidekiq for background jobs (e.g., SyncJob, ImportJob, AssistantResponseJob)
Applied to files:
app/jobs/identify_recurring_transactions_job.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/controllers/**/*.rb : Use Rails strong parameters and CSRF protection throughout the application
Applied to files:
config/routes.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/controllers/**/*.rb : Use strong parameters and CSRF protection throughout the application.
Applied to files:
config/routes.rb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{rb,erb,html.erb} : Use `Current.family` for the current family context. Do NOT use `current_family`.
Applied to files:
app/controllers/recurring_transactions_controller.rb
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to **/*.{rb,erb} : Use `Current.family` for accessing the current family. Do NOT use `current_family`
Applied to files:
app/controllers/recurring_transactions_controller.rb
📚 Learning: 2025-11-24T16:55:59.156Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.156Z
Learning: Applies to db/migrate/*.rb : ActiveRecord migrations must inherit from `ActiveRecord::Migration[7.2]`. Do NOT use version 8.0 yet
Applied to files:
db/migrate/20251215100443_add_recurring_transactions_disabled_to_families.rbdb/schema.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Use Turbo Frames for page sections instead of client-side solutions
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb} : Leverage Turbo frames for page sections over client-side solutions.
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.rb : Use Rails pluralization in i18n: `t("transactions.count", count: transactions.count)`.
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Prefer native HTML elements over JavaScript-based components. Use semantic HTML: <dialog> for modals, <details><summary> for disclosures
Applied to files:
app/views/recurring_transactions/index.html.erb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Document breaking changes in rules
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:56:44.818Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.818Z
Learning: Update rules after major refactors
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to db/migrate/*.rb : Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to db/migrate/**/*.rb : Use simple validations (null checks, unique indexes) in database layer
Applied to files:
db/schema.rb
🧬 Code graph analysis (5)
test/models/recurring_transaction_test.rb (1)
app/models/recurring_transaction.rb (1)
identify_patterns_for!(47-49)
app/jobs/identify_recurring_transactions_job.rb (1)
app/models/recurring_transaction/identifier.rb (1)
identify_recurring_patterns(10-139)
app/controllers/recurring_transactions_controller.rb (1)
app/models/recurring_transaction.rb (1)
identify_patterns_for!(47-49)
app/models/recurring_transaction/identifier.rb (1)
app/models/recurring_transaction.rb (1)
calculate_next_expected_date(269-280)
app/models/recurring_transaction.rb (1)
app/jobs/identify_recurring_transactions_job.rb (1)
schedule_for(26-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (16)
app/models/family/sync_complete_event.rb (1)
32-37: LGTM! Async scheduling is appropriate here.The non-bang variant correctly schedules the debounced job to run after sync completion, while error handling ensures sync resilience. This aligns with the PR's goal of running identification once for all syncs rather than per-provider.
config/locales/views/recurring_transactions/en.yml (1)
12-15: LGTM! Localization follows conventions.The new i18n keys follow hierarchical structure and provide clear user-facing strings for the settings toggle feature.
db/schema.rb (1)
13-13: LGTM! Schema correctly reflects migration.The schema version bump and column addition match the migration exactly, maintaining consistency between the migration and schema definition.
Also applies to: 411-411
config/routes.rb (1)
185-185: LGTM! Route follows RESTful conventions.The PATCH route for update_settings is appropriately placed in the collection and follows Rails conventions for updating resources.
test/models/recurring_transaction_test.rb (1)
30-30: LGTM! Tests correctly use synchronous variant.All tests appropriately use
identify_patterns_for!to enable synchronous execution and immediate result verification, which is necessary for the test assertions to work correctly.Also applies to: 59-59, 162-162, 190-190, 238-238, 264-264, 323-323, 566-566
app/models/recurring_transaction.rb (2)
46-49: LGTM! Bang variant preserves synchronous behavior.The new
identify_patterns_for!method correctly delegates toIdentifier.new(family).identify_recurring_patterns, maintaining the original synchronous execution path for manual triggers.
40-44: The return value change is safe. The only caller of the non-bang variant (Family::SyncCompleteEventat line 34) does not use the return value—it simply schedules the job and ignores the result, which is the correct async behavior. The bang variant correctly returns the count for synchronous callers (controller and tests). No breaking change occurs.Likely an incorrect or invalid review comment.
app/controllers/recurring_transactions_controller.rb (2)
8-9: LGTM! Family exposure for settings form.Adding
@familyto the index action appropriately exposes the family context for the settings toggle UI.
22-31: LGTM! Correct use of synchronous variant for manual triggers.The identify action appropriately uses
identify_patterns_for!(bang variant) to execute pattern identification synchronously when manually triggered from the UI, ensuring the count is available for the flash message.app/models/recurring_transaction/identifier.rb (1)
84-132: Solid race-condition handling.The pattern of catching
RecordNotUniqueand re-fetching the existing record is the correct approach for handling concurrent creation. Good use of defensive retry flow.One observation: in the rescue path (line 126-131),
statusis not explicitly set, whereas in the main path (line 110) you preserve status for existing records. This is likely intentional since the rescued record already exists with a status, but worth confirming this aligns with expected behavior.app/views/recurring_transactions/index.html.erb (3)
30-38: Toggle logic looks correct but is subtle.The hidden field inverts the current state (
disabled? ? "false" : "true") so clicking the toggle submits the opposite value. This is correct but could be confusing to maintain.The form uses
data: { turbo_frame: "_top" }which will perform a full-page Turbo visit on submit, and theauto-submit-formcontroller withautotarget will trigger submission on toggle change. This aligns with the Hotwire-first approach from the coding guidelines.
42-57: Good use of conditional rendering and semantic structure.The info panel is properly guarded by the feature flag and uses appropriate design tokens (
bg-container-inset,border-secondary,text-primary, etc.). The icon helper is correctly used per coding guidelines.
59-74: Clean empty state implementation.The empty state is well-structured with proper vertical centering and clear call-to-action. Good use of the
DS::Linkcomponent withvariant: "primary"for the main action.app/jobs/identify_recurring_transactions_job.rb (3)
78-82: Advisory lock key generation is sound but has theoretical collision risk.The MD5 hash modulo
2^31approach creates a positive 31-bit integer suitable for PostgreSQL advisory locks. While collision probability is extremely low for practical family counts, it's worth documenting this design choice.
45-59: Multiple existence queries for sync checks are acceptable here.Each
exists?query is efficient (usesEXISTSsubquery), and the earlyreturn truepattern short-circuits once any incomplete sync is found. Therespond_to?guards handle optional provider associations gracefully.
8-24: Well-structured guard clauses with proper early returns.The perform method correctly handles:
- Missing family (soft delete or race condition)
- Feature disabled check
- Stale job detection via cache comparison
- Incomplete sync prevention
- Advisory lock for concurrent execution safety
This multi-layered guard approach aligns well with the PR objective of running identify as a job scheduled once for all syncs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/models/user.rb (1)
237-258: Transactions preferences implementation is consistent; consider DRYing with existing helpersThe namespacing (
"transactions_#{key}") andtransactions_collapsed_sectionslookup align correctly, and the pessimistic-lock + deep_dup pattern matches the existing dashboard/reports flows, so behavior looks sound and concurrency-safe.You now have three very similar updaters (
update_dashboard_preferences,update_reports_preferences,update_transactions_preferences). It may be worth extracting a small private helper (e.g.,update_preferences_with_namespace(namespace, prefs)) to centralize the merge logic and reduce future drift.app/controllers/transactions_controller.rb (1)
247-249: Alignpreferences_paramswith dashboard preferences pattern for clearer hashes
params.require(:preferences).permit(collapsed_sections: {})works, but you’re now passing controller parameter objects down into the model, whereasPagesController#preferences_paramsmaterializes plain Ruby hashes (to_unsafe_h).For consistency and to keep models decoupled from strong-params types, consider mirroring that approach:
- def preferences_params - params.require(:preferences).permit(collapsed_sections: {}) - end + def preferences_params + prefs = params.require(:preferences) + + {}.tap do |permitted| + permitted["collapsed_sections"] = prefs[:collapsed_sections].to_unsafe_h if prefs[:collapsed_sections] + end + endThis makes the shape of data reaching
User#update_transactions_preferencesexplicit and uniform with other preference paths.app/views/transactions/index.html.erb (1)
76-103: Collapsible upcoming recurring section wiring looks solidThe Stimulus wiring (
data-controller,*-target,*-value) matches the controller API, the collapsed state is correctly sourced fromCurrent.user.transactions_section_collapsed?, and ARIA/keyboard affordances are in place, with dates and labels localized (t,l). This fits the existing dashboard/reports collapsible patterns well.If you later generalize more sections, this whole block is a good candidate for a small ViewComponent shared across pages.
app/javascript/controllers/transactions_section_controller.js (1)
1-96: Well-scoped Stimulus controller; behavior and persistence are coherentThe controller is nicely focused: targets/values are minimal,
connecthonors the initial collapsed state without extra writes, andcollapse/expandkeep DOM visibility, chevron rotation, andaria-expandedin sync before optimistically persisting viasavePreference. Error handling is non-intrusive (console-only) and CSRF is handled correctly.If you find this firing a lot in practice, you could later add a small debounce/throttle around
savePreferenceor make the endpoint URL a value (e.g.,urlValue) to share the controller across contexts, but it’s not necessary for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/controllers/transactions_controller.rb(2 hunks)app/javascript/controllers/transactions_section_controller.js(1 hunks)app/models/user.rb(1 hunks)app/views/transactions/index.html.erb(1 hunks)config/locales/views/transactions/en.yml(1 hunks)config/routes.rb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- config/routes.rb
🧰 Additional context used
📓 Path-based instructions (33)
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/javascript/controllers/transactions_section_controller.jsapp/views/transactions/index.html.erb
app/javascript/controllers/**/*_controller.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/javascript/controllers/**/*_controller.js: Keep Stimulus controllers lightweight and simple with fewer than 7 targets
Keep Stimulus controllers small with private methods and clear public API
Files:
app/javascript/controllers/transactions_section_controller.js
app/javascript/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use D3.js for financial visualizations (time series, donut, sankey charts)
Files:
app/javascript/controllers/transactions_section_controller.js
{app/components/**/*.js,app/components/**/*.ts,app/javascript/controllers/**/*.{js,ts}}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Stimulus Controllers for interactivity, organized alongside components in
app/components/or globally inapp/javascript/controllers/.
Files:
app/javascript/controllers/transactions_section_controller.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use D3.js for financial visualizations including time series, donut, and sankey charts.
Files:
app/javascript/controllers/transactions_section_controller.js
{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts},app/components/**/*.html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use declarative Stimulus actions in HTML with
data-actionanddata-*-targetattributes. Keep controllers lightweight and simple (< 7 targets).
Files:
app/javascript/controllers/transactions_section_controller.js
{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}}
📄 CodeRabbit inference engine (CLAUDE.md)
{app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}}: Use private methods in Stimulus controllers and expose a clear public API.
Component Stimulus controllers should stay in component directory (app/components/). Global controllers should be inapp/javascript/controllers/.
Files:
app/javascript/controllers/transactions_section_controller.js
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/javascript/controllers/transactions_section_controller.jsapp/controllers/transactions_controller.rbapp/views/transactions/index.html.erbapp/models/user.rb
**/*_controller.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
**/*_controller.{js,ts}: Use declarative actions in Stimulus controllers, not imperative event listeners - define actions in ERB templates using data-action attributes instead of binding events in the controller's connect() method
Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity
Use private methods in Stimulus controllers and expose a clear public API
Keep Stimulus controllers focused on single or highly related responsibilities - domain logic should not belong in Stimulus controllers
Files:
app/javascript/controllers/transactions_section_controller.js
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/javascript/controllers/transactions_section_controller.jsapp/views/transactions/index.html.erb
app/javascript/controllers/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Use Stimulus targets to reference DOM elements, not manual getElementById calls
Files:
app/javascript/controllers/transactions_section_controller.js
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/javascript/controllers/transactions_section_controller.jsapp/controllers/transactions_controller.rbapp/models/user.rb
app/javascript/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript code should use
lowerCamelCasefor variables and functions, andPascalCasefor classes and components
Files:
app/javascript/controllers/transactions_section_controller.js
app/**/*.{js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript and CSS code should be formatted using Biome via
npm run lintandnpm run format
Files:
app/javascript/controllers/transactions_section_controller.js
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/controllers/transactions_controller.rbapp/models/user.rb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/transactions_controller.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/controllers/transactions_controller.rbapp/views/transactions/index.html.erbapp/models/user.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/controllers/transactions_controller.rbapp/views/transactions/index.html.erbapp/models/user.rb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/controllers/transactions_controller.rbapp/views/transactions/index.html.erbapp/models/user.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/controllers/transactions_controller.rbapp/views/transactions/index.html.erbapp/models/user.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/controllers/transactions_controller.rbapp/models/user.rb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-sideViews should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by
erb-lintper.erb_lint.yml)
Files:
app/views/transactions/index.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/transactions/index.html.erb
**/*.{erb,html,vue,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined inapp/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g.,text-primarynottext-white).
Prefer semantic HTML elements over JavaScript components (e.g., use<dialog>for modals,<details><summary>for disclosures).
Files:
app/views/transactions/index.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb}: Useiconhelper for icons, never uselucide_icondirectly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript.
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/transactions/index.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep domain logic out of view templates. Logic belongs in component files, not template files.
Files:
app/views/transactions/index.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bundle exec erb_lint ./app/**/*.erb -afor ERB linting with auto-correct before pull requests.
Files:
app/views/transactions/index.html.erb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/transactions/index.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/transactions/index.html.erb
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/views/transactions/en.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/views/transactions/en.yml
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/user.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/user.rb
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/jobs/**/*.rb : Handle account syncing, import processing, and AI responses via background jobs (Sidekiq)
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers small with private methods and clear public API
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts},app/components/**/*.html.erb} : Use declarative Stimulus actions in HTML with `data-action` and `data-*-target` attributes. Keep controllers lightweight and simple (< 7 targets).
Applied to files:
app/javascript/controllers/transactions_section_controller.jsapp/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.js,app/components/**/*.ts,app/javascript/controllers/**/*.{js,ts}} : Use Stimulus Controllers for interactivity, organized alongside components in `app/components/` or globally in `app/javascript/controllers/`.
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to {app/components/**/*.{js,ts},app/javascript/controllers/**/*.{js,ts}} : Use private methods in Stimulus controllers and expose a clear public API.
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers focused on UI behavior; avoid domain/business logic and prefer single, closely related responsibilities; use Stimulus callbacks, actions, targets, values, and classes
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets and expose a small public API with other logic in private methods
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations
Applied to files:
app/javascript/controllers/transactions_section_controller.jsapp/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers lightweight and simple with fewer than 7 targets
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Use private methods in Stimulus controllers and expose a clear public API
Applied to files:
app/javascript/controllers/transactions_section_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Use declarative actions in Stimulus controllers, not imperative event listeners - define actions in ERB templates using data-action attributes instead of binding events in the controller's connect() method
Applied to files:
app/javascript/controllers/transactions_section_controller.jsapp/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to app/controllers/**/*.rb : Use query params for state over localStorage/sessions.
Applied to files:
app/controllers/transactions_controller.rb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use ViewComponents when elements have complex logic/styling, are reused across contexts, need variants/slots/accessibility, or require Stimulus interactivity
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
app/views/transactions/index.html.erbconfig/locales/views/transactions/en.yml
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to **/*.{erb,html.erb} : Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Prefer native HTML elements over JavaScript-based components. Use semantic HTML: <dialog> for modals, <details><summary> for disclosures
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to **/*.{erb,html.erb} : Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Use Turbo Frames for page sections instead of client-side solutions
Applied to files:
app/views/transactions/index.html.erb
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Applied to files:
config/locales/views/transactions/en.yml
📚 Learning: 2025-11-24T16:55:43.069Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.069Z
Learning: Applies to **/*.rb : Use Rails pluralization in i18n: `t("transactions.count", count: transactions.count)`.
Applied to files:
config/locales/views/transactions/en.yml
🧬 Code graph analysis (2)
app/controllers/transactions_controller.rb (3)
app/controllers/pages_controller.rb (2)
update_preferences(24-30)preferences_params(58-64)app/models/current.rb (1)
user(8-10)app/models/user.rb (1)
update_transactions_preferences(242-258)
app/models/user.rb (4)
app/javascript/controllers/transactions_section_controller.js (1)
preferences(57-61)app/javascript/controllers/dashboard_section_controller.js (1)
preferences(57-61)app/javascript/controllers/reports_section_controller.js (1)
preferences(57-61)app/controllers/onboardings_controller.rb (1)
preferences(10-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (2)
config/locales/views/transactions/en.yml (2)
51-51: Potential key duplication: "Import" appears under bothheaderandindex.The key
import: Importis defined at line 51 (headersection) and again at line 55 (indexsection) with identical translations. If these contexts require different wording or if one is redundant, this should be clarified or refactored to avoid maintenance issues.Is this duplication intentional, or should you reuse the existing
header.importkey across both contexts?Also applies to: 55-55
56-56: The key placement is correct and matches the code usage. Inapp/views/transactions/index.html.erb, the key is accessed ast("transactions.toggle_recurring_section"), which exactly matches the current structure wheretoggle_recurring_sectionresides at thetransactionsroot level. Moving it totransactions.index.toggle_recurring_sectionwould break this reference.The hierarchical structure is appropriate:
toggle_recurring_sectioncontrols a feature-level toggle for upcoming recurring transactions, not a view-specific subsection, so root-level placement undertransactionsis justified per i18n guidelines.The duplicate "Import" keys (lines 51 and 55) are not a problem—they exist in different contexts (
headervsindex) and serve distinct purposes.No changes needed.
Likely an incorrect or invalid review comment.
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.