Skip to content

fix: declare regex as a core dependency (CitationMixin crashes with ModuleNotFoundError)#2443

Open
chuenchen309 wants to merge 1 commit into
567-labs:mainfrom
chuenchen309:fix/citation-missing-regex-dependency
Open

fix: declare regex as a core dependency (CitationMixin crashes with ModuleNotFoundError)#2443
chuenchen309 wants to merge 1 commit into
567-labs:mainfrom
chuenchen309:fix/citation-missing-regex-dependency

Conversation

@chuenchen309

Copy link
Copy Markdown

Problem

CitationMixin._get_span (instructor/v2/dsl/citation.py:81) does import regex at call time, but regex is not declared anywhere in pyproject.toml's core dependencies or any [project.optional-dependencies] extra (checked all ~25 extras — none of them pull it in).

A stock pip install instructor therefore has CitationMixin — a documented public feature — crash with ModuleNotFoundError the moment validate_sources() runs. The feature is entirely broken on a clean install.

This isn't hypothetical: the repo's own existing test tests/coverage/test_dsl_small_coverage.py::test_citation_keeps_quotes_without_context_and_recovers_fuzzy_quotes already exercises this exact path and fails on a fresh checkout without regex manually installed.

Fix

Add "regex>=2023.0.0" to [project.dependencies] and update uv.lock accordingly.

Not a duplicate of #2430

#2430 ("fix(citations): escape substring quotes before fuzzy span matching", open) fixes a different bug — regex.error on quotes containing metacharacters — and does not touch pyproject.toml. Its own new test uses pytest.importorskip("regex"), i.e. it works around this exact missing-dependency issue rather than fixing it. Both fixes are needed independently.

Testing

  • The previously-failing test now passes: 16/16 in test_dsl_small_coverage.py.
  • Full tests/dsl/ + test_dsl_small_coverage.py: 88 passed, 2 pre-existing skips (unrelated to this change).
  • ruff check clean.

AI-Generated disclosure

Found via an AI-assisted code review pass (Claude Code) over instructor/v2/dsl/. I personally reproduced the ModuleNotFoundError against the repo's own pre-existing test, verified the fix, checked for overlap with #2430, and ran the relevant test suites before submitting.

…Error)

instructor/v2/dsl/citation.py's CitationMixin._get_span does `import
regex` at call time, but `regex` is not declared anywhere in
pyproject.toml's core dependencies or any optional-dependencies extra
(checked all ~25 extras). A stock `pip install instructor` therefore
has CitationMixin -- a documented public feature -- crash with
ModuleNotFoundError the moment validate_sources() runs, i.e. the
feature is entirely broken on a clean install.

This isn't a hypothetical: the repo's own existing test
tests/coverage/test_dsl_small_coverage.py::test_citation_keeps_quotes_without_context_and_recovers_fuzzy_quotes
already exercises this path and fails on a stock checkout without
`regex` manually installed.

Fix: add "regex>=2023.0.0" to [project.dependencies] and update
uv.lock accordingly.

Not a duplicate of 567-labs#2430 (open, "fix(citations): escape substring
quotes before fuzzy span matching") -- that PR fixes a different bug
(regex.error on quotes containing metacharacters) and does not touch
pyproject.toml; its own new test uses pytest.importorskip("regex"),
i.e. it works around this exact missing-dependency issue rather than
fixing it.

Testing: previously-failing test now passes (16/16 in
test_dsl_small_coverage.py); full tests/dsl/ + test_dsl_small_coverage.py
suite (88 passed, 2 pre-existing skips unrelated to this change); ruff
clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@ErenAta16 ErenAta16 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked where regex is actually used: it's a local import regex inside CitationMixin's fuzzy-match method in instructor/v2/dsl/citation.py, not a module-level import, so strictly only the fuzzy-citation path needs it, not every CitationMixin user. Declaring it as a core dependency rather than gating it behind a lazy import or an extras group is still the right call here: regex is small and has no heavy transitive dependencies, and the alternative (lazy-guarded import with a custom error message) would be more code for a package this lightweight. Straightforward fix for a real crash.

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.

2 participants