LCORE-2572: Minor fixes#1944
Conversation
WalkthroughDocumentation-only updates across four files: ChangesOpenAPI Documentation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/openapi.md (1)
2531-2532:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the
Returnsbullet indentation.The extra leading spaces make Markdown treat this as an indented block instead of a list item.
📝 Suggested fix
- - RAGListResponse: List of RAG identifiers. +- RAGListResponse: List of RAG identifiers.🤖 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 `@docs/openapi.md` around lines 2531 - 2532, The indentation of the RAGListResponse bullet point under the Returns section has extra leading spaces that cause Markdown to treat it as an indented code block instead of a proper list item. Remove the extra leading spaces before the dash in the RAGListResponse line so it aligns as a standard list item with minimal indentation beneath the Returns heading.Source: Linters/SAST tools
🤖 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/openapi.json`:
- Around line 2728-2732: The docs/openapi.json file is out of date and must be
regenerated from source code rather than manually edited. First, verify that the
Python docstrings in src/app/endpoints/prompts.py and src/app/endpoints/rags.py
contain the description updates matching the hunks shown (specifically for the
update_prompt_handler and any related RAG endpoints). Then run the provided
script to regenerate the OpenAPI schema, which will update docs/openapi.json to
match the current docstrings. Finally, commit the regenerated openapi.json file.
In `@docs/openapi.md`:
- Around line 5471-5481: The markdown headings (### Parameters:, ### Raises:,
### Returns:) are missing blank lines before and after them, which violates the
markdownlint MD022 rule and causes rendering inconsistencies. Add a blank line
before each of these headings and after the final line of each section to
properly space them. This should be applied at the three locations mentioned:
around lines 5471-5481 for the first set of headings, around lines 5502-5512 for
the second set, and around lines 5596-5603 for the third set.
In `@src/app/endpoints/prompts.py`:
- Around line 360-367: Add HTTPException with status 400 to the Raises section
in the docstring of the delete_prompt_handler function to document that a 400
response is returned when check_suid_prompt(prompt_id) fails, indicating an
invalid prompt ID format. This status code is already implemented in the
function logic and registered in the prompt_delete_responses dict, so the
docstring must be updated to reflect all possible HTTP responses the endpoint
can return.
In `@src/app/endpoints/rags.py`:
- Around line 150-166: The docstring for this GET handler incorrectly claims it
can raise an HTTPException with status 422 for incorrect request payload. Since
GET requests have no request body, this status code is impossible to return from
this endpoint. Remove the entire line mentioning HTTPException with status 422
for incorrect request payload from the Raises section of the docstring, keeping
only the status codes that are actually possible: 401, 403, 404, 500, and 503.
---
Outside diff comments:
In `@docs/openapi.md`:
- Around line 2531-2532: The indentation of the RAGListResponse bullet point
under the Returns section has extra leading spaces that cause Markdown to treat
it as an indented code block instead of a proper list item. Remove the extra
leading spaces before the dash in the RAGListResponse line so it aligns as a
standard list item with minimal indentation beneath the Returns heading.
🪄 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: 66e9a21e-e62f-42ee-9e88-ea7c00f31fd7
📒 Files selected for processing (4)
docs/openapi.jsondocs/openapi.mdsrc/app/endpoints/prompts.pysrc/app/endpoints/rags.py
📜 Review details
⏰ 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). (14)
- GitHub Check: unit_tests (3.13)
- GitHub Check: mypy
- GitHub Check: unit_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: Pylinter
- GitHub Check: E2E: library mode / ci / group 2
- 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: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Useasync deffor I/O operations and external API calls
Use standard log levels with clear purposes:debug()for diagnostic info,info()for program execution,warning()for unexpected events,error()for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes must use ABC with@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/app/endpoints/prompts.pysrc/app/endpoints/rags.py
src/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/app/**/*.py: FastAPI dependencies: Import fromfastapimodule forAPIRouter,HTTPException,Request,status,Depends
Use FastAPIHTTPExceptionwith appropriate status codes for API endpoints and handleAPIConnectionErrorfrom Llama Stack
Files:
src/app/endpoints/prompts.pysrc/app/endpoints/rags.py
🧠 Learnings (1)
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.
Applied to files:
src/app/endpoints/prompts.pysrc/app/endpoints/rags.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json
[error] 1-1: OpenAPI schema is out of date. Pipeline failed because 'diff -u docs/openapi.json /tmp/openapi-generated.json' reported differences. Regenerate with: 'uv run scripts/generate_openapi_schema.py docs/openapi.json'.
🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json
[error] 1-1: Mismatch detected in OpenAPI schema file 'docs/openapi.json'. The file does not match the newly generated schema at '/tmp/openapi-generated.json'.
[error] 1-1: Command used to validate: 'diff -u docs/openapi.json /tmp/openapi-generated.json' (non-zero exit indicates differences).
🪛 markdownlint-cli2 (0.22.1)
docs/openapi.md
[warning] 2619-2619: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2624-2624: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2634-2634: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5471-5471: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5474-5474: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5480-5480: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5502-5502: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5505-5505: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5511-5511: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5596-5596: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5599-5599: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5602-5602: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (4)
src/app/endpoints/prompts.py (1)
1-398: LGTM for all other segments!The imports, type annotations, async patterns, docstring conventions, and error handling across all handlers align with the coding guidelines. The update to
update_prompt_handlerdocstring (lines 293–302) is complete and accurate.docs/openapi.md (2)
2619-2635: Same 422 mismatch as the source docstring.This should stay in sync with
src/app/endpoints/rags.py; once that source docstring is corrected, regenerate this section so the published spec matches.
5913-5913: LGTM!Also applies to: 6009-6013, 6182-6185, 6615-6630
src/app/endpoints/rags.py (1)
58-112: LGTM!
| "prompts" | ||
| ], | ||
| "summary": "Update Prompt Handler", | ||
| "description": "Handle requests to the PUT /prompts/{prompt_id} endpoint.\n\nProcess requests to update a stored prompt; Llama Stack increments the\nversion. The body includes the new text, the current version being\nreplaced, and optional fields such as ``set_as_default`` and ``variables``.\nFor example:\n\n curl -X PUT http://localhost:8080/v1/prompts/pmpt_abc123 \\\\\n -H 'Content-Type: application/json' \\\\\n -d '{\"prompt\": \"Hi\", \"version\": 1, \"set_as_default\": true}'\n\n### Parameters:\n- request: The incoming HTTP request (used by middleware).\n- prompt_id: The Llama Stack prompt identifier.\n- auth: Authentication tuple from the auth dependency (used by middleware).\n- body: Prompt update parameters.\n\n### Raises:\n- HTTPException: If configuration is not loaded, if the prompt is not\n found, if unable to connect to Llama Stack, or if the prompts API returns\n an error response.\n\n### Returns:\n- PromptResourceResponse: The updated prompt object returned by Llama Stack.", | ||
| "description": "Handle requests to the PUT /prompts/{prompt_id} endpoint.\n\nProcess requests to update a stored prompt; Llama Stack increments the\nversion. The body includes the new text, the current version being\nreplaced, and optional fields such as ``set_as_default`` and ``variables``.\nFor example:\n\n curl -X PUT http://localhost:8080/v1/prompts/pmpt_abc123 \\\\\n -H 'Content-Type: application/json' \\\\\n -d '{\"prompt\": \"Hi\", \"version\": 1, \"set_as_default\": true}'\n\n### Parameters:\n- request: The incoming HTTP request (used by middleware).\n- prompt_id: The Llama Stack prompt identifier.\n- auth: Authentication tuple from the auth dependency (used by middleware).\n- body: Prompt update parameters.\n\n### Raises:\n- HTTPException: with status 400 when request format is not valid.\n- HTTPException: with status 401 for unauthorized access.\n- HTTPException: with status 403 if permission is denied.\n- HTTPException: with status 404 if prompt is not found.\n- HTTPException: with status 422 if request payload is corrupted.\n- HTTPException: with status 500 and a detail object containing `response`\n and `cause` when service configuration is wrong or incomplete.\n- HTTPException: with status 503 and a detail object containing `response`\n and `cause` when unable to connect to Llama Stack.\n\n### Returns:\n- PromptResourceResponse: The updated prompt object returned by Llama Stack.", | ||
| "operationId": "update_prompt_handler_v1_prompts__prompt_id__put", |
There was a problem hiding this comment.
OpenAPI schema is out of date and fails CI validation.
The pipeline has flagged that docs/openapi.json does not match the schema generated by the build process. The diff command shows a mismatch against /tmp/openapi-generated.json. OpenAPI JSON files should be auto-generated from source docstrings rather than manually edited.
You must regenerate the OpenAPI schema using the provided script before this PR can merge.
Regenerate the OpenAPI schema:
uv run scripts/generate_openapi_schema.py docs/openapi.jsonEnsure the Python docstrings in src/app/endpoints/prompts.py and src/app/endpoints/rags.py match the description updates shown in these hunks, then commit the regenerated docs/openapi.json.
Also applies to: 2980-2984, 3211-3214, 3384-3387
🤖 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 `@docs/openapi.json` around lines 2728 - 2732, The docs/openapi.json file is
out of date and must be regenerated from source code rather than manually
edited. First, verify that the Python docstrings in src/app/endpoints/prompts.py
and src/app/endpoints/rags.py contain the description updates matching the hunks
shown (specifically for the update_prompt_handler and any related RAG
endpoints). Then run the provided script to regenerate the OpenAPI schema, which
will update docs/openapi.json to match the current docstrings. Finally, commit
the regenerated openapi.json file.
Source: Pipeline failures
| ### Parameters: | ||
| - auth: Authentication tuple from the auth dependency (used by middleware). | ||
|
|
||
| ### Raises: | ||
| - HTTPException: with status 500 and a detail object containing `response` | ||
| and `cause` when service configuration is wrong or incomplete. | ||
| - HTTPException: with status 503 and a detail object containing `response` | ||
| and `cause` when unable to connect to Llama Stack. | ||
|
|
||
| ### Returns: | ||
| - AgentCard: The agent card describing this agent's capabilities. |
There was a problem hiding this comment.
Add blank lines around these headings.
These blocks hit the markdownlint MD022 warning, and the current layout can render inconsistently in Markdown viewers.
🛠️ Suggested fix
-This endpoint provides the agent card that describes Lightspeed's
-capabilities according to the A2A protocol specification.
-### Parameters:
+This endpoint provides the agent card that describes Lightspeed's
+capabilities according to the A2A protocol specification.
+
+### Parameters:
+
- auth: Authentication tuple from the auth dependency (used by middleware).Also applies to: 5502-5512, 5596-5603
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 5471-5471: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5474-5474: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5480-5480: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@docs/openapi.md` around lines 5471 - 5481, The markdown headings (###
Parameters:, ### Raises:, ### Returns:) are missing blank lines before and after
them, which violates the markdownlint MD022 rule and causes rendering
inconsistencies. Add a blank line before each of these headings and after the
final line of each section to properly space them. This should be applied at the
three locations mentioned: around lines 5471-5481 for the first set of headings,
around lines 5502-5512 for the second set, and around lines 5596-5603 for the
third set.
Source: Linters/SAST tools
| ### Raises: | ||
| - HTTPException: If configuration is not loaded, if unable to connect to | ||
| Llama Stack, or if the prompts API returns an error response. | ||
| - HTTPException: with status 401 for unauthorized access. | ||
| - HTTPException: with status 403 if permission is denied. | ||
| - HTTPException: with status 422 if request payload is corrupted. | ||
| - HTTPException: with status 500 and a detail object containing `response` | ||
| and `cause` when service configuration is wrong or incomplete. | ||
| - HTTPException: with status 503 and a detail object containing `response` | ||
| and `cause` when unable to connect to Llama Stack. |
There was a problem hiding this comment.
Missing HTTP 400 status code in delete_prompt_handler docstring.
The Raises section omits status 400, but the implementation at line 378–381 raises HTTPException with BadRequestResponse (status 400) when check_suid_prompt(prompt_id) fails. Additionally, the response dict prompt_delete_responses (lines 85–94) explicitly includes a 400 response definition, and the decorator @router.delete(..., responses=prompt_delete_responses) at line 336 registers it with FastAPI.
The docstring should document all status codes that the endpoint can return, including 400 for invalid prompt ID format.
📋 Proposed docstring update
### Raises:
+ - HTTPException: with status 400 when request format is not valid.
- HTTPException: with status 401 for unauthorized access.
- HTTPException: with status 403 if permission is denied.
- HTTPException: with status 422 if request payload is corrupted.🤖 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 `@src/app/endpoints/prompts.py` around lines 360 - 367, Add HTTPException with
status 400 to the Raises section in the docstring of the delete_prompt_handler
function to document that a 400 response is returned when
check_suid_prompt(prompt_id) fails, indicating an invalid prompt ID format. This
status code is already implemented in the function logic and registered in the
prompt_delete_responses dict, so the docstring must be updated to reflect all
possible HTTP responses the endpoint can return.
| ### Parameters: | ||
| - request: The incoming HTTP request (used by middleware). | ||
| - rag_id: rag_id or llama-stack vector_store_id | ||
| - auth: Authentication tuple from the auth dependency (used by middleware). | ||
|
|
||
| ### Raises: | ||
| - HTTPException: with status 401 for unauthorized access. | ||
| - HTTPException: with status 403 if permission is denied. | ||
| - HTTPException: with status 404 if rag_id is not found. | ||
| - HTTPException: with status 422 for incorrect request payload. | ||
| - HTTPException: with status 500 and a detail object containing `response` | ||
| and `cause` when service configuration is wrong or incomplete. | ||
| - HTTPException: with status 503 and a detail object containing `response` | ||
| and `cause` when unable to connect to Llama Stack. | ||
|
|
||
| ### Returns: | ||
| - RAGInfoResponse: A single RAG's details. |
There was a problem hiding this comment.
Remove the 422 claim from this GET handler.
This endpoint has no request body, and the code path shown here only maps to 404/503. Keeping 422 here makes the generated contract misleading.
✏️ Suggested fix
- - HTTPException: with status 422 for incorrect request payload.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Parameters: | |
| - request: The incoming HTTP request (used by middleware). | |
| - rag_id: rag_id or llama-stack vector_store_id | |
| - auth: Authentication tuple from the auth dependency (used by middleware). | |
| ### Raises: | |
| - HTTPException: with status 401 for unauthorized access. | |
| - HTTPException: with status 403 if permission is denied. | |
| - HTTPException: with status 404 if rag_id is not found. | |
| - HTTPException: with status 422 for incorrect request payload. | |
| - HTTPException: with status 500 and a detail object containing `response` | |
| and `cause` when service configuration is wrong or incomplete. | |
| - HTTPException: with status 503 and a detail object containing `response` | |
| and `cause` when unable to connect to Llama Stack. | |
| ### Returns: | |
| - RAGInfoResponse: A single RAG's details. | |
| ### Parameters: | |
| - request: The incoming HTTP request (used by middleware). | |
| - rag_id: rag_id or llama-stack vector_store_id | |
| - auth: Authentication tuple from the auth dependency (used by middleware). | |
| ### Raises: | |
| - HTTPException: with status 401 for unauthorized access. | |
| - HTTPException: with status 403 if permission is denied. | |
| - HTTPException: with status 404 if rag_id is not found. | |
| - HTTPException: with status 500 and a detail object containing `response` | |
| and `cause` when service configuration is wrong or incomplete. | |
| - HTTPException: with status 503 and a detail object containing `response` | |
| and `cause` when unable to connect to Llama Stack. | |
| ### Returns: | |
| - RAGInfoResponse: A single RAG's details. |
🤖 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 `@src/app/endpoints/rags.py` around lines 150 - 166, The docstring for this GET
handler incorrectly claims it can raise an HTTPException with status 422 for
incorrect request payload. Since GET requests have no request body, this status
code is impossible to return from this endpoint. Remove the entire line
mentioning HTTPException with status 422 for incorrect request payload from the
Raises section of the docstring, keeping only the status codes that are actually
possible: 401, 403, 404, 500, and 503.
1 participant
Description
LCORE-2572: Minor fixes
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit