Fixes llama-stack model-id #325
Conversation
WalkthroughThe model selection logic in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/endpoints/query.py(2 hunks)tests/unit/app/endpoints/test_query.py(4 hunks)
🔇 Additional comments (6)
src/app/endpoints/query.py (3)
219-219: LGTM: Composite model identifier creationThe creation of the composite model identifier follows the expected "provider_id/model_id" format, which aligns with llama-stack 0.2.16 requirements.
236-236: No downstream compatibility issues detected with composite model identifiersAll call sites—including
query_endpoint_handlerandstreaming_query_endpoint_handler—pass through the returnedmodel_idand tests forselect_model_and_provider_idalready assert theprovider/modelformat. No further changes are needed.
222-225: No changes needed: validation logic aligns with composite identifiers
I’ve confirmed that all existing tests forselect_model_and_provider_idmock models withidentifier="provider/model"(e.g."provider1/model1"), and the lookup insrc/app/endpoints/query.py(lines 222–225) along with metric labeling insrc/metrics/utils.pyall consistently treatidentifieras the compositeprovider_id/model_id. No update to the validation logic is required at this time.tests/unit/app/endpoints/test_query.py (3)
194-205: LGTM: Test data updated for composite identifiersThe mock model objects correctly use composite identifiers matching the "provider_id/model_id" format, which aligns with the updated function behavior.
215-216: LGTM: Assertion updated for composite identifierThe assertion correctly expects the composite identifier format "provider2/model2" returned by the updated function.
249-250: LGTM: Assertion updated for default configuration scenarioThe assertion correctly expects the composite identifier "default_provider/default_model" when using default configuration values.
Fixes llama-stack model-id introduced with version 0.2.16, This allows lightspeed-stack to preserve its interface and any configuration and clients to remain compatible. Signed-off-by: Djebran Lezzoum <ldjebran@gmail.com>
ldjebran
force-pushed
the
fixes-llama-stack-model-id
branch
from
8770e76 to
7d49636
Compare
3 participants
Description
Fixes llama-stack model-id introduced with version 0.2.16, This allows lightspeed-stack to preserve its interface and any configuration and clients to remain compatible.
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Bug Fixes
Tests