fix acts_as delegation to return self instead of RubyLLM#82
fix acts_as delegation to return self instead of RubyLLM#82crmne merged 11 commits intocrmne:mainfrom
Conversation
The issue was that and methods were returning the RubyLLM::Chat instance instead of the ActiveRecord model, which broke the persistence chain when calling .ask() after .with_tools(). This fix: 1. Overrides the with_tool and with_tools methods to return self (the ActiveRecord model) 2. Removes these methods from the delegation list 3. Adds test to verify user messages are persisted when using method chains Fixes issue where user messages weren't being saved to the database when using the method chain.
Extended the fix to apply to all chainable methods: - with_model - with_temperature - on_new_message - on_end_message These methods now return the ActiveRecord model instead of the RubyLLM::Chat instance, ensuring method chains maintain persistence capabilities. Added comprehensive test case that verifies all chainable methods return the correct type and properly persist user messages when used in chains.
Refactored the acts_as_chat and acts_as_message methods to improve readability by removing unnecessary comments and ensuring consistent formatting. Updated the on_new_message and on_end_message methods to use shorthand block syntax for better clarity. Additionally, introduced shared examples in tests to validate chainable methods and callbacks, ensuring they return the correct Chat instance. This enhances the maintainability of the code and improves test coverage for method chaining scenarios.
|
Hey @kieranklaassen when are the messages not created? |
|
When you do this it won't create user messages: You can see that because the class that with_tools return is a RubyLLM::Chat and not a Chat from active record. |
crmne
left a comment
There was a problem hiding this comment.
I can accept it if you can keep the original test as well!
| it 'persists chat history' do # rubocop:disable RSpec/ExampleLength,RSpec/MultipleExpectations | ||
| chat = Chat.create!(model_id: 'gpt-4o-mini') | ||
| chat.ask("What's your favorite Ruby feature?") | ||
|
|
||
| expect(chat.messages.count).to eq(2) | ||
| expect(chat.messages.first.role).to eq('user') | ||
| expect(chat.messages.last.role).to eq('assistant') | ||
| expect(chat.messages.last.content).to be_present | ||
| expect(chat.messages.last.input_tokens).to be_positive | ||
| expect(chat.messages.last.output_tokens).to be_positive |
There was a problem hiding this comment.
why removing this very important test?
There was a problem hiding this comment.
Yes, adding that back was a mistake. I was moving quickly yesterday.
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@crmne looked it over and should be good to go now. TY |
|
@crmne would you mind guiding me in the failing tests on CI? Locally all pass |
|
@kieranklaassen the error message there tells you all you need to know: there are some VCR cassettes missing. I tried it locally and that was the case. |
…th tool functionality Added new VCR cassettes to persist messages and tool calls for ActiveRecord acts_as chainable methods. These cassettes capture interactions with the OpenAI API for arithmetic operations, ensuring accurate testing of user message persistence and tool functionality. - Created `activerecord_actsas_chainable_methods_persists_messages_after_chaining.yml` - Created `activerecord_actsas_with_tools_functionality_persists_user_messages.yml` This addition enhances the test coverage for the integration of tools within the ActiveRecord methods.
|
@crmne I deleted and added VCR cassettes again. How do I run the CI here? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
+ Coverage 92.11% 92.27% +0.15%
==========================================
Files 71 71
Lines 2588 2640 +52
Branches 378 380 +2
==========================================
+ Hits 2384 2436 +52
Misses 204 204 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I had this issue in Rails:
with_tools does not return the proper class so the messages are not created.