Skip to content

Conversation

@mhmaguire
Copy link
Contributor

Ignore rails specifics in zeitwerk configuration

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.71%. Comparing base (b3eaaca) to head (0f63e0b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #59   +/-   ##
=======================================
  Coverage   92.70%   92.71%           
=======================================
  Files          54       54           
  Lines        1658     1660    +2     
  Branches      284      284           
=======================================
+ Hits         1537     1539    +2     
  Misses        121      121           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@crmne crmne merged commit cc2e45e into crmne:main Mar 22, 2025
2 checks passed
OriPekelman pushed a commit to OriPekelman/ruby_llm that referenced this pull request Mar 24, 2025
trevorturk added a commit to trevorturk/ruby_llm that referenced this pull request Nov 2, 2025
## Problem

Applications using ruby_llm crash during eager loading (production boot):

```
uninitialized constant RubyLLM::Rails (NameError)
  class Railtie < Rails::Railtie
                  ^^^^^
```

This occurs when:
- `config.eager_load = true` (standard in production)
- Zeitwerk eager loads all files including railtie.rb
- railtie.rb references `Rails::Railtie` unconditionally
- Rails isn't loaded yet or app doesn't use Rails

##Root Cause

PR crmne#59 added conditional require (`require 'ruby_llm/railtie' if defined?(Rails::Railtie)`),
but this is insufficient because Zeitwerk's eager loading happens BEFORE that
conditional check runs.

When Zeitwerk eager loads, it loads railtie.rb and expects `RubyLLM::Railtie`
to be defined, but the class definition fails because `Rails::Railtie` doesn't exist.

## Solution

Two complementary fixes (both required):

**1. Wrap class definition** in `if defined?(Rails::Railtie)` (railtie.rb)
   - Prevents error when file is loaded without Rails
   - Pattern used by github/secure_headers gem

**2. Ignore from Zeitwerk** with `loader.ignore()` (ruby_llm.rb)
   - Prevents Zeitwerk from expecting a constant from this file
   - Per Zeitwerk docs: "files not following conventions" should be ignored

## Evidence

**Zeitwerk maintainer guidance** (issue crmne#143):
> "have the strategy defined in `lib`, perform a `require` in the initializer"

**Zeitwerk docs**:
> Use `loader.ignore()` for "files not following conventions"

**Real-world precedent** (github/secure_headers):
```ruby
if defined?(Rails::Railtie)
  module SecureHeaders
    class Railtie < Rails::Railtie
      # ...
    end
  end
end
```

## Testing

Before (ruby_llm 1.8.2):
```bash
$ bundle exec ruby -e "require 'swarm_sdk'; Zeitwerk::Loader.eager_load_all"
NameError: uninitialized constant RubyLLM::Rails
```

After (with both fixes):
```bash
$ bundle exec ruby -e "require 'swarm_sdk'; Zeitwerk::Loader.eager_load_all"
✓ SUCCESS
```

## Related

- PR crmne#59 - Partial fix (conditional require only)
- Zeitwerk issue crmne#143 - Guidance on conditional loading
- github/secure_headers - Real-world example of pattern
crmne added a commit that referenced this pull request Nov 4, 2025
## Summary

Completes the fix from PR #59 to prevent crashes during eager loading in
non-Rails contexts.

## Problem

```
uninitialized constant RubyLLM::Rails (NameError)
  class Railtie < Rails::Railtie
```

Occurs when Zeitwerk eager loads (e.g., in gems that depend on ruby_llm
but don't use Rails).

## Root Cause

**PR #59** added conditional require: `require 'ruby_llm/railtie' if
defined?(Rails::Railtie)`

This is insufficient because Zeitwerk's eager loading happens *before*
that check runs. When Zeitwerk loads `railtie.rb`, the class definition
fails because `Rails::Railtie` doesn't exist.

## Solution

Two complementary fixes (both required per Zeitwerk documentation):

**1. Wrap class definition** in `if defined?(Rails::Railtie)`:
```ruby
if defined?(Rails::Railtie)
  module RubyLLM
    class Railtie < Rails::Railtie
      # ...
    end
  end
end
```

**2. Ignore from Zeitwerk**:
```ruby
loader.ignore("#{__dir__}/ruby_llm/railtie.rb")
```

## Evidence

- **Zeitwerk issue #143**: Maintainer confirms conditional loading
pattern
- **Zeitwerk docs**: Use `loader.ignore()` for "files not following
conventions"
- **Real-world example**: github/secure_headers uses same pattern

## Testing

**Before**: `Zeitwerk::Loader.eager_load_all` → ❌ NameError  
**After**: `Zeitwerk::Loader.eager_load_all` → ✅ Success

Thank you for reviewing!

Co-authored-by: Carmine Paolino <[email protected]>
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.

3 participants