Skip to content

Conversation

@Elbehery
Copy link
Collaborator

@Elbehery Elbehery commented Sep 30, 2025

What does this PR do?

This PR bumps LLS version to upstream 0.2.23 tag.

https://github.com/llamastack/llama-stack/releases/tag/v0.2.23

Closes: https://issues.redhat.com/browse/RHAIENG-1311

Summary by CodeRabbit

  • Chores

    • Bumped Llama Stack and multiple provider dependency versions, deduplicated and reordered dependency installation steps, and removed exact version pins for some provider modules.
  • Documentation

    • Updated distribution README and build metadata to reflect the new provider and stack versions.
  • Notes

    • No runtime behavior or public API changes; build/configuration updates only.

@coderabbitai
Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Bumped llama-stack from 0.2.22 → 0.2.23 across pre-commit and distribution files; updated several provider module versions; added/adjusted pip packages in container files; introduced package deduplication/sorting in the build script; relaxed exact pins in distribution/run.yaml. No control-flow changes.

Changes

Cohort / File(s) Summary of Changes
Pre-commit hook
\.pre-commit-config.yaml
Bumped local pre-commit dependency llama-stack from 0.2.220.2.23.
Container images / templates
distribution/Containerfile, distribution/Containerfile.in
Added/updated pip packages (e.g., datasets>=4.0.0, mcp>=1.8.1, pymilvus[milvus-lite]>=2.4.10); removed duplicate package entries; reordered torch install; set llama-stack to 0.2.23 and used --no-cache. No entrypoint/runtime logic changes.
Build script
distribution/build.py
Bumped BASE_REQUIREMENTS llama-stack to 0.2.23; added package deduplication/sorting: packages = sorted(set(packages)) before constructing pip command.
Distribution specs
distribution/build.yaml, distribution/run.yaml
Updated provider module versions: llama_stack_provider_lmeval 0.2.4 → 0.3.0; llama_stack_provider_ragas (and ragas[remote]) 0.3.0 → 0.3.1. In run.yaml removed exact version pins for trustyai_fms and trustyai_lmeval (converted to unpinned references).
Documentation
distribution/README.md
Updated referenced Llama Stack and provider versions to reflect the bumps and adjusted related table entries/notes accordingly.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer / CI
    participant BuildPy as distribution/build.py
    participant Container as distribution/Containerfile
    participant Pip as pip

    rect #F0F9E8
    Dev->>BuildPy: request dependency list
    BuildPy->>BuildPy: collect packages\npackages = sorted(set(packages))
    BuildPy-->>Dev: deduped/sorted package list
    end

    rect #FFF7E6
    Dev->>Container: build image
    Container->>Pip: pip install (updated packages,\nllama-stack==0.2.23, --no-cache)
    Pip-->>Container: install results
    Container-->>Dev: image produced (no runtime control changes)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I twitch my whiskers for a tidy bump,
Versions hopped up, duplicates jumped;
Packages sorted in a neat little row,
Builds prance lightly where changes go —
A rabbit cheers the pipeline's hum! 🥕

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 succinctly describes the primary change in the PR, namely bumping the llama-stack dependency to version 0.2.23. It follows a conventional commit style with a clear “feat:” prefix, and it is concise and specific enough that a teammate can understand the nature of the update at a glance. There is no extraneous information or ambiguity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5b62b5 and a21e7f3.

📒 Files selected for processing (4)
  • distribution/Containerfile (2 hunks)
  • distribution/README.md (2 hunks)
  • distribution/build.yaml (1 hunks)
  • distribution/run.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • distribution/run.yaml
  • distribution/README.md
  • distribution/build.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
📚 Learning: 2025-09-15T14:25:54.837Z
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.

Applied to files:

  • distribution/Containerfile
⏰ 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: build-test-push (linux/amd64)
  • GitHub Check: Summary
🔇 Additional comments (4)
distribution/Containerfile (4)

9-11: Good consolidation of package dependencies.

These packages are now properly deduplicated and consolidated in the main pip install block, which improves maintainability and reduces the risk of version conflicts.


43-47: Provider version bumps align with llama-stack 0.2.23.

The updated provider versions (lmeval 0.3.0, ragas 0.3.1) are compatible with llama-stack 0.2.23 as validated through the related PRs (#63, #65) and passing CI checks.


50-50: Torch installation reordering already reviewed.

This change was already discussed and approved in previous review comments. The argument order doesn't affect pip's dependency resolution.


52-52: Approve llama-stack bump to 0.2.23
This upgrade includes the moderate RCE fix introduced in 0.2.20 (advisory published 2025-09-24), and CI is green.


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

@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)
distribution/build.py (1)

91-97: Fix pymilvus replacement logic to prevent duplicate extras.

