[LEADS-444] Support Python 3.14 in lightspeed-evaluation#262
[LEADS-444] Support Python 3.14 in lightspeed-evaluation#262bsatapat-jpg wants to merge 1 commit into
Conversation
WalkthroughPython 3.14 support is added by updating ChangesPython 3.14 support, dependency refresh, and instructor warmup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@requirements-local-embeddings.txt`:
- Around line 104-128: The greenlet dependency marker is too restrictive and
blocks Python 3.14 even though greenlet==3.5.3 supports it. Update the
`greenlet==3.5.3` entry in the generated requirements so the
`python_full_version < '3.14'` guard is removed, while keeping any platform
constraints that still apply, and make sure the corresponding generated
requirements files stay consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5170cdd3-bc7e-4e0b-b92d-80ac40749b9f
⛔ Files ignored due to path filters (2)
uv-gpu.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/tests.yamllsc_agent_eval/pyproject.tomlpyproject.tomlrequirements-all-extras.txtrequirements-local-embeddings.txtrequirements-nlp-metrics.txtrequirements.txt
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/tests.yaml
- pyproject.toml
- requirements-all-extras.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- lsc_agent_eval/pyproject.toml
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
There are some unrelated changes. I have commented PTAL
| ] | ||
|
|
||
| [[package]] | ||
| name = "langfuse" |
There was a problem hiding this comment.
Why is it being added instead of update ? Did we miss to re-generate lock and requirement files when we added the langfuse as optional dependency ?
There was a problem hiding this comment.
Yes, I have missed to push uv.lock file in the langfuse PR's.
There was a problem hiding this comment.
Let's add this in a differnt PR
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _warmup_instructor_registry() -> None: |
There was a problem hiding this comment.
While running the evaluation with Python 3.14, I found an issue regarding thread-safety race in instructor's mode registry that crashes. The registry uses dict.pop() on lazy loaders without locking — when our ThreadPoolExecutor threads hit it concurrently, one thread pops the loader while another gets KeyError. Pre-warming it once from the main thread before threads start eliminates the race. Wrapped in try/except for forward compatibility.
There was a problem hiding this comment.
Could you please check with previous versions - seems weird that it appeared in 3.14
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
@bsatapat-jpg Please check if all required packages actually support python 3.14 ?
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lightspeed_evaluation/core/llm/ragas.py (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the skipped warmup instead of swallowing it silently.
If this workaround stops matching instructor’s behavior,
passleaves no signal in logs. Please emit a debug log here so registry warmup failures remain diagnosable. As per coding guidelines, "Use structured logging with appropriate log levels."Suggested change
try: mode_registry.get_handlers(Provider.OPENAI, Mode.JSON) - except KeyError: - pass + except KeyError as exc: + logger.debug( + "Skipping instructor mode registry warmup; handlers are not available yet", + exc_info=exc, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lightspeed_evaluation/core/llm/ragas.py` around lines 28 - 29, The exception handling in the Ragas warmup path currently swallows a KeyError silently, which leaves no trace when registry warmup is skipped. Update the except block in the warmup logic around the instructor-related lookup to emit a structured debug log with appropriate context instead of using pass. Use the existing logger in this module (or the same logging pattern used elsewhere in ragas.py) so the skipped warmup remains diagnosable without changing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lightspeed_evaluation/core/llm/ragas.py`:
- Around line 28-29: The exception handling in the Ragas warmup path currently
swallows a KeyError silently, which leaves no trace when registry warmup is
skipped. Update the except block in the warmup logic around the
instructor-related lookup to emit a structured debug log with appropriate
context instead of using pass. Use the existing logger in this module (or the
same logging pattern used elsewhere in ragas.py) so the skipped warmup remains
diagnosable without changing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e07a319-5b54-4a06-95e2-1742774a3a29
⛔ Files ignored due to path filters (2)
uv-gpu.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/tests.yamlpyproject.tomlrequirements-all-extras.txtrequirements-local-embeddings.txtrequirements-nlp-metrics.txtrequirements.txtsrc/lightspeed_evaluation/core/llm/ragas.py
✅ Files skipped from review due to trivial changes (2)
- pyproject.toml
- requirements-nlp-metrics.txt
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/tests.yaml
- requirements.txt
- requirements-local-embeddings.txt
- requirements-all-extras.txt
All packages support python 3.14, litellm declares requires-python: <3.14 in its metadata (all versions through 1.90.0), but it's pure Python and works fine on 3.14. |
Two important aspects
Let's keep this PR open, but we can fix other things in.separate PR |
2 participants
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit