LCORE-1427: Normalize Tool and Inline RAG default behavior#2146
LCORE-1427: Normalize Tool and Inline RAG default behavior#2146Jazzcort wants to merge 2 commits into
Conversation
…nfigured Previously, when neither rag.tool nor rag.inline were configured, prepare_tools would fetch and register all vector stores from llama-stack as a backward-compatibility fallback. This implicit behavior is removed — tool RAG now requires explicit configuration via rag.tool. Updated docs, config model, example YAML, and tests to reflect the new behavior.
WalkthroughRAG documentation now states that inline and Tool RAG are disabled unless explicitly configured. Tool preparation no longer implicitly registers all vector stores, while direct vector-store listing remains supported and tests reflect the updated behavior. ChangesRAG strategy behavior
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/utils/test_responses.py (1)
3062-3065: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename test to reflect the updated function signature.
Since
get_vector_store_idsno longer accepts avector_store_idsparameter, the "when_no_ids_provided" suffix is obsolete. Consider renaming the test for clarity.♻️ Proposed fix
- async def test_fetches_all_when_no_ids_provided( + async def test_fetches_all_vector_stores( self, mocker: MockerFixture ) -> None: - """Test that all vector stores are fetched when no IDs provided.""" + """Test that all available vector stores are fetched."""🤖 Prompt for 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. In `@tests/unit/utils/test_responses.py` around lines 3062 - 3065, Rename the test method test_fetches_all_when_no_ids_provided to reflect that get_vector_store_ids no longer accepts a vector_store_ids argument, removing the obsolete “when_no_ids_provided” wording while preserving the test behavior.
🤖 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.
Outside diff comments:
In `@tests/unit/utils/test_responses.py`:
- Around line 3062-3065: Rename the test method
test_fetches_all_when_no_ids_provided to reflect that get_vector_store_ids no
longer accepts a vector_store_ids argument, removing the obsolete
“when_no_ids_provided” wording while preserving the test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8e521c5-292b-4e00-a6df-6ba018037946
📒 Files selected for processing (8)
docs/byok_guide.mddocs/config.mddocs/models/responses_succ.mddocs/openapi.mdexamples/lightspeed-stack-byok-okp-rag.yamlsrc/models/config.pysrc/utils/responses.pytests/unit/utils/test_responses.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: Pylinter
- GitHub Check: build-pr
- GitHub Check: mypy
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
⚠️ CI failures not shown inline (3)
GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt: LCORE-1427: Normalize Tool and Inline RAG default behavior
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/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json"�[0m
GitHub Actions: Pydocstyle / 0_pydocstyle.txt: LCORE-1427: Normalize Tool and Inline RAG default behavior
Conclusion: failure
##[group]Run uv tool run pydocstyle -v src tests
�[36;1muv tool run pydocstyle -v src tests�[0m
shell: /usr/bin/bash -e {0}
env:
UV_PYTHON: 3.12
VIRTUAL_ENV: /home/runner/work/lightspeed-stack/lightspeed-stack/.venv
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
Installed 2 packages in 3ms
Checking file src/configuration.py.
Checking file src/__init__.py.
Checking file src/version.py.
Checking file src/sentry.py.
Checking file src/lightspeed_stack.py.
Checking file src/client.py.
Checking file src/constants.py.
Checking file src/log.py.
Checking file src/llama_stack_configuration.py.
Checking file src/quota/quota_exceed_error.py.
Checking file src/quota/sql.py.
Checking file src/quota/__init__.py.
Checking file src/quota/user_quota_limiter.py.
Checking file src/quota/connect_pg.py.
Checking file src/quota/revokable_quota_limiter.py.
Checking file src/quota/quota_limiter.py.
Checking file src/quota/quota_limiter_factory.py.
Checking file src/quota/connect_sqlite.py.
Checking file src/quota/token_usage_history.py.
Checking file src/quota/cluster_quota_limiter.py.
Checking file src/utils/vector_search.py.
Checking file src/utils/stream_interrupts.py.
Checking file src/utils/prompts.py.
Checking file src/utils/__init__.py.
Checking file src/utils/mcp_oauth_probe.py.
Checking file src/utils/checks.py.
Checking file src/utils/config_dumper.py.
Checking file src/utils/openapi_schema_dumper.py.
Checking file src/utils/quota_utils.py.
Checking file src/utils/endpoints.py.
Checking file src/utils/responses.py.
Checking file src/utils/rh_identity.py.
Checking file src/utils/reranker.py.
Checking file src/utils/token_counter.py.
Checking file src/utils/pydantic_ai_helpers.py.
Checking file src/utils/mcp_auth_headers.py.
Checking file src/utils/degraded_mode.py.
Checking file src/utils/json_schema_updater.py.
Checking file src/utils/llama_stack_version.py.
Checking file src/utils/mcp_headers.py.
Checkin...
GitHub Actions: OpenAPI (Spectral) / spectral: LCORE-1427: Normalize Tool and Inline RAG default behavior
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/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json"�[0m
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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.
Files:
examples/lightspeed-stack-byok-okp-rag.yamldocs/byok_guide.mddocs/models/responses_succ.mddocs/config.mddocs/openapi.mdsrc/utils/responses.pysrc/models/config.pytests/unit/utils/test_responses.py
**/*.{py,toml,yaml,yml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run and satisfy the project's configured quality checks, including Black, Pylint, Pyright, Ruff, pydocstyle, type checks, and Bandit security scanning.
Files:
examples/lightspeed-stack-byok-okp-rag.yamlsrc/utils/responses.pysrc/models/config.pytests/unit/utils/test_responses.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules and checkpyproject.tomlfor existing dependencies and versions before adding dependencies.
All Python modules must begin with descriptive docstrings; package__init__.pyfiles must contain brief package descriptions.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
UseFinal[type]for constants and checkconstants.pyfor shared constants before defining new ones.
Define type aliases at module level for clarity.
All functions must have Google-style docstrings and complete parameter and return type annotations.
Usetyping_extensions.Selffor model validators, modern union syntax such asstr | int, andOptional[Type]for optional values.
Use descriptive, action-oriented snake_case names for functions, such asget_,validate_, andcheck_.
Do not modify parameter objects in place when producing results; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack integrations.
Use standard logger levels appropriately:debugfor diagnostics,infofor normal execution,warningfor unexpected or potentially problematic conditions, anderrorfor serious failures.
All classes must have descriptive Google-style docstrings, complete attribute type annotations, and specific types instead ofAny.
Use PascalCase for classes with descriptive suffixes, includingConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
UseABCand@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions and includeParameters,Returns,Raises, andAttributessections when applicable.
Files:
src/utils/responses.pysrc/models/config.pytests/unit/utils/test_responses.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use
BaseModelfor data models,ConfigurationBasefor configuration models, and Pydantic@model_validatorand@field_validatorfor validation.
Files:
src/models/config.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all unit tests; do not use unittest. Put shared fixtures in
conftest.py, usepytest-mockfor mocks, mark async tests withpytest.mark.asyncio, and maintain at least 60% unit-test coverage.
Files:
tests/unit/utils/test_responses.py
🧠 Learnings (6)
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
examples/lightspeed-stack-byok-okp-rag.yaml
📚 Learning: 2026-02-23T14:56:59.186Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1198
File: src/utils/responses.py:184-192
Timestamp: 2026-02-23T14:56:59.186Z
Learning: In the lightspeed-stack codebase (lightspeed-core/lightspeed-stack), do not enforce de-duplication of duplicate client.models.list() calls in model selection flows (e.g., in src/utils/responses.py prepare_responses_params). These calls are considered relatively cheap and removing duplicates could add unnecessary complexity to the flow. Apply this guideline specifically to this file/context unless similar performance characteristics and design decisions are documented elsewhere.
Applied to files:
src/utils/responses.py
📚 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:
src/utils/responses.pysrc/models/config.pytests/unit/utils/test_responses.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/utils/responses.pysrc/models/config.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
🪛 GitHub Actions: Pydocstyle / 0_pydocstyle.txt
src/utils/responses.py
[error] 133-133: pydocstyle: D401: First line should be in imperative mood (perhaps 'Fetch', not 'Fetche')
🪛 GitHub Actions: Pydocstyle / pydocstyle
src/utils/responses.py
[error] 133-133: pydocstyle: D401 violation in public function get_vector_store_ids: First line should be in imperative mood (perhaps 'Fetch', not 'Fetche').
🔇 Additional comments (6)
docs/byok_guide.md (1)
264-268: LGTM!docs/config.md (1)
609-616: LGTM!docs/models/responses_succ.md (1)
1843-1850: LGTM!docs/openapi.md (1)
8158-8165: LGTM!examples/lightspeed-stack-byok-okp-rag.yaml (1)
63-63: LGTM!src/models/config.py (1)
2261-2262: LGTM!Also applies to: 2277-2277
Description
Remove fallback to all registered vector stores when tool RAG is unconfigured.
Previously, when neither rag.tool nor rag.inline were configured, prepare_tools would fetch and register all vector stores from llama-stack as a backward-compatibility fallback. This implicit behavior is removed — tool RAG now requires explicit configuration via rag.tool. Updated docs, config model, example YAML, and tests to reflect the new behavior.
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
rag.toolto an empty list or simply omit it.Summary by CodeRabbit
Behavior Changes
Documentation
Tests