The string replacement logic will match "pymilvus" even within "pymilvus[milvus-lite]", causing duplicate extras like pymilvus[milvus-lite][milvus-lite]>=2.4.10 (visible in distribution/Containerfile line 31). This invalid syntax may cause pip installation failures.

Apply this diff to fix the logic:

                    # Modify pymilvus package to include milvus-lite extra
                    packages = [
-                        package.replace("pymilvus", "pymilvus[milvus-lite]")
-                        if "pymilvus" in package
+                        package.replace("pymilvus>=", "pymilvus[milvus-lite]>=")
+                        if package.startswith("pymilvus>=")
                        else package
                        for package in packages
                    ]

Alternatively, use a more robust approach with regex:

import re

# Modify pymilvus package to include milvus-lite extra
packages = [
    re.sub(r'^pymilvus(?!\[)', 'pymilvus[milvus-lite]', package)
    if package.startswith("pymilvus")
    else package
    for package in packages
]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b2d68f and cab3105.

📒 Files selected for processing (4)
  • .pre-commit-config.yaml (1 hunks)
  • distribution/Containerfile (2 hunks)
  • distribution/Containerfile.in (1 hunks)
  • distribution/build.py (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
📚 Learning: 2025-09-15T14:25:54.837Z
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.

Applied to files:

  • distribution/Containerfile.in
  • distribution/Containerfile
⏰ 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: Summary
🔇 Additional comments (4)
distribution/build.py (1)

16-16: LGTM! Version bump is consistent with PR objective.

The llama-stack version bump to 0.2.23 aligns with the PR's stated goal.

distribution/Containerfile.in (1)

6-6: LGTM! Template version bump is correct.

The llama-stack version bump to 0.2.23 in the Containerfile template is consistent with the PR objective.

.pre-commit-config.yaml (1)

59-59: LGTM! Pre-commit dependency version is consistent.

The llama-stack version bump to 0.2.23 in the pre-commit hook dependency matches the version in distribution/build.py BASE_REQUIREMENTS.

distribution/Containerfile (1)

49-49: LGTM! Version bump is correct.

The llama-stack version bump to 0.2.23 is consistent with the PR objective.

Elbehery added a commit that referenced this pull request Sep 30, 2025
# What does this PR do?
<!-- Provide a short summary of what this PR does and why. Link to
relevant issues if applicable. -->

This PR ensures there is only one pymilvus dependnecy in req.txt.

During bumping LLS version in
#54, the
CI failed upon illegal `pymilvus` dependency in `req.txt`. See CI
[run](https://github.com/opendatahub-io/llama-stack-distribution/actions/runs/18126623537/job/51583337085).

<!-- If resolving an issue, uncomment and update the line below -->
<!-- Closes #[issue-number] -->
Closes https://issues.redhat.com/browse/RHAIENG-1312.
## Test Plan
<!-- Describe the tests you ran to verify your changes with result
summaries. *Provide clear instructions so the plan can be easily
re-executed.* -->


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Corrected dependency handling to only add the milvus-lite extra to
pymilvus when it’s not already specified, preventing duplicate or
malformed package specifications.
* **Chores**
* Refined build-time package transformation for more reliable
distribution behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from cab3105 to a0c8dc5 Compare September 30, 2025 14:21
@Elbehery
Copy link
Collaborator Author

rebased 👍🏽

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from a0c8dc5 to 9876ddd Compare September 30, 2025 14:22
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 9876ddd to 6ab2324 Compare September 30, 2025 14:36
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch 3 times, most recently from 5a812fe to cd441f2 Compare September 30, 2025 15:03
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch 2 times, most recently from 73a0df0 to 0eaa51a Compare September 30, 2025 21:22
@Elbehery
Copy link
Collaborator Author

Elbehery commented Oct 1, 2025

the failure is due to older version of trustyai-fms that does not support providers

A new version of trustyai-fms has been released yesterday https://test.pypi.org/project/llama-stack-provider-trustyai-fms/

bumping down and mid stream ...

@Elbehery
Copy link
Collaborator Author

Elbehery commented Oct 1, 2025

raised #56 to bump trustyai-fms version.

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 0eaa51a to 5fa4aef Compare October 1, 2025 12:06
@Elbehery
Copy link
Collaborator Author

Elbehery commented Oct 1, 2025

rebased #56 👍🏽

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 5fa4aef to 6f68f76 Compare October 1, 2025 12:12
@mergify mergify bot added the needs-rebase label Oct 7, 2025
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 70e6584 to 1a56a71 Compare October 7, 2025 16:28
@mergify mergify bot removed the needs-rebase label Oct 7, 2025
Copy link

@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)
distribution/Containerfile (1)

1-2: Regenerate this auto-generated file to resolve pre-commit failures.

This file is auto-generated by distribution/build.py (as noted in the header). The pre-commit hook failure indicates the committed version doesn't match what the generator produces. After fixing the generator or configuration in build.yaml, regenerate this file to ensure consistency.

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 70e6584 and 1a56a71.

📒 Files selected for processing (7)
  • .pre-commit-config.yaml (1 hunks)
  • distribution/Containerfile (2 hunks)
  • distribution/Containerfile.in (1 hunks)
  • distribution/README.md (1 hunks)
  • distribution/build.py (2 hunks)
  • distribution/build.yaml (1 hunks)
  • distribution/run.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • distribution/build.yaml
  • distribution/build.py
  • distribution/README.md
  • .pre-commit-config.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
📚 Learning: 2025-09-15T14:25:54.837Z
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.

Applied to files:

  • distribution/Containerfile.in
  • distribution/Containerfile
🪛 GitHub Actions: Pre-commit
distribution/Containerfile.in

[error] 1-1: Pre-commit hook 'pkg-gen' reported distribution build changes: files were modified by this hook, and the hook ultimately failed. Exit code 1.

distribution/Containerfile

[error] 1-1: Pre-commit hook 'pkg-gen' reported distribution build changes: files were modified by this hook, and the hook ultimately failed. Exit code 1.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build-test-push (linux/amd64)
  • GitHub Check: Summary
🔇 Additional comments (2)
distribution/run.yaml (1)

107-107: LGTM! Version bump aligns with llama-stack 0.2.23.

The llama_stack_provider_lmeval upgrade to 0.3.0 is consistent with the llama-stack 0.2.23 upgrade and reflects the hard dependency noted in the PR comments.

distribution/Containerfile (1)

9-11: Version bumps and dependency consolidation look correct.

The changes consolidate dependencies into the initial pip install block and update:

  • llama_stack_provider_lmeval: 0.2.4 → 0.3.0
  • llama-stack: 0.2.22 → 0.2.23

These changes align with the PR objectives and are consistent with the dependency deduplication logic added to distribution/build.py. Once the pre-commit hook issue is resolved by regenerating the file, these changes should be correct.

Based on learnings

Also applies to: 43-43, 50-50

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 1a56a71 to 2d20d51 Compare October 7, 2025 16:32
@Elbehery
Copy link
Collaborator Author

Elbehery commented Oct 7, 2025

I think the CI failure is due to RAGAS provider version ..

I have bumped it in #65

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 2d20d51 to c1ca410 Compare October 7, 2025 18:17
Copy link

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a56a71 and c1ca410.

📒 Files selected for processing (7)
  • .pre-commit-config.yaml (1 hunks)
  • distribution/Containerfile (2 hunks)
  • distribution/Containerfile.in (1 hunks)
  • distribution/README.md (2 hunks)
  • distribution/build.py (2 hunks)
  • distribution/build.yaml (1 hunks)
  • distribution/run.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • distribution/build.py
  • distribution/Containerfile.in
  • distribution/build.yaml
  • distribution/Containerfile
🧰 Additional context used
🪛 GitHub Actions: Pre-commit
distribution/run.yaml

[error] 104-105: pre-commit check-yaml failed: could not find expected ':' while scanning a simple key in distribution/run.yaml (lines 104-105).


[error] 104-105: pre-commit Distribution Documentation hook (doc-gen) failed due to YAML parsing error in distribution/run.yaml (lines 104-105).

🪛 YAMLlint (1.37.1)
distribution/run.yaml

[error] 105-105: syntax error: could not find expected ':'

(syntax)

⏰ 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: build-test-push (linux/amd64)
  • GitHub Check: Summary

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from c1ca410 to 0d4dec1 Compare October 7, 2025 18:22
@nathan-weinberg nathan-weinberg requested a review from leseb October 7, 2025 18:37
@nathan-weinberg
Copy link
Collaborator

Checks are passing so other than my comment we are looking good!

Copy link
Collaborator

@cdoern cdoern left a comment

Choose a reason for hiding this comment

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

+1 to Nathans comment, but lgtm otherwise.

@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 0d4dec1 to 07ee03f Compare October 7, 2025 20:56
@nathan-weinberg nathan-weinberg requested a review from cdoern October 7, 2025 20:59
@nathan-weinberg nathan-weinberg removed the do-not-merge Apply to PRs that should not be merged (yet) label Oct 8, 2025
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from 07ee03f to e5b62b5 Compare October 8, 2025 08:38
@Elbehery Elbehery force-pushed the 20250929_bump_lls_0.2.23 branch from e5b62b5 to a21e7f3 Compare October 8, 2025 11:59
@skamenan7
Copy link
Collaborator

LGTM, thanks.

@Elbehery Elbehery merged commit b2ab458 into opendatahub-io:main Oct 8, 2025
6 checks passed
@Elbehery Elbehery deleted the 20250929_bump_lls_0.2.23 branch October 8, 2025 12:34
@nathan-weinberg
Copy link
Collaborator

@Elbehery as a reminder, you should be using the bot to merge, not merging things yourself

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.

5 participants