Skip to content

Conversation

@raptorsun
Copy link
Contributor

@raptorsun raptorsun commented Jan 12, 2026

Description

due to this issue in pybuild-deps, we have to pin the pip package to version below 25 to make pybuild-deps working, so that we can generate the requirements-build.txt required by prefetch step in Konflux

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Chores
    • Updated development dependency pip to 24.3.1 and bumped build tool versions (maturin, pathspec, trove-classifiers).
    • Enhanced build pipeline to more precisely filter and generate build requirements; refined exclusion handling for a heavy package.
    • Expanded and clarified indirect/optional dependency hints within the build manifest (additional commented entries and omission adjustments).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

Walkthrough

Makefile: changes to konflux-requirements filtering to extract package==version lines from requirements.x86_64.txt then exclude faiss-cpu. pyproject.toml: adds pip==24.3.1 to dev deps. requirements-build.txt: several dependency version updates and comment adjustments.

Changes

Cohort / File(s) Summary
Build pipeline
Makefile
konflux-requirements target now builds requirements-build.txt using a second input that first extracts lines matching ^[a-z]+==[^ ]+ from requirements.x86_64.txt then excludes faiss-cpu (replaces prior simple exclusion).
Project metadata
pyproject.toml
Added dev dependency pip==24.3.1.
Dependency pins & metadata
requirements-build.txt
Bumped/edited several entries (e.g., maturin → 1.11.5, pathspec → 1.0.3, trove-classifiers → 2026.1.12.15), added/commented indirect dependency hints, and adjusted omission/comment blocks.

Sequence Diagram(s)

(omitted — changes are small configuration/dependency edits without a new multi-component control flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • radofuchs
  • tisnik
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly summarizes the main change: pinning pip to version 24.3.1 to fix pybuild-deps compatibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b93195 and 1f6da72.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Makefile
  • pyproject.toml
  • requirements-build.txt
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: Check pyproject.toml for supported Python versions before development
Always check pyproject.toml for existing dependencies before adding new ones
Always verify current library versions in pyproject.toml rather than assuming versions
Use pylint with source-roots = "src" configuration

Files:

  • pyproject.toml
🧠 Learnings (3)
📓 Common learnings
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:43-45
Timestamp: 2025-08-18T10:55:18.914Z
Learning: The lightspeed-stack project updates dependencies every sprint as part of their regular maintenance cycle, which explains their preference for exact dependency pins rather than version ranges.
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:59-59
Timestamp: 2025-08-18T10:57:39.266Z
Learning: In the lightspeed-stack project, transitive dependencies like faiss-cpu are intentionally pinned as top-level dependencies to maintain better control over the dependency graph and avoid version conflicts when bundling ML/LLM tooling packages.
📚 Learning: 2026-01-11T16:30:41.767Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.767Z
Learning: Applies to pyproject.toml : Always check `pyproject.toml` for existing dependencies before adding new ones

Applied to files:

  • pyproject.toml
📚 Learning: 2026-01-11T16:30:41.768Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.768Z
Learning: Use `uv sync --group dev --group llslibdev` to install development dependencies

Applied to files:

  • pyproject.toml
⏰ 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). (4)
  • GitHub Check: build-pr
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci
🔇 Additional comments (3)
requirements-build.txt (1)

1-6: Auto-generated file - no manual review concerns.

This file is auto-generated by pybuild-deps compile as indicated in the header. The version updates and expanded dependency hints are expected outcomes of regenerating the build requirements with the updated tooling (pinned pip version) and current dependency graph.

Makefile (1)

119-121: LGTM - Improved filtering for pybuild-deps input.

The regex ^[a-zA-Z0-9][-a-zA-Z0-9._]*==[^ ]+ correctly extracts package==version entries while stripping hashes, comments, and other metadata that could interfere with pybuild-deps. The continued exclusion of faiss-cpu aligns with the project's handling of this transitive dependency. Based on learnings, faiss-cpu is intentionally managed separately.

