Commit 1ae2407
authored
Fix ActiveRecord dependency check for Zeitwerk eager loading (#504)
## Summary
Fixes RubyLLM's optional ActiveRecord integration so the core gem can be
required and Zeitwerk-eager-loaded without loading Rails-only
ActiveRecord mixins too early.
The PR keeps normal Rails usage intact: in a Rails app, `acts_as_chat`,
`acts_as_message`, `acts_as_model`, and `acts_as_tool_call` are still
installed on `ActiveRecord::Base` by the RubyLLM railtie after
ActiveRecord loads.
## Why This Was Needed
`lib/ruby_llm/active_record` is optional Rails integration code. It
should not be part of RubyLLM's standalone gem eager-load path.
Before this change, the gem-level Zeitwerk loader still managed
`lib/ruby_llm/active_record`. That meant a standalone eager-load like
this could load ActiveRecord mixin files even when a Rails app had not
booted ActiveRecord:
```sh
bundle exec appraisal rails-7.2 ruby -e "require 'ruby_llm'; Zeitwerk::Loader.eager_load_all"
```
That exposed load-order assumptions inside the ActiveRecord integration.
The immediate CI failure was:
```text
NoMethodError: undefined method `delegate' for module RubyLLM::ActiveRecord::ModelMethods
```
The failing line was in `RubyLLM::ActiveRecord::ModelMethods`, which
uses ActiveSupport's `delegate`. The broader issue was not just that one
missing require; the optional ActiveRecord directory itself was being
eager-loaded by the core gem loader.
## How Normal Rails Loading Works
The intended Rails path is:
1. Rails boots and loads its railties.
2. Bundler requires `ruby_llm`.
3. `ruby_llm` requires `ruby_llm/railtie` when `Rails::Railtie` is
defined.
4. `RubyLLM::Railtie` registers an `ActiveSupport.on_load
:active_record` hook.
5. When ActiveRecord loads, the hook explicitly loads RubyLLM's
ActiveRecord support files.
6. The railtie includes either `RubyLLM::ActiveRecord::ActsAs` or
`RubyLLM::ActiveRecord::ActsAsLegacy` into `ActiveRecord::Base`.
7. Application models can call the `acts_as_*` APIs normally.
This PR makes that boundary explicit: core RubyLLM eager-loading does
not load ActiveRecord integration files; the Rails railtie loads them
when ActiveRecord is available.
## Changes
### Isolate ActiveRecord integration from gem eager-load
`lib/ruby_llm.rb` now ignores `lib/ruby_llm/active_record` in the
gem-level Zeitwerk loader:
```ruby
loader.ignore("#{__dir__}/ruby_llm/active_record")
```
This matches the existing treatment for tasks, generators, and the
railtie itself. It prevents `Zeitwerk::Loader.eager_load_all` from
loading optional Rails integration code during standalone `require
"ruby_llm"` usage.
### Stop loading ActiveRecord mixins from `ruby_llm.rb`
`lib/ruby_llm.rb` no longer directly requires
`ruby_llm/active_record/acts_as`.
It only requires the railtie when Rails is present:
```ruby
require 'ruby_llm/railtie' if defined?(Rails::Railtie)
```
That keeps the Rails integration behind the railtie instead of coupling
it to the core gem entrypoint.
### Make the railtie explicitly own ActiveRecord integration loading
`lib/ruby_llm/railtie.rb` now loads the ActiveRecord support files
inside `ActiveSupport.on_load :active_record`:
- `ruby_llm/active_record/payload_helpers`
- `ruby_llm/active_record/chat_methods`
- `ruby_llm/active_record/message_methods`
- `ruby_llm/active_record/model_methods`
- `ruby_llm/active_record/tool_call_methods`
Then it loads the configured public API module:
- `ruby_llm/active_record/acts_as` when `RubyLLM.config.use_new_acts_as`
is true
- `ruby_llm/active_record/acts_as_legacy` otherwise
This preserves normal Rails behavior while removing reliance on
gem-level Zeitwerk autoloads for an ignored directory.
### Make ActiveRecord files explicit about their dependencies
Several files previously relied on incidental load order or Zeitwerk
autoloads. The PR adds explicit requires so each file is safe when
loaded by the railtie:
- `acts_as.rb` now requires `active_support/concern` and
`active_support/inflector`.
- `acts_as_legacy.rb` now requires `active_support/concern` and
`active_support/inflector`.
- `chat_methods.rb` now requires `active_support/concern`.
- `message_methods.rb` now requires `active_support/concern` and
`ruby_llm/active_record/payload_helpers`.
- `model_methods.rb` now requires `active_support/concern` and
`active_support/core_ext/module/delegation`.
- `payload_helpers.rb` now requires
`active_support/core_ext/object/blank` and `json`.
- `tool_call_methods.rb` now requires `active_support/concern` and
`ruby_llm/active_record/payload_helpers`.
These changes are intentionally small, but they are important because
ignoring the ActiveRecord directory removes the previous incidental
Zeitwerk autoload fallback.
### Keep the standalone helper spec independent
`spec/ruby_llm/active_record_tool_error_helpers_spec.rb` tests
`MessageMethods` and `ToolCallMethods` without booting the dummy Rails
app. After isolating `lib/ruby_llm/active_record` from the gem Zeitwerk
loader, that spec must require the helper modules it exercises directly.
This keeps the test independent of suite ordering and matches the new
explicit-loading contract.
### Add CI coverage for the failure mode
The test matrix now runs an eager-load guard for each Rails appraisal
after dependencies install:
```sh
bundle exec appraisal ${{ matrix.rails-version }} ruby -e "require 'ruby_llm'; Zeitwerk::Loader.eager_load_all"
```
This catches regressions where optional Rails integration code leaks
into the standalone gem eager-load path.
## Reviewer Notes
This diff is easier to review with whitespace hidden:
https://github.com/crmne/ruby_llm/pull/504/files?w=1
The code changes are mostly explicit `require` lines plus one Zeitwerk
ignore. Hiding whitespace makes it easier to see that the behavioral
change is the load boundary, not a broad refactor.
## Validation
Verified against the current PR head (`aa73ba7a`) after checking out the
GitHub PR branch locally.
Standalone RubyLLM eager-load:
```sh
bundle exec ruby -e 'require "ruby_llm"; Zeitwerk::Loader.eager_load_all; puts :ok'
```
Rails appraisal eager-load guards:
```sh
bundle exec appraisal rails-7.1 ruby -e 'require "ruby_llm"; Zeitwerk::Loader.eager_load_all; puts :ok'
bundle exec appraisal rails-7.2 ruby -e 'require "ruby_llm"; Zeitwerk::Loader.eager_load_all; puts :ok'
bundle exec appraisal rails-8.0 ruby -e 'require "ruby_llm"; Zeitwerk::Loader.eager_load_all; puts :ok'
bundle exec appraisal rails-8.1 ruby -e 'require "ruby_llm"; Zeitwerk::Loader.eager_load_all; puts :ok'
```
Rails integration still installs `acts_as_chat` through the railtie:
```sh
bundle exec appraisal rails-8.1 ruby -e 'require_relative "spec/dummy/config/application"; Rails.application.initialize!; puts ActiveRecord::Base.respond_to?(:acts_as_chat); puts ActiveRecord::Base.method(:acts_as_chat).owner'
```
Output confirmed:
```text
true
RubyLLM::ActiveRecord::ActsAs::ClassMethods
```
Direct explicit ActiveRecord support-file loading works without relying
on RubyLLM's gem Zeitwerk loader:
```sh
bundle exec ruby -Ilib -e 'module RubyLLM; module ActiveRecord; end; end; require "ruby_llm/active_record/payload_helpers"; require "ruby_llm/active_record/chat_methods"; require "ruby_llm/active_record/message_methods"; require "ruby_llm/active_record/model_methods"; require "ruby_llm/active_record/tool_call_methods"; require "ruby_llm/active_record/acts_as"; require "ruby_llm/active_record/acts_as_legacy"; puts :ok'
```
The CI-failing standalone helper spec also passes through `rspec-queue`,
matching the worker loading path from the failing job:
```sh
env TEST_QUEUE_WORKERS=2 SKIP_COVERAGE=true bundle exec appraisal rails-7.1 bin/rspec-queue spec/ruby_llm/active_record_tool_error_helpers_spec.rb
```
Targeted ActiveRecord specs:
```sh
SKIP_COVERAGE=true bundle exec appraisal rails-8.1 rspec spec/ruby_llm/active_record spec/ruby_llm/active_record_tool_error_helpers_spec.rb
```
Result:
```text
100 examples, 0 failures
```
RuboCop on changed Ruby files:
```sh
bundle exec rubocop --cache false spec/ruby_llm/active_record_tool_error_helpers_spec.rb lib/ruby_llm.rb lib/ruby_llm/railtie.rb lib/ruby_llm/active_record/acts_as.rb lib/ruby_llm/active_record/acts_as_legacy.rb lib/ruby_llm/active_record/chat_methods.rb lib/ruby_llm/active_record/message_methods.rb lib/ruby_llm/active_record/model_methods.rb lib/ruby_llm/active_record/payload_helpers.rb lib/ruby_llm/active_record/tool_call_methods.rb
```
Result:
```text
10 files inspected, no offenses detected
```
Whitespace check:
```sh
git diff --check 776d92a...HEAD
```
Result: no whitespace errors.1 parent 776d92a commit 1ae2407
11 files changed
Lines changed: 33 additions & 4 deletions
File tree
- .github/workflows
- lib
- ruby_llm
- active_record
- spec/ruby_llm
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
55 | 58 | | |
56 | 59 | | |
57 | 60 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
36 | 37 | | |
37 | 38 | | |
38 | 39 | | |
| |||
107 | 108 | | |
108 | 109 | | |
109 | 110 | | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
| 111 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
3 | 5 | | |
4 | 6 | | |
5 | 7 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
3 | 6 | | |
4 | 7 | | |
5 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
15 | 21 | | |
16 | 22 | | |
17 | 23 | | |
| |||
0 commit comments