fix: lazy-import document_search in evaluate pipelines to avoid breaking CLI#947
Open
MichaelMcCulloch-deepsense-ai wants to merge 2 commits intodevelopfrom
Open
fix: lazy-import document_search in evaluate pipelines to avoid breaking CLI#947MichaelMcCulloch-deepsense-ai wants to merge 2 commits intodevelopfrom
MichaelMcCulloch-deepsense-ai wants to merge 2 commits intodevelopfrom
Conversation
…ing CLI The top-level import of ragbits.document_search in the evaluate pipelines __init__.py caused a ModuleNotFoundError when ragbits-document-search was not installed, breaking all ragbits CLI commands due to eager autoregister. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Code Coverage SummaryDetailsDiff against mainResults for commit: 9b63733 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
mkoruszowic
reviewed
Mar 3, 2026
Collaborator
There was a problem hiding this comment.
Wouldn't it be better to just add ragbits-document-search as a dependency of ragbits-evaluate? It's imported unconditionally at the module level, so without it the package is effectively broken anyway.
If the intent is for these to be truly optional (i.e., you can use ragbits-evaluate without document search), then the try/except approach is fine but in that case they should be declared as optional dependencies (extras), e.g. pip install ragbits-evaluate[document-search], so the installation path is clear to users.
Also worth noting that ragbits-agents has the same problem. It's not listed as a dependency either, but modules like question_answer.py and the agent_simulation subpackage import from it unconditionally, so the same ModuleNotFoundError will happen there too.
Longer term, if we expect more pipeline types to be added by different packages, it might be worth considering Python's entry points mechanism instead of try/except blocks. Each package would self-register its pipeline in its own pyproject.toml:
# in ragbits-document-search/pyproject.toml
[project.entry-points."ragbits.evaluate.pipelines"]
document_search = "ragbits.evaluate.pipelines.document_search:DocumentSearchPipeline"And ragbits-evaluate would discover them at runtime via importlib.metadata.entry_points() so no hard imports, no try/except, and zero changes needed in ragbits-evaluate when a new pipeline type is added. But that's a bigger refactor and probably out of scope for this PR.
…f ragbits-evaluate Both packages are imported unconditionally throughout ragbits-evaluate (pipelines, metrics, dataloaders, agent_simulation) but were not declared as dependencies. This caused ModuleNotFoundError that broke all CLI commands because autoregister() eagerly imports every ragbits.*.cli module. Rather than guarding every import with try/except, declare them as proper dependencies since ragbits-evaluate cannot function without them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
The issue is that |
Collaborator
@sleter Makes sense, keeping it lightweight is a good call. The |
It sounds like what I need to do is reintroduce the try/except blocks, and make the dependency optional, and for Is that right? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ragbits.document_searchimports inragbits/evaluate/pipelines/__init__.pywith atry/except ImportErrorguardragbits-document-searchis not a dependency ofragbits-evaluate, but a hard top-level import causedModuleNotFoundErrorwhen it wasn't installedautoregister()eagerly imports everyragbits.*.climodule on startup, this broke all CLI commands — not justragbits evaluate