Skip to content

Conversation

@raptorsun
Copy link
Contributor

@raptorsun raptorsun commented Oct 31, 2025

Do Not Merge!

Description

activate hermetic build

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

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

  • New Features

    • Pipeline options to build source images, prefetch inputs, and enable hermetic runs.
    • Conditional hermetic build path for container builds with a non-hermetic fallback.
    • Hermetic Python tooling group, new package index entry, and a hermetic dependency group to support isolated installs.
    • Tightened Python compatibility range.
  • Chores

    • Deterministic RPM configuration and lockfile for x86_64 and aarch64.
    • Added UBI9 repository configuration and a hermetic requirements file.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

Adds hermetic build support: Tekton PipelineRun params, RPM input and lockfiles, a UBI9 repo, hermetic Python requirements and pyproject tooling/groups, and Containerfile changes to conditionally source /cachi2/cachi2.env and run a hermetic uv sync path. (50 words)

Changes

Cohort / File(s) Summary
Tekton pipeline
\.tekton/lightspeed-stack-pull-request.yaml
Adds public PipelineRun spec.params: build-source-image, prefetch-input, and hermetic; retains build-platforms and image-expires-after; inserts inline comments about arm64 and on-push pipeline notes.
RPM inputs & lockfile
rpms.in.yaml, rpms.lock.yaml
Adds rpms.in.yaml (packages: gcc, jq, patch; contentOrigin.repofiles; arches x86_64/aarch64) and rpms.lock.yaml (lockfileVersion/vendor and per-arch packaged RPM entries with url/size/sha256/name/evr/sourcerpm; empty source/module_metadata).
UBI repo config
ubi.repo
Adds UBI9 repository definitions for BaseOS, AppStream, and CodeReady Builder with RPM / Debug / SRPM variants (name, baseurl, enabled, gpgkey, gpgcheck).
Hermetic Python deps & tooling
requirements.hermetic.txt, pyproject.toml
Adds requirements.hermetic.txt (uv==0.8.15, pdm>=2.21.0, pdm-backend) and updates pyproject.toml to declare tool-hermetic, add a hermetic dependency group llslibdev-hermetic, UV index/source additions, and a conflicts matrix separating hermetic/non-hermetic groups.
Container build
Containerfile
Copies requirements.txt into the build and changes RUN logic to check for /cachi2/cachi2.env; if present, sources it and runs hermetic uv sync/pdm path (--no-index, --find-links, python version pinned); otherwise falls back to existing non-hermetic uv sync flow.

Sequence Diagram(s)

sequenceDiagram
    participant Tekton as Tekton Pipeline
    participant Builder as Container build
    participant Cachi as /cachi2/cachi2.env
    participant UV as uv (package sync)

    rect rgb(245,250,255)
      note right of Tekton: PipelineRun invoked with params: hermetic, prefetch-input, build-source-image
      Tekton->>Builder: start build
      Builder->>Cachi: test for /cachi2/cachi2.env
      alt cachi2 present (hermetic)
        Builder->>Cachi: source env
        Cachi-->>Builder: provide PIP_FIND_LINKS / PIP_NO_INDEX
        Builder->>UV: uv sync --locked --group llslibdev-hermetic --no-index --find-links ...
      else cachi2 absent (non-hermetic)
        Builder->>UV: uv sync --locked --group llslibdev ...
      end
      UV->>Builder: sync result
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Files to scrutinize:
    • pyproject.toml — new hermetic group, UV indices, conflicts matrix, and long dependency lists.
    • Containerfile — shell conditional, environment sourcing, hermetic uv/pdm flags, and added COPY steps.
    • rpms.lock.yaml — validate RPM URLs, sizes, and sha256 checksums per-arch.
    • .tekton/lightspeed-stack-pull-request.yaml — public params ordering and comment insertions.

Possibly related PRs

Suggested reviewers

  • tisnik

Poem

"I hopped through builds with nimble paws,
Locked RPMs snug behind soft jaws,
cachi2 hummed a hermetic tune,
uv synced under the silver moon,
Carrots, code, and cozy bytes." 🐇

Pre-merge checks and finishing touches

✅ 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 '[WIP] konflux: hermetic build' is specific and clearly relates to the main changeset which introduces hermetic build support across multiple configuration files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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: 0

🧹 Nitpick comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)

30-30: Replace TODO comments with tracked issues.

Informal TODO comments should be converted to tracked issues or linked via ADR/documentation. Line 30 references platform expansion; line 34 mentions coordinating changes with the on-push pipeline.

These items would be easier to track and communicate if converted to:

  • GitHub issues with acceptance criteria
  • An ADR (Architecture Decision Record) documenting the multi-platform strategy
  • Comments in docs linking to the implementation plan

For now, consider adding issue numbers or links to clarify ownership and timeline.

Also applies to: 34-34

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd3291c and 27c8343.

📒 Files selected for processing (1)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
⏰ 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_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (3)
.tekton/lightspeed-stack-pull-request.yaml (3)

35-40: Verify hermetic build enablement and coordinate on-push pipeline updates.

This change activates hermetic (network-isolated) builds alongside source-image generation and dependency prefetching. These are significant behavioral changes that require:

  1. Verify the hermetic build works end-to-end: Confirm all build steps (especially custom ones in your Containerfile) remain compatible with network isolation.
  2. Confirm on-push pipeline sync: The TODO on line 34 indicates the on-push pipeline also needs these parameter additions. Ensure both pipelines are kept in sync to maintain consistent behavior across PR and merge workflows.
  3. Test the prefetch configuration: The prefetch-input JSON (line 38) pre-downloads pip dependencies. Verify this works with your project's dependency structure and any custom pip configurations.

Run the following checks if available:

  • Trigger a test build on this PR branch and verify the build completes successfully with hermetic mode enabled.
  • Inspect the on-push pipeline configuration to confirm it also needs updating (see .tekton/lightspeed-stack-on-push.yaml or similar).
  • Validate that all build-time dependencies are captured by the prefetch configuration.

1-40: PR requires completion before merge: checklist items outstanding.

Per the PR objectives, this PR is marked WIP and the following checklist items remain unchecked:

  • Self-review performed
  • PR passed pre-merge test jobs
  • Core feature tests added (if applicable)

The Testing section contains no concrete test steps or results. Before this can be merged:

  1. Complete self-review, documenting any concerns or design rationale.
  2. Run the full pre-merge test suite and provide results.
  3. Add or update tests to cover the hermetic build behavior if not already present.
  4. Update the on-push pipeline to maintain consistency (as noted in the TODO).

Please confirm:

  • Have pre-merge tests been executed? If so, share the results.
  • Are there existing tests for the build pipeline configuration? Do they need updates?
  • Is the on-push pipeline update being handled in this PR or a follow-up?

38-38: JSON syntax is valid; manual verification of task compatibility required.

The prefetch-input JSON at line 38 is syntactically valid and properly passed to the prefetch-dependencies-oci-ta task via the input parameter (line 186). Path "." correctly resolves to the cloned project root since the task runs after clone-repository. However, verify that the remote task bundle (version 0.2) accepts the string value "allow_binary": "true" as configured—consult the task documentation or test execution to confirm compatibility.

@raptorsun raptorsun force-pushed the hermetic branch 2 times, most recently from 279ac6d to 57b1036 Compare November 3, 2025 16:00
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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 279ac6d and 57b1036.

📒 Files selected for processing (5)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • rpms.in.yaml
  • ubi.repo
  • rpms.lock.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Use uv as the package manager; prefix commands with uv run

Applied to files:

  • requirements.hermetic.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_tests (azure)
  • GitHub Check: e2e_tests (ci)
🔇 Additional comments (3)
requirements.hermetic.txt (1)

1-1: Upgrade uv to 0.9.7 after verifying hermetic build reproducibility.

uv 0.8.15 is outdated; uv 0.9.7 was released October 30, 2025. However, the default --python-platform linux target changed from manylinux_2_17 to manylinux_2_28, which affects wheel resolution. Since the project pins a custom PyTorch CPU wheel index and uses a locked requirements.txt generated via uv pip compile, upgrading to 0.9.7 may cause requirements.txt to diverge, breaking hermetic reproducibility. Regenerate requirements.txt with uv 0.9.7 and verify the lock file is identical or intentionally updated before committing the upgrade.

.tekton/lightspeed-stack-pull-request.yaml (2)

37-38: The allow_binary: true setting is standard Cachi2/Hermeto practice and does not conflict with hermetic builds.

The allow_binary parameter in Cachi2 controls whether the prefetcher will accept binary distributions (wheels) for Python packages instead of forcing source distributions. This setting operates during the prefetch phase—precisely where dependencies are cached before the hermetic build phase begins. Hermetic builds emphasize no external network access during the build itself, not during prefetching. Pre-fetching binary wheels (when available) is a standard optimization and does not weaken hermetic guarantees; the wheels are vendored and available for reproducible builds.

The pipeline includes security scanning tasks (clair-scan, sast-snyk-check) that can audit the final artifacts, and the setting is intentional and documented. No action required.


35-40: All specified parameters are properly integrated and configuration files exist.

The verification confirms:

  1. Parameters are defined in the pipeline spec: build-source-image (lines 91-94), prefetch-input (lines 82-85), hermetic (lines 78-81) are all declared as string parameters with appropriate defaults.

  2. Parameters are passed to downstream tasks:

    • prefetch-input is correctly passed to the prefetch-dependencies task (line 186 as "input") and buildah-remote-oci-ta task (line 225 as "PREFETCH_INPUT")
    • hermetic is passed to buildah-remote-oci-ta (line 223 as "HERMETIC")
    • build-source-image is used in the source-build-oci-ta task's when condition (line 320)
  3. Task references are correct: prefetch-dependencies-oci-ta:0.2, buildah-remote-oci-ta:0.6, and source-build-oci-ta:0.3 are all referenced at the specified lines.

  4. Configuration files exist: rpms.in.yaml, rpms.lock.yaml, and ubi.repo are present in the repository root.

  5. prefetch-input JSON is well-formed with valid fields (type, path, allow_binary).

Note: The prefetch-input JSON schema validation against the external Konflux task bundle schema cannot be performed locally; verify this against the official Konflux CI task catalog documentation if needed.

value: quay.io/redhat-user-workloads/lightspeed-core-tenant/lightspeed-stack:on-pr-{{revision}}
- name: image-expires-after
value: 5d
# todo: add arm64. refer to https://konflux.pages.redhat.com/docs/users/getting-started/multi-platform-builds.html#arm64-2
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete TODOs block mergeability.

Lines 30 and 34 contain TODO comments that indicate incomplete work:

  • Line 30: ARM64 multi-platform support not yet added
  • Line 34: on-push pipeline not updated to match PR pipeline changes

These must be resolved or tracked as separate issues before merge.

Create follow-up issues or PRs to address:

  1. Add ARM64 platform support (refer to the linked Konflux documentation)
  2. Apply the same hermetic/prefetch changes to .tekton/lightspeed-stack-on-push.yaml

Also applies to: 34-34

🤖 Prompt for AI Agents
In .tekton/lightspeed-stack-pull-request.yaml around lines 30 and 34, there are
TODO comments left for adding ARM64 multi-platform support and updating the
on-push pipeline to match PR-pipeline hermetic/prefetch changes; resolve by
either implementing ARM64 build support per the Konflux multi-platform docs (add
arm64 entries, multi-platform build config and any necessary image/platform
matrix) and by applying the same hermetic/prefetch changes to
.tekton/lightspeed-stack-on-push.yaml, or open two follow-up issues/PRs that
clearly track: (1) adding ARM64 support with a link to the Konflux guidance, and
(2) updating the on-push pipeline to mirror the PR pipeline changes — reference
the new issues/PR numbers in these TODOs and remove or replace the inline TODO
comments so the file is mergeable.

@raptorsun raptorsun force-pushed the hermetic branch 3 times, most recently from be8d77a to 96fbb5e Compare November 4, 2025 10:32
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

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)

1-195: PR is marked "[WIP] Do Not Merge!" — address before review.

The PR objectives indicate this is marked as work-in-progress with incomplete author checklists (self-review, pre-merge tests, feature tests all unchecked). This should be finalized before requesting substantive code review.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be8d77a and 96fbb5e.

📒 Files selected for processing (6)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • pyproject.toml (1 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • rpms.lock.yaml
  • .tekton/lightspeed-stack-pull-request.yaml
  • rpms.in.yaml
  • requirements.hermetic.txt
  • ubi.repo
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

Files:

  • pyproject.toml
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to pyproject.toml : ALWAYS check pyproject.toml for existing dependencies before adding new ones

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). (1)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request

@raptorsun raptorsun force-pushed the hermetic branch 2 times, most recently from deee0f1 to 3acd44b Compare November 4, 2025 16:40
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: 4

♻️ Duplicate comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)

30-30: Resolve TODOs before merge.

The TODO comments at lines 30 and 34 were previously flagged as merge blockers. These must be addressed by either:

  1. Implementing the ARM64 support and updating the on-push pipeline, or
  2. Creating tracked issues and updating the TODOs with issue references.

Also applies to: 34-34

pyproject.toml (1)

208-209: Reconsider pdm in hermetic group—already flagged in previous review.

The hermetic group includes both uv==0.8.15 and pdm>=2.21.0. A previous review comment identified that pdm is already the project's build backend and conflicts with the hermetic tooling approach that uses uv. Including both package managers in the same group creates confusion about which tool is intended for hermetic builds.

Based on the Containerfile (line 30) which uses uv sync, the hermetic group should only include uv, not pdm. Remove pdm from this group.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between deee0f1 and 3acd44b.

📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • requirements.hermetic.txt
  • ubi.repo
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

Files:

  • pyproject.toml
🧠 Learnings (2)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to pyproject.toml : ALWAYS check pyproject.toml for existing dependencies before adding new ones

Applied to files:

  • pyproject.toml
📚 Learning: 2025-08-18T10:57:39.266Z
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.

Applied to files:

  • pyproject.toml
🪛 GitHub Actions: E2E Tests
Containerfile

[error] 29-33: RUN ... uv sync --locked --no-dev --group llslibdev; failed: The lockfile at uv.lock needs to be updated. To update, run 'uv lock'.

⏰ 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: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: build-pr
🔇 Additional comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)

35-40: LGTM: Hermetic build parameters correctly configured.

The three new parameters (build-source-image, prefetch-input, hermetic) are properly structured for Konflux hermetic builds. The prefetch-input correctly specifies both rpm and pip dependency types, with allow_binary enabled for pip packages.

rpms.lock.yaml (1)

1-40: Lockfile structure is correct but may be incomplete.

The lockfile is well-formed and includes oniguruma (jq's dependency) and jq for both architectures. However, gcc and patch declared in rpms.in.yaml are not present here. This was flagged in the rpms.in.yaml review.

@raptorsun raptorsun force-pushed the hermetic branch 7 times, most recently from d46f456 to 5ebdc69 Compare November 5, 2025 11:07
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: 0

🧹 Nitpick comments (1)
Containerfile (1)

28-33: Consider extracting the common command to reduce duplication.

Both branches execute the same uv sync command. You can simplify the conditional to only source the cachi2 environment when present, then run the command once:

-# Source cachi2 environment for hermetic builds if available, otherwise use normal installation
-RUN if [ -f /cachi2/cachi2.env ]; then \
-        . /cachi2/cachi2.env && uv sync --locked --no-dev --group llslibdev; \
-    else \
-        uv sync --locked --no-dev --group llslibdev; \
-    fi
+# Source cachi2 environment for hermetic builds if available
+RUN if [ -f /cachi2/cachi2.env ]; then . /cachi2/cachi2.env; fi && \
+    uv sync --locked --no-dev --group llslibdev

This reduces duplication and makes future command changes easier to maintain.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6eef4cf and 5ebdc69.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • rpms.in.yaml
  • .tekton/lightspeed-stack-pull-request.yaml
  • rpms.lock.yaml
  • requirements.hermetic.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). (3)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
ubi.repo (1)

17-17: The ubi.repo SRPM URLs are correctly configured.

The verification confirms that UBI9 source RPM repositories require the $basearch variable in the URL path. Testing shows:

  • URLs with $basearch return HTTP 302 (valid redirects)
  • URLs without $basearch return HTTP 404 (not found)

Red Hat's CDN organizes source packages with architecture-specific URL paths, making the current configuration correct. No changes are needed.

Likely an incorrect or invalid review comment.

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: 0

♻️ Duplicate comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)

30-30: Unresolved TODOs block mergeability — duplicate of past review.

Lines 30 and 34 contain TODO comments that remain unresolved:

  • Line 30: ARM64 multi-platform support not yet added
  • Line 34: on-push pipeline not updated to match PR pipeline changes

These were flagged in the previous review as blocking issues. Either resolve them or create and reference follow-up tracking issues before merging.

Also applies to: 34-34

🧹 Nitpick comments (1)
pyproject.toml (1)

178-221: Eliminate duplication: llslibdev-hermetic mirrors entire llslibdev group.

Lines 178–221 duplicate the entire llslibdev group (lines 130–177) verbatim. This creates a maintenance burden: any change to llslibdev must be manually synchronized to llslibdev-hermetic. The comment at line 179 acknowledges this ("the same as llslibdev, just using default index"), but the actual difference in index routing is handled separately at lines 95–98.

Explore one of these alternatives:

Option 1: Use group composition (if uv supports it)

llslibdev-hermetic = [
    { include-group = "llslibdev" },
]

This inherits all dependencies from llslibdev while allowing the torch source scoping to apply per-group routing.

Option 2: Keep duplication with a maintenance warning
Add a prominent comment at line 178 explicitly warning that changes to llslibdev (lines 130–177) must be mirrored to llslibdev-hermetic, and clarify that torch variant routing is controlled by the source scoping at lines 95–98, not by group differences.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ebdc69 and c5d3f13.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • rpms.in.yaml
  • requirements.hermetic.txt
  • ubi.repo
  • rpms.lock.yaml
  • Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

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_tests (ci)
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (2)
pyproject.toml (2)

95-98: Verify torch variant routing for llslibdev-hermetic.

Line 96 routes llslibdev group to pytorch-cpu (CPU variant), but line 97 routes llslibdev-hermetic to pypi-default, which defaults to the GPU variant on PyPI. The past review suggested both groups should use the CPU variant for consistency. Clarify whether the GPU variant is intentional for hermetic builds, or whether line 97 should also reference pytorch-cpu.


222-225: The review comment is based on an incorrect premise and should be dismissed.

The review claims that requirements.hermetic.txt specifies uv==0.8.15 as the hermetic tool, not pdm, but the actual file contents show that requirements.hermetic.txt contains both uv==0.8.15 and pdm>=2.21.0. Therefore, including pdm>=2.21.0 in the tool-hermetic group is correct and maintains consistency with the hermetic requirements file. Having pdm as both the build-backend and in the hermetic tooling group is intentional for ensuring reproducible builds.

Likely an incorrect or invalid review comment.

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

♻️ Duplicate comments (2)
pyproject.toml (2)

178-221: Consider group composition to reduce duplication.

The llslibdev-hermetic group duplicates all 44 dependencies from llslibdev. While this works, any future changes to llslibdev must be manually synchronized. If your tooling supports it, consider using group composition/inclusion to reference the base group and avoid the maintenance burden.


95-98: Torch variant mismatch between groups—hermetic will use GPU instead of CPU.

The torch source configuration directs llslibdev to the pytorch-cpu index but routes llslibdev-hermetic to pypi-default, which defaults to the GPU variant. This creates an unintended hardware dependency difference between the two groups that should otherwise be equivalent.

Apply this diff to align both groups to the CPU variant:

 torch = [
     { index = "pytorch-cpu", group = "llslibdev" },
-    { index = "pypi-default", group = "llslibdev-hermetic" }
+    { index = "pytorch-cpu", group = "llslibdev-hermetic" }
 ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5d3f13 and fadad34.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • rpms.in.yaml
  • requirements.hermetic.txt
  • ubi.repo
  • rpms.lock.yaml
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

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_tests (azure)
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (5)
Containerfile (1)

28-33: LGTM! Clean hermetic build conditional.

The conditional correctly switches between hermetic and standard builds by detecting the cachi2 environment file and selecting the appropriate dependency group. Both branches maintain lockfile integrity with the --locked flag.

.tekton/lightspeed-stack-pull-request.yaml (2)

30-30: TODOs are tracked but need resolution before merge.

The TODO comments for ARM64 support (line 30) and on-push pipeline updates (line 34) were flagged in previous reviews as blocking mergeability. Since this PR is still marked WIP, these are acceptable for now, but ensure they're addressed or converted to tracked issues before removing the WIP status.

Also applies to: 34-34


35-40: LGTM! Hermetic build parameters are correctly configured.

The new parameters enable hermetic builds with appropriate prefetch configuration for both RPM and Python dependencies. The allow_binary: true setting for pip is appropriate for this Python project.

pyproject.toml (2)

89-93: LGTM! PyPI default index correctly configured.

The explicit pypi-default index is properly configured for hermetic builds that need to use the standard PyPI repository.


99-105: LGTM! Conflict declaration prevents mixing incompatible groups.

The mutual exclusion between llslibdev and llslibdev-hermetic is correctly configured to prevent simultaneous installation.

Comment on lines 222 to 226
tool-hermetic = [
"uv==0.8.15",
"pdm>=2.21.0",
]
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify pdm is needed for hermetic tooling.

The tool-hermetic group includes both uv==0.8.15 and pdm>=2.21.0. A previous critical review comment suggested removing pdm from this group since uv is the hermetic build tool and pdm is already the build backend. Confirm that pdm is intentionally included here for hermetic builds, or remove it if uv alone is sufficient.


🏁 Script executed:

#!/bin/bash
# Check if pdm is used in hermetic build context beyond being the build backend
echo "=== Check Containerfile for pdm usage ==="
rg -n "pdm" Containerfile || echo "No pdm references in Containerfile"

echo "=== Check hermetic requirements file ==="
if [ -f "requirements.hermetic.txt" ]; then
  cat requirements.hermetic.txt
else
  echo "requirements.hermetic.txt not found"
fi

echo "=== Check Tekton pipeline for pdm usage ==="
rg -n "pdm" .tekton/lightspeed-stack-pull-request.yaml || echo "No pdm references in pipeline"

Length of output: 214


🏁 Script executed:

#!/bin/bash
# Check pyproject.toml for build-backend and hermetic-related info
echo "=== Check build-backend in pyproject.toml ==="
rg -n "build-backend|tool-hermetic" pyproject.toml -A2 -B2

# Look for build/hermetic documentation or scripts
echo "=== Check for build-related documentation ==="
fd -e md -e txt -e rst | xargs rg -l "hermetic\|build.*tool" 2>/dev/null | head -10

# Check if any script invokes pdm during build/hermetic context
echo "=== Check for pdm invocation in scripts/docs ==="
rg -n "pdm\s+(install|build|sync|lock)" --type=py --type=sh --type=bash 2>/dev/null || echo "No pdm invocation patterns found"

# Check Makefile if exists
echo "=== Check Makefile for hermetic/build targets ==="
if [ -f "Makefile" ]; then
  rg -n "hermetic|pdm|uv" Makefile
else
  echo "Makefile not found"
fi

Length of output: 297


🏁 Script executed:

#!/bin/bash
# Check pyproject.toml for build-backend configuration
echo "=== Extract build-backend and tool-hermetic from pyproject.toml ==="
rg -n "build-backend" pyproject.toml -A1 -B1

echo "=== Extract tool-hermetic section ==="
rg -n "\[tool\.hermetic\]" pyproject.toml -A10

echo "=== Extract build-system section ==="
rg -n "\[build-system\]" pyproject.toml -A10

echo "=== Search for any hermetic-related comments in pyproject.toml ==="
rg -n "hermetic" pyproject.toml -B2 -A2

echo "=== Check if uv is documented as the build tool ==="
rg -n "uv" pyproject.toml | head -20

Length of output: 1462


🏁 Script executed:

#!/bin/bash
# Search for any documentation or comments about hermetic builds or why pdm is needed
echo "=== Search for hermetic documentation in README/docs ==="
rg -n "hermetic" README.md docs/ 2>/dev/null || echo "No hermetic references in README/docs"

echo "=== Check git history comments for tool-hermetic context ==="
git log -p --all -S "tool-hermetic" -- pyproject.toml 2>/dev/null | head -100 || echo "Git history not available"

echo "=== Check if pdm is imported or called in any Python files ==="
rg -n "import pdm|from pdm" --type=py 2>/dev/null || echo "No pdm imports found"

echo "=== Check requirements.hermetic.txt history if it exists ==="
if [ -f "requirements.hermetic.txt" ]; then
  git log -p --all -- requirements.hermetic.txt 2>/dev/null | head -100 || echo "Git history not available"
fi

Length of output: 431


🏁 Script executed:

#!/bin/bash
# Check README context around hermetic builds mention
echo "=== README context for hermetic builds ==="
sed -n '730,750p' README.md

# Double-check: is pdm ever invoked as a command in any files?
echo "=== Search for pdm command invocations (not just imports) ==="
rg -n "pdm" --type=sh --type=bash --type=yaml --type=makefile 2>/dev/null || echo "No pdm found in build files"

# Check the full tool-hermetic group definition
echo "=== Full tool-hermetic group from pyproject.toml ==="
rg -n "tool-hermetic" pyproject.toml -A5

Length of output: 783


Remove pdm from tool-hermetic group; it is unused in hermetic builds.

Verification confirms that pdm is not invoked during hermetic builds. No pdm references appear in the Containerfile, Tekton pipeline, or any build scripts. The hermetic build context uses only uv. Since pdm's sole role is as the project's build backend (specified in build-system), it does not belong in the tool-hermetic dependency group. Remove "pdm>=2.21.0" from lines 222-225.

🤖 Prompt for AI Agents
In pyproject.toml around lines 222 to 225, the tool-hermetic dependency group
incorrectly includes "pdm>=2.21.0" which is unused in hermetic builds; remove
the pdm entry so the group contains only "uv==0.8.15". Update the file by
deleting the "pdm>=2.21.0" line and ensure trailing commas/formatting remain
valid (leave the tool-hermetic list with only the uv entry).

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: 0

♻️ Duplicate comments (1)
pyproject.toml (1)

222-225: Remove pdm from tool-hermetic; it is unused in hermetic builds.

Previous review verified that pdm is not invoked during hermetic builds (no references in Containerfile, Tekton pipeline, or build scripts). The hermetic context uses only uv. Since pdm's sole role is the project's build backend (pdm.backend in build-system at line 247), it does not belong in tool-hermetic. Remove line 224 ("pdm>=2.21.0") to keep only the uv entry.

Apply this diff:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
 ]
🧹 Nitpick comments (1)
pyproject.toml (1)

178-225: Eliminate duplication between llslibdev and llslibdev-hermetic groups; document synchronization requirement.

Lines 178–221 duplicate all 44 lines of the llslibdev group verbatim (lines 130–177), then append build tools in lines 222–225. This creates a maintenance burden: any dependency update to llslibdev must be manually mirrored to llslibdev-hermetic.

If uv's dependency group syntax supports group composition (e.g., include-group), refactor to reference the base group and only override/append differing entries. Otherwise, add a prominent comment warning maintainers to mirror llslibdev changes to llslibdev-hermetic.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fadad34 and 63b5f04.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • rpms.in.yaml
  • .tekton/lightspeed-stack-pull-request.yaml
  • rpms.lock.yaml
  • ubi.repo
  • requirements.hermetic.txt
  • Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

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: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
pyproject.toml (1)

90-98: Clarify intentionality of torch variant split across indices.

Past review (marked resolved in fadad34) recommended extending CPU variant to both llslibdev and llslibdev-hermetic. However, the current code specifies pytorch-cpu for llslibdev but pypi-default for llslibdev-hermetic, causing hermetic builds to resolve torch from the default GPU-first PyPI index—contradicting the stated intent.

Is this split intentional (perhaps hermetic builds require GPU torch despite non-hermetic using CPU)? If so, add an explanatory comment. If not, apply the suggested fix to use pytorch-cpu for both groups.

@raptorsun
Copy link
Contributor Author

/retest

3 similar comments
@raptorsun
Copy link
Contributor Author

/retest

@raptorsun
Copy link
Contributor Author

/retest

@raptorsun
Copy link
Contributor Author

/retest

@raptorsun
Copy link
Contributor Author

/retest

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

♻️ Duplicate comments (2)
pyproject.toml (2)

178-221: Acknowledge: duplication between llslibdev groups flagged in earlier review.

The complete duplication of the llslibdev group into llslibdev-hermetic (43 lines) was already flagged as a major refactor suggestion in previous reviews. This creates a maintenance burden where changes to llslibdev must be manually synchronized to llslibdev-hermetic.

The earlier review suggested either using group composition (if supported by uv) or adding prominent documentation about the need to keep both groups synchronized.


222-225: Remove pdm from tool-hermetic—it's unused and already the build backend.

The tool-hermetic group includes pdm>=2.21.0, but pdm is not used in hermetic builds. As noted in earlier reviews, pdm is already specified as the build backend (pdm-backend in [build-system] at line 246) and serves no purpose in the hermetic tooling group. The hermetic build workflow only requires uv.

Apply this diff to remove pdm:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
 ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63b5f04 and be34f46.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • rpms.lock.yaml
  • requirements.hermetic.txt
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

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: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: e2e_tests (ci)
🔇 Additional comments (6)
.tekton/lightspeed-stack-pull-request.yaml (1)

35-40: LGTM—hermetic build parameters correctly configured.

The new parameters (build-source-image: 'true', prefetch-input with rpm and pip types, and hermetic: 'true') are correctly configured for Konflux hermetic builds. The prefetch-input JSON structure properly specifies both RPM and Python dependency prefetching with allow_binary enabled for pip packages.

Note: The TODO comments at lines 30 and 34 regarding ARM64 support and on-push pipeline updates were already flagged in previous reviews.

pyproject.toml (2)

90-93: LGTM—pypi-default index correctly configured.

The explicit pypi-default index is properly configured to provide an alternative source for packages when not using the specialized pytorch-cpu index. The explicit = true flag ensures it's only used when explicitly referenced.


99-105: LGTM—conflicts declaration prevents mixed group usage.

The conflicts block correctly prevents simultaneous use of the llslibdev and llslibdev-hermetic groups, ensuring users explicitly choose between the standard and hermetic build paths.

rpms.in.yaml (1)

1-4: LGTM—RPM input configuration correctly structured.

The configuration properly declares the required packages (gcc, jq, patch), references the UBI repository file, and specifies both x86_64 and aarch64 architectures. This aligns with the hermetic build requirements and multi-platform support indicated in the Tekton pipeline configuration.

Note: The earlier review comment about lockfile completeness (gcc and patch missing from rpms.lock.yaml) was marked as addressed in commits d9da223 to c621aaa.

ubi.repo (1)

1-62: LGTM—UBI9 repository configuration correctly structured.

The repository configuration properly defines all three UBI9 repository categories (BaseOS, AppStream, CodeReady Builder) with their respective RPM, Debug, and Source variants. The configuration follows standard yum/dnf repository file format:

  • Correct baseurl patterns using $basearch variable for multi-architecture support
  • GPG verification enabled (gpgcheck = 1) with proper key path for all repositories
  • Main RPM repositories appropriately enabled (enabled = 1)
  • Debug and Source repositories appropriately disabled (enabled = 0)
  • URLs point to the public Red Hat UBI CDN

This configuration provides the reproducible package sources required for hermetic builds.

Containerfile (1)

28-33: No action required—requirements.txt exists and is correctly referenced.

The verification confirms that requirements.txt exists in the repository, and the Containerfile correctly references it on line 30 for the hermetic build path. The code is valid as written.

@raptorsun raptorsun force-pushed the hermetic branch 3 times, most recently from 79004b6 to a0b7bd9 Compare November 7, 2025 09:35
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

♻️ Duplicate comments (3)
pyproject.toml (3)

222-225: Remove pdm from tool-hermetic—unresolved from past review.

Despite past review comments claiming this was addressed (commit showing as resolved), the tool-hermetic group still includes pdm>=2.21.0. Past verification confirmed that pdm is not invoked during hermetic builds and serves only as the project's build backend. This contradicts the hermetic tooling purpose and should be removed.

Apply this diff to remove pdm:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
 ]

178-221: Eliminate 44-line duplication—hermetic group copies entire llslibdev.

Despite past review comments claiming this was addressed (commit be34f46), the llslibdev-hermetic group still duplicates all 44 lines of llslibdev verbatim. This creates significant maintenance burden: any change to llslibdev must be manually synchronized to llslibdev-hermetic. The comment at line 179 claims "the same as llslibdev, just using default index", but the actual package pins are identical—only the torch source routing differs (line 95-98).

Consider documenting the duplication risk prominently if tool-level group composition is not supported:

 llslibdev-hermetic = [
+    # WARNING: This group duplicates llslibdev verbatim. Any changes to llslibdev
+    # must be manually synchronized here. The torch GPU/CPU variant is controlled
+    # by [tool.uv.sources] torch configuration (lines 95-98), not by these pins.
     # the same as llslibdev, just using default index.
     "matplotlib>=3.10.0",

Alternatively, if UV supports group references or composition, refactor to reference llslibdev instead of duplicating it.


95-98: Torch variant mismatch persists—hermetic group resolves GPU variant from PyPI.

Despite past review comments claiming this was addressed (commit fadad34), the torch source configuration still specifies index = "pytorch-cpu" for llslibdev but index = "pypi-default" for llslibdev-hermetic. This means:

  • llslibdev: CPU-only torch variant (~200MB smaller)
  • llslibdev-hermetic: GPU torch variant from PyPI (larger, includes CUDA)

This inconsistency contradicts the comment at line 179 stating "the same as llslibdev" and will cause hermetic builds to pull a different, heavier torch package.

Apply this diff to use the CPU variant for both groups:

 torch = [
     { index = "pytorch-cpu", group = "llslibdev" },
-    { index = "pypi-default", group = "llslibdev-hermetic" }
+    { index = "pytorch-cpu", group = "llslibdev-hermetic" }
 ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79004b6 and a0b7bd9.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .tekton/lightspeed-stack-pull-request.yaml
  • rpms.in.yaml
  • Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

Files:

  • pyproject.toml
🧠 Learnings (1)
📚 Learning: 2025-09-18T16:46:33.353Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Use uv as the package manager; prefix commands with uv run

Applied to files:

  • requirements.hermetic.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_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (4)
pyproject.toml (2)

99-105: LGTM—conflict prevents accidental mixing of hermetic and non-hermetic groups.

The UV conflicts declaration correctly ensures that llslibdev and llslibdev-hermetic cannot be installed together, preventing unintended dependency resolution issues.


90-93: Based on my verification, I found that the pyproject.toml configuration uses two explicit index entries to support different build modes:

  1. pytorch-cpu index for standard builds (llslibdev group)
  2. pypi-default index for hermetic builds (llslibdev-hermetic group)

The torch package is explicitly pinned to each index per group. The configuration uses dependency groups with explicit conflicts to ensure builds use only one mode at a time. The hermetic build variant (llslibdev-hermetic) uses PyPI directly rather than pytorch.org/whl, likely because Konflux hermetic builds have limited network access to external registries.

Marking pypi-default as explicit = true actually reduces variability by restricting torch to a pinned source, which is appropriate for reproducible hermetic builds. This is the opposite of the concern stated in the review comment.


Configuration is correct for hermetic build support.

The pypi-default index is necessary and correctly configured. It explicitly pins torch to PyPI for the llslibdev-hermetic group, ensuring reproducibility in hermetic builds. Marking it as explicit = true restricts its use to only the pinned package (torch), reducing—not increasing—variability. The dual-index setup appropriately supports both standard (pytorch-cpu) and hermetic (pypi-default) build workflows with dependency group conflicts to prevent mixing them.

rpms.lock.yaml (1)

1-355: I need to search for scripts or tooling that generates the lock file.

Now let me search the codebase for generation scripts and build documentation:Based on my verification, the review comment's concerns are valid but the request is incomplete. Here are my findings:

Verified Facts:

  1. The rpms.lock.yaml file is auto-generated from rpms.in.yaml using the rpm-lockfile-prototype tool via: rpm-lockfile-prototype --outfile=rpms.lock.yaml rpms.in.yaml
  2. The rpm-lockfile-prototype tool uses live dnf metadata to resolve a given rpms.in.yaml file into an rpms.lock.yaml file in which every RPM is pinned to a repository and version
  3. The packages in the lock file (27+) correctly include gcc, jq, patch plus all their transitive dependencies
  4. Missing: The codebase has no documented process for regenerating the lockfile

Verify lockfile is auto-generated and matches rpms.in.yaml.

The rpms.lock.yaml file is correctly auto-generated from rpms.in.yaml. To generate the lockfile, the rpm-lockfile-prototype tool is run with: rpm-lockfile-prototype --outfile=shared-ingress/rpms.lock.yaml shared-ingress/rpms.in.yaml

However, the codebase lacks documentation on how developers should regenerate this lockfile when updating rpms.in.yaml. Add regeneration instructions to README or CONTRIBUTING.md, specifying:

  • The tool to use (rpm-lockfile-prototype)
  • Required dependencies (dnf, python3)
  • Command to run for regeneration
  • When regeneration is needed (whenever rpms.in.yaml changes)
ubi.repo (1)

1-62: Security configuration is correct; verification request is based on incorrect assumptions about build process.

The ubi.repo file is used by Cachi2 (Red Hat's hermetic build tool) for build-time RPM dependency resolution via rpms.in.yaml, not as runtime container configuration. The file is never copied into the final container image, as evidenced by the absence of any COPY/ADD commands for it in the Containerfile. The base UBI image already includes the standard Red Hat GPG key (RPM-GPG-KEY-redhat-release), and GPG key validation during builds is handled by the Konflux/Tekton CI/CD pipeline, not the container itself.

The original review comment's security assessment remains valid:

  • HTTPS URLs are used
  • GPG checking is enabled
  • Only main RPM repositories are enabled at runtime
  • Standard Red Hat GPG key location is referenced

No changes needed.

@@ -0,0 +1,2 @@
uv==0.8.15
pdm>=2.21.0
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove pdm from hermetic requirements—it's the build backend, not a hermetic tool.

Based on past verification, pdm is not invoked during hermetic builds and serves only as the project's build backend (pdm-backend in [build-system]). The hermetic build workflow uses uv. Including pdm>=2.21.0 here is inconsistent with the hermetic tooling purpose.

Apply this diff to remove pdm:

 uv==0.8.15
-pdm>=2.21.0

Based on learnings

📝 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.

Suggested change
pdm>=2.21.0
uv==0.8.15
🤖 Prompt for AI Agents
In requirements.hermetic.txt around line 2, the file incorrectly lists
"pdm>=2.21.0" which should be removed because pdm is the build backend, not a
hermetic runtime tool; edit the file to delete that line so the hermetic
requirements only contain tools actually used at runtime by the hermetic build
workflow (ensure no leftover whitespace or blank-line artifacts remain).

@raptorsun
Copy link
Contributor Author

/retest

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: 2

🧹 Nitpick comments (1)
rpms.lock.yaml (1)

1-355: RPM lockfile structure looks correct.

The lockfile format is valid and includes:

  • Proper metadata (lockfileVersion, lockfileVendor)
  • Complete package information for both aarch64 and x86_64
  • SHA256 checksums for integrity verification
  • Official Red Hat UBI repository URLs

This supports reproducible hermetic builds by pinning exact RPM versions. Ensure this file is regenerated when RPM dependencies change (via rpms.in.yaml updates).

Maintenance note: This lockfile should be version-controlled and regenerated using the appropriate tooling (e.g., cachi2) whenever rpms.in.yaml is updated to ensure consistency between the input specification and the locked versions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0b7bd9 and 8460b61.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • .tekton/lightspeed-stack-pull-request.yaml
  • rpms.in.yaml
  • ubi.repo
  • pyproject.toml
  • requirements.hermetic.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_tests (azure)
  • GitHub Check: e2e_tests (ci)

# (avoid accidental inclusion of local directories or env files or credentials)
COPY ${LSC_SOURCE_DIR}/src ./src
COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ./
COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ${LSC_SOURCE_DIR}/requirements.txt ./
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify requirements.txt usage in hermetic build path.

The requirements.txt file is now copied but doesn't appear to be used in either the hermetic or standard build paths (lines 29-33). Ensure this file serves a purpose in the hermetic build workflow, or consider if it should be removed to avoid confusion.

Run the following script to check if requirements.txt is referenced elsewhere:


🏁 Script executed:

#!/bin/bash
# Description: Verify requirements.txt usage across the codebase

echo "=== Searching for requirements.txt references ==="
rg -n "requirements\.txt" --type dockerfile --type yaml --type toml -C2

echo -e "\n=== Check if requirements.hermetic.txt exists ==="
fd -t f "requirements.*\.txt$"

Length of output: 178


I need to examine the Containerfile more closely and search for actual usage of requirements.txt in the build process.The web search gave general context about hermetic builds, but I need to examine the actual Containerfile. Let me search for the specific build logic in the codebase.


🏁 Script executed:

#!/bin/bash
# Search for requirements.txt usage in build commands and view Containerfile around lines 29-33
echo "=== View Containerfile lines 20-40 ==="
head -40 Containerfile | tail -20

echo -e "\n=== Search for pip or requirements usage in Containerfile ==="
rg -n "pip|requirements" Containerfile -C1

echo -e "\n=== Search for hermetic or RUN commands in Containerfile ==="
rg -n "hermetic|RUN" Containerfile -C1

Length of output: 344


I need to clone the repository to access the Containerfile. Let me retry:


🏁 Script executed:

#!/bin/bash
# View Containerfile and check requirements.txt usage
echo "=== View entire Containerfile ==="
cat Containerfile

echo -e "\n=== Search for requirements.txt usage ==="
rg -n "requirements\.txt" -C2

echo -e "\n=== Search for hermetic build logic ==="
rg -n "hermetic" -C3

Length of output: 43388


requirements.txt is copied but unused in the Containerfile build—remove it from line 25.

The hermetic build uses uv pip sync --group llslibdev-hermetic pyproject.toml, and the standard build uses uv sync --locked --no-dev --group llslibdev. Neither references requirements.txt. The file is generated as an artifact (per Makefile and README.md), but does not serve a purpose in the container build process. Copying it adds unnecessary bloat and creates confusion.

Remove ${LSC_SOURCE_DIR}/requirements.txt from the COPY statement on line 25.

🤖 Prompt for AI Agents
In Containerfile around line 25, the COPY statement currently includes
${LSC_SOURCE_DIR}/requirements.txt which is unused in the container build;
remove ${LSC_SOURCE_DIR}/requirements.txt from that COPY invocation so only
pyproject.toml, LICENSE, README.md, uv.lock (and any other needed files) are
copied, then rebuild to verify image size and that the hermetic and standard
build flows still succeed.

@raptorsun raptorsun force-pushed the hermetic branch 4 times, most recently from 5f4c673 to 90686ca Compare November 7, 2025 17:08
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: 0

♻️ Duplicate comments (3)
pyproject.toml (3)

95-98: Torch variant mismatch—llslibdev-hermetic should use pytorch-cpu, not pypi-default.

The torch configuration (lines 95–98) specifies pytorch-cpu index for llslibdev (CPU-only, smaller) but pypi-default for llslibdev-hermetic (GPU variant, much larger). This contradicts the intent that hermetic builds should match the dev environment. A past review flagged this same issue and marked it addressed, but the code still shows the inconsistency.

Apply this diff to align both groups on the CPU variant:

 torch = [
     { index = "pytorch-cpu", group = "llslibdev" },
-    { index = "pypi-default", group = "llslibdev-hermetic" }
+    { index = "pytorch-cpu", group = "llslibdev-hermetic" }
 ]

Verify: Was commit fadad34 actually merged to this branch, and if so, why does the torch variant still differ?


178-221: Eliminate duplication—llslibdev-hermetic copies entire llslibdev group.

Lines 178–221 duplicate all 44 lines of the llslibdev group verbatim, creating maintenance risk: future changes to llslibdev must be manually mirrored to avoid divergence. A past review flagged this same maintenance burden and marked it addressed, but the full duplication remains.

If your uv version supports group composition, refactor to reference the existing group:

llslibdev-hermetic = [
    { include-group = "llslibdev" },
    "torch==2.7.1",  # re-pin here if needed to override index scoping
]

Otherwise, add a prominent comment warning that changes to llslibdev must be manually synced to llslibdev-hermetic, and clarify that the torch GPU/CPU variant is determined by the index scoping (line 97), not by duplicate pins here.


222-226: Remove pdm and pdm-backend from tool-hermetic—not used in hermetic builds.

The tool-hermetic dependency group includes pdm>=2.21.0 and pdm-backend==2.4.5, but hermetic builds use only uv (line 223). Since pdm is already the project's build backend (specified in [build-system] line 248), it does not belong in this group. A past review flagged this and marked it addressed, but both entries remain.

Apply this diff to remove the unused build-system entries:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
-    "pdm-backend==2.4.5",
 ]
🧹 Nitpick comments (1)
.tekton/lightspeed-stack-pull-request.yaml (1)

30-30: TODO comments block merge—resolve or track as separate issues.

Lines 30 and 34 contain incomplete work markers that prevent merging:

  • Line 30: ARM64 multi-platform support not yet implemented
  • Line 34: On-push pipeline not synchronized with PR pipeline changes

Given that the PR is marked [WIP] and the author checklist is unchecked, these are documented known gaps. Before merge, either implement both features or open two follow-up issues with explicit references and remove/replace the TODO comments so the file is merge-ready.

I can help generate the follow-up issue templates or implement ARM64 support per the Konflux multi-platform documentation if you'd like.

Also applies to: 34-34

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f4c673 and 90686ca.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • rpms.lock.yaml
  • ubi.repo
  • requirements.hermetic.txt
  • Containerfile
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

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: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: e2e_tests (ci)
🔇 Additional comments (2)
.tekton/lightspeed-stack-pull-request.yaml (1)

35-40: Parameter additions are properly integrated.

The three new parameters (build-source-image, prefetch-input, hermetic) align with pipeline spec definitions and are correctly forwarded to dependent tasks (buildah-remote, source-build, etc.). The prefetch-input JSON structure matches the prefetch-dependencies task expectations.

rpms.in.yaml (1)

1-4: Structure and architecture alignment look correct.

The RPM configuration correctly declares all required packages (gcc, jq, patch), references the ubi.repo repository, and specifies both target architectures. This aligns with the Tekton prefetch-input configuration and the build-platforms specified in the PipelineRun.

@raptorsun raptorsun force-pushed the hermetic branch 4 times, most recently from d3c6138 to 9a447a7 Compare November 12, 2025 13:49
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: 3

♻️ Duplicate comments (2)
Containerfile (1)

28-28: Remove unused requirements.txt from COPY statement.

This issue was previously flagged: requirements.txt is copied but not used in either the hermetic or standard build paths. Both invoke uv sync with pyproject.toml. Remove ${LSC_SOURCE_DIR}/requirements.txt from the COPY statement.

pyproject.toml (1)

222-226: Remove pdm and pdm-backend from tool-hermetic group.

A previous review comment identified that pdm is not invoked during hermetic builds and requested its removal from this group. Verification confirmed no pdm usage in the Containerfile, pipeline, or build scripts. The hermetic build uses only uv. Since pdm's role is limited to being the build backend ([build-system]), it should not be in this dependency group.

Apply this diff:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
-    "pdm-backend==2.4.5",
 ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3c6138 and 9a447a7.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • .tekton/lightspeed-stack-pull-request.yaml
  • ubi.repo
  • rpms.in.yaml
  • requirements.hermetic.txt
  • rpms.lock.yaml
⏰ 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_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
pyproject.toml (1)

89-105: Incomplete hermetic torch configuration—intentional staging for WIP?

The torch source for llslibdev-hermetic (line 97), the pypi-default index (lines 90-93), and the conflicts block (lines 99-105) are all commented out. This is consistent with the llslibdev-hermetic dependency group also being commented out (lines 178-221), but it leaves hermetic build support incomplete. Given the PR is marked [WIP] and "Do Not Merge," this may be intentional staging.

If hermetic builds are intended to be testable in this PR, uncomment these blocks. Otherwise, document that hermetic configuration is staged but not yet active.

Comment on lines +22 to +23
# Install pdm and pdm-backend for hermetic builds
RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused pdm installation—hermetic builds use only uv.

This conditional installation of pdm and pdm-backend was previously flagged as unnecessary. Verification confirms that the hermetic build path (line 36) invokes only uv sync, and no pdm commands appear elsewhere in the Containerfile or pipeline configuration. Since pdm's sole role is as the build backend (specified in [build-system] of pyproject.toml), installing it here adds unnecessary dependencies and image bloat.

Remove these lines:

-# Install pdm and pdm-backend for hermetic builds
-RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi
-
📝 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.

Suggested change
# Install pdm and pdm-backend for hermetic builds
RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi
🤖 Prompt for AI Agents
In Containerfile around lines 22 to 23, remove the conditional RUN that installs
pdm and pdm-backend (the line starting with "RUN if [ -f /cachi2/cachi2.env ];
then pip3.12 install \"pdm>=2.21.0\" \"pdm-backend\"; fi") because hermetic
builds only use "uv sync" and no pdm commands are used elsewhere; delete that
RUN step to avoid unnecessary dependencies and image bloat.

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: 0

♻️ Duplicate comments (1)
pyproject.toml (1)

222-226: Remove pdm from tool-hermetic; it is unused in hermetic builds.

The tool-hermetic group includes "pdm>=2.21.0" (line 224), but verification from past review confirms pdm is not invoked during hermetic builds. No pdm references appear in the Containerfile, Tekton pipeline, or build scripts. Since pdm's sole role is as the project's build backend (specified in [build-system] line 247), it does not belong in the hermetic tooling group.

Additionally, the version constraint 2.21.0 is outdated; the latest stable release is 2.26.1 (October 2025).

Apply this diff to remove pdm and keep only uv for hermetic builds:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
     "pdm-backend==2.4.5",
 ]

Verify whether pdm-backend==2.4.5 is also needed in hermetic builds or should be removed as well.

🧹 Nitpick comments (1)
pyproject.toml (1)

178-221: Duplication of llslibdev group remains; consider refactoring for maintainability.

The llslibdev-hermetic group duplicates all 43 lines of llslibdev verbatim before adding hermetic tools. This creates a maintenance burden: any future change to llslibdev (dependencies, versions, comments) must be manually mirrored to llslibdev-hermetic to keep them in sync.

Past review suggested using group composition (e.g., include-group) if the tool supports it, or at minimum documenting the duplication risk with a prominent comment. The current implementation has neither.

If uv supports dependency group composition, refactor to:

llslibdev-hermetic = [
    { include-group = "llslibdev" },
    "uv==0.8.15",
]

If composition is not supported, add a comment warning that changes to llslibdev must be mirrored here to prevent divergence.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a447a7 and 4af3772.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (2 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • Containerfile
  • .tekton/lightspeed-stack-pull-request.yaml
  • ubi.repo
  • requirements.hermetic.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_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (3)
rpms.in.yaml (1)

1-4: ✅ RPM configuration is complete and consistent with lockfile.

All three declared packages (gcc, jq, patch) are now present in rpms.lock.yaml for both x86_64 and aarch64 architectures with resolved NEVRA entries. The reference to ./ubi.repo is valid, and the architecture list matches the lockfile structure.

rpms.lock.yaml (1)

1-355: ✅ Lockfile structure and content are valid.

The lockfile includes all declared packages with proper NEVRA entries, sha256 checksums, and architecture-specific URLs for both aarch64 and x86_64. Repository IDs (ubi-9-baseos-rpms, ubi-9-appstream-rpms) align with the ubi.repo configuration.

pyproject.toml (1)

95-98: Torch source still uses PyPI default for hermetic group (GPU variant mismatch).

Line 97 specifies index = "pypi-default" for llslibdev-hermetic, which resolves torch to the PyPI GPU variant. This contradicts the CPU-only variant used by llslibdev (line 96: pytorch-cpu), causing the hermetic build to pull a significantly larger, architecture-incompatible torch package.

Past review flagged this issue as "Addressed in commit f35507a" but the fix was not applied to the current code.

Apply this diff to align hermetic with non-hermetic torch resolution:

 torch = [
     { index = "pytorch-cpu", group = "llslibdev" },
-    { index = "pypi-default", group = "llslibdev-hermetic" }
+    { index = "pytorch-cpu", group = "llslibdev-hermetic" }
 ]

Signed-off-by: Haoyu Sun <[email protected]>
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

♻️ Duplicate comments (6)
pyproject.toml (3)

178-221: Duplication remains between llslibdev groups—consider composition.

The llslibdev-hermetic group duplicates 43 of 44 lines from llslibdev, differing only in torch's platform marker (line 212). Past review flagged this maintenance burden: changes to llslibdev must be manually mirrored. While marked as addressed in a previous commit, the duplication persists.

If UV supports group composition, consider:

llslibdev-hermetic = [
    { include-group = "llslibdev" },
    # Override torch with platform marker
    "torch==2.7.1; sys_platform == 'linux'",
]

Otherwise, add a prominent comment warning that llslibdev changes must be mirrored here.

Based on learnings


95-98: Torch variant mismatch persists—hermetic group uses GPU variant.

This issue remains from past review: llslibdev uses pytorch-cpu index (CPU-only), while llslibdev-hermetic uses pypi-default (GPU variant, significantly larger). This contradicts the stated intent that hermetic should mirror llslibdev except for build tools.

Apply this diff to use CPU variant for both:

 torch = [
     { index = "pytorch-cpu", group = "llslibdev" },
-    { index = "pypi-default", group = "llslibdev-hermetic" }
+    { index = "pytorch-cpu", group = "llslibdev-hermetic" }
 ]

Based on learnings


222-226: Remove pdm from tool-hermetic—not used in hermetic builds.

Past verification confirmed pdm is not invoked during hermetic builds. The hermetic workflow uses only uv. Since pdm's role is solely as the build backend (in [build-system]), it does not belong in the tool-hermetic dependency group.

Apply this diff:

 tool-hermetic = [
     "uv==0.8.15",
-    "pdm>=2.21.0",
-    "pdm-backend==2.4.5",
 ]

Based on learnings

Containerfile (2)

22-23: Remove unused pdm installation—hermetic builds use only uv.

This conditional pdm installation remains from a previous review. Verification shows hermetic builds invoke only uv sync (line 36), and no pdm commands are used elsewhere. Installing pdm here adds unnecessary dependencies and image bloat.

Apply this diff:

-# Install pdm and pdm-backend for hermetic builds
-RUN if [ -f /cachi2/cachi2.env ]; then pip3.12 install "pdm>=2.21.0" "pdm-backend"; fi
-

Based on learnings


28-28: Remove unused requirements.txt from COPY statement.

This file is copied but never used in the build. Past review confirmed neither the hermetic build (uv sync --group llslibdev-hermetic) nor the standard build (uv sync --group llslibdev) references requirements.txt. It adds unnecessary bloat.

Apply this diff:

-COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ${LSC_SOURCE_DIR}/requirements.txt ./
+COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ./

Based on learnings

requirements.hermetic.txt (1)

1-3: Remove pdm and pdm-backend—only uv is used in hermetic builds.

Past review confirmed pdm is not invoked during hermetic builds and serves only as the build backend. The hermetic workflow uses uv exclusively. Including these packages is inconsistent with hermetic tooling requirements.

Apply this diff:

 uv==0.8.15
-pdm>=2.21.0
-pdm-backend

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4af3772 and 110f717.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .tekton/lightspeed-stack-pull-request.yaml (1 hunks)
  • Containerfile (1 hunks)
  • pyproject.toml (3 hunks)
  • requirements.hermetic.txt (1 hunks)
  • rpms.in.yaml (1 hunks)
  • rpms.lock.yaml (1 hunks)
  • ubi.repo (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • rpms.in.yaml
  • .tekton/lightspeed-stack-pull-request.yaml
  • ubi.repo
⏰ 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). (3)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (4)
pyproject.toml (2)

90-93: LGTM: pypi-default index added for hermetic group.

The explicit pypi-default index enables hermetic builds to use PyPI packages while the standard build uses pytorch-cpu index. The explicit = true flag ensures packages must explicitly reference this index.


99-105: LGTM: UV conflicts prevent mixing hermetic and non-hermetic groups.

The conflict declaration correctly prevents simultaneous installation of llslibdev and llslibdev-hermetic groups, enforcing clean separation between standard and hermetic builds.

Containerfile (1)

31-39: Hermetic build conditional verified as correct.

The uv.lock lockfile exists and contains the llslibdev-hermetic group, confirming that past critical issues (missing hermetic group and outdated lockfile) have been resolved. The conditional logic correctly routes to the hermetic path with proper environment sourcing and Python version specification.

rpms.lock.yaml (1)

1-355: Lockfile is current and valid.

All three packages specified in rpms.in.yaml are present in rpms.lock.yaml for both architectures:

  • gcc (lines 18, 197)
  • jq (lines 137, 309)
  • patch (lines 74, 246)

Additional packages are transitive dependencies, which is correct for a hermetic build lockfile. All packages include required metadata (URL, checksum, size, EVR, source RPM). The lockfile is properly formatted and up-to-date with the input specification.

"RAG"
]
requires-python = ">=3.12,<3.14"
requires-python = ">=3.12,<3.13"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Python version restricted to 3.12.x—verify intentional for hermetic builds.

The Python requirement is narrowed from a broader range to >=3.12,<3.13, restricting to Python 3.12.x only. This is typical for hermetic builds requiring specific versions, but verify this doesn't break compatibility for users on different Python versions.

Verify this restriction is documented and intentional:


🏁 Script executed:

#!/bin/bash
# Description: Check if Python version restriction is documented

echo "=== Check README for Python version requirements ==="
rg -n "python.*3\.12|requires.*python" README.md -i -A2 -B2 || echo "Not documented in README"

echo -e "\n=== Check if CI uses Python 3.12 ==="
rg -n "python.*3\.12|python-version.*3\.12" .github/workflows/ .tekton/ -A1 -B1 || echo "Not found in CI config"

Length of output: 2664


Python version restriction breaks documented 3.13 support.

The change restricts to >=3.12,<3.13 (3.12.x only), but the README documents support for both Python 3.12 and 3.13, and CI tests run on both versions. Update to >=3.12,<3.14 to maintain advertised compatibility, or clarify documentation if 3.13 support is being dropped.

🤖 Prompt for AI Agents
In pyproject.toml around line 21, the Python version constraint was narrowed to
">=3.12,<3.13" which contradicts README and CI that claim support for Python
3.13; update the constraint to ">=3.12,<3.14" to allow both 3.12 and 3.13, or if
3.13 is intentionally dropped, update README and CI to reflect the stricter
requirement — make the change consistent across pyproject.toml, README, and CI
matrix.

@openshift-ci
Copy link

openshift-ci bot commented Nov 12, 2025

@raptorsun: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/images 110f717 link true /test images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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.

1 participant