Conversation
Signed-off-by: Anxhela Coba <acoba@redhat.com>
WalkthroughRepository-wide migration from PDM to uv for developer tooling and CI; Makefile and GitHub workflows updated to use uv; build backends migrated to Hatchling in pyproject files; PDM-specific config and CPU-specific torch source removed; Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant Runner as Local/CI Runner
participant UV as uv
participant Tools as Tools (black/ruff/mypy/pytest/build)
Developer->>Runner: run Makefile target or trigger CI job
Runner->>UV: uv lock --check
Runner->>UV: uv sync
alt needs dev deps
Runner->>UV: uv sync --group dev
end
Runner->>UV: uv run <tool> (black/ruff/mypy/pytest, python -m build)
Tools-->>Runner: results & exit codes
Runner-->>Developer: output / status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsc_agent_eval/pyproject.toml (1)
1-16: Switch CI workflows to uv & adjust PyTorch version specsI ran the suggested grep and fd checks and found:
- Two explicit
torch==2.7.0pins in:
pyproject.toml(root) at line 24lsc_eval/pyproject.tomlat line 21- A committed lockfile at
lsc_eval/uv.lock- No GitHub Actions workflows under
.github/workflows/referencing eitherpdmoruvPlease address the following before merging:
• CI updates (mandatory)
– Add or update your workflows in.github/workflows/*.ymlto install and invoke uv:
• Install uv (e.g.pip install uv)
• Runuv syncagainst your committeduv.lock
• Useuv run <task>instead ofpdm install• Dependency pins (mandatory)
– Change the hardtorch==2.7.0pins to a semantic range, e.g.:
torch>=2.7.0,<3.0.0
– If you need a specific build (CPU vs GPU), source from the PyTorch index with extras (e.g.[cpu]) instead of build-string-pinned versions• Lockfile verification (optional)
– Confirm you’ve committed everyuv.lock(e.g. at root if also using uv there) and that CI exercises the correct lockfileThese changes will ensure CI passes under uv and prevent future variant-conflicts with PyTorch builds.
🧹 Nitpick comments (7)
lsc_eval/README.md (2)
20-21: Use the actual GitHub org to avoid copy-paste errorsReplace placeholder with the real org to make the command runnable as-is.
-uv add git+https://github.com/your-org/lightspeed-evaluation.git#subdirectory=lsc_eval +uv add git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_eval
33-34: Consider module form for portabilityRunning via module avoids path issues if
runner.pymoves under a package.-uv run python runner.py --system-config config/system.yaml --eval-data config/evaluation_data.yaml +uv run python -m lsc_eval.runner --system-config config/system.yaml --eval-data config/evaluation_data.yamlREADME.md (1)
6-11: Tighten grammar and fix markdown list styleMinor grammar and markdownlint nits.
-- Install `uv` -- Check `uv --version` is working -- If running Python 3.11 from `venv`, make sure no conflicting packages are installed. In case of problems create a clean venv for Python 3.11 and `uv`. -- Run `uv sync` +- Install `uv` +- Check `uv --version` is working +- If running Python 3.11 from `venv`, make sure no conflicting packages are installed. In case of problems, create a clean venv for Python 3.11 and `uv`. +- Run `uv sync` - Optional: For development, run `make install-tools` - + if `uv` is not installed this will install `uv` by running `pip install uv` in your current Python environment. + - If `uv` is not installed, this will install `uv` by running `pip install uv` in your current Python environment.CONTRIBUTING.md (1)
56-60: Good: dev env via uv sync with groupsNice switch from PDM to
uv sync --group dev. Consider--frozenin CI to enforce lockfile when you update workflows.-uv sync --group dev +uv sync --group dev --frozenlsc_agent_eval/pyproject.toml (1)
37-39: Ensure YAML data files are included in your distributionWe ran a scan for non-Python assets and discovered the following YAML config under your package directory:
- lsc_agent_eval/sample_data/agent_goal_eval_example.yaml
To guarantee these files (and any future YAML/config assets) end up in your wheels and sdists, add a Hatch build section in lsc_agent_eval/pyproject.toml:
[tool.hatch.build] include = [ "lsc_agent_eval/**/*.py", "lsc_agent_eval/**/*.yaml", "README.md", "LICENSE", ]• Double-check whether any other YAML/config files (e.g., under lsc_eval/config or src/generate_answers) are needed at runtime; if so, extend the include patterns accordingly.
Makefile (2)
36-39: Prefer explicit ‘uv lock’ before sync.This makes lock updates deterministic and avoids network work during check-only runs.
update-deps: ## Check pyproject.toml for changes, update the lock file if needed, then sync. - uv sync - uv sync --group dev + uv lock + uv sync + uv sync --group dev
17-17: Avoid duplicate ‘uv sync’ in install-tools.You already sync in install-deps/install-deps-test. Running it here lengthens runs and couples tool bootstrap with env install.
- @for a in 1 2 3 4 5; do uv sync --group dev && break || sleep 15; done + @echo "Skipping env sync in install-tools; use install-deps or install-deps-test."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
lsc_eval/uv.lockis excluded by!**/*.lockpdm.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
CONTRIBUTING.md(4 hunks)Makefile(2 hunks)README-generate-answers.md(1 hunks)README.md(2 hunks)lsc_agent_eval/pyproject.toml(2 hunks)lsc_eval/README.md(3 hunks)lsc_eval/pyproject.toml(2 hunks)pyproject.toml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:17:48.640Z
Learnt from: asamal4
PR: lightspeed-core/lightspeed-evaluation#28
File: lsc_eval/runner.py:99-103
Timestamp: 2025-08-26T11:17:48.640Z
Learning: The lsc_eval generic evaluation tool is intended to become the primary evaluation framework, replacing an existing evaluation tool in the lightspeed-evaluation repository.
Applied to files:
lsc_eval/README.md
🪛 GitHub Actions: Ruff
lsc_agent_eval/pyproject.toml
[error] 1-1: Step 'pdm install' failed due to dependency resolution conflict for torch. Conflicting versions: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core.git). Loosen dependency constraints in pyproject.toml or adjust dependencies. ResolutionError: Unable to find a resolution.
pyproject.toml
[error] 1-1: Step 'pdm install' failed due to dependency resolution conflict for torch. Conflicting versions: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core.git). Loosen dependency constraints in pyproject.toml or adjust dependencies. ResolutionError: Unable to find a resolution.
lsc_eval/pyproject.toml
[error] 1-1: Step 'pdm install' failed due to dependency resolution conflict for torch. Conflicting versions: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core.git). Loosen dependency constraints in pyproject.toml or adjust dependencies. ResolutionError: Unable to find a resolution.
🪛 GitHub Actions: Pydocstyle
lsc_agent_eval/pyproject.toml
[error] 1-1: pdm install failed: Unable to find a resolution for torch; conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core). Loosen dependency constraints in pyproject.toml.
pyproject.toml
[error] 1-1: pdm install failed: Unable to find a resolution for torch; conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core). Loosen dependency constraints in pyproject.toml.
lsc_eval/pyproject.toml
[error] 1-1: pdm install failed: Unable to find a resolution for torch; conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core). Loosen dependency constraints in pyproject.toml.
🪛 GitHub Actions: Pyright
lsc_agent_eval/pyproject.toml
[error] 1-1: pdm install failed due to dependency resolution: Unable to find a resolution for torch. Conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core@git+https://github.com/road-core/service.git). Lockfile update failed. See /home/runner/.local/state/pdm/log/pdm-lock-x7h94jvv.log for details. Consider loosening constraints in pyproject.toml.
pyproject.toml
[error] 1-1: pdm install failed due to dependency resolution: Unable to find a resolution for torch. Conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core@git+https://github.com/road-core/service.git). Lockfile update failed. See /home/runner/.local/state/pdm/log/pdm-lock-x7h94jvv.log for details. Consider loosening constraints in pyproject.toml.
lsc_eval/pyproject.toml
[error] 1-1: pdm install failed due to dependency resolution: Unable to find a resolution for torch. Conflicts: torch==2.7.0 (from project) vs torch==2.7.0+cpu (from road-core@git+https://github.com/road-core/service.git). Lockfile update failed. See /home/runner/.local/state/pdm/log/pdm-lock-x7h94jvv.log for details. Consider loosening constraints in pyproject.toml.
🪛 LanguageTool
README.md
[grammar] ~8-~8: There might be a mistake here.
Context: ...e a clean venv for Python 3.11 and uv. - Run uv sync - Optional: For developmen...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
11-11: Unordered list style
Expected: dash; Actual: plus
(MD004, ul-style)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (11)
README-generate-answers.md (1)
37-37: Console script verified; no changes requiredI’ve confirmed that
generate_answersis declared under[project.scripts]inpyproject.toml(line 50), so invoking it viauv run generate_answers -hcorrectly uses the console script entry point. No module invocation changes are needed.lsc_eval/README.md (1)
128-133: Nice consistency on dev tools via uv runThe dev commands are coherent and align with the repo-wide tooling shift.
README.md (1)
34-35: Console scriptevaluateconfirmed inpyproject.tomlVerified that
evaluateis declared under[project.scripts]in the mainpyproject.toml(line 47), pointing tolightspeed_core_evaluation.driver:main. No changes required.
- Location:
pyproject.toml:47–evaluate = "lightspeed_core_evaluation.driver:main"CONTRIBUTING.md (2)
44-46: uv install instructions OKStraightforward install and version check steps; clear and platform-agnostic.
77-78:evaluateCLI entrypoint confirmed in pyproject.toml
Theevaluatescript is defined under[project.scripts], so the quickstart command will work as documented.
pyproject.toml:47–evaluate = "lightspeed_core_evaluation.driver:main"lsc_agent_eval/pyproject.toml (1)
18-28: Dev dependency group migration looks good
[dependency-groups]works well with uv; tool versions are sensible for Py3.11.pyproject.toml (1)
52-53: Comment-only change looks good.lsc_eval/pyproject.toml (2)
24-34: Dev dependency group migration to [dependency-groups] looks correct.
42-43: Hatchling backend switch is consistent with repo-wide uv migration.Makefile (2)
61-63: Test target update looks good.
1-84: No PDM invocations in CI workflows—review comment no longer applies.I searched the entire repository (including .github/workflows) for any
pdm install,pdm run, orpdm.lockreferences and found none in the CI configs. The only remaining “pdm” mentions are in lsc_agent_eval/README.md for local developer instructions, not in any GitHub Actions workflows.Likely an incorrect or invalid review comment.
| @command -v uv > /dev/null || { echo >&2 "uv is not installed. Installing..."; pip install uv; } | ||
| uv --version |
There was a problem hiding this comment.
Installing “uv” via pip likely installs the wrong package. Use official installer or brew.
Astral’s uv is a standalone binary; pip install uv usually pulls an unrelated PyPI package, breaking subsequent steps.
Use a robust installer path:
- @command -v uv > /dev/null || { echo >&2 "uv is not installed. Installing..."; pip install uv; }
- uv --version
+ @if ! command -v uv >/dev/null; then \
+ echo "uv is not installed. Installing..."; \
+ if command -v brew >/dev/null 2>&1; then \
+ brew install uv; \
+ else \
+ curl -LsSf https://astral.sh/uv/install.sh | sh; \
+ export PATH="$$HOME/.cargo/bin:$$HOME/.local/bin:$$PATH"; \
+ fi; \
+ fi
+ uv --version📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @command -v uv > /dev/null || { echo >&2 "uv is not installed. Installing..."; pip install uv; } | |
| uv --version | |
| @if ! command -v uv >/dev/null; then \ | |
| echo "uv is not installed. Installing..."; \ | |
| if command -v brew >/dev/null 2>&1; then \ | |
| brew install uv; \ | |
| else \ | |
| curl -LsSf https://astral.sh/uv/install.sh | sh; \ | |
| export PATH="$$HOME/.cargo/bin:$$HOME/.local/bin:$$PATH"; \ | |
| fi; \ | |
| fi | |
| uv --version |
🤖 Prompt for AI Agents
In Makefile around lines 8-9, the current install step runs `pip install uv`
which installs an unrelated PyPI package; replace this with a robust installer
path for Astral’s uv binary: check for uv as done, if missing attempt a native
package manager install (e.g., `brew install uv` on macOS or `apt-get install -y
uv` where appropriate) or download the official release binary from the
project's GitHub releases URL (verify checksum), make it executable and move it
into PATH; ensure the Makefile checks the install result and exits with an error
if uv is still not available.
| uv-lock-check: ## Check that the uv.lock file is in a good shape | ||
| uv lock --check | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make uv-lock-check generate a lock if missing to aid first-time clones.
uv lock --check fails when uv.lock is absent. Fall back to creating it.
-uv-lock-check: ## Check that the uv.lock file is in a good shape
- uv lock --check
+uv-lock-check: ## Check that the uv.lock file exists and is valid (create if missing)
+ uv lock --check || uv lock🤖 Prompt for AI Agents
In Makefile around lines 27-29, the uv-lock-check target currently runs only "uv
lock --check" which fails when uv.lock is missing; change the target to first
run the check and, if it fails due to a missing lock, fall back to generating a
lock (run "uv lock") so first-time clones get a created uv.lock; implement this
by chaining the commands (check || generate) and ensure non-zero exit is
preserved for real check failures.
Signed-off-by: Anxhela Coba <acoba@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/tests.yaml (1)
15-15: CI matrix includes Python 3.13 but pyproject.toml excludes itThe GitHub Actions workflow is set to test against Python 3.13, but none of the
requires-pythonconstraints currently admit 3.13—this mismatch will cause CI failures.•
.github/workflows/tests.yaml(line 15)- python-version: ["3.11", "3.12", "3.13"]•
pyproject.toml(root, line 30):requires-python = ">=3.11.1,<=3.12.8"•
lsc_eval/pyproject.toml(line 6):requires-python = ">=3.11,<3.13"•
lsc_agent_eval/pyproject.toml(line 6):requires-python = ">=3.11,<3.13"Suggested fixes (choose one):
Remove
"3.13"from the test matrix:- python-version: ["3.11", "3.12", "3.13"] + python-version: ["3.11", "3.12"]Or widen your package constraints (and README) to support 3.13, for example:
- requires-python = ">=3.11.1,<=3.12.8" + requires-python = ">=3.11.1,<=3.13.*"
🧹 Nitpick comments (11)
.github/workflows/outdated_dependencies.yaml (1)
23-23: Consider pinning behavior for “outdated” query to lockfileIf you intend to compare against the committed lock (instead of live index), use the locked view:
- run: uv tree --outdated + run: uv tree --locked --outdatedConfirm your desired semantics: “current env vs latest” (keep as-is) or “locked vs latest” (use --locked).
.github/workflows/mypy.yaml (1)
23-25: Avoid redundant sync; one ‘uv sync --group dev’ is sufficientuv installs the default group by default; adding --group dev installs dev in addition to default. Running plain uv sync first duplicates work.
- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (dev) + run: uv sync --group dev.github/workflows/pydocstyle.yaml (1)
23-25: Consolidate sync steps to a single dev installReduce CI time by avoiding duplicate resolution/install.
- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (dev) + run: uv sync --group dev.github/workflows/black.yaml (1)
23-27: Option: run black via uvx to avoid env couplingIf black isn’t a declared dev dependency, uv run will fail. Using uvx black runs the tool without requiring it in the project env.
- - name: Install devel dependencies - run: uv sync --group dev - - name: Black version - run: uv run black --version - - name: Black check - run: uv run black --check . + - name: Install devel dependencies + run: uv sync --group dev + - name: Black version + run: uvx black --version + - name: Black check + run: uvx black --check .If black is already in the dev group, current approach is fine.
.github/workflows/check_dependencies.yaml (1)
25-25: Export semantics: confirm desired groups and lock usage
- If you want production-only, current export (defaults) is fine.
- If you want to enforce lockfile versions, add --locked.
- run: uv export --format requirements-txt --output-file requirements.txt --no-hashes + run: uv export --format requirements-txt --output-file requirements.txt --no-hashes --lockedIf you need extras/groups, append: --group default --group .
.github/workflows/tests.yaml (2)
27-29: Avoid double resolution; install dev once
uv syncfollowed byuv sync --group devdoes the work twice. Oneuv sync --group devinstalls default+dev.- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (default + dev) + run: uv sync --group dev
33-33: Use working-directory instead of cdImproves readability and avoids compound shell commands.
- - name: Run agent tests - run: cd lsc_agent_eval && uv sync --group dev && uv run python -m pytest tests --cov=src --cov-report term-missing + - name: Install agent deps + run: uv sync --group dev + working-directory: lsc_agent_eval + - name: Run agent tests + run: uv run python -m pytest tests --cov=src --cov-report term-missing + working-directory: lsc_agent_eval.github/workflows/pylint.yaml (1)
24-26: Single sync for default+devConsolidate to one sync step to reduce CI time.
- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (default + dev) + run: uv sync --group dev.github/workflows/ruff.yaml (1)
23-25: Collapse sync stepsInstall dev in one pass.
- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (default + dev) + run: uv sync --group dev.github/workflows/pyright.yaml (1)
24-26: One sync step is enoughInstall default+dev together.
- - name: Install dependencies - run: uv sync - - name: Install devel dependencies - run: uv sync --group dev + - name: Install dependencies (default + dev) + run: uv sync --group devlsc_agent_eval/README.md (1)
49-51: Clarify env scope foruv syncIf this section targets end users (not contributors), consider
uv pip install -e .to mirror the pip example; keepuv syncfor the Development section.-# Or install with uv -uv sync +# Or install with uv (editable install) +uv pip install -e .
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.github/workflows/black.yaml(1 hunks).github/workflows/check_dependencies.yaml(1 hunks).github/workflows/mypy.yaml(1 hunks).github/workflows/outdated_dependencies.yaml(1 hunks).github/workflows/pydocstyle.yaml(1 hunks).github/workflows/pylint.yaml(1 hunks).github/workflows/pyright.yaml(1 hunks).github/workflows/ruff.yaml(1 hunks).github/workflows/tests.yaml(1 hunks)lsc_agent_eval/README.md(4 hunks)
🔇 Additional comments (5)
.github/workflows/pylint.yaml (1)
28-28: Makefile’spylinttarget already usesuv runThe
pylinttarget in the Makefile (around line 73–74) invokespylint: uv run pylint src lsc_agent_eval/srcso running
make pylintfrom the workflow will correctly use the uv virtual environment. No changes required in.github/workflows/pylint.yaml..github/workflows/ruff.yaml (1)
27-27: No changes needed—make ruffalready uses a UV-managed environment.The
ruffMakefile target invokesuv run ruff check …, so the workflow step (run: make ruff) is correctly using UV..github/workflows/pyright.yaml (1)
28-28: pyright target correctly invokesuv runThe Makefile’s
pyrighttarget on line 77 already usesuv run pyright …, ensuring execution within the UV environment—no changes are needed.lsc_agent_eval/README.md (2)
24-24: Align Python version support with CIREADME states “Python 3.11 or 3.12” while CI runs 3.11–3.13. Either update README or narrow CI matrix.
283-295: Nice: unified dev workflow with uvThe dev commands are consistent and discoverable. No action needed.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
Add PATH for uv after pip --user install
Prevents “uv: command not found.”
- name: Install uv
- run: pip install --user uv
+ run: pip install --user uv
+ - name: Add user base to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Add user base to PATH | |
| run: echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
🤖 Prompt for AI Agents
In .github/workflows/black.yaml around lines 20-21, after the step that runs
"pip install --user uv", add a step or command to ensure the user-local bin
directory is on PATH (e.g. export PATH="$HOME/.local/bin:$PATH" or set PATH in
the workflow step) so the installed "uv" executable can be found; update the
step to either run the export before invoking uv or add the directory to the
job's PATH environment.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv not on PATH after pip --user install
Add ~/.local/bin to PATH or use a setup action.
- name: Install uv
- run: pip install --user uv
+ run: pip install --user uv
+ - name: Add user base to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Add user base to PATH | |
| run: echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
🤖 Prompt for AI Agents
.github/workflows/check_dependencies.yaml lines 20-21: the workflow installs the
'uv' package with pip --user but does not ensure ~/.local/bin is on PATH so the
installed executable isn't found; update the workflow to either add ~/.local/bin
to the PATH before the install/run steps (so pip --user-installed binaries are
discoverable) or replace the manual install with a setup action that installs
Python packages system-wide or manages PATH (for example use
actions/setup-python and a step that adds ~/.local/bin to PATH via the workflow
env), ensuring subsequent steps can execute the 'uv' command.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv may not be on PATH when installed with pip --user
Same PATH issue as other workflows; uv calls may fail.
- name: Install uv
- run: pip install --user uv
+ run: pip install --user uv
+ - name: Add user base to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv may not be on PATH when installed with pip --user on GitHub runners
pip --user drops binaries in ~/.local/bin, which isn’t on PATH by default in Actions. Subsequent steps calling uv can fail with “command not found.”
Apply this diff to add the user base bin dir to PATH right after installation:
- name: Install uv
- run: pip install --user uv
+ run: pip install --user uv
+ - name: Add user base to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"Optional: replace the install with the dedicated setup action for reliability and caching:
- uses: astral-sh/setup-uv@vX
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Add user base to PATH | |
| run: echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
🤖 Prompt for AI Agents
.github/workflows/outdated_dependencies.yaml lines 20-21: the workflow installs
uv with pip --user which places the uv binary under ~/.local/bin (or the user
base bin) not on PATH on GitHub runners; after the install step add a step that
prepends the user base bin directory to PATH (e.g. determine it with python -m
site --user-base and append its /bin to $GITHUB_ENV) so subsequent steps can
find uv, or alternatively replace the pip install step with the dedicated action
(uses: astral-sh/setup-uv@vX) for more reliable setup and caching.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
Ensure uv is discoverable on PATH
Add ~/.local/bin to PATH after pip --user install.
- name: Install uv
- run: pip install --user uv
+ run: pip install --user uv
+ - name: Add user base to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Add user base to PATH | |
| run: echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
🤖 Prompt for AI Agents
.github/workflows/pydocstyle.yaml around lines 20-21: the workflow installs a
pip package with --user but doesn't make ~/.local/bin available on PATH; after
the pip install --user uv step, add a step that appends $HOME/.local/bin to the
runner PATH (for example: echo "$HOME/.local/bin" >> $GITHUB_PATH) so the
installed uv executable is discoverable by subsequent steps.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv install step may leave binary off PATH
Same concern as tests workflow: prefer astral-sh/setup-uv@v4 or add ~/.local/bin to PATH.
- - name: Install uv
- run: pip install --user uv
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v4 |
🤖 Prompt for AI Agents
.github/workflows/pylint.yaml lines 21-22: the current step installs uv via `pip
install --user uv` which may place the uv binary in ~/.local/bin and leave it
off PATH; replace this step with the astral-sh/setup-uv@v4 action (preferred)
or, if keeping pip install, export ~/.local/bin into PATH (e.g., add `echo
"::add-path::$HOME/.local/bin"` or adjust PATH) so the uv executable is
available to later steps.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv install via pip --user risks PATH issues
Switch to setup action or export PATH.
- - name: Install uv
- run: pip install --user uv
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v4 |
🤖 Prompt for AI Agents
In .github/workflows/pyright.yaml around lines 21-22, installing the 'uv'
package with 'pip install --user uv' can leave the binary outside the runner
PATH; replace this step by ensuring the workflow uses actions/setup-python to
set up the interpreter and then run a normal pip install (without --user) or
explicitly add the user's pip bin dir to PATH (export
PATH="$HOME/.local/bin:$PATH") so the installed executable is available to
subsequent steps.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
Ensure uv is on PATH
Use the official setup action or export PATH to avoid command-not-found errors.
- - name: Install uv
- run: pip install --user uv
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4🤖 Prompt for AI Agents
.github/workflows/ruff.yaml around lines 20-21: the workflow installs the uv
package with pip --user but doesn't ensure the install location is on PATH,
which causes command-not-found errors; fix by either (A) use the official
actions/setup-python to configure Python and ensure user-site binaries are on
PATH, or (B) after pip install --user, export PATH="$HOME/.local/bin:$PATH" (or
the correct user-base bin) in the step so the uv executable is discoverable;
implement one of these approaches and remove the failing assumption that --user
puts the binary on PATH.
| - name: Install uv | ||
| run: pip install --user uv |
There was a problem hiding this comment.
uv may not be on PATH; use official setup action or export PATH
pip install --user uv places the binary under ~/.local/bin, which isn’t guaranteed to be on PATH in GitHub runners. Prefer the official action or add PATH export to avoid “uv: command not found.”
Option A (recommended):
- - name: Install uv
- run: pip install --user uv
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4Option B (if keeping pip):
- name: Install uv
run: pip install --user uv
+ - name: Add uv to PATH
+ run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v4 |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Install uv | |
| run: pip install --user uv | |
| - name: Add uv to PATH | |
| run: echo "$HOME/.local/bin" >> "$GITHUB_PATH" |
🤖 Prompt for AI Agents
.github/workflows/tests.yaml around lines 24-25: the workflow installs uv via
`pip install --user uv` but the installer puts the binary in ~/.local/bin which
may not be on PATH on GitHub runners causing “uv: command not found”; fix by
either replacing the pip step with the official setup action for uv (preferred)
or, if keeping pip, export PATH to include ~/.local/bin (e.g., add a step that
sets PATH="$HOME/.local/bin:$PATH" for subsequent steps) so the uv binary is
discoverable.
| # Or install with uv | ||
| uv add git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_agent_eval | ||
| ``` |
There was a problem hiding this comment.
Use uv pip install for one-off installs from Git
uv add modifies the caller’s pyproject, which is not equivalent to pip install. For parity with the pip example, use uv pip install.
-# Or install with uv
-uv add git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_agent_eval
+# Or install with uv (one-off install into current environment)
+uv pip install git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_agent_eval📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Or install with uv | |
| uv add git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_agent_eval | |
| ``` | |
| # Or install with uv (one-off install into current environment) | |
| uv pip install git+https://github.com/lightspeed-core/lightspeed-evaluation.git#subdirectory=lsc_agent_eval |
🤖 Prompt for AI Agents
In lsc_agent_eval/README.md around lines 35 to 37, the README uses `uv add`
which mutates the caller’s pyproject; change the example to use `uv pip install`
for a one-off install to match the pip example (i.e. replace the `uv add
git+...` line with the equivalent `uv pip install git+...` command and keep the
same URL/subdirectory).
Signed-off-by: Anxhela Coba <acoba@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
7-11: Misplaced tool configs; move to proper [tool.*] sections so CI linters pick them up.Top-level
target-version,line-length, andlint.pydocstyle.conventionare ignored by Black/Ruff/pydocstyle.Apply:
-# always generate Python 3.11-compatible code. -target-version = "py311" -lint.pydocstyle.convention = "google" -line-length = 100 +# Formatting/linting configuration + +[tool.black] +line-length = 100 +target-version = ["py311"] + +[tool.ruff] +line-length = 100 +target-version = "py311" + +[tool.pydocstyle] +convention = "google"
♻️ Duplicate comments (1)
pyproject.toml (1)
66-67: Build fails with Hatchling due to VCS/direct dependency; enable direct references.Hatchling rejects
project.dependenciescontaining a direct reference (road-core @ git+...) unless explicitly allowed.Apply:
+[tool.hatch.metadata] +allow-direct-references = true + [build-system] requires = ["hatchling"] build-backend = "hatchling.build"
🧹 Nitpick comments (4)
pyproject.toml (4)
52-53: Comment/docs vs dependency mismatch.The note says “Modern PyTorch versions are available on PyPI directly,” yet the dependency uses a
+cpubuild tag. Align the comment and the spec (e.g., remove+cpuas suggested above), or document the extra index/source required for+cpu.
30-30: Broaden Python upper bound to avoid unnecessary patch-level lock-in.
<=3.12.8blocks 3.12.9+; prefer a minor-bound.-requires-python = ">=3.11.1,<=3.12.8" +requires-python = ">=3.11,<3.13"
32-32: Use SPDX identifier for license for better tooling compatibility.-license = {text = "Apache"} +license = { text = "Apache-2.0" }
25-25: Pin VCS dependency to a commit/tag for reproducible builds.
road-core @ git+https://...without a ref is mutable and harms reproducibility; also interacts with the Hatchling direct-reference setting.-road-core @ git+https://github.com/road-core/service.git +road-core @ git+https://github.com/road-core/service.git@<commit-sha-or-tag>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lsc_eval/pyproject.toml(2 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lsc_eval/pyproject.toml
🔇 Additional comments (1)
pyproject.toml (1)
24-25: Relax Torch pin to a compatible range & verify locallyIt’s best to drop the hard
+cpusuffix in favor of a version range that your environment’s resolver can satisfy (and letroad-coreor your CI handle any extra index configuration if needed).• In pyproject.toml, replace:
- "torch==2.7.0+cpu", + "torch>=2.7,<2.8",• Then, in a local Python 3.11 environment with
venvsupport, confirm resolution succeeds:python3.11 -m venv .venv-torchcheck source .venv-torchcheck/bin/activate pip install --upgrade pip pip install --no-deps 'torch>=2.7,<2.8' deactivateIf that install fails, you may need to point pip at the CPU-wheel index or further adjust the range. Please run this check in your development environment to ensure no conflicts recur.
Signed-off-by: Anxhela Coba <acoba@redhat.com>
Signed-off-by: Anxhela Coba <acoba@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
7-11: Fix PEP 621 schema violations: move tooling config out of [project].
target-version,lint.pydocstyle.convention, andline-lengthare not valid PEP 621 fields under[project]. Put them under the appropriate tool sections to avoid metadata validation/build issues.[project] name = "lightspeed-core-evaluation" version = "0.1.0" description = "Evaluation tooling for lightspeed-core" authors = [] -# always generate Python 3.11-compatible code. -target-version = "py311" -lint.pydocstyle.convention = "google" -line-length = 100 +# Tooling configuration moved to [tool.*] sections below. ... +[tool.ruff] +target-version = "py311" +line-length = 100 + +[tool.ruff.lint.pydocstyle] +convention = "google" + +[tool.black] +line-length = 100 +target-version = ["py311"]
♻️ Duplicate comments (1)
pyproject.toml (1)
24-24: Unblock installs: relax or remove strict torch pin.Re-introducing
torch==2.7.0risks resolver conflicts with CPU/GPU build variants (e.g.,+cpu) and with downstream projects (see earlier CI issue). Prefer a compatible range so the resolver can pick the correct local build.Apply:
- "torch==2.7.0", + "torch>=2.7,<2.8",Alternative: drop the top-level torch dependency if a transitive dependency (e.g., from a core lib) already pins the exact build required.
🧹 Nitpick comments (2)
pyproject.toml (2)
29-29: Loosen Python upper bound to avoid unnecessary pinning to a patch release.
<=3.12.8blocks newer 3.12 patch updates. Prefer a minor cap.-requires-python = ">=3.11.1,<=3.12.8" +requires-python = ">=3.11,<3.13"
51-52: Clarify install guidance for CPU vs CUDA wheels.Good note, but devs often need CUDA. Add a tip on selecting CUDA wheels with uv/pip.
-# Note: torch[cpu] variant configuration removed for uv compatibility -# Modern PyTorch versions are available on PyPI directly +# Note: torch variant selection +# - CPU: default PyPI wheels resolve automatically. +# - CUDA: install with the appropriate index URL (example for cu124): +# uv pip install --index-url https://download.pytorch.org/whl/cu124 torch
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
README-generate-answers.md(2 hunks)pyproject.toml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README-generate-answers.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Pylinter
- GitHub Check: mypy
🔇 Additional comments (1)
pyproject.toml (1)
65-66: Hatchling migration verified—builds clean, no direct dependency references detected
- The
build-systemsection in bothpyproject.tomlfiles specifies onlywith no VCS or file URLs underrequires = ["hatchling"] build-backend = "hatchling.build"requires—so no direct references to enable.- The GitHub URLs found by the scan are under
[project.urls](metadata) and don’t affect the build backend’s dependency policy.- Non-isolated builds produced both sdist and wheel artifacts successfully.
No changes needed; migration is complete.
tisnik
left a comment
tisnik
left a comment
There was a problem hiding this comment.
LGTM
seems like those "OLS import errors" need to be solved in separate PR?
|
@tisnik I'm good to merge this one and we can open new PR for decoupling OLS to avoid an even bigger PR :) |
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
LGTM, ols related error will be fixed (when this gets removed)
4 participants
Changes to use uv project manager instead of pdm for faster development and ease for mac users.
Summary by CodeRabbit