LCORE-2572: Updated OpenAPI specification (1st part)#1926
Conversation
WalkthroughDocstrings for the ChangesEndpoint error documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 (2)
src/app/endpoints/tools.py (1)
72-92: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winReturn a normalized copy instead of mutating
tool_dict.
_normalize_tool_dict()rewrites and deletes keys on its argument, so callers have to remember it has side effects. Returning a new dict keeps the transformation local and makes the assignment at the call site explicit. As per coding guidelines, avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters.♻️ Proposed fix
-def _normalize_tool_dict(tool_dict: dict[str, Any], toolgroup: Any) -> None: +def _normalize_tool_dict(tool_dict: dict[str, Any], toolgroup: Any) -> dict[str, Any]: + normalized = dict(tool_dict) - if "name" in tool_dict and not tool_dict.get("identifier"): - tool_dict["identifier"] = tool_dict["name"] - tool_dict.pop("name", None) + if "name" in normalized and not normalized.get("identifier"): + normalized["identifier"] = normalized["name"] + normalized.pop("name", None) - if "input_schema" in tool_dict and not tool_dict.get("parameters"): - tool_dict["parameters"] = _input_schema_to_parameters(tool_dict["input_schema"]) - tool_dict.pop("input_schema", None) + if "input_schema" in normalized and not normalized.get("parameters"): + normalized["parameters"] = _input_schema_to_parameters(normalized["input_schema"]) + normalized.pop("input_schema", None) - if not tool_dict.get("provider_id"): - tool_dict["provider_id"] = toolgroup.provider_id - if not tool_dict.get("type"): - tool_dict["type"] = getattr(toolgroup, "type", None) or "tool" + if not normalized.get("provider_id"): + normalized["provider_id"] = toolgroup.provider_id + if not normalized.get("type"): + normalized["type"] = getattr(toolgroup, "type", None) or "tool" + + return normalized @@ - _normalize_tool_dict(tool_dict, toolgroup) + tool_dict = _normalize_tool_dict(tool_dict, toolgroup)Also applies to: 221-221
🤖 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/tools.py` around lines 72 - 92, Refactor the _normalize_tool_dict function to avoid mutating its input parameter. Instead of modifying tool_dict in-place, create a copy of the input dictionary at the start of the function, apply all the key remapping and propagation logic to this copy, and return the normalized copy at the end. Update all call sites where _normalize_tool_dict is invoked (including the one around line 221) to capture and use the returned value instead of relying on side effects, making the assignment explicit at each call location.Source: Coding guidelines
src/app/endpoints/models.py (1)
5-6:⚠️ Potential issue | 🟡 MinorImport
Dependsfromfastapiinstead offastapi.params.As per the coding guidelines for
src/app/**/*.pyfiles, FastAPI dependencies should be imported from thefastapimodule. Currently, this file importsDependsfromfastapi.params, which is inconsistent with the pattern used throughout the rest of the codebase.♻️ Proposed fix
-from fastapi import APIRouter, HTTPException, Query, Request -from fastapi.params import Depends +from fastapi import APIRouter, Depends, HTTPException, Query, Request🤖 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/models.py` around lines 5 - 6, The import statement for Depends is inconsistent with FastAPI coding guidelines. Move the Depends import from the separate fastapi.params line into the main fastapi import statement on the first line, alongside APIRouter, HTTPException, Query, and Request. Remove the now-empty fastapi.params import line to maintain consistency with the codebase pattern.Source: Coding guidelines
🤖 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`:
- Line 31: The docs/openapi.json file is a generated artifact that is currently
out of date due to pipeline failure. This file should never be manually edited.
Instead, update any documentation or description changes in the source endpoint
handler docstrings located in src/app/endpoints/*.py files, then regenerate the
OpenAPI schema by running the command: uv run scripts/generate_openapi_schema.py
docs/openapi.json. This will ensure the generated file stays in sync with your
source code documentation.
- Line 31: The openapi.json file contains duplicate "status" words in
HTTPException documentation ("with status status XXX"). Since this is a
generated file, locate and fix the source docstrings in the endpoint handler
functions across the endpoints directory. In the Raises sections of the
docstrings in root.py, info.py, models.py, and tools.py, change all instances of
"with status status" to "with status" for the HTTPException descriptions. After
fixing all source docstrings, regenerate the openapi.json file to apply these
corrections.
In `@docs/openapi.md`:
- Around line 476-478: Remove the repeated "status status" typo from the
HTTPException descriptions in the Raises sections across four endpoint
documentations in docs/openapi.md. At lines 476-478 (for the anchor endpoint),
change "with status status 401/403" to "with status 401/403". Apply the same fix
at lines 539-542 (for the /v1/info endpoint), lines 674-681 (for the /v1/models
endpoint), and lines 768-775 (for the /v1/tools endpoint). Each location
contains two HTTPException entries with the same repeated word issue that needs
to be corrected.
In `@src/app/endpoints/root.py`:
- Around line 804-806: Remove the duplicated "status" word in the Raises
sections of the docstrings across all affected files. In
src/app/endpoints/root.py at lines 804-806, src/app/endpoints/info.py at lines
55-58, src/app/endpoints/models.py at lines 101-108, and
src/app/endpoints/tools.py at lines 124-131, replace all occurrences of "with
status status" with either "with status" or "HTTP status" to eliminate the typo
that will appear in generated API documentation.
---
Outside diff comments:
In `@src/app/endpoints/models.py`:
- Around line 5-6: The import statement for Depends is inconsistent with FastAPI
coding guidelines. Move the Depends import from the separate fastapi.params line
into the main fastapi import statement on the first line, alongside APIRouter,
HTTPException, Query, and Request. Remove the now-empty fastapi.params import
line to maintain consistency with the codebase pattern.
In `@src/app/endpoints/tools.py`:
- Around line 72-92: Refactor the _normalize_tool_dict function to avoid
mutating its input parameter. Instead of modifying tool_dict in-place, create a
copy of the input dictionary at the start of the function, apply all the key
remapping and propagation logic to this copy, and return the normalized copy at
the end. Update all call sites where _normalize_tool_dict is invoked (including
the one around line 221) to capture and use the returned value instead of
relying on side effects, making the assignment explicit at each call location.
🪄 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: f223359a-bdc5-4777-b5e4-024b54013d97
📒 Files selected for processing (6)
docs/openapi.jsondocs/openapi.mdsrc/app/endpoints/info.pysrc/app/endpoints/models.pysrc/app/endpoints/root.pysrc/app/endpoints/tools.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.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: Pylinter
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- 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-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-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/tools.pysrc/app/endpoints/info.pysrc/app/endpoints/models.pysrc/app/endpoints/root.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/tools.pysrc/app/endpoints/info.pysrc/app/endpoints/models.pysrc/app/endpoints/root.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/tools.pysrc/app/endpoints/info.pysrc/app/endpoints/models.pysrc/app/endpoints/root.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json
[error] 1-1: docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json
🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json
[error] 1-1: CI check failed: docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json (diff -u between docs/openapi.json and /tmp/openapi-generated.json failed).
🪛 markdownlint-cli2 (0.22.1)
docs/openapi.md
[warning] 476-476: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 767-767: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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)
src/app/endpoints/models.py (1)
64-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd 422 response declarations to match docstring claims about improper parameters.
Both
models.pyandtools.pydocumentHTTPException: with status 422in their Raises sections, but neither file'sresponsesdict explicitly declares the 422 response. While FastAPI automatically generates a 422 response for query/dependency validation errors, the docstrings' explicit claims should be matched by explicit 422 entries in the respectiveresponsesdicts to keep OpenAPI documentation consistent and enable customized examples.
src/app/endpoints/models.py#L64-L72: Add 422 entry tomodels_responsesdict (e.g.,UnprocessableEntityResponse.openapi_response(examples=["invalid value"])) to match docstring line 103.src/app/endpoints/tools.py#L94-L102: Add 422 entry totools_responsesdict (e.g.,UnprocessableEntityResponse.openapi_response(examples=["invalid value"])) to match docstring line 126.Both fixes require importing
UnprocessableEntityResponsefrommodels.api.responses.error.🤖 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/models.py` around lines 64 - 72, In both src/app/endpoints/models.py (lines 64-72, the models_responses dict) and src/app/endpoints/tools.py (lines 94-102, the tools_responses dict), add an explicit 422 entry to match the HTTPException claims in the docstrings. Add a new line to each responses dict with the pattern "422: UnprocessableEntityResponse.openapi_response(examples=[...])". To support this, import UnprocessableEntityResponse from models.api.responses.error at the top of both files.
🤖 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.md`:
- Around line 476-478: The `### Raises:` headings in the markdown file are not
preceded by blank lines, causing markdownlint to flag them. Insert one blank
line before each `### Raises:` heading to comply with markdown formatting
standards. This applies at two locations in docs/openapi.md: at the anchor
location starting around line 476 where the first `### Raises:` section appears,
and at the sibling location around line 767 where another `### Raises:` section
appears. Add a blank line immediately before each of these headings to separate
them from the preceding list content.
---
Outside diff comments:
In `@src/app/endpoints/models.py`:
- Around line 64-72: In both src/app/endpoints/models.py (lines 64-72, the
models_responses dict) and src/app/endpoints/tools.py (lines 94-102, the
tools_responses dict), add an explicit 422 entry to match the HTTPException
claims in the docstrings. Add a new line to each responses dict with the pattern
"422: UnprocessableEntityResponse.openapi_response(examples=[...])". To support
this, import UnprocessableEntityResponse from models.api.responses.error at the
top of both files.
🪄 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: 46518767-2414-4b24-be75-a5bec027960e
📒 Files selected for processing (6)
docs/openapi.jsondocs/openapi.mdsrc/app/endpoints/info.pysrc/app/endpoints/models.pysrc/app/endpoints/root.pysrc/app/endpoints/tools.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). (2)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-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/tools.pysrc/app/endpoints/models.pysrc/app/endpoints/root.pysrc/app/endpoints/info.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/tools.pysrc/app/endpoints/models.pysrc/app/endpoints/root.pysrc/app/endpoints/info.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/tools.pysrc/app/endpoints/models.pysrc/app/endpoints/root.pysrc/app/endpoints/info.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json
[error] 1-1: OpenAPI schema is out of date. diff -u docs/openapi.json /tmp/openapi-generated.json failed. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json
🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json
[error] 1-1: OpenAPI schema mismatch detected. diff -u docs/openapi.json /tmp/openapi-generated.json failed, so docs/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json.
🪛 markdownlint-cli2 (0.22.1)
docs/openapi.md
[warning] 476-476: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 767-767: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (1)
docs/openapi.json (1)
31-31: Regeneratedocs/openapi.json; current generated artifact is still stale.Line 31, Line 175, Line 326, and Line 539 are in a generated file that currently fails sync checks. The pipeline reports
diff -u docs/openapi.json /tmp/openapi-generated.jsonmismatch, so this file must be regenerated from source docstrings rather than hand-edited:
uv run scripts/generate_openapi_schema.py docs/openapi.json.Also applies to: 175-175, 326-326, 539-539
Source: Pipeline failures
| ### Raises: | ||
| - HTTPException: with status 401 for unauthorized access. | ||
| - HTTPException: with status 403 if permission is denied. |
There was a problem hiding this comment.
Add the missing blank lines before both Raises headings.
markdownlint still flags these headings because they follow the preceding list immediately. Insert one blank line before each ### Raises: block to keep the generated docs lint-clean.
🛠️ Suggested fix
- auth: Authentication tuple from the auth dependency (used by middleware).
### Raises:
@@
- mcp_headers: Headers that should be passed to MCP servers.
### Raises:Also applies to: 767-775
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 476-476: 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 476 - 478, The `### Raises:` headings in the
markdown file are not preceded by blank lines, causing markdownlint to flag
them. Insert one blank line before each `### Raises:` heading to comply with
markdown formatting standards. This applies at two locations in docs/openapi.md:
at the anchor location starting around line 476 where the first `### Raises:`
section appears, and at the sibling location around line 767 where another `###
Raises:` section appears. Add a blank line immediately before each of these
headings to separate them from the preceding list content.
Source: Linters/SAST tools
1 participant
Description
LCORE-2572: Updated OpenAPI specification (1st part)
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
HTTPExceptionstatus codes across the main routes, including401,403,422,500, and503.detailstructures that includeresponseandcausewhere applicable.