Skip to content

Add Zeitwerk eager loading check to CI#95

Open
trevorturk wants to merge 1 commit into
patvice:mainfrom
trevorturk:add-zeitwerk-eager-load-check
Open

Add Zeitwerk eager loading check to CI#95
trevorturk wants to merge 1 commit into
patvice:mainfrom
trevorturk:add-zeitwerk-eager-load-check

Conversation

@trevorturk
Copy link
Copy Markdown
Contributor

@trevorturk trevorturk commented Nov 18, 2025

Summary

Problem

Without automated Zeitwerk eager loading validation, autoloading issues can slip into production causing crashes. These issues include:

  • Inflector configuration errors
  • Constant definition problems
  • Missing conditional checks for optional dependencies

Solution

Add a simple validation step that runs Zeitwerk::Loader.eager_load_all after dependency installation. This structural validation provides fast feedback before the rest of the test job continues.

Upstream Status

The original blocker in RubyLLM has been fixed on ruby_llm main:

  • ruby_llm#504 merged on May 6, 2026 and fixes the ActiveRecord dependency/eager-loading failure.
  • ruby_llm#505 was closed unmerged on March 1, 2026, so this PR no longer treats it as a blocker.

This PR remains draft until a published ruby_llm gem release includes #504. The current latest RubyGems release is ruby_llm 1.14.1 from April 2, 2026, which still predates the fix.

Testing

Local verification after rebasing still fails with the expected upstream gem issue because Bundler resolves ruby_llm 1.14.1:

uninitialized constant RubyLLM::ActiveRecord::ActsAs::ActiveSupport (NameError)

Command run locally:

RBENV_VERSION=3.4.9 rbenv exec bundle exec ruby -e "require 'ruby_llm/mcp'; Zeitwerk::Loader.eager_load_all"

@patvice
Copy link
Copy Markdown
Owner

patvice commented Nov 19, 2025

Hey! Thanks for the PR -- I'm open to this if your downstream changes merged into RubyLLM

Only question is that Zeitwerk::Loader.eager_load_all is going to load all ruby files within a project. Right now it's failing in my CI due to ActiveSupport not be available when loading RubyLLM gem. That is not a hard dependency of RubyLLM or MCP gem, so I am wondering if there is not some work to do to exclude those files or had checks within.

Whatever RubyLLM provides as it solution for this, I'm happy to follow.

@patvice
Copy link
Copy Markdown
Owner

patvice commented Nov 19, 2025

Looks like I commented to soon and #94 provides that solution.

Happy to merge this once RubyLLM does

@trevorturk
Copy link
Copy Markdown
Contributor Author

Sounds good, thanks! Also note this is entirely optional and pedantic, so if it's annoying later on of course feel free to remove!

@trevorturk trevorturk force-pushed the add-zeitwerk-eager-load-check branch from 8d5d025 to 5d045b3 Compare May 6, 2026 17:19
@trevorturk
Copy link
Copy Markdown
Contributor Author

crmne/ruby_llm#504 was merged, so I'll keep an eye out for a RubyLLM release and then mark this ready for review...!

@trevorturk
Copy link
Copy Markdown
Contributor Author

@trevorturk trevorturk marked this pull request as ready for review May 7, 2026 01:01
Copilot AI review requested due to automatic review settings May 7, 2026 01:01
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a CI validation step to catch Zeitwerk autoload/eager-load issues early by running Zeitwerk::Loader.eager_load_all after Ruby dependencies are installed.

Changes:

  • Add a “Check Zeitwerk eager loading” step to the Ruby test job in CI.
  • Run the eager-load check before installing Bun dependencies / running specs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

run: |
bundle install

- name: Check Zeitwerk eager loading
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