RHIDP-14076: add top-level vector store config#2200
Conversation
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAdds dynamic FAISS and pgvector vector-store provider configuration, validates provider IDs and defaults, enriches unified Llama Stack synthesis with providers and storage settings, and documents the configuration with tests and examples. ChangesDynamic vector-store providers
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Configuration
participant synthesize_configuration
participant enrich_vector_store_providers
participant LlamaStackConfig
Configuration->>synthesize_configuration: vector_store_providers
synthesize_configuration->>enrich_vector_store_providers: provider entries
enrich_vector_store_providers->>LlamaStackConfig: enriched vector I/O and defaults
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
Split vector store provider enrichment to satisfy pylint, and refresh docs/devel_doc/openapi.json for the new config models. Signed-off-by: Jordan Dubrick <jdubrick@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user_doc/config.md`:
- Line 327: Update the FAISS provider ID description at
docs/user_doc/config.md:327-327 to document the shared validator’s
surrounding-whitespace normalization instead of claiming IDs are emitted as-is;
apply the same correction to the pgvector description at
docs/user_doc/config.md:528-528, preserving the existing format and byok_
constraints.
In `@docs/user_doc/rag_guide.md`:
- Around line 445-447: Update the unified synthesis documentation near
“vector_store_providers” to clarify that only inference models/providers require
separate configuration; distinguish embedding models as being registered
automatically by the enricher when needed.
In `@src/llama_stack_configuration.py`:
- Around line 632-651: Update _apply_vector_stores_defaults so
vector_stores["default_provider_id"] uses the same stripped string normalization
as the emitted vector_io provider_id, applying str(...).strip() to
designated["id"] before assignment. Leave the embedding-model handling
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a8441f0-94b4-43f6-8d0c-83d66b18dc20
📒 Files selected for processing (10)
docs/user_doc/byok_guide.mddocs/user_doc/config.mddocs/user_doc/rag_guide.mdsrc/llama_stack_configuration.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_vector_store_providers.pytests/unit/test_llama_stack_configuration.pytests/unit/test_llama_stack_synthesize.pytests/unit/utils/test_models_dumper.py
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
⚠️ CI failures not shown inline (2)
GitHub Actions: OpenAPI (Spectral) / spectral: RHIDP-14076: add top-level vector store config
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt: RHIDP-14076: add top-level vector store config
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.Never commit secrets or keys; use environment variables for sensitive data.
Files:
docs/user_doc/byok_guide.mdtests/unit/test_llama_stack_synthesize.pytests/unit/utils/test_models_dumper.pytests/unit/models/config/test_vector_store_providers.pydocs/user_doc/config.mdtests/unit/test_llama_stack_configuration.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pydocs/user_doc/rag_guide.mdsrc/llama_stack_configuration.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for unit tests, shared fixtures inconftest.py,pytest-mockfor mocks, andpytest.mark.asynciofor async tests.
Maintain at least 60% unit-test coverage; add unit tests for new functionality.
Files:
tests/unit/test_llama_stack_synthesize.pytests/unit/utils/test_models_dumper.pytests/unit/models/config/test_vector_store_providers.pytests/unit/test_llama_stack_configuration.pytests/unit/models/config/test_dump_configuration.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal Python modules, complete type annotations, descriptive snake_case names, and module docstrings.
Every module, class, and function must have a descriptive Google-style docstring; document parameters, returns, raises, and class attributes where applicable.
Uselogger = get_logger(__name__)fromlog.pyfor module logging and apply debug, info, warning, and error levels appropriately.
UseFinal[type]annotations for constants and define shared constants inconstants.pywith descriptive comments.
Useasync deffor I/O operations and external API calls, and avoid modifying mutable parameters in place; return newly constructed data instead.
Use modern union syntax such asstr | int,Optional[Type]for optional values, andtyping_extensions.Selffor model validators.
All configuration must use Pydantic models extendingConfigurationBase; configuration must reject unknown fields withextra="forbid".
Use@field_validatorand@model_validatorfor custom Pydantic validation; use specific types such asOptional[FilePath],PositiveInt, andSecretStr.
Classes require descriptive docstrings, PascalCase names, complete typed attributes, and specific types instead ofAny.
UseABCand@abstractmethodfor abstract interfaces; use standard suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Files:
src/models/config.pysrc/llama_stack_configuration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Pydantic configuration models extend
ConfigurationBase, while data models extendBaseModel; use Pydantic validators for validation.
Files:
src/models/config.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/unit/test_llama_stack_synthesize.pytests/unit/utils/test_models_dumper.pytests/unit/models/config/test_vector_store_providers.pytests/unit/test_llama_stack_configuration.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pysrc/llama_stack_configuration.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/config.pysrc/llama_stack_configuration.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/config.pysrc/llama_stack_configuration.py
🪛 ast-grep (0.44.1)
tests/unit/test_llama_stack_synthesize.py
[info] 646-646: Do not hardcode temporary file or directory names
Context: "/tmp/notebooks.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
tests/unit/models/config/test_vector_store_providers.py
[warning] 18-18: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(_BASE_CONFIG_PATH, "r", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[info] 79-79: Do not hardcode temporary file or directory names
Context: "/tmp/x.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 109-109: Do not hardcode temporary file or directory names
Context: "/tmp/x.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 123-123: Do not hardcode temporary file or directory names
Context: "/tmp/x.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 137-137: Do not hardcode temporary file or directory names
Context: "/tmp/x.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 154-154: Do not hardcode temporary file or directory names
Context: "/tmp/a.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 155-155: Do not hardcode temporary file or directory names
Context: "/tmp/b.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 165-165: Do not hardcode temporary file or directory names
Context: "/tmp/a.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 166-166: Do not hardcode temporary file or directory names
Context: "/tmp/b.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 185-185: Do not hardcode temporary file or directory names
Context: "/tmp/other.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
tests/unit/test_llama_stack_configuration.py
[info] 909-909: Do not hardcode temporary file or directory names
Context: "/tmp/kv.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 1133-1133: Do not hardcode temporary file or directory names
Context: "/tmp/n.db"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🔇 Additional comments (23)
docs/user_doc/byok_guide.md (1)
419-420: LGTM!docs/user_doc/config.md (3)
250-250: LGTM!
319-326: LGTM!Also applies to: 328-345
520-526: LGTM!Also applies to: 529-550
docs/user_doc/rag_guide.md (5)
19-19: LGTM!
38-42: LGTM!
139-196: LGTM!
397-444: LGTM!Also applies to: 446-447
465-469: LGTM!src/models/config.py (4)
2132-2189: LGTM!
2192-2282: LGTM!
2864-2872: LGTM!
2941-2970: LGTM!tests/unit/models/config/test_vector_store_providers.py (1)
1-191: LGTM!tests/unit/models/config/test_dump_configuration.py (1)
197-197: LGTM!Also applies to: 421-421, 781-781, 1040-1040, 1294-1294, 1513-1513, 1892-1892, 2117-2117, 2342-2342, 2574-2574
tests/unit/utils/test_models_dumper.py (1)
9168-9169: LGTM!Also applies to: 9241-9242
src/llama_stack_configuration.py (5)
84-88: LGTM!
362-398: LGTM!
510-630: LGTM!Also applies to: 654-723
1121-1123: LGTM!
646-651: 🗄️ Data Integrity & IntegrationNo action needed:
vector_stores.default_embedding_model.model_idis already the provider model id.registered_resources.models[*].model_idis the synthetic internal id; the raw embedding path belongs inprovider_model_id, so this default is not pointing at an unregistered model.> Likely an incorrect or invalid review comment.tests/unit/test_llama_stack_configuration.py (1)
3-4: LGTM!Also applies to: 21-21, 887-1139
tests/unit/test_llama_stack_synthesize.py (1)
633-658: LGTM!
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
|
cc @tisnik |
1 participant
Description
/v1/vector-storesAPI that is available for pre-creating destinationsExample for RHDH Notebooks:
Maps to the following in the synthesized
run.yaml:Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Documentation
Tests