Skip to content

Commit 8a65723

Browse files
Fix namespaced models foreign key (#460)
First of all, thank you very much for all the effort you put into this gem. It is amazing. 🚀 ## What this does Version 1.8.2 breaks namespaced models with custom foreign keys. Consider the following example: ```rb ActiveRecord::Migration.create_table :support_conversations, force: true do |t| t.string :model_id t.timestamps end ActiveRecord::Migration.create_table :support_messages, force: true do |t| t.references :conversation, foreign_key: { to_table: :support_conversations } t.string :role t.text :content t.string :model_id t.integer :input_tokens t.integer :output_tokens t.references :tool_call, foreign_key: { to_table: :support_tool_calls } t.timestamps end ActiveRecord::Migration.create_table :support_tool_calls, force: true do |t| t.references :message, foreign_key: { to_table: :support_messages } t.string :tool_call_id t.string :name t.json :arguments t.timestamps end ``` ```rb module Support def self.table_name_prefix 'support_' end class Conversation < ActiveRecord::Base acts_as_chat message_class: 'Support::Message' end class Message < ActiveRecord::Base acts_as_message chat: :conversation, chat_class: 'Support::Conversation', tool_call_class: 'Support::ToolCall' end class ToolCall < ActiveRecord::Base acts_as_tool_call message_class: 'Support::Message' end end ``` ```rb Support::Conversation.last.messages => #<ActiveRecord::StatementInvalid:"PG::UndefinedColumn: ERROR: column support_messages.support_conversation_id does not exist ``` It does not work, since the correct foreign key should have been `support_messages.conversation_id`. I have added a test showcasing this and implemented a fix. Since we cannot infer foreign keys automatically when it doesn't match the model-name, we expose options to pass in a foreign key, similarly to [associations in Rails](https://guides.rubyonrails.org/association_basics.html#foreign-key): ```rb class Support::Message < ActiveRecord::Base acts_as_message chat_class: 'Conversation', chat_foreign_key: 'conversation_id' end ``` Generators have also been updated to add foreign keys if needed. ## Type of change - [X] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation - [ ] Performance improvement ## Scope check - [X] I read the [Contributing Guide](https://github.com/crmne/ruby_llm/blob/main/CONTRIBUTING.md) - [X] This aligns with RubyLLM's focus on **LLM communication** - [X] This isn't application-specific logic that belongs in user code - [X] This benefits most users, not just my specific use case ## Quality check - [X] I ran `overcommit --install` and all hooks pass - [X] I tested my changes thoroughly - [ ] For provider changes: Re-recorded VCR cassettes with `bundle exec rake vcr:record[provider_name]` - [X] All tests pass: `bundle exec rspec` - [ ] I updated documentation if needed - [X] I didn't modify auto-generated files manually (`models.json`, `aliases.json`) ## API changes - [ ] Breaking change - [ ] New public methods/classes - [X] Changed method signatures - [ ] No API changes ## Related issues Fixes bug introduced in #425 --------- Co-authored-by: Carmine Paolino <[email protected]>
1 parent 76021b4 commit 8a65723

File tree

3 files changed

+111
-28
lines changed

3 files changed

+111
-28
lines changed

lib/generators/ruby_llm/generator_helpers.rb

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,34 +52,41 @@ def parse_model_mappings
5252
def acts_as_chat_declaration
5353
params = []
5454

55-
add_association_params(params, :messages, message_table_name, message_model_name, plural: true)
56-
add_association_params(params, :model, model_table_name, model_model_name)
55+
add_association_params(params, :messages, message_table_name, message_model_name,
56+
owner_table: chat_table_name, plural: true)
57+
add_association_params(params, :model, model_table_name, model_model_name,
58+
owner_table: chat_table_name)
5759

5860
"acts_as_chat#{" #{params.join(', ')}" if params.any?}"
5961
end
6062

6163
def acts_as_message_declaration
6264
params = []
6365

64-
add_association_params(params, :chat, chat_table_name, chat_model_name)
65-
add_association_params(params, :tool_calls, tool_call_table_name, tool_call_model_name, plural: true)
66-
add_association_params(params, :model, model_table_name, model_model_name)
66+
add_association_params(params, :chat, chat_table_name, chat_model_name,
67+
owner_table: message_table_name)
68+
add_association_params(params, :tool_calls, tool_call_table_name, tool_call_model_name,
69+
owner_table: message_table_name, plural: true)
70+
add_association_params(params, :model, model_table_name, model_model_name,
71+
owner_table: message_table_name)
6772

6873
"acts_as_message#{" #{params.join(', ')}" if params.any?}"
6974
end
7075

7176
def acts_as_model_declaration
7277
params = []
7378

74-
add_association_params(params, :chats, chat_table_name, chat_model_name, plural: true)
79+
add_association_params(params, :chats, chat_table_name, chat_model_name,
80+
owner_table: model_table_name, plural: true)
7581

7682
"acts_as_model#{" #{params.join(', ')}" if params.any?}"
7783
end
7884

7985
def acts_as_tool_call_declaration
8086
params = []
8187

82-
add_association_params(params, :message, message_table_name, message_model_name)
88+
add_association_params(params, :message, message_table_name, message_model_name,
89+
owner_table: tool_call_table_name)
8390

8491
"acts_as_tool_call#{" #{params.join(', ')}" if params.any?}"
8592
end
@@ -134,13 +141,21 @@ def table_exists?(table_name)
134141

135142
private
136143

137-
def add_association_params(params, default_assoc, table_name, model_name, plural: false)
144+
def add_association_params(params, default_assoc, table_name, model_name, owner_table:, plural: false) # rubocop:disable Metrics/ParameterLists
138145
assoc = plural ? table_name.to_sym : table_name.singularize.to_sym
139146

140-
return if assoc == default_assoc
147+
default_foreign_key = "#{default_assoc}_id"
148+
# has_many/has_one: foreign key is on the associated table pointing back to owner
149+
# belongs_to: foreign key is on the owner table pointing to associated table
150+
foreign_key = if plural || default_assoc.to_s.pluralize == default_assoc.to_s # has_many or has_one
151+
"#{owner_table.singularize}_id"
152+
else # belongs_to
153+
"#{table_name.singularize}_id"
154+
end
141155

142-
params << "#{default_assoc}: :#{assoc}"
156+
params << "#{default_assoc}: :#{assoc}" if assoc != default_assoc
143157
params << "#{default_assoc.to_s.singularize}_class: '#{model_name}'" if model_name != assoc.to_s.classify
158+
params << "#{default_assoc}_foreign_key: :#{foreign_key}" if foreign_key != default_foreign_key
144159
end
145160

146161
# Convert namespaced model names to proper table names

lib/ruby_llm/active_record/acts_as.rb

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ def load_from_database!
3131
end
3232

3333
class_methods do # rubocop:disable Metrics/BlockLength
34-
def acts_as_chat(messages: :messages, message_class: nil,
35-
model: :model, model_class: nil)
34+
def acts_as_chat(messages: :messages, message_class: nil, messages_foreign_key: nil, # rubocop:disable Metrics/ParameterLists
35+
model: :model, model_class: nil, model_foreign_key: nil)
3636
include RubyLLM::ActiveRecord::ChatMethods
3737

3838
class_attribute :messages_association_name, :model_association_name, :message_class, :model_class
@@ -45,12 +45,12 @@ def acts_as_chat(messages: :messages, message_class: nil,
4545
has_many messages,
4646
-> { order(created_at: :asc) },
4747
class_name: self.message_class,
48-
foreign_key: ActiveSupport::Inflector.foreign_key(table_name.singularize),
48+
foreign_key: messages_foreign_key,
4949
dependent: :destroy
5050

5151
belongs_to model,
5252
class_name: self.model_class,
53-
foreign_key: ActiveSupport::Inflector.foreign_key(model.to_s.singularize),
53+
foreign_key: model_foreign_key,
5454
optional: true
5555

5656
delegate :add_message, to: :to_llm
@@ -68,7 +68,7 @@ def acts_as_chat(messages: :messages, message_class: nil,
6868
end
6969
end
7070

71-
def acts_as_model(chats: :chats, chat_class: nil)
71+
def acts_as_model(chats: :chats, chat_class: nil, chats_foreign_key: nil)
7272
include RubyLLM::ActiveRecord::ModelMethods
7373

7474
class_attribute :chats_association_name, :chat_class
@@ -80,18 +80,16 @@ def acts_as_model(chats: :chats, chat_class: nil)
8080
validates :provider, presence: true
8181
validates :name, presence: true
8282

83-
has_many chats,
84-
class_name: self.chat_class,
85-
foreign_key: ActiveSupport::Inflector.foreign_key(table_name.singularize)
83+
has_many chats, class_name: self.chat_class, foreign_key: chats_foreign_key
8684

8785
define_method :chats_association do
8886
send(chats_association_name)
8987
end
9088
end
9189

92-
def acts_as_message(chat: :chat, chat_class: nil, touch_chat: false, # rubocop:disable Metrics/ParameterLists
93-
tool_calls: :tool_calls, tool_call_class: nil,
94-
model: :model, model_class: nil)
90+
def acts_as_message(chat: :chat, chat_class: nil, chat_foreign_key: nil, touch_chat: false, # rubocop:disable Metrics/ParameterLists
91+
tool_calls: :tool_calls, tool_call_class: nil, tool_calls_foreign_key: nil,
92+
model: :model, model_class: nil, model_foreign_key: nil)
9593
include RubyLLM::ActiveRecord::MessageMethods
9694

9795
class_attribute :chat_association_name, :tool_calls_association_name, :model_association_name,
@@ -106,12 +104,12 @@ def acts_as_message(chat: :chat, chat_class: nil, touch_chat: false, # rubocop:d
106104

107105
belongs_to chat,
108106
class_name: self.chat_class,
109-
foreign_key: ActiveSupport::Inflector.foreign_key(chat.to_s.singularize),
107+
foreign_key: chat_foreign_key,
110108
touch: touch_chat
111109

112110
has_many tool_calls,
113111
class_name: self.tool_call_class,
114-
foreign_key: ActiveSupport::Inflector.foreign_key(table_name.singularize),
112+
foreign_key: tool_calls_foreign_key,
115113
dependent: :destroy
116114

117115
belongs_to :parent_tool_call,
@@ -126,7 +124,7 @@ def acts_as_message(chat: :chat, chat_class: nil, touch_chat: false, # rubocop:d
126124

127125
belongs_to model,
128126
class_name: self.model_class,
129-
foreign_key: ActiveSupport::Inflector.foreign_key(model.to_s.singularize),
127+
foreign_key: model_foreign_key,
130128
optional: true
131129

132130
delegate :tool_call?, :tool_result?, to: :to_llm
@@ -144,8 +142,8 @@ def acts_as_message(chat: :chat, chat_class: nil, touch_chat: false, # rubocop:d
144142
end
145143
end
146144

147-
def acts_as_tool_call(message: :message, message_class: nil,
148-
result: :result, result_class: nil)
145+
def acts_as_tool_call(message: :message, message_class: nil, message_foreign_key: nil, # rubocop:disable Metrics/ParameterLists
146+
result: :result, result_class: nil, result_foreign_key: nil)
149147
class_attribute :message_association_name, :result_association_name, :message_class, :result_class
150148

151149
self.message_association_name = message
@@ -155,11 +153,11 @@ def acts_as_tool_call(message: :message, message_class: nil,
155153

156154
belongs_to message,
157155
class_name: self.message_class,
158-
foreign_key: ActiveSupport::Inflector.foreign_key(message.to_s.singularize)
156+
foreign_key: message_foreign_key
159157

160158
has_one result,
161159
class_name: self.result_class,
162-
foreign_key: ActiveSupport::Inflector.foreign_key(table_name.singularize),
160+
foreign_key: result_foreign_key,
163161
dependent: :nullify
164162

165163
define_method :message_association do

spec/ruby_llm/active_record/acts_as_spec.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,76 @@ class BotToolCall < ActiveRecord::Base # rubocop:disable Lint/ConstantDefinition
393393
end
394394
end
395395

396+
describe 'namespaced chat models with custom foreign keys' do
397+
before(:all) do # rubocop:disable RSpec/BeforeAfterAll
398+
# Create additional tables for testing edge cases
399+
ActiveRecord::Migration.suppress_messages do
400+
ActiveRecord::Migration.create_table :support_conversations, force: true do |t|
401+
t.string :model_id
402+
t.timestamps
403+
end
404+
405+
ActiveRecord::Migration.create_table :support_messages, force: true do |t|
406+
t.references :conversation, foreign_key: { to_table: :support_conversations }
407+
t.string :role
408+
t.text :content
409+
t.string :model_id
410+
t.integer :input_tokens
411+
t.integer :output_tokens
412+
t.references :tool_call, foreign_key: { to_table: :support_tool_calls }
413+
t.timestamps
414+
end
415+
416+
ActiveRecord::Migration.create_table :support_tool_calls, force: true do |t|
417+
t.references :message, foreign_key: { to_table: :support_messages }
418+
t.string :tool_call_id
419+
t.string :name
420+
t.json :arguments
421+
t.timestamps
422+
end
423+
end
424+
end
425+
426+
after(:all) do # rubocop:disable RSpec/BeforeAfterAll
427+
ActiveRecord::Migration.suppress_messages do
428+
if ActiveRecord::Base.connection.table_exists?(:support_tool_calls)
429+
ActiveRecord::Migration.drop_table :support_tool_calls
430+
end
431+
if ActiveRecord::Base.connection.table_exists?(:support_messages)
432+
ActiveRecord::Migration.drop_table :support_messages
433+
end
434+
if ActiveRecord::Base.connection.table_exists?(:support_conversations)
435+
ActiveRecord::Migration.drop_table :support_conversations
436+
end
437+
end
438+
end
439+
440+
module Support # rubocop:disable Lint/ConstantDefinitionInBlock,RSpec/LeakyConstantDeclaration
441+
def self.table_name_prefix
442+
'support_'
443+
end
444+
445+
class Conversation < ActiveRecord::Base # rubocop:disable RSpec/LeakyConstantDeclaration
446+
acts_as_chat message_class: 'Support::Message'
447+
end
448+
449+
class Message < ActiveRecord::Base # rubocop:disable RSpec/LeakyConstantDeclaration
450+
acts_as_message chat: :conversation, chat_class: 'Support::Conversation', tool_call_class: 'Support::ToolCall'
451+
end
452+
453+
class ToolCall < ActiveRecord::Base # rubocop:disable RSpec/LeakyConstantDeclaration
454+
acts_as_tool_call message_class: 'Support::Message'
455+
end
456+
end
457+
458+
it 'creates messages successfully' do
459+
conversation = Support::Conversation.create!(model: model)
460+
461+
expect { conversation.messages.create!(role: 'user', content: 'Test') }.not_to raise_error
462+
expect(conversation.messages.count).to eq(1)
463+
end
464+
end
465+
396466
describe 'to_llm conversion' do
397467
it 'correctly converts custom messages to RubyLLM format' do
398468
bot_chat = Assistants::BotChat.create!(model: model)

0 commit comments

Comments
 (0)