Update dependencies and spaCy model loading#64
Conversation
pyproject.toml: Add en_core_web_sm as a direct dependency via URL so the model is installed at project setup time rather than downloaded at runtime via spacy.cli.download(), which uses pip internally and fails in non-pip environments (e.g. uv). Also narrow litellm version upper bound from <1.80.0 to <1.60.0 to avoid a regression where newer versions accidentally import litellm.proxy.proxy_server, pulling in proxy-only dependencies (apscheduler) even when the proxy is not used. ner_tool.py: Remove automatic spaCy model download at runtime; raise an explicit error if the model is not installed, prompting the user to install it manually.
Summary of ChangesHello @puja-trivedi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the project's dependency management and enhances the robustness of spaCy model loading. It addresses issues with runtime model downloads and prevents unintended dependency imports by adjusting version constraints, leading to a more stable and predictable development environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request improves dependency management by making the spaCy model a direct project dependency and removing runtime downloads, which is a great change for robustness. I've identified a potential version incompatibility issue with the spacy package and its model that could break the application in the future. Additionally, I've suggested an improvement to an error message to make it consistent with the new dependency strategy.
pyproject.toml: Tighten spacy version constraint from ^3.8.11 to >=3.8.11,<3.9.0 to ensure compatibility with the pinned en_core_web_sm-3.8.0 model, which requires spaCy 3.8.x. ner_tool.py: Update error message to direct users to run 'poetry install' or 'uv sync' instead of 'python -m spacy download', consistent with the model now being a project dependency.
| spacy = "^3.8.11" | ||
| litellm = {version = ">=1.17.0,<1.80.0"} | ||
| spacy = ">=3.8.11,<3.9.0" | ||
| en-core-web-sm = {url = "https://github.com/explosion/spacy-models/releases/download/en_core_web_sm-3.8.0/en_core_web_sm-3.8.0-py3-none-any.whl"} |
There was a problem hiding this comment.
@puja-trivedi any particular reason for including the model in pyproject.toml? Why not put it in the code, you can download the model automatically. For now we've pre-defined models, but say we decided to take it from env files then it won't work.
There was a problem hiding this comment.
I ran into an issue when using uv; however, I was able to fix that now. I also understand your point about the env files so I have reverted that change. However the version changes I made for spacy and litellm should stay because of the non-fatal runtime error I was getting.
Further explanation about the error:
The error is happening because a newer version of LiteLLM has an internal code path in its logging/cold-storage handler that accidentally imports litellm.proxy.proxy_server, which requires apscheduler (a proxy-only dependency). This happens even when you're not using the LiteLLM proxy — it's a bug/regression in LiteLLM itself.
Key points:
1. It's a non-fatal ERROR log, not a crash. The extraction pipeline should still work — LiteLLM catches this exception internally and continues.
2. Don't need litellm[proxy] — that would pull in a bunch of proxy dependencies we don't need.
3. The real fix is to pin LiteLLM to a version that doesn't have this regression. Try tightening the upper bound.
pyproject.toml: Remove en-core-web-sm direct dependency; the spaCy model will be downloaded at runtime via spacy.cli.download() instead, which works in uv environments by running 'uv add pip'. ner_tool.py: Restore spacy.cli.download() fallback and re-add the from spacy.cli import download import.
Current Changes
pyproject.toml: narrow litellm version upper bound from <1.80.0 to <1.60.0 to avoid a regression where newer versions accidentally import litellm.proxy.proxy_server, pulling in proxy-only dependencies (apscheduler) even when the proxy is not used.
Reverted Changes to Original PR- Outdated: