LCORE-1348: better docstrings for REST API endpoints, part 2#1491
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR standardizes endpoint documentation across the codebase by reformatting parameter and return descriptions from an ad-hoc "Args/Returns/Raises" style to a consistent "### Parameters / ### Returns / ### Raises" markdown structure. Middleware-resolved parameters like Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openapi.json`:
- Line 770: The OpenAPI description incorrectly describes the operation body as
headers; update the operation description to reflect that the request body is a
MCPServerRegistrationRequest (the server registration payload) and summarize its
fields (e.g., name, url, optional headers, metadata) instead of saying "Headers
that should be pass to MCP servers"; locate the text around the MCP server
registration operation and replace the `body` description with a concise note
that the body is an MCPServerRegistrationRequest containing registration
attributes (mention MCPServerRegistrationRequest by name) and, if helpful, list
the key fields that are part of that schema.
- Line 559: Replace the grammar typo "should be pass" with "should be passed" in
the OpenAPI description strings where mcp_headers is documented; specifically
update the description text that lists "mcp_headers: Headers that should be pass
to MCP servers." to read "mcp_headers: Headers that should be passed to MCP
servers." in the docs/openapi.json entries referenced (the description blocks
containing the MCPClientAuthOptionsResponse and related parameter lists).
- Line 934: The OpenAPI description for the Delete endpoint incorrectly reuses
the registration "request" description; update the documentation under the
Unregister/Delete operation to accurately describe the incoming request context
(e.g., that the delete uses path parameter "name" and/or an empty body or
authentication context) instead of describing dynamic registration model; locate
the "request" param description near the "Unregister a dynamically registered
MCP server." text and replace it with a concise description such as "Delete
request context (no body); uses path parameter 'name' and auth tuple from the
auth dependency" and keep the rest of the Raises/Returns text intact (reference:
the "request" parameter and "MCPServerDeleteResponse" in the same operation).
- Line 666: The parameter description for `request` in the MCP servers list
endpoint is incorrect: replace the line that describes `request` as "Model
containing attributes to dynamically registering an MCP server" with a brief
description stating it represents the incoming HTTP request / middleware context
(e.g., "Incoming HTTP request object used by middleware, containing headers,
client info and context"); keep the rest of the endpoint doc (including
references to `auth` and `MCPServerListResponse`) unchanged so the GET handler
documentation correctly reflects that `request` is the HTTP request, not a
registration model.
In `@docs/openapi.md`:
- Around line 69-70: Multiple markdown headings like "### Parameters:", "###
Returns:", and "### Raises:" are missing required surrounding blank lines
(MD022); edit docs/openapi.md to insert a blank line before and after each of
these headings wherever they appear (e.g., the "### Parameters:", "###
Returns:", and "### Raises:" blocks noted in the comment) so that each heading
is separated from adjacent paragraph/list content, and normalize similar
occurrences at the other ranges listed to satisfy markdownlint.
- Line 437: Replace the incorrect phrase "should be pass" with "should be
passed" in the OpenAPI docs for the parameter descriptions (e.g., the
mcp_headers parameter and any other parameter descriptions flagged), updating
every occurrence noted in the review so the user-facing text reads "Headers that
should be passed to MCP servers." Search for the exact string "should be pass"
and update those parameter descriptions to use "should be passed" consistently.
- Around line 507-509: The OpenAPI parameter descriptions are incorrect: update
the "Parameters:" blocks that currently list "request: Model containing
attributes to dynamically registering an MCP server." and "auth: Authentication
tuple..." so they match each endpoint's semantics — remove or change the
"request"/"body" description for GET and DELETE endpoints (they should not
describe a registration body), and for POST endpoints ensure "body" describes
the MCP registration model (and move header descriptions to a "headers" or
explicit header parameter). Locate the offending "Parameters:" sections (the
block with "- request: Model containing attributes to dynamically registering an
MCP server." and "- auth: Authentication tuple...") and replace the descriptions
so GET/DELETE show no request body, POST body describes the registration model,
and any headers are documented as headers rather than body.
In `@src/app/endpoints/info.py`:
- Around line 52-55: Update the docstring in src/app/endpoints/info.py to
reflect the actual status code returned on Llama Stack connection failures:
replace the incorrect "500" with "503" and mention that the handler raises
ServiceUnavailableResponse (HTTP 503) in the APIConnectionError path; ensure the
Raises section references APIConnectionError and ServiceUnavailableResponse by
name so it matches the implementation.
In `@src/app/endpoints/mcp_auth.py`:
- Line 58: The docstring for get_mcp_client_auth_options incorrectly documents a
parameter named mcp_headers that the function does not accept; remove the
undocumented/ non-existent mcp_headers entry from the function's docstring and
update the remaining parameter descriptions to match the actual signature of
get_mcp_client_auth_options (ensure any references to mcp_headers in the
surrounding comments or endpoint docs are also removed or replaced with the
correct parameter names like the actual auth options the function returns).
In `@src/app/endpoints/mcp_servers.py`:
- Around line 146-148: Update the parameter docstring to correctly describe the
FastAPI request object and auth tuple: change the `request` description from
"Model containing attributes to dynamically registering an MCP server" to
something like "FastAPI Request object for the incoming HTTP request (used by
middleware)", and keep `auth` described as "Authentication tuple from the auth
dependency (used by middleware)"; make this change in the docstring/block that
documents the endpoint parameters where `request` and `auth` are referenced.
- Around line 62-65: Update the docstring parameter descriptions for the MCP
server endpoint: change the `request` description to indicate it is the FastAPI
HTTP request object used by middleware, correct `body` to describe it as an
instance of the MCPServerRegistrationRequest model containing the registration
details (fix grammar to "should be passed" if mentioning headers), and keep
`auth` described as the authentication tuple from the auth dependency; locate
these in the endpoint function's docstring in src/app/endpoints/mcp_servers.py
where `request`, `auth`, and `body` are documented and replace the inaccurate
lines accordingly.
- Around line 201-204: Update the docstring for the endpoint function that
declares the parameters request, auth, and name so the request parameter is
accurately described: replace "Model containing attributes to dynamically
registering an MCP server" with a concise description like "FastAPI Request
object (incoming HTTP request; used by middleware)" while leaving auth and name
descriptions unchanged; ensure the updated text appears in the function's
parameter block (the docstring/sphinx-style params) next to the request
parameter.
In `@src/app/endpoints/query.py`:
- Line 113: Fix the docstring typo for the mcp_headers parameter in the
function/method docstring in src/app/endpoints/query.py by changing "Headers
that should be pass to MCP servers." to "Headers that should be passed to MCP
servers."—update the description for the parameter name mcp_headers accordingly.
In `@src/app/endpoints/stream_interrupt.py`:
- Around line 59-60: The docstring Raises section for the stream interrupt
handler is incomplete; update the handler's docstring (e.g.,
interrupt_stream_handler / interrupt_stream) to state that it raises
HTTPException both when no active stream exists for the given request ID and
when interruption is forbidden (CancelStreamResult.FORBIDDEN), specifying the
404 case for missing stream and 403 for forbidden interruption (include
CancelStreamResult.FORBIDDEN in the description).
In `@src/app/endpoints/streaming_query.py`:
- Around line 164-182: Edit the docstring for the endpoint containing the
parameter list and correct the grammar for the mcp_headers description: change
"Headers that should be pass to MCP servers" to "Headers that should be passed
to MCP servers". Locate the parameter block associated with the streaming query
endpoint (look for the parameters request, query_request, auth, mcp_headers in
the streaming_query handler) and update only that string in the docstring.
In `@src/app/endpoints/tools.py`:
- Around line 117-120: The docstring for tools_endpoint_handler is missing the
mcp_headers parameter; update the ### Parameters section to include an entry for
mcp_headers (matching the style of request and auth), describing its type (e.g.,
dict or Mapping[str, str]) and purpose (MCP headers passed from middleware for
downstream services), and place it alongside `request` and `auth` so the
docstring reflects the function signature exactly.
🪄 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: a3d2acdb-bfce-45bf-a8d3-00b37642fb53
📒 Files selected for processing (18)
docs/openapi.jsondocs/openapi.mdsrc/app/endpoints/authorized.pysrc/app/endpoints/config.pysrc/app/endpoints/feedback.pysrc/app/endpoints/health.pysrc/app/endpoints/info.pysrc/app/endpoints/mcp_auth.pysrc/app/endpoints/mcp_servers.pysrc/app/endpoints/metrics.pysrc/app/endpoints/models.pysrc/app/endpoints/providers.pysrc/app/endpoints/query.pysrc/app/endpoints/root.pysrc/app/endpoints/shields.pysrc/app/endpoints/stream_interrupt.pysrc/app/endpoints/streaming_query.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). (10)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: Pyright
- GitHub Check: integration_tests (3.12)
- GitHub Check: unit_tests (3.12)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E: library mode / ci
- GitHub Check: build-pr
- GitHub Check: E2E: server mode / ci
- GitHub Check: Pylinter
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules (e.g.,from authentication import get_auth_dependency)
Usefrom llama_stack_client import AsyncLlamaStackClientfor Llama Stack imports
Checkconstants.pyfor shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Use complete type annotations for function parameters and return types
Use union types with modern syntax:str | intinstead ofUnion[str, int]
UseOptional[Type]for optional types in type annotations
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 parameters
Useasync deffor I/O operations and external API calls
HandleAPIConnectionErrorfrom Llama Stack in error handling
Uselogger.debug()for detailed diagnostic information
Uselogger.info()for general information about program execution
Uselogger.warning()for unexpected events or potential problems
Uselogger.error()for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes:Configuration,Error/Exception,Resolver,Interface
Use complete type annotations for all class attributes; avoid usingAny
Follow Google Python docstring conventions for all modules, classes, and functions
IncludeParameters:,Returns:,Raises:sections in function docstrings as needed
Files:
src/app/endpoints/mcp_auth.pysrc/app/endpoints/stream_interrupt.pysrc/app/endpoints/query.pysrc/app/endpoints/config.pysrc/app/endpoints/info.pysrc/app/endpoints/metrics.pysrc/app/endpoints/authorized.pysrc/app/endpoints/shields.pysrc/app/endpoints/feedback.pysrc/app/endpoints/models.pysrc/app/endpoints/mcp_servers.pysrc/app/endpoints/providers.pysrc/app/endpoints/root.pysrc/app/endpoints/health.pysrc/app/endpoints/streaming_query.pysrc/app/endpoints/tools.py
src/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/app/**/*.py: Usefrom fastapi import APIRouter, HTTPException, Request, status, Dependsfor FastAPI dependencies
Use FastAPIHTTPExceptionwith appropriate status codes for API endpoint error handling
Files:
src/app/endpoints/mcp_auth.pysrc/app/endpoints/stream_interrupt.pysrc/app/endpoints/query.pysrc/app/endpoints/config.pysrc/app/endpoints/info.pysrc/app/endpoints/metrics.pysrc/app/endpoints/authorized.pysrc/app/endpoints/shields.pysrc/app/endpoints/feedback.pysrc/app/endpoints/models.pysrc/app/endpoints/mcp_servers.pysrc/app/endpoints/providers.pysrc/app/endpoints/root.pysrc/app/endpoints/health.pysrc/app/endpoints/streaming_query.pysrc/app/endpoints/tools.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-05T12:19:36.009Z
Learning: Applies to src/**/*.py : Include `Parameters:`, `Returns:`, `Raises:` sections in function docstrings as needed
📚 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/mcp_auth.pysrc/app/endpoints/stream_interrupt.pysrc/app/endpoints/query.pysrc/app/endpoints/config.pysrc/app/endpoints/info.pysrc/app/endpoints/metrics.pysrc/app/endpoints/authorized.pysrc/app/endpoints/shields.pysrc/app/endpoints/feedback.pysrc/app/endpoints/models.pysrc/app/endpoints/mcp_servers.pysrc/app/endpoints/providers.pysrc/app/endpoints/root.pysrc/app/endpoints/health.pysrc/app/endpoints/streaming_query.pysrc/app/endpoints/tools.py
📚 Learning: 2026-01-14T09:37:51.612Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 988
File: src/app/endpoints/query.py:319-339
Timestamp: 2026-01-14T09:37:51.612Z
Learning: In the lightspeed-stack repository, when provider_id == "azure", the Azure provider with provider_type "remote::azure" is guaranteed to be present in the providers list. Therefore, avoid defensive StopIteration handling for next() when locating the Azure provider in providers within src/app/endpoints/query.py. This change applies specifically to this file (or nearby provider lookup code) and relies on the invariant that the Azure provider exists; if the invariant could be violated, keep the existing StopIteration handling.
Applied to files:
src/app/endpoints/query.py
🪛 markdownlint-cli2 (0.22.0)
docs/openapi.md
[warning] 69-69: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 128-128: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 134-134: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 263-263: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 267-267: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 346-346: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 350-350: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 354-354: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 434-434: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 439-439: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 507-507: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 511-511: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 514-514: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 583-583: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 588-588: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 592-592: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 695-695: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 700-700: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 704-704: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 813-813: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 817-817: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 820-820: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 902-902: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 905-905: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 992-992: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 995-995: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1301-1301: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1307-1307: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1310-1310: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1584-1584: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1590-1590: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1593-1593: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1864-1864: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1869-1869: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1872-1872: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1949-1949: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2022-2022: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2029-2029: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2032-2032: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2148-2148: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2151-2151: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2174-2174: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 2179-2179: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3710-3710: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3714-3714: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3791-3791: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3794-3794: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3858-3858: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3861-3861: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3926-3926: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 3930-3930: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (15)
src/app/endpoints/config.py (1)
50-51: Docstring update looks good.Parameter and return documentation are clear and consistent with the endpoint behavior.
Also applies to: 54-54
src/app/endpoints/metrics.py (1)
55-61: Nice docstring clarification for middleware args.The added parameter and return details are precise and readable.
src/app/endpoints/root.py (1)
797-803: Looks good.The parameter and return docs are clear and aligned with implementation.
src/app/endpoints/shields.py (1)
52-61: LGTM! Docstring formatting improved.The docstring has been successfully reformatted to use the standardized Markdown structure with clear parameter, return, and exception documentation. The middleware usage of
requestandauthparameters is appropriately noted.src/app/endpoints/authorized.py (1)
36-40: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter and return documentation following the standardized Markdown structure.
src/app/endpoints/models.py (2)
36-36: LGTM! Docstring section header updated.The docstring section header has been updated from "Args:" to "Parameters:" to align with the repository's documentation standards.
94-104: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter, return, and exception documentation. The middleware usage and optional query parameter are appropriately documented.
src/app/endpoints/providers.py (2)
64-75: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter, return, and exception documentation using the standardized Markdown structure.
131-144: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter, return, and exception documentation. All HTTP status codes are properly documented.
src/app/endpoints/feedback.py (3)
104-117: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter, return, and exception documentation using the standardized Markdown structure.
189-193: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter and return documentation following the standardized Markdown structure.
215-221: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter and return documentation. The middleware usage of the
authparameter is appropriately noted.src/app/endpoints/health.py (2)
111-118: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter and return documentation. The middleware usage of both
responseandauthparameters is appropriately noted, and the return value description clearly explains the readiness response structure.
152-156: LGTM! Docstring formatting improved.The docstring has been successfully reformatted with clear parameter and return documentation following the standardized Markdown structure.
docs/openapi.json (1)
31-31: Good standardization of operation descriptionsThe move to structured
### Parameters / ### Returns / ### Raisessections is clear and improves readability across endpoints.Also applies to: 105-105, 256-256, 421-421, 5140-5140, 5215-5215
| ], | ||
| "summary": "Get Mcp Client Auth Options", | ||
| "description": "Get MCP servers that accept client-provided authorization.\n\nReturns a list of MCP servers configured to accept client-provided\nauthorization tokens, along with the header names where clients\nshould provide these tokens.\n\nThis endpoint helps clients discover which MCP servers they can\nauthenticate with using their own tokens.\n\nArgs:\n request: The incoming HTTP request (used by middleware).\n auth: Authentication tuple from the auth dependency (used by middleware).\n\nReturns:\n MCPClientAuthOptionsResponse: List of MCP servers and their\n accepted client authentication headers.", | ||
| "description": "Get MCP servers that accept client-provided authorization.\n\nReturns a list of MCP servers configured to accept client-provided\nauthorization tokens, along with the header names where clients\nshould provide these tokens.\n\nThis endpoint helps clients discover which MCP servers they can\nauthenticate with using their own tokens.\n\n### Parameters:\n- request: The incoming HTTP request (used by middleware).\n- auth: Authentication tuple from the auth dependency (used by middleware).\n- mcp_headers: Headers that should be pass to MCP servers.\n\n### Returns:\n- MCPClientAuthOptionsResponse: List of MCP servers and their\n accepted client authentication headers.", |
There was a problem hiding this comment.
Fix repeated grammar typo: “should be pass”
At Lines 559, 770, 1828, and 2146, use “should be passed” (or “to pass”) for clarity and professionalism in API docs.
Also applies to: 770-770, 1828-1828, 2146-2146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi.json` at line 559, Replace the grammar typo "should be pass"
with "should be passed" in the OpenAPI description strings where mcp_headers is
documented; specifically update the description text that lists "mcp_headers:
Headers that should be pass to MCP servers." to read "mcp_headers: Headers that
should be passed to MCP servers." in the docs/openapi.json entries referenced
(the description blocks containing the MCPClientAuthOptionsResponse and related
parameter lists).
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Scanning docs for the phrase 'should be pass'..."
rg -n -S $'should be pass' docs || true
cat > /tmp/issue_body.md << 'EOF'
A minor grammar typo appears in multiple API docs descriptions: the phrase "should be pass" should be "should be passed".
Scope:
- File: docs/openapi.json
- Likely affected endpoint descriptions where mcp_headers is documented:
- GET /v1/mcp-auth/client-options
- POST /v1/mcp-servers (parameter list currently says "body", but the issue is the phrase)
- POST /v1/query
- POST /v1/streaming_query
What to do:
- Replace the text "should be pass" with "should be passed".
- Ensure the bullet reads consistently as:
- mcp_headers: Headers that should be passed to MCP servers.
- After editing openapi.json, regenerate docs/openapi.md if it is derived from it, and fix any remaining occurrences across docs.
Acceptance criteria:
- No occurrences of "should be pass" remain in docs/openapi.json and docs/openapi.md.
- Generated docs render without lint warnings for these lines.
References:
- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/1491
- Comment: https://github.com/lightspeed-core/lightspeed-stack/pull/1491#discussion_r3071757192
- Requested by: `@tisnik`
Search evidence (line:col):
EOF
# Append the actual grep results (if any)
rg -n -S $'should be pass' docs | sed 's/^/- /' >> /tmp/issue_body.md || true
TITLE="Docs: fix grammar 'should be pass' -> 'should be passed' in docs/openapi.json (mcp_headers bullets)"
echo
echo "Creating the GitHub issue in lightspeed-core/lightspeed-stack..."
issue_output=$(gh issue create \
--repo lightspeed-core/lightspeed-stack \
--title "$TITLE" \
--assignee tisnik \
--body-file /tmp/issue_body.md 2>&1 || true)
echo "$issue_output"
# Try to extract the created issue URL from gh output
issue_url=$(echo "$issue_output" | grep -Eo 'https://github\.com/[^ ]+/issues/[0-9]+' | tail -n1 || true)
if [[ -n "${issue_url}" ]]; then
echo "ISSUE_URL=${issue_url}"
else
echo "ISSUE_URL="
fi
Length of output: 4292
@coderabbit-user Issue created: #1492
Details:
- Found 8 occurrences across docs/openapi.json and docs/openapi.md.
- Issue includes scope, acceptance criteria, and backlinks to this PR and the specific comment.
- Assigned to
@tisnik.
Want me to also open a quick follow-up PR to fix these lines now?
| ], | ||
| "summary": "List Mcp Servers Handler", | ||
| "description": "List all registered MCP servers.\n\nReturns both statically configured (from YAML) and dynamically\nregistered (via API) MCP servers.\n\nRaises:\n HTTPException: If configuration is not loaded.\n\nReturns:\n MCPServerListResponse: List of all registered MCP servers with source info.", | ||
| "description": "List all registered MCP servers.\n\nReturns both statically configured (from YAML) and dynamically\nregistered (via API) MCP servers.\n\n### Parameters:\n- request: Model containing attributes to dynamically registering an MCP server.\n- auth: Authentication tuple from the auth dependency (used by middleware).\n\n### Raises:\n- HTTPException: If configuration is not loaded.\n\n### Returns:\n- MCPServerListResponse: List of all registered MCP servers with source info.", |
There was a problem hiding this comment.
Incorrect parameter description for request in MCP servers list endpoint
At Line 666, request is documented as a model for registering an MCP server, which is unrelated to this GET endpoint. Please update it to describe the incoming HTTP request (middleware context).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi.json` at line 666, The parameter description for `request` in
the MCP servers list endpoint is incorrect: replace the line that describes
`request` as "Model containing attributes to dynamically registering an MCP
server" with a brief description stating it represents the incoming HTTP request
/ middleware context (e.g., "Incoming HTTP request object used by middleware,
containing headers, client info and context"); keep the rest of the endpoint doc
(including references to `auth` and `MCPServerListResponse`) unchanged so the
GET handler documentation correctly reflects that `request` is the HTTP request,
not a registration model.
| ], | ||
| "summary": "Register Mcp Server Handler", | ||
| "description": "Register an MCP server dynamically at runtime.\n\nAdds the MCP server to the runtime configuration and registers it\nas a toolgroup with Llama Stack so it becomes available for queries.\n\nRaises:\n HTTPException: On duplicate name, Llama Stack connection error,\n or registration failure.\n\nReturns:\n MCPServerRegistrationResponse: Details of the newly registered server.", | ||
| "description": "Register an MCP server dynamically at runtime.\n\nAdds the MCP server to the runtime configuration and registers it\nas a toolgroup with Llama Stack so it becomes available for queries.\n\n### Parameters:\n- request: Model containing attributes to dynamically registering an MCP server.\n- auth: Authentication tuple from the auth dependency (used by middleware).\n- body: Headers that should be pass to MCP servers.\n\n### Raises:\n- HTTPException: On duplicate name, Llama Stack connection error, or\n registration failure.\n\n### Returns:\n- MCPServerRegistrationResponse: Details of the newly registered server.", |
There was a problem hiding this comment.
body parameter description is inaccurate for MCP server registration
At Line 770, body is documented as “Headers that should be pass to MCP servers,” but this operation’s body is MCPServerRegistrationRequest (server registration payload). Please align the description with the actual request schema fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi.json` at line 770, The OpenAPI description incorrectly describes
the operation body as headers; update the operation description to reflect that
the request body is a MCPServerRegistrationRequest (the server registration
payload) and summarize its fields (e.g., name, url, optional headers, metadata)
instead of saying "Headers that should be pass to MCP servers"; locate the text
around the MCP server registration operation and replace the `body` description
with a concise note that the body is an MCPServerRegistrationRequest containing
registration attributes (mention MCPServerRegistrationRequest by name) and, if
helpful, list the key fields that are part of that schema.
| ], | ||
| "summary": "Delete Mcp Server Handler", | ||
| "description": "Unregister a dynamically registered MCP server.\n\nRemoves the MCP server from the runtime configuration and unregisters\nits toolgroup from Llama Stack. Only servers registered via the API\ncan be deleted; statically configured servers cannot be removed.\n\nRaises:\n HTTPException: If the server is not found, is statically configured,\n or Llama Stack unregistration fails.\n\nReturns:\n MCPServerDeleteResponse: Confirmation of the deletion.", | ||
| "description": "Unregister a dynamically registered MCP server.\n\nRemoves the MCP server from the runtime configuration and unregisters\nits toolgroup from Llama Stack. Only servers registered via the API\ncan be deleted; statically configured servers cannot be removed.\n\n### Parameters:\n- request: Model containing attributes to dynamically registering an MCP server.\n- auth: Authentication tuple from the auth dependency (used by middleware).\n- name: MCP server name\n\n### Raises:\n- HTTPException: If the server is not found, is statically configured, or\n Llama Stack unregistration fails.\n\n### Returns:\n- MCPServerDeleteResponse: Confirmation of the deletion.", |
There was a problem hiding this comment.
Delete endpoint request description appears copied from registration flow
At Line 934, request is described as a model for dynamic registration, which does not match the delete operation. This should document the incoming request context instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi.json` at line 934, The OpenAPI description for the Delete
endpoint incorrectly reuses the registration "request" description; update the
documentation under the Unregister/Delete operation to accurately describe the
incoming request context (e.g., that the delete uses path parameter "name"
and/or an empty body or authentication context) instead of describing dynamic
registration model; locate the "request" param description near the "Unregister
a dynamically registered MCP server." text and replace it with a concise
description such as "Delete request context (no body); uses path parameter
'name' and auth tuple from the auth dependency" and keep the rest of the
Raises/Returns text intact (reference: the "request" parameter and
"MCPServerDeleteResponse" in the same operation).
| ### Returns: | ||
| HTMLResponse: The HTML content of the index page, including a heading, | ||
| embedded image with the service icon, and links to the API documentation | ||
| via Swagger UI and ReDoc. | ||
| - HTMLResponse: The HTML content of the index page, including a heading, |
There was a problem hiding this comment.
Markdown heading spacing is violating MD022 across changed sections
Changed ### Parameters:, ### Returns:, and ### Raises: headings frequently miss required surrounding blank lines. This is already flagged by markdownlint and should be normalized to keep docs CI clean.
Also applies to: 128-129, 346-347, 434-435, 1301-1302, 1584-1585, 1864-1865, 2022-2023, 2148-2151, 3710-3715, 3791-3795, 3858-3862, 3926-3931
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 69-69: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi.md` around lines 69 - 70, Multiple markdown headings like "###
Parameters:", "### Returns:", and "### Raises:" are missing required surrounding
blank lines (MD022); edit docs/openapi.md to insert a blank line before and
after each of these headings wherever they appear (e.g., the "### Parameters:",
"### Returns:", and "### Raises:" blocks noted in the comment) so that each
heading is separated from adjacent paragraph/list content, and normalize similar
occurrences at the other ranges listed to satisfy markdownlint.
| - request: The incoming HTTP request (used by middleware). | ||
| - query_request: Request to the LLM. | ||
| - auth: Auth context tuple resolved from the authentication dependency. | ||
| - mcp_headers: Headers that should be pass to MCP servers. |
There was a problem hiding this comment.
Fix docstring typo in parameter description.
Use “should be passed to MCP servers” instead of “should be pass.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/endpoints/query.py` at line 113, Fix the docstring typo for the
mcp_headers parameter in the function/method docstring in
src/app/endpoints/query.py by changing "Headers that should be pass to MCP
servers." to "Headers that should be passed to MCP servers."—update the
description for the parameter name mcp_headers accordingly.
| ### Raises: | ||
| - HTTPException: If no active stream for the given request ID can be interrupted. |
There was a problem hiding this comment.
Raises section is incomplete for this handler.
The function also raises HTTPException for forbidden interruption attempts (CancelStreamResult.FORBIDDEN), not only missing stream IDs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/endpoints/stream_interrupt.py` around lines 59 - 60, The docstring
Raises section for the stream interrupt handler is incomplete; update the
handler's docstring (e.g., interrupt_stream_handler / interrupt_stream) to state
that it raises HTTPException both when no active stream exists for the given
request ID and when interruption is forbidden (CancelStreamResult.FORBIDDEN),
specifying the 404 case for missing stream and 403 for forbidden interruption
(include CancelStreamResult.FORBIDDEN in the description).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Description
LCORE-1348: better docstrings for REST API endpoints, part 2
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
mcp_headersfor query, streaming, and MCP-related endpoints