pyproject.toml (1)

132-133: LGTM - Appropriate workaround for pybuild-deps dependency management.

The exact pin pip==24.3.1 in the dev group is correct since it only affects build tooling. The git history confirms this pin is necessary to ensure pybuild-deps functions properly during the build process.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @Makefile:
- Around line 119-121: The grep regex `^[a-z]+==[^ ]+` in the Makefile silently
excludes many valid package names; update the pattern used in the `grep -Eo
"^[a-z]+==[^ ]+" requirements.x86_64.txt` invocation to allow letters, digits,
dots, hyphens and underscores (e.g. use a character class like [A-Za-z0-9._-]+
before `==`) so package names such as `a2a-sdk`, `httpx-sse`, `jinja2`,
`email-validator`, etc. are matched and included.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6682b7a and 0b93195.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Makefile
  • pyproject.toml
  • requirements-build.txt
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: Check pyproject.toml for supported Python versions before development
Always check pyproject.toml for existing dependencies before adding new ones
Always verify current library versions in pyproject.toml rather than assuming versions
Use pylint with source-roots = "src" configuration

Files:

  • pyproject.toml
🧠 Learnings (4)
📓 Common learnings
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:43-45
Timestamp: 2025-08-18T10:55:18.914Z
Learning: The lightspeed-stack project updates dependencies every sprint as part of their regular maintenance cycle, which explains their preference for exact dependency pins rather than version ranges.
📚 Learning: 2026-01-11T16:30:41.767Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.767Z
Learning: Applies to pyproject.toml : Always check `pyproject.toml` for existing dependencies before adding new ones

Applied to files:

  • pyproject.toml
📚 Learning: 2026-01-11T16:30:41.768Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.768Z
Learning: Use `uv sync --group dev --group llslibdev` to install development dependencies

Applied to files:

  • pyproject.toml
📚 Learning: 2026-01-11T16:30:41.767Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.767Z
Learning: Applies to **/*.py : Use Llama Stack imports: `from llama_stack_client import AsyncLlamaStackClient`

Applied to files:

  • requirements-build.txt
⏰ 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). (4)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci
  • GitHub Check: E2E: library mode / ci
🔇 Additional comments (2)
requirements-build.txt (1)

1-6: LGTM! Auto-generated file with expected updates.

This file is auto-generated by pybuild-deps compile. The version bumps and pruned transitive dependency comments are expected results of regenerating the build requirements with the pinned pip version.

pyproject.toml (1)

132-133: Reconsider pinning pip to 24.3.1 due to known security vulnerabilities.

While pip 24.3.1 is a valid version (released October 27, 2024), it contains multiple known security vulnerabilities:

  • CVE-2025-8869: Symlink validation bypass in tar extraction (affects pip < 25.2)
  • Arbitrary code execution in pip self-update (affects pip < 25.0)
  • CVE-2025-50181: Bundled urllib3 vulnerability

These issues are fixed in pip >= 25.0 (for arbitrary code execution) and >= 25.2 (for symlink validation). Pinning to 24.3.1 should only proceed if there is a documented incompatibility with newer versions that cannot be resolved. If pybuild-deps compatibility with pip 25.x has been verified or fixed, use a newer pip version instead.

⛔ Skipped due to learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T16:30:41.767Z
Learning: Applies to pyproject.toml : Always verify current library versions in `pyproject.toml` rather than assuming versions

Signed-off-by: Haoyu Sun <hasun@redhat.com>
@raptorsun raptorsun force-pushed the fix-requirement-gen branch from 0b93195 to 1f6da72 Compare January 12, 2026 17:19
Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

LGTM

@tisnik
Copy link
Contributor

tisnik commented Jan 12, 2026

/retest

@tisnik tisnik merged commit 06686ba into lightspeed-core:main Jan 13, 2026
19 of 23 checks passed
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