LLM Cache reset #260
LLM Cache reset #260
Conversation
- We shouldn't cache LLM results if they returned empty category or merchant - Add button to clear AI cache ( good to make it clear to users this will cause costs? ) - Added a job to clear the cache, with low prio
WalkthroughAdds an AI cache clearing flow (UI button, route, controller action, background job) and implements cache-clear methods on Enrichable. Also changes auto-categorizer/merchant-detector to lock attributes only when detected values are present, and updates related tests and Setting setter to invalidate cache on model change. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RulesUI as Rules Index UI
participant Controller as RulesController
participant Queue as Job Queue
participant Job as ClearAiCacheJob
participant TxnModel as Transaction
participant EntryModel as Entry
User->>RulesUI: Click "Reset AI cache"
RulesUI->>Controller: POST /rules/clear_ai_cache
Controller->>Queue: Enqueue ClearAiCacheJob.perform_later
Controller-->>RulesUI: Redirect with notice
Note right of Queue: Job executes asynchronously
Queue->>Job: Run ClearAiCacheJob
Job->>TxnModel: Transaction.clear_ai_cache
TxnModel->>TxnModel: Find AI enrichments → unlock attrs → delete enrichments
Job->>EntryModel: Entry.clear_ai_cache
EntryModel->>EntryModel: Find AI enrichments → unlock attrs → delete enrichments
Job-->>Job: Log completion
sequenceDiagram
participant User
participant SettingsUI as Settings UI
participant SettingModel as Setting
participant Queue as Job Queue
User->>SettingsUI: Change OpenAI model value
SettingsUI->>SettingModel: set openai_model=(new_value)
SettingModel->>SettingModel: compare with previous value
alt previous non-blank and changed
SettingModel->>Queue: Enqueue ClearAiCacheJob.perform_later
SettingModel->>SettingModel: Log change
end
SettingModel-->>SettingsUI: Persist new raw_openai_model
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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)
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: 2
🧹 Nitpick comments (2)
app/views/rules/index.html.erb (1)
6-16: Consider adding cost warning to the confirmation dialog.The confirmation dialog clearly describes the action but doesn't mention potential costs. The PR description notes "the UI should indicate this will cause costs." While a cost warning exists at lines 35-42 for self-hosted instances, consider adding a cost note directly in the confirmation dialog body to ensure users are aware before clicking "Reset Cache."
Apply this diff to add a cost warning:
confirm: CustomConfirm.new( title: "Reset AI cache?", - body: "Are you sure you want to reset the AI cache? This will allow AI rules to re-process all transactions.", + body: "Are you sure you want to reset the AI cache? This will allow AI rules to re-process all transactions. Warning: This may incur significant API costs if filters are not used properly.", btn_text: "Reset Cache" )) %>app/models/concerns/enrichable.rb (1)
106-123: Instance method implementation is cleaner, but verify thread safety.The instance method correctly uses the
data_enrichmentsassociation and avoids N+1 queries. However, there are two considerations:
- update_column usage: Line 118 bypasses callbacks and doesn't update
updated_at. This may be intentional for performance, but consider whether maintaining timestamp integrity is important for cache operations.- Race conditions: If another process enriches attributes while this method is executing, there could be a race condition between unlocking and concurrent enrichment.
Consider using
update_columns(plural) to includeupdated_at:-update_column(:locked_attributes, new_locked_attributes) +update_columns(locked_attributes: new_locked_attributes, updated_at: Time.current)To verify the race condition concern, please confirm whether concurrent enrichment during cache clearing is a realistic scenario in your system. If the cache clearing is infrequent and typically run during maintenance windows, the current implementation may be acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/components/DS/menu_item.rb(1 hunks)app/controllers/rules_controller.rb(1 hunks)app/jobs/clear_ai_cache_job.rb(1 hunks)app/models/concerns/enrichable.rb(2 hunks)app/models/family/auto_categorizer.rb(1 hunks)app/models/family/auto_merchant_detector.rb(1 hunks)app/models/setting.rb(2 hunks)app/views/rules/index.html.erb(1 hunks)config/routes.rb(1 hunks)test/models/family/auto_categorizer_test.rb(1 hunks)test/models/family/auto_merchant_detector_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (19)
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/controllers/rules_controller.rbapp/jobs/clear_ai_cache_job.rbapp/models/setting.rbapp/components/DS/menu_item.rbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbapp/models/family/auto_merchant_detector.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/controllers/rules_controller.rbapp/jobs/clear_ai_cache_job.rbapp/models/setting.rbapp/components/DS/menu_item.rbtest/models/family/auto_merchant_detector_test.rbconfig/routes.rbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbtest/models/family/auto_categorizer_test.rbapp/models/family/auto_merchant_detector.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/controllers/rules_controller.rbapp/jobs/clear_ai_cache_job.rbapp/models/setting.rbapp/components/DS/menu_item.rbtest/models/family/auto_merchant_detector_test.rbconfig/routes.rbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbtest/models/family/auto_categorizer_test.rbapp/models/family/auto_merchant_detector.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/controllers/rules_controller.rbapp/models/setting.rbapp/views/rules/index.html.erbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbapp/models/family/auto_merchant_detector.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Domain models should not call Provider::Registry directly; use a Provided concern within the model’s namespace to select providers and expose convenience methods
Use ActiveRecord validations for forms and complex domain constraints.
app/models/**/*.rb: Place business logic in POROs and model classes under app/models
Models should answer questions about themselves (e.g., prefer account.balance_series over service objects)
Implement complex validations and business logic with ActiveRecord validations
Model-level validations may mirror DB constraints but are not strictly required
Files:
app/models/setting.rbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbapp/models/family/auto_merchant_detector.rb
app/{helpers,models}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Files:
app/models/setting.rbapp/models/concerns/enrichable.rbapp/models/family/auto_categorizer.rbapp/models/family/auto_merchant_detector.rb
app/components/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/components/**/*: Prefer ViewComponents for reusable or complex UI components; keep domain logic out of view templates.
Logic belongs in component files, not in component template (*.html.erb, *.html.slim) files.
Files:
app/components/DS/menu_item.rb
app/views/**/*.erb
📄 CodeRabbit inference engine (AGENTS.md)
app/views/**/*.erb: Keep heavy logic out of ERB views; prefer helpers/components instead
ERB templates are linted by erb-lint per .erb_lint.ymlAlways use the icon helper (icon) for icons; never call lucide_icon directly
Files:
app/views/rules/index.html.erb
app/views/**/*.html.*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.html.*: Use partials only for simple, context-specific, mostly static HTML content.
Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Use query params for state, not localStorage or sessionStorage.
Always perform server-side formatting for currencies, numbers, and dates.
Files:
app/views/rules/index.html.erb
**/*.{html,erb,slim,js,jsx,ts,tsx,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Files:
app/views/rules/index.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/views/**/*.html.erb: Prefer native HTML elements (e.g., ,) over JS-based components
Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Prefer native client-side form validation when possible
Files:
app/views/rules/index.html.erb
{app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Files:
app/views/rules/index.html.erb
{app/components/**/*.{js,ts,erb},app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component-scoped Stimulus controllers in app/components must be used only within their component views, not in app/views
Files:
app/views/rules/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/**}: Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Prefer functional tokens from the design system over raw Tailwind values (e.g., use text-primary, bg-container, border border-primary instead of text-white, bg-white, border-gray-200)
Files:
app/views/rules/index.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/rules/index.html.erb
{app/views,app/components}/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views,app/components}/**/*.html.erb: Keep domain logic out of ERB templates; compute values in component/controller code and reference them in the template
Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Files:
app/views/rules/index.html.erb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Name Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
test/**/*_test.rb: Always use Minitest for tests; do not use RSpec
Do not use factories (e.g., FactoryBot) in tests; rely on fixtures instead
For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Only write tests for critical and important code paths
Avoid tests that merely verify framework/ActiveRecord behavior
Test boundaries correctly: for queries, assert returned values; for commands, assert collaborators are called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct for mock instances; use a mock class for complex cases
Only mock what’s necessary; don’t mock return values unless they are under test
Files:
test/models/family/auto_merchant_detector_test.rbtest/models/family/auto_categorizer_test.rb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/routes.rb
app/models/concerns/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use Rails concerns organized around model traits (not as generic code dumps)
Files:
app/models/concerns/enrichable.rb
🧠 Learnings (1)
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/models/**/*.rb : Implement complex validations and business logic with ActiveRecord validations
Applied to files:
app/models/concerns/enrichable.rb
🧬 Code graph analysis (7)
app/controllers/rules_controller.rb (1)
app/models/concerns/enrichable.rb (2)
clear_ai_cache(29-53)clear_ai_cache(107-123)
app/jobs/clear_ai_cache_job.rb (2)
app/controllers/rules_controller.rb (1)
clear_ai_cache(81-84)app/models/concerns/enrichable.rb (2)
clear_ai_cache(29-53)clear_ai_cache(107-123)
app/models/setting.rb (1)
app/models/provider/configurable.rb (2)
value(188-202)present?(205-207)
app/components/DS/menu_item.rb (1)
app/helpers/custom_confirm.rb (1)
to_data_attribute(23-30)
app/models/concerns/enrichable.rb (3)
app/models/family/auto_categorizer.rb (1)
scope(78-82)app/models/family/auto_merchant_detector.rb (1)
scope(92-96)app/controllers/rules_controller.rb (1)
clear_ai_cache(81-84)
app/models/family/auto_categorizer.rb (1)
app/models/concerns/enrichable.rb (1)
lock_attr!(92-94)
app/models/family/auto_merchant_detector.rb (1)
app/models/concerns/enrichable.rb (1)
lock_attr!(92-94)
⏰ 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 (10)
config/routes.rb (1)
170-170: LGTM!The collection route for clearing AI cache is correctly implemented and follows Rails conventions.
app/components/DS/menu_item.rb (1)
53-54: LGTM!The duck-typed check for
to_data_attributeelegantly supports both simple confirmation strings and rich CustomConfirm objects while maintaining backward compatibility.app/models/setting.rb (2)
38-38: LGTM!The alias correctly captures the original setter before redefinition, following the same pattern used for
onboarding_state. This prevents circular reference when the new setter callsraw_openai_model=.
53-62: Verify the cache clearing condition is intentional.The condition on Line 58 only clears the cache when changing from one non-blank value to another. Setting the model for the first time (from blank to a value) will not clear the cache. This seems intentional to avoid clearing non-existent cache on initial setup, but please confirm this is the desired behavior.
app/models/family/auto_merchant_detector.rb (1)
53-54: LGTM! Important correctness fix.Moving the lock inside the conditional ensures that only successful merchant detections are cached. This prevents the rule from being permanently blocked when the LLM returns empty results, aligning with the PR's objective to avoid caching unsuccessful AI results.
app/models/family/auto_categorizer.rb (1)
41-41: LGTM! Important correctness fix.Moving the lock inside the conditional ensures that only successful categorizations are cached. This prevents the rule from being permanently blocked when the LLM returns empty results, consistent with the same fix in
auto_merchant_detector.rb.app/jobs/clear_ai_cache_job.rb (1)
4-14: The code is correct—verification confirmed.Both Transaction and Entry models have access to the
clear_ai_cacheclass method:
- Transaction includes Entryable, which includes Enrichable
- Entry includes Enrichable directly
The job correctly calls the method on both models with appropriate logging.
test/models/family/auto_merchant_detector_test.rb (1)
37-39: LGTM! Test correctly reflects conditional locking behavior.The updated assertion correctly validates that only transactions with successfully detected merchants are locked, leaving txn3 (with nil merchant) enrichable. This aligns with the PR's goal of not caching empty LLM results.
test/models/family/auto_categorizer_test.rb (1)
36-38: LGTM! Test correctly reflects conditional locking behavior.The updated assertion properly validates that only successfully categorized transactions are locked, leaving txn3 (with nil category) enrichable. This mirrors the merchant detection logic and aligns with the PR's goal.
app/models/concerns/enrichable.rb (1)
18-18: LGTM! Association enables cache clearing functionality.The polymorphic association with
dependent: :destroyis correctly configured to support the new cache clearing methods.
| def clear_ai_cache | ||
| ClearAiCacheJob.perform_later | ||
| redirect_to rules_path, notice: "AI cache is being cleared. This may take a few moments." | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Consider verifying authorization.
The action correctly enqueues the background job and provides clear user feedback. Given the PR description warns this is an "aggressive feature" with potential cost implications, verify that appropriate authorization checks are in place (either at the application controller level or through the UI confirmation).
Run the following script to check if ApplicationController has authorization checks:
🏁 Script executed:
#!/bin/bash
# Description: Verify authorization setup in ApplicationController
# Check ApplicationController for authentication/authorization
rg -n -A10 'class ApplicationController' app/controllers/application_controller.rb
# Check if RulesController has additional authorization
rg -n 'before_action|authorize|authenticate' app/controllers/rules_controller.rbLength of output: 501
🏁 Script executed:
#!/bin/bash
# Check for clear_ai_cache authorization and method definition
rg -n -B5 -A10 'def clear_ai_cache' app/controllers/rules_controller.rb
# Check for authorization patterns in RulesController (policy, authorize, etc.)
rg -n 'authorize|policy|pundit|cancan|skip_before_action' app/controllers/rules_controller.rb
# Check Authentication module to understand what it provides
fd -t f 'authentication' app/concerns/ --exec cat {}Length of output: 531
🏁 Script executed:
#!/bin/bash
# Check Authentication module implementation
fd -t f 'authentication.rb' app/concerns/ --exec cat {}
# Check if clear_ai_cache has explicit before_action or authorization
rg -n -B2 'clear_ai_cache' app/controllers/rules_controller.rb
# Check FeatureGuardable for clear_ai_cache patterns
rg -n 'FeatureGuardable|feature_guard|feature_flag' app/concerns/ app/controllers/Length of output: 347
🏁 Script executed:
#!/bin/bash
# Find and display Authentication concern
fd -type f -name '*authentication*' app/concerns/ --exec cat {}
# Check full context around clear_ai_cache definition
rg -n -B5 -A15 'def clear_ai_cache' app/controllers/rules_controller.rb
# Check if routes file has any special handling for clear_ai_cache
rg -n 'clear_ai_cache' config/routes.rb
# Check FeatureGuardable concern
fd -type f -name '*feature_guardable*' app/concerns/ --exec cat {}Length of output: 1098
🏁 Script executed:
#!/bin/bash
# Display FeatureGuardable concern
cat app/controllers/concerns/feature_guardable.rb
# Find and display Authentication concern (likely in app/controllers/concerns/)
fd 'authentication' app/controllers/concerns/ -x cat {}
# Check for any skip_before_action or explicit authorization in full RulesController
rg -n 'skip_before_action|authorize|private.*def' app/controllers/rules_controller.rb | head -20
# Check ClearAiCacheJob for authorization
fd 'clear_ai_cache_job' -x cat {}Length of output: 2572
🏁 Script executed:
#!/bin/bash
# Display FeatureGuardable concern
cat app/controllers/concerns/feature_guardable.rb
# Find and display Authentication concern
fd 'authentication' app/controllers/concerns/ -x cat {}
# Check for skip_before_action patterns in RulesController
rg -n 'skip_before_action' app/controllers/rules_controller.rb
# Check ClearAiCacheJob for any authorization
fd 'clear_ai_cache_job' -x cat {}
# Check if there's a policy file for RulesController (Pundit pattern)
fd 'rules_policy|rule_policy' -x cat {}Length of output: 2572
Add authorization check to prevent unauthorized cache clearing.
The action is only protected by authentication (via the Authentication module), meaning any authenticated user can trigger a global cache clear affecting all users. For an "aggressive feature" with cost implications, this needs explicit authorization:
- ClearAiCacheJob clears cache globally for all transactions and entries (not scoped to Current.family like other actions)
- No role check, feature flag guard, or permission control
- Unlike
destroy_all(line 76-79, also risky), this has no visibility that it's system-wide
Add authorize :rule, :clear_ai_cache? (if using Pundit) or implement a before_action to verify admin/appropriate permissions before clearing the global cache.
🤖 Prompt for AI Agents
In app/controllers/rules_controller.rb around lines 81 to 84, the clear_ai_cache
action lacks an authorization check allowing any authenticated user to clear the
global AI cache; add an explicit authorization before allowing this destructive,
system-wide operation—either call authorize :rule, :clear_ai_cache? (Pundit) at
the start of the action or add a before_action (e.g., ensure_admin or
authorize_clear_ai_cache) that verifies the current user has admin/permission to
clear global caches, and only then invoke ClearAiCacheJob and redirect; ensure
the authorization uses the same policy/role checks used elsewhere in the
controller for consistency.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: soky srm <[email protected]>
1 participant
This affects the data_enrichments table with source = 'ai' and attribute_name = 'merchant_id' or attribute_name = 'category_id'
This is a VERY aggressive feature. If the cache is cleared, running a rule will send ALL of your transactions again to an LLM.
This could incur in a lot of costs, if filters are not used properly.
@jjmata good to discuss
Summary by CodeRabbit
New Features
Bug Fixes