Skip to content

Conversation

manuelmeurer
Copy link

@manuelmeurer manuelmeurer commented Oct 2, 2025

What this does

This fixes the same problem as #380, i.e. when you call RubyLLM::Chat.ask and pass a ActiveStorage::Attached::One object as the with parameter, it would try to call blobs on the record instead of blob.

The reason that this went unnoticed was that the test that was supposed to test it, didn't. 😀
In fact, the "handles multiple attachments" and "handles attachments in ask method" tests in acts_as_attachment_spec.rb were duplicates of the tests with the same names in acts_as_spec.rb. Probably copy-pasted and then forgotten to adjust.
I renamed and reordered the specs in acts_as_spec to make them easier to understand (but otherwise left the tests untouched) and renamed and rewrote the two tests in acts_as_attachment_spec.rb, so that they actually test that passing ActiveStorage::Attached::One and ActiveStorage::Attached::Many objects work as expected.

Unfortunately, the "handles ActiveStorage::Attached::Many in ask method" test fails locally for me with this error:

  1) RubyLLM::ActiveRecord::ActsAs attachment handling handles ActiveStorage::Attached::Many in ask method
     Failure/Error: message_record.attachments.attach(attachables) if attachables.any?

     NoMethodError:
       undefined method 'encode' for nil
     # ./lib/ruby_llm/active_record/chat_methods.rb:302:in 'RubyLLM::ActiveRecord::ChatMethods#persist_content'
     # ./lib/ruby_llm/active_record/chat_methods.rb:178:in 'RubyLLM::ActiveRecord::ChatMethods#create_user_message'
     # ./lib/ruby_llm/active_record/chat_methods.rb:183:in 'RubyLLM::ActiveRecord::ChatMethods#ask'
     # ./spec/ruby_llm/active_record/acts_as_attachment_spec.rb:83:in 'block (3 levels) in <top (required)>'
     # ./spec/support/rspec_configuration.rb:17:in 'block (3 levels) in <top (required)>'
     # ./spec/support/rspec_configuration.rb:16:in 'block (2 levels) in <top (required)>'

I suspect it's a problem with my local setup, but I can't even commit the failing test to git due to pre-commit hooks, so I don't know how to trigger CI with it to check if it passes. 🤷‍♂️

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Performance improvement

Scope check

  • I read the Contributing Guide
  • This aligns with RubyLLM's focus on LLM communication
  • This isn't application-specific logic that belongs in user code
  • This benefits most users, not just my specific use case

Quality check

  • I ran overcommit --install and all hooks pass
  • I tested my changes thoroughly
    • For provider changes: Re-recorded VCR cassettes with bundle exec rake vcr:record[provider_name]
    • All tests pass: bundle exec rspec
  • I updated documentation if needed
  • I didn't modify auto-generated files manually (models.json, aliases.json)

API changes

  • Breaking change
  • New public methods/classes
  • Changed method signatures
  • No API changes

Related issues

#380

@manuelmeurer
Copy link
Author

Alright, I figured it out! 🥳
Both ActiveStorage::Attached::One and ActiveStorage::Attached::Many are now handled property and the specs pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant