Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .tekton/lightspeed-stack-pull-request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ spec:
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.

- name: build-platforms
value:
- linux/x86_64
# todo: change on-push pipeline,too
- name: build-source-image
value: 'true'
- name: prefetch-input
value: '[{"type": "rpm", "path": "."}, {"type": "pip", "path": ".", "allow_binary": "true", "requirements_files": ["requirements.txt", "requirements.hermetic.txt"]}]'
- name: hermetic
value: 'true'
Comment on lines +35 to +40
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

🧩 Analysis chain

Verify requirements.hermetic.txt is included in Containerfile and properly used.

The prefetch-input parameter references both "requirements.txt" and "requirements.hermetic.txt" in the pip requirements_files array (line 38), but the Containerfile hermetic build path (line 33) only installs from requirements.txt. If requirements.hermetic.txt contains additional hermetic-specific dependencies, they may not be installed.

Run the following to check if requirements.hermetic.txt exists and is used:


🏁 Script executed:

#!/bin/bash
# Check if requirements.hermetic.txt exists and its usage
echo "=== Check if requirements.hermetic.txt exists ==="
if [ -f requirements.hermetic.txt ]; then
    echo "✓ requirements.hermetic.txt exists"
    echo "First 10 lines:"
    head -10 requirements.hermetic.txt
else
    echo "✗ requirements.hermetic.txt not found"
fi

echo -e "\n=== Check Containerfile for requirements.hermetic.txt usage ==="
rg -n "requirements\.hermetic\.txt" Containerfile

Length of output: 806


🏁 Script executed:

#!/bin/bash
# Examine Containerfile for pip/requirements usage and build logic
echo "=== Containerfile pip install and requirements handling ==="
rg -n -A3 -B3 "requirements\.txt|pip install" Containerfile

echo -e "\n=== Check Containerfile structure for hermetic build ==="
cat Containerfile

Length of output: 4420


Add requirements.hermetic.txt to Containerfile and use it in the hermetic build path.

Verification confirms the issue: requirements.hermetic.txt exists with pinned dependencies (autogenerated by uv), but the Containerfile never references it. Line 25 only copies requirements.txt, and line 33 (hermetic build path) installs from requirements.txt, not requirements.hermetic.txt. This defeats the purpose of prefetching hermetic-specific dependencies in the Tekton task.

Fix required:

  1. Add ${LSC_SOURCE_DIR}/requirements.hermetic.txt to the COPY statement (line 25)
  2. Update line 33 to install from requirements.hermetic.txt instead of requirements.txt for reproducible hermetic builds
🤖 Prompt for AI Agents
.tekton/lightspeed-stack-pull-request.yaml (and the referenced Containerfile
around lines 25 and 33): the Containerfile currently only copies and installs
requirements.txt which means the hermetic build path cannot use the pre-fetched
requirements.hermetic.txt; update the COPY at line ~25 to also copy
${LSC_SOURCE_DIR}/requirements.hermetic.txt into the image (same destination as
requirements.txt), and change the hermetic install step at line ~33 to
pip-install from requirements.hermetic.txt instead of requirements.txt so the
hermetic build uses the pinned, reproducible dependency list.

- name: dockerfile
value: Containerfile
pipelineSpec:
Expand Down
7 changes: 7 additions & 0 deletions .tekton/lightspeed-stack-push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,16 @@ spec:
value: '{{revision}}'
- name: output-image
value: quay.io/redhat-user-workloads/lightspeed-core-tenant/lightspeed-stack:{{revision}}
# todo: add arm64. refer to https://konflux.pages.redhat.com/docs/users/getting-started/multi-platform-builds.html#arm64-2
- name: build-platforms
value:
- linux/x86_64
- name: build-source-image
value: 'true'
- name: prefetch-input
value: '[{"type": "rpm", "path": "."}, {"type": "pip", "path": ".", "allow_binary": "true", "requirements_files": ["requirements.txt", "requirements.hermetic.txt"]}]'
- name: hermetic
value: 'true'
- name: dockerfile
value: Containerfile
pipelineSpec:
Expand Down
12 changes: 10 additions & 2 deletions Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,18 @@ RUN pip3.12 install "uv==0.8.15"
# Add explicit files and directories
# (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 | 🟠 Major

🧩 Analysis chain

Verify requirements.txt is properly generated and kept in sync.

The hermetic build path relies on requirements.txt (line 33), but a past review comment noted that the Makefile still uses an outdated pdm export command to generate this file, even though the repo has migrated to uv. This creates a risk of drift between the checked-in requirements.txt and the actual generation method.

Run the following to verify requirements.txt generation is using uv:


🏁 Script executed:

#!/bin/bash
# Check how requirements.txt is generated
echo "=== Check Makefile requirements.txt target ==="
rg -n "requirements\.txt" Makefile -A 2 -B 2

echo -e "\n=== Check header of requirements.txt for generation method ==="
head -5 requirements.txt

Length of output: 769


Update Makefile to use uv for requirements.txt generation.

The Makefile still uses pdm export to generate requirements.txt (line 44), but the actual file was generated using uv pip compile (as shown in its header). The Makefile has a "TODO uv migration" comment but hasn't been updated. This creates a critical maintenance issue: running make requirements.txt would regenerate the file using the outdated pdm method, causing potential drift from pyproject.toml. Additionally, the methods use different dependency groups (pdm: --no-extras --without evaluation vs. uv: --group llslibdev-hermetic), which could result in different locked dependencies.

Update line 44 in the Makefile to:

uv pip compile pyproject.toml -o requirements.txt --generate-hashes --group llslibdev-hermetic
🤖 Prompt for AI Agents
In Makefile around line 44, the rule still uses "pdm export" to regenerate
requirements.txt which is inconsistent with the actual file generated by uv;
replace the pdm export invocation with the uv command used to generate the file
so the Makefile runs: uv pip compile pyproject.toml -o requirements.txt
--generate-hashes --group llslibdev-hermetic, ensuring the TODO uv migration is
resolved and the requirements.txt will be recreated with the same tool and
dependency group as the committed lockfile.


# Bundle additional dependencies for library mode.
RUN uv sync --locked --no-dev --group llslibdev
# Source cachi2 environment for hermetic builds if available, otherwise use normal installation
# cachi2.env has these env vars:
# PIP_FIND_LINKS=/cachi2/output/deps/pip
# PIP_NO_INDEX=true
RUN if [ -f /cachi2/cachi2.env ]; then \
. /cachi2/cachi2.env && uv venv --seed --no-index --find-links ${PIP_FIND_LINKS} && . .venv/bin/activate && pip install --no-index --find-links ${PIP_FIND_LINKS} -r requirements.txt; \
else \
uv sync --locked --no-dev --group llslibdev; \
fi

# Explicitly remove some packages to mitigate some CVEs
# - GHSA-wj6h-64fc-37mp: python-ecdsa package won't fix it upstream.
Expand Down
61 changes: 60 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,23 @@ Issues = "https://github.com/lightspeed-core/lightspeed-stack/issues"
name = "pytorch-cpu"
url = "https://download.pytorch.org/whl/cpu"
explicit = true

[[tool.uv.index]]
name = "pypi-default"
url = "https://pypi.org/simple"
explicit = true
[tool.uv.sources]
torch = [{ index = "pytorch-cpu" }]
torch = [
{ index = "pytorch-cpu", group = "llslibdev" },
{ index = "pypi-default", group = "llslibdev-hermetic" }
]
[tool.uv]
conflicts = [
[
{ group = "llslibdev" },
{ group = "llslibdev-hermetic" },
],
]

[dependency-groups]
dev = [
Expand Down Expand Up @@ -159,6 +174,50 @@ llslibdev = [
"blobfile>=3.0.0",
"psutil>=7.0.0",
]
llslibdev-hermetic = [
# the same as llslibdev, just using default index.
"matplotlib>=3.10.0",
"pillow>=11.1.0",
"pandas>=2.2.3",
"scikit-learn>=1.5.2",
"psycopg2-binary>=2.9.10",
# API eval: inline::meta-reference
"tree_sitter>=0.24.0",
"pythainlp>=3.0.10",
"langdetect>=1.0.9",
"emoji>=2.1.0",
"nltk>=3.8.1",
# API inference: remote::gemini
"litellm>=1.75.5.post1",
# API inference: inline::sentence-transformers
"sentence-transformers>=5.0.0",
# API vector_io: inline::faiss
"faiss-cpu>=1.11.0",
# API scoring: inline::basic
"requests>=2.32.4",
# API datasetio: inline::localfs
"aiosqlite>=0.21.0",
# API datasetio: remote::huggingface
"datasets>=3.6.0",
# API telemetry: inline::meta-reference
"opentelemetry-sdk>=1.34.1",
"opentelemetry-exporter-otlp>=1.34.1",
# API tool_runtime: inline::rag-runtime
"transformers>=4.34.0",
"numpy==2.2.6",
# API tool_runtime: remote::model-context-protocol
"mcp>=1.9.4",
# API post_training: inline::huggingface
"torch==2.7.1; sys_platform == 'linux'",
"trl>=0.18.2",
"peft>=0.15.2",
# Other
"autoevals>=0.0.129",
"fire>=0.7.0",
"opentelemetry-instrumentation>=0.55b0",
"blobfile>=3.0.0",
"psutil>=7.0.0",
]
Comment on lines +177 to +220
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

Inconsistent torch platform marker—creates behavioral divergence.

The llslibdev-hermetic group at line 211 specifies torch==2.7.1; sys_platform == 'linux', but the regular llslibdev group at line 167 has no platform marker. This means:

  • llslibdev: torch installs on all platforms
  • llslibdev-hermetic: torch installs only on Linux

This contradicts the comment at line 178 stating the groups are "the same". If the Linux restriction is intentional for hermetic builds, document why. Otherwise, remove the platform marker for consistency.

Apply this diff if the platform restriction is unintentional:

-   "torch==2.7.1; sys_platform == 'linux'",
+   "torch==2.7.1",
🤖 Prompt for AI Agents
In pyproject.toml around lines 177 to 220 the llslibdev-hermetic extras list
contains a platform marker on the torch entry ("torch==2.7.1; sys_platform ==
'linux'") while the llslibdev group (around line 167) has torch with no marker,
causing inconsistent behavior; if the Linux-only restriction is unintentional,
remove the platform marker so both groups specify torch identically (or if
intentional, update the comment to document why hermetic excludes non-Linux),
ensuring both extras are kept semantically the same.


build = [
"build>=1.2.2.post1",
Expand Down
2 changes: 2 additions & 0 deletions requirements.hermetic.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
uv==0.8.15
pip==24.2
27 changes: 27 additions & 0 deletions requirements.hermetic.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# This file was autogenerated by uv via the following command:
# uv pip compile requirements.hermetic.in -o requirements.hermetic.txt --generate-hashes
pip==24.2 \
--hash=sha256:2cd581cf58ab7fcfca4ce8efa6dcacd0de5bf8d0a3eb9ec927e07405f4d9e2a2 \
--hash=sha256:5b5e490b5e9cb275c879595064adce9ebd31b854e3e803740b72f9ccf34a45b8
# via -r requirements.hermetic.in
uv==0.8.15 \
--hash=sha256:069eed78b79d1e88bced23e3d4303348edb0a0209e7cae0f20024c42430bf50f \
--hash=sha256:07765f99fd5fd3b257d7e210e8d0844c0a8fd111612e31fcca66a85656cc728e \
--hash=sha256:226360003e71084e0a73cbec72170e88634b045e95529654d067ea3741bba242 \
--hash=sha256:3330bb4f206a6180679a75a8b2e77ff0f933fcb06c028b6f4da877b10a5e4f95 \
--hash=sha256:333a93bb6af64f3b95ee99e82b4ea227e2af6362c45f91c89a24e2bfefb628f9 \
--hash=sha256:3780d2f3951d83e55812fdeb7eee233787b70c774497dbfc55b0fdf6063aa345 \
--hash=sha256:3ec78a54a8eb0bbb9a9c653982390af84673657c8a48a0be6cdcb81d7d3e95c3 \
--hash=sha256:56f2451c9193ee1754ce1d8390ded68e9cb8dee0aaf7e2f38a9bd04d99be1be7 \
--hash=sha256:6aa824ab933dfafe11efe32e6541c6bcd65ecaa927e8e834ea6b14d3821020f6 \
--hash=sha256:7d5b19ac2bdda3d1456b5d6013af50b443ffb0e40c66d42874f71190a5364711 \
--hash=sha256:89c7c10089e07d944c72d388fd88666c650dec2f8c79ca541e365f32843882c6 \
--hash=sha256:8ea57b78be9f0911a2a50b6814d15aec7d1f8aa6517059dc8250b1414156f93a \
--hash=sha256:9488260536b35b94a79962fea76837f279c0cd0ae5021c761e66b311f47ffa70 \
--hash=sha256:a022a752d20da80d2a49fc0721522a81e3a32efe539152d756d84ebdba29dbc3 \
--hash=sha256:a395fa1fc8948eacdd18e4592ed489fad13558b13fea6b3544cb16e5006c5b02 \
--hash=sha256:b0461bb1ad616c8bcb59c9b39ae9363245ca33815ebb1d11130385236eca21b9 \
--hash=sha256:c4868e6a4e1a8c777a5ba3cff452c405837318fb0b272ff203bfda0e1b8fc54d \
--hash=sha256:de9896ad4fa724ab317a8048f4891b9b23df1403b3724e96606f3be2dbbbf009 \
--hash=sha256:f02e6b8be08b840f86b8d5997b658b657acdda95bc216ecf62fce6c71414bdc7
# via -r requirements.hermetic.in
Loading
Loading