chore: pin upper bound in pyproject, add req text files & add make targets#203
Open
asamal4 wants to merge 1 commit intolightspeed-core:mainfrom
Open
chore: pin upper bound in pyproject, add req text files & add make targets#203asamal4 wants to merge 1 commit intolightspeed-core:mainfrom
asamal4 wants to merge 1 commit intolightspeed-core:mainfrom
Conversation
Contributor
WalkthroughReplaced single-step requirements export with a lock-first workflow: Make targets now regenerate uv.lock/uv-gpu.lock and produce multiple pinned requirements files; CI updated to run Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant UV as uv Tool
participant Lock as Lock Files (uv.lock, uv-gpu.lock)
participant Req as Requirements Files
participant CI as CI Workflow
participant Git as Git Repo
Dev->>Make: make sync-lock-and-requirements
Make->>Make: uv-lock-regenerate (edit pyproject for GPU mode)
Make->>UV: uv lock (GPU)
UV->>Lock: write uv-gpu.lock
Make->>Make: restore pyproject (CPU)
Make->>UV: uv lock (CPU)
UV->>Lock: write uv.lock
Make->>Make: generate-requirements
Make->>UV: uv export --frozen --all-extras
UV->>Req: create requirements-all-extras.txt
Make->>UV: uv export --extra local-embeddings
UV->>Req: create requirements-local-embeddings.txt
Make->>UV: uv export --extra nlp-metrics
UV->>Req: create requirements-nlp-metrics.txt
Make->>UV: uv export (base)
UV->>Req: create requirements.txt
Dev->>Git: commit uv.lock, uv-gpu.lock, requirements*.txt
CI->>Make: run make generate-requirements
Make->>Req: ensure requirements-*.txt generated
CI->>Git: git diff --exit-code requirements*.txt
alt diffs found
CI->>Dev: fail with message to run make generate-requirements and commit
else
CI->>Dev: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/check_dependencies.yaml:
- Around line 24-27: After the "Generate pinned requirements files" step (named
"Generate pinned requirements files") and before or after the "List generated
requirements files" step, add a CI step that fails when the working tree is
dirty by running a git diff check (e.g., run git diff --exit-code && git diff
--staged --exit-code) so any regenerated/modified requirements*.txt files cause
the workflow to fail; ensure the new step's name makes its purpose clear (e.g.,
"Fail on regenerated manifests drift") and that it runs in the same workspace
where generation occurs so stale checked-in artifacts cannot silently pass CI.
In `@Makefile`:
- Around line 41-54: The uv-lock-regenerate Makefile target is not
transactional: if the sed edit or either uv lock call fails you can leave
pyproject.toml mutated or lose a valid uv.lock; update the uv-lock-regenerate
target to perform the edit and both uv lock operations inside a single shell
with an EXIT trap that restores pyproject.toml on error, write both new lock
outputs to temporary files (e.g. uv.lock.tmp and uv-gpu.lock.tmp), and only mv
the temps to uv.lock and uv-gpu.lock after both uv lock commands succeed; ensure
cleanup removes any .tmp files and always restores pyproject.toml.bak on failure
or exit.
- Around line 56-74: The generated requirements files from the Makefile target
generate-requirements pin a local-version torch (TORCH_VERSION) but omit the
PyTorch index URL; update the uv export invocations that produce
requirements-local-embeddings.txt and requirements-all-extras.txt to either
include the PyTorch CPU index URL (add --index-url
https://download.pytorch.org/whl/cpu to those uv export commands) so the +cpu
wheels can be resolved, or instead exclude torch from exports (add
--no-emit-package torch) and keep the existing TORCH_VERSION echo and README
instructions to install torch separately; change the uv export calls referenced
in generate-requirements accordingly and keep the TORCH_VERSION extraction for
messaging.
🪄 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: 164b0445-9d55-48f9-bea6-9682f0f97185
⛔ Files ignored due to path filters (2)
uv-gpu.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/check_dependencies.yamlMakefileREADME.mdpyproject.tomlrequirements-all-extras.txtrequirements-local-embeddings.txtrequirements-nlp-metrics.txtrequirements.txt
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pyproject.toml (1)
10-29: Cross-major-version ranges may introduce breaking changes.A few dependency ranges span major versions:
langchain[huggingface]>=0.3.30,<=1.2.12(0.x → 1.x)numpy>=1.23.0,<=2.3.2(1.x → 2.x)datasets>=3.0.0,<=4.0.0(includes major bump boundary)Major version changes often include breaking API changes. Consider tightening these to stay within a single major version (e.g.,
numpy>=1.23.0,<2.0.0ornumpy>=2.0.0,<=2.3.2) to avoid unexpected runtime failures when the resolver picks different versions across environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 10 - 29, The dependency ranges in pyproject.toml are too broad across major versions (notably langchain[huggingface], numpy, datasets) which can introduce breaking changes; update each problematic entry (e.g., "langchain[huggingface]>=0.3.30,<=1.2.12", "numpy>=1.23.0,<=2.3.2", "datasets>=3.0.0,<=4.0.0") to a single-major-version range—either pin to the current major (like <2.0.0 for numpy 1.x) or require the newer major explicitly (>=2.0.0,<3.0.0) depending on tested compatibility—and similarly restrict langchain and datasets to a single major (e.g., >=0.3.30,<1.0.0 or >=1.0.0,<2.0.0) to ensure deterministic, non-breaking upgrades.Makefile (1)
50-52: The sed pattern is fragile ifpyproject.tomlstructure changes.The range pattern
/^\[tool\.uv\.sources\]/,/^torch = /dassumes thetorch =line immediately follows the section header. If other sources are added between them, or if the section is restructured, this pattern could delete unintended content or fail silently.Consider a more targeted approach that only removes the specific
torchsource line:♻️ Suggested alternative
- sed '/^\[tool\.uv\.sources\]/,/^torch = /d' pyproject.toml > pyproject.toml.tmp; \ + sed '/^torch = { index = "pytorch-cpu" }/d' pyproject.toml > pyproject.toml.tmp; \Or to also remove an empty
[tool.uv.sources]section if torch is the only entry, use a more explicit multi-line removal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 50 - 52, The sed range `/^\[tool\.uv\.sources\]/,/^torch = /d` is brittle; update the Makefile line that runs sed on pyproject.toml so it only removes the specific torch source entry instead of a whole section range and optionally removes the now-empty `[tool.uv.sources]` header. Concretely, replace the current sed invocation that targets the range with a command that deletes lines matching the torch assignment (e.g., lines matching `^\s*torch\s*=`) but only when inside the `[tool.uv.sources]` section, and add a follow-up step to delete an empty `[tool.uv.sources]` section header if it has no remaining entries; locate and modify the sed invocation in the Makefile (the line containing "sed '/^\\[tool\\.uv\\.sources\\]/,/^torch = /d' pyproject.toml") to implement this targeted removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Makefile`:
- Around line 50-52: The sed range `/^\[tool\.uv\.sources\]/,/^torch = /d` is
brittle; update the Makefile line that runs sed on pyproject.toml so it only
removes the specific torch source entry instead of a whole section range and
optionally removes the now-empty `[tool.uv.sources]` header. Concretely, replace
the current sed invocation that targets the range with a command that deletes
lines matching the torch assignment (e.g., lines matching `^\s*torch\s*=`) but
only when inside the `[tool.uv.sources]` section, and add a follow-up step to
delete an empty `[tool.uv.sources]` section header if it has no remaining
entries; locate and modify the sed invocation in the Makefile (the line
containing "sed '/^\\[tool\\.uv\\.sources\\]/,/^torch = /d' pyproject.toml") to
implement this targeted removal.
In `@pyproject.toml`:
- Around line 10-29: The dependency ranges in pyproject.toml are too broad
across major versions (notably langchain[huggingface], numpy, datasets) which
can introduce breaking changes; update each problematic entry (e.g.,
"langchain[huggingface]>=0.3.30,<=1.2.12", "numpy>=1.23.0,<=2.3.2",
"datasets>=3.0.0,<=4.0.0") to a single-major-version range—either pin to the
current major (like <2.0.0 for numpy 1.x) or require the newer major explicitly
(>=2.0.0,<3.0.0) depending on tested compatibility—and similarly restrict
langchain and datasets to a single major (e.g., >=0.3.30,<1.0.0 or
>=1.0.0,<2.0.0) to ensure deterministic, non-breaking upgrades.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87e00b51-bfe7-4d6e-bd12-d89c764686c1
⛔ Files ignored due to path filters (2)
uv-gpu.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/check_dependencies.yamlMakefileREADME.mdpyproject.tomlrequirements-all-extras.txtrequirements-local-embeddings.txtrequirements-nlp-metrics.txtrequirements.txt
✅ Files skipped from review due to trivial changes (4)
- requirements-nlp-metrics.txt
- requirements.txt
- README.md
- requirements-local-embeddings.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/check_dependencies.yaml
- requirements-all-extras.txt
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
1 participant
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.
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
Documentation
Chores