-
Notifications
You must be signed in to change notification settings - Fork 52
LCORE-390: fields in response structures #399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LCORE-390: fields in response structures #399
Conversation
WalkthroughExpanded OpenAPI/Markdown docs and Pydantic response metadata: added an Attachment schema; extended QueryRequest with attachments, no_tools, and media_type; enriched endpoint descriptions, error semantics, and many field descriptions/examples; consolidated DatabaseConfiguration and tightened Configuration.authentication. No runtime logic changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (10)
src/models/responses.py (2)
11-34: Consider introducing a typed model formodelsinstead ofdict[str, Any].Good addition of description and examples. For stronger schema guarantees, consider defining a dedicated Pydantic model (e.g.,
ModelDescriptor) so docs and validation are consistent (and avoid double fields likeapi_model_typevsmodel_typein examples).If you want, I can draft a
ModelDescriptormodel and update the response schema accordingly.
56-61: Optional: Typeconversation_idasUUIDfor stronger validation.If you intend to guarantee UUID format, consider
Optional[UUID]rather thanOptional[str]. This will improve validation and OpenAPI output (adds aformat: uuid).Would you like me to update this to
Optional[UUID]and scan usages to ensure compatibility?docs/openapi.md (3)
211-217: Nit: Minor grammar fix for the conversation retrieval description.Consider smoothing the narrative (“It then fetches…”, “and returns it…”) for readability.
-Retrieve a conversation's chat history by its ID. Then fetches -the conversation session from the Llama Stack backend, -simplifies the session data to essential chat history, and -returns it in a structured response. Raises HTTP 400 for +Retrieve a conversation's chat history by its ID. It then fetches +the conversation session from the Llama Stack backend, +simplifies the session data to essential chat history, and +returns it in a structured response. Raises HTTP 400 for invalid IDs, 404 if not found, 503 if the backend is unavailable, and 500 for unexpected errors.
218-224: Add parameter description to the table below.You’ve added a clear “Parameters” section, but the parameters table further down (Line 231) has an empty “Description” cell for
conversation_id. Mirror the description there for consistency.
419-421: Use “Username” (one word).Update the wording for consistency and common usage.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |docs/output.md (3)
185-189: Use “cannot” (one word) in 500 description.Minor language polish.
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | +| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |
233-239: Nit: Minor grammar tweak for the conversation retrieval description.Align with similar suggestion in openapi.md.
-Retrieve a conversation's chat history by its ID. Then fetches +Retrieve a conversation's chat history by its ID. It then fetches the conversation session from the Llama Stack backend, simplifies the session data to essential chat history, and returns it in a structured response. Raises HTTP 400 for
441-443: Use “Username” (one word).Keep term usage consistent across docs.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |docs/openapi.json (2)
810-824: Use “Username” (one word) in description; consider “ID” capitalization.Minor consistency: description currently says “User name”. Prefer “Username”. Titles also use “User Id/Username”; consider “User ID”.
- "title": "Username", - "description": "User name", + "title": "Username", + "description": "Username",
347-347: Use “cannot” (one word) in the 500 description.Minor language tweak for consistency.
- "description": "User feedback can not be stored", + "description": "User feedback cannot be stored",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/openapi.json(11 hunks)docs/openapi.md(8 hunks)docs/output.md(16 hunks)src/models/responses.py(4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/output.md
[style] ~188-~188: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...biddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
[style] ~442-~442: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
docs/openapi.md
[style] ~420-~420: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
⏰ 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: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (26)
src/models/responses.py (2)
62-67: LGTM: Field metadata improves schema clarity.The
responsefield description and example are appropriate and helpful for consumers.
98-106: LGTM: Clearer InfoResponse field docs.Descriptions and examples for
nameandversionlook good and align with the documented endpoints.docs/openapi.md (6)
174-179: LGTM: Feedback status endpoint description and return type are clear.The brief description and explicit return type improve consumer understanding.
248-255: LGTM: DELETE conversation endpoint docs are much clearer.The validation/error pathways and explicit return type are helpful.
604-605: LGTM: ErrorResponse.detail description aligns with code.The concise description “Error details” is appropriate.
736-738: LGTM: InfoResponse field descriptions are crisp.These match the Pydantic metadata and improve docs.
818-819: LGTM: ModelsResponse field description is clear.
902-904: LGTM: QueryResponse field descriptions are accurate.docs/output.md (14)
59-68: LGTM: Expanded /v1/models docs add useful behavior and error context.The “Raises” and “Returns” sections are helpful for clients.
157-159: LGTM: Feedback endpoint docs tightened.Clear description and explicit 500-path for storage failures.
Also applies to: 170-172
196-201: LGTM: Feedback status docs and return type are explicit.
210-210: LGTM: Explicit component reference for status response.
225-225: LGTM: Conversations list response correctly references the component.
260-260: LGTM: Response component links are correct.Also applies to: 263-263
270-277: LGTM: DELETE conversation docs improved.
291-291: LGTM: Readiness response codes correctly mapped to components.Also applies to: 294-294
332-333: LGTM: Liveness response codes correctly mapped to components.
354-356: LGTM: Authorized endpoint response mappings are explicit.
626-626: LGTM: ErrorResponse.detail description reads well.
748-750: LGTM: InfoResponse field descriptions match the code.
830-830: LGTM: ModelsResponse field description is precise.
914-916: LGTM: QueryResponse field descriptions are aligned.docs/openapi.json (4)
872-873: Schema change:Configuration.authenticationno longer nullable — verify runtime compatibility.This moves from
anyOf [AuthenticationConfiguration, null]to a direct$refwith a default. If the server previously allowedauthentication: null, this is now disallowed per the schema.Do we intend to disallow null? Ensure the runtime config loader rejects/handles null accordingly. I can scan the codebase for
authenticationusage to confirm assumptions if you’d like.
1406-1420: LGTM: InfoResponse field metadata and examples are well-formed.Matches the Pydantic definitions and improves generated docs.
1579-1600: LGTM: ModelsResponsemodelsfield has clear title/description/examples.This will render nicely in tools.
1891-1904: LGTM: QueryResponse field metadata is clear and consistent.Examples and descriptions align with intent.
| "description": "Handle requests to the /models endpoint.\n\nProcess GET requests to the /models endpoint, returning a list of available\nmodels from the Llama Stack service.\n\nRaises:\n HTTPException: If unable to connect to the Llama Stack server or if\n model retrieval fails for any reason.\n\nReturns:\n ModelsResponse: An object containing the list of available models.", | ||
| "operationId": "models_endpoint_handler_v1_models_get", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
OpenAPI responses contain invalid top-level keys (examples placed outside content).
Several path responses (e.g., 200 for /v1/models, /v1/info, /v1/query, /v1/conversations, etc.) include properties like models, conversation_id, response, chat_history, etc., at the same level as description/content. In OpenAPI, examples must be placed under content -> media type -> example or examples. Keeping them at the response root violates the spec and will break validators/clients.
Run this script to detect non-standard keys under response objects:
As a fix pattern, move those objects under content.application/json.example. For instance, for /v1/models 200:
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": { "$ref": "#/components/schemas/ModelsResponse" },
"example": {
"models": [ { "identifier": "...", "type": "model" } ]
}
}
}
}🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Finds response objects that contain keys other than the allowed set:
# description, headers, content, links, $ref, and extension keys (x-*)
jq -r '
.paths
| to_entries[] as $p
| $p.value
| to_entries[] as $m
| ($m.value.responses // {})
| to_entries[]
| . as $r
| ($r.value | keys_unsorted
| map(select(. as $k | . != "description" and . != "headers" and . != "content" and . != "links" and . != "$ref" and (startswith("x-") | not)))
) as $bad
| select($bad | length > 0)
| "\($p.key) \($m.key) status \($r.key) invalid keys: \($bad|join(", "))"
' docs/openapi.jsonLength of output: 1445
Move non-standard response keys into content (fix required)
The jq check confirms multiple responses in docs/openapi.json contain invalid top-level keys (examples/fields placed outside content), which violates the OpenAPI spec and will break validators/clients.
Locations found (file: docs/openapi.json):
- /v1/info GET 200 — invalid keys: name, version
- /v1/models GET 200 — invalid keys: models
- /v1/query POST 200 — invalid keys: conversation_id, response
- /v1/query POST 503 — invalid keys: detail
- /v1/config GET 200 — invalid keys: name, service, llama_stack, user_data_collection, mcp_servers
- /v1/config GET 503 — invalid keys: detail
- /v1/conversations GET 200 — invalid keys: conversations
- /v1/conversations GET 503 — invalid keys: detail
- /v1/conversations/{conversation_id} GET 200 — invalid keys: conversation_id, chat_history
- /v1/conversations/{conversation_id} GET 404 — invalid keys: detail
- /v1/conversations/{conversation_id} GET 503 — invalid keys: detail
- /v1/conversations/{conversation_id} DELETE 200 — invalid keys: conversation_id, success, message
- /v1/conversations/{conversation_id} DELETE 404 — invalid keys: detail
- /v1/conversations/{conversation_id} DELETE 503 — invalid keys: detail
Fix pattern (example) — move these fields under content -> application/json -> example (and include a matching schema):
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": { "$ref": "#/components/schemas/ModelsResponse" },
"example": {
"models": [{ "identifier": "ggml-model", "type": "model" }]
}
}
}
}Error example (503):
"503": {
"description": "Service Unavailable",
"content": {
"application/json": {
"schema": { "$ref": "#/components/schemas/ErrorResponse" },
"example": { "detail": "Llama Stack unreachable" }
}
}
}Notes: allowed response root keys are description, headers, content, links, $ref, and vendor extensions (x-*). Move all other top-level fields (models, conversation_id, response, detail, etc.) into content.application/json.example or content.application/json.examples and ensure corresponding schemas exist under #/components/schemas. Re-run an OpenAPI validator after updating.
🤖 Prompt for AI Agents
In docs/openapi.json around lines 77-78, multiple response objects include
non-standard top-level keys (e.g., models, conversation_id, response, detail,
name, version, etc.) which violate OpenAPI; move those fields into the
response.content.application/json.example (or examples) and attach a matching
schema under components/schemas (or reference an existing one) for each response
code (e.g., 200 -> ModelsResponse, 503 -> ErrorResponse); ensure each response
object only uses allowed root keys (description, headers, content, links, $ref,
x-*) and then re-run an OpenAPI validator to confirm all invalid top-level keys
are removed.
| "title": "Detail", | ||
| "description": "Error details", | ||
| "examples": [ | ||
| { | ||
| "detail": { | ||
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | ||
| "response": "Error while validation question" | ||
| } | ||
| }, | ||
| { | ||
| "detail": { | ||
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | ||
| "response": "Error retrieving conversation history" | ||
| } | ||
| } | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix: ErrorResponse.detail schema-level examples are incorrectly nested.
Examples for the detail property should illustrate the map value (response/cause), not include another detail key.
Apply this diff to flatten examples:
- "examples": [
- { "detail": { "cause": "...", "response": "..." } },
- { "detail": { "cause": "...", "response": "..." } }
- ]
+ "examples": [
+ { "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text",
+ "response": "Error validating question" },
+ { "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000",
+ "response": "Error retrieving conversation history" }
+ ]Also corrected the grammar (“Error validating question”).
📝 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.
| "title": "Detail", | |
| "description": "Error details", | |
| "examples": [ | |
| { | |
| "detail": { | |
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | |
| "response": "Error while validation question" | |
| } | |
| }, | |
| { | |
| "detail": { | |
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | |
| "response": "Error retrieving conversation history" | |
| } | |
| } | |
| ] | |
| "title": "Detail", | |
| "description": "Error details", | |
| "examples": [ | |
| { | |
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | |
| "response": "Error validating question" | |
| }, | |
| { | |
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | |
| "response": "Error retrieving conversation history" | |
| } | |
| ] |
🤖 Prompt for AI Agents
In docs/openapi.json around lines 1152 to 1167, the schema-level examples for
ErrorResponse.detail are incorrectly nested under an extra "detail" key and one
message has bad grammar; update the examples to be maps that directly contain
"cause" and "response" (remove the redundant "detail" wrapper) and change "Error
while validation question" to "Error validating question" so each example looks
like { "cause": "...", "response": "..." }.
| detail: dict[str, str] = Field( | ||
| description="Error details", | ||
| examples=[ | ||
| { | ||
| "detail": { | ||
| "response": "Error while validation question", | ||
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | ||
| }, | ||
| }, | ||
| { | ||
| "detail": { | ||
| "response": "Error retrieving conversation history", | ||
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | ||
| }, | ||
| }, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix: ErrorResponse.detail examples are incorrectly shaped (nested detail).
The examples provided here should represent the value of the detail field itself (a mapping of string->string), not an object containing another detail key. This shape mismatch will propagate to generated OpenAPI and confuse consumers.
Apply this diff to correct the examples:
- detail: dict[str, str] = Field(
+ detail: dict[str, str] = Field(
description="Error details",
examples=[
- {
- "detail": {
- "response": "Error while validation question",
- "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text",
- },
- },
- {
- "detail": {
- "response": "Error retrieving conversation history",
- "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000",
- },
- },
+ {
+ "response": "Error validating question",
+ "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text",
+ },
+ {
+ "response": "Error retrieving conversation history",
+ "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000",
+ },
],
)Note: also corrected the grammar from “Error while validation question” to “Error validating question”.
📝 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.
| detail: dict[str, str] = Field( | |
| description="Error details", | |
| examples=[ | |
| { | |
| "detail": { | |
| "response": "Error while validation question", | |
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | |
| }, | |
| }, | |
| { | |
| "detail": { | |
| "response": "Error retrieving conversation history", | |
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | |
| }, | |
| }, | |
| ], | |
| ) | |
| detail: dict[str, str] = Field( | |
| description="Error details", | |
| examples=[ | |
| { | |
| "response": "Error validating question", | |
| "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", | |
| }, | |
| { | |
| "response": "Error retrieving conversation history", | |
| "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", | |
| }, | |
| ], | |
| ) |
🤖 Prompt for AI Agents
In src/models/responses.py around lines 526 to 542, the examples for
ErrorResponse.detail are incorrectly nested under another "detail" key and one
message has bad grammar; update the examples so each example is directly a
mapping of string->string (e.g. {"response": "Error validating question",
"cause": "..."}) instead of {"detail": {...}}, and fix the grammar "Error while
validation question" -> "Error validating question".
4ae4e74 to
6421f35
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
docs/openapi.json (2)
530-542: Schema mismatch: “message” vs. “response” in ConversationDeleteResponseThe example uses “message”, but the schema defines “response”. Also, example fields should be under content.example, not at the response root.
- "conversation_id": "123e4567-e89b-12d3-a456-426614174000", - "success": true, - "message": "Conversation deleted successfully" + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ConversationDeleteResponse" }, + "example": { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "success": true, + "response": "Conversation deleted successfully" + } + } + }
934-1002: Docs/schema inconsistency: ConversationDetails examples use “model”, but schema defines “last_used_model/last_used_provider”The examples under ConversationsListResponse and the narrative in docs reference “model”, while the schema defines “last_used_model” and “last_used_provider”. Align examples and docs with the schema.
- "message_count": 5, - "model": "gemini/gemini-2.0-flash" + "message_count": 5, + "last_used_model": "gemini/gemini-2.0-flash", + "last_used_provider": "gemini"Apply similar changes to all conversation examples across the spec and markdown docs.
♻️ Duplicate comments (6)
docs/openapi.json (6)
56-68: OpenAPI violation: invalid top-level keys under responses (must be under content.example)The 200 response includes “name” and “version” at the same level as “description”/“content”, which is invalid in OpenAPI. Move example payloads under content.application/json.example.
Apply this pattern:
"responses": { "200": { "description": "Successful Response", - "content": { - "application/json": { - "schema": { "$ref": "#/components/schemas/InfoResponse" } - } - }, - "name": "Service name", - "version": "Service version" + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/InfoResponse" }, + "example": { + "name": "Lightspeed Stack", + "version": "0.1.3" + } + } + } } }Replicate for all other responses with stray fields.
80-114: OpenAPI violation: stray “models” at response rootThe 200 response for /v1/models places “models” at the response root. Move the example under content.application/json.example.
"200": { "description": "Successful Response", - "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ModelsResponse" } } }, - "models": [ { ... }, { ... } ] + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ModelsResponse" }, + "example": { + "models": [ + { "identifier": "all-MiniLM-L6-v2", "metadata": { "embedding_dimension": 384 }, "api_model_type": "embedding", "provider_id": "ollama", "provider_resource_id": "all-minilm:latest", "type": "model", "model_type": "embedding" }, + { "identifier": "llama3.2:3b-instruct-fp16", "metadata": {}, "api_model_type": "llm", "provider_id": "ollama", "provider_resource_id": "llama3.2:3b-instruct-fp16", "type": "model", "model_type": "llm" } + ] + } + } + } }
137-175: OpenAPI violation: response example fields at root; 503 error lacks schema/content
- 200 response has “conversation_id” and “response” at the root.
- 503 response has “detail” at the root and misses content/schema.
Move examples under content and attach schemas (QueryResponse for 200, ErrorResponse for 503).
"200": { "description": "Successful Response", - "content": { "application/json": { "schema": { "$ref": "#/components/schemas/QueryResponse" } } }, - "conversation_id": "123e4567-e89b-12d3-a456-426614174000", - "response": "LLM answer" + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/QueryResponse" }, + "example": { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "response": "LLM answer" + } + } + } }, "503": { - "description": "Service Unavailable", - "detail": { "response": "Unable to connect to Llama Stack", "cause": "Connection error." } + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Unable to connect to Llama Stack", + "cause": "Connection error." + } + } + } + } }
239-294: OpenAPI violation: /v1/config responses carry payload at rootBoth 200 and 503 include payload keys at the response root. Move into content.application/json.example and reference appropriate schema (Configuration and ErrorResponse).
#!/usr/bin/env bash # Detect invalid response keys across the spec (allowed: description, headers, content, links, $ref, x-*) jq -r ' .paths | to_entries[] as $p | $p.value | to_entries[] as $m | ($m.value.responses // {}) | to_entries[] | . as $r | ($r.value | keys_unsorted | map(select(. as $k | . != "description" and . != "headers" and . != "content" and . != "links" and . != "$ref" and (startswith("x-") | not))) ) as $bad | select($bad | length > 0) | "\($p.key) \($m.key) status \($r.key) invalid keys: \($bad|join(", "))" ' docs/openapi.json
409-435: OpenAPI violation: /v1/conversations (list) responses not under content; 503 lacks schema
- 200: “conversations” array is at the root, not inside content.example.
- 503: “detail” at root; attach ErrorResponse schema.
Also ensure list examples match the ConversationDetails schema (see separate note below).
467-508: OpenAPI violation and schema mismatch in GET conversation
- 200: “conversation_id” and “chat_history” are placed at the response root — move into content.example.
- 404/503: “detail” at root — move under content with ErrorResponse schema.
🧹 Nitpick comments (6)
docs/openapi.md (1)
419-421: Use “Username” as a single wordMinor wording improvement for consistency with contemporary usage and other schemas.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |docs/output.md (3)
185-189: Prefer “cannot” over “can not”Improve wording in the 500 description.
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | +| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |
210-210: Markdown tables should be surrounded by blank linesMultiple tables trigger MD058. Add a blank line before/after each table to satisfy linters and improve readability. Apply globally to all tables (also at Lines 226, 263, 294, 314, 333, 356).
-### ✅ Responses -| Status Code | Description | Component | +### ✅ Responses + +| Status Code | Description | Component | ... -| 200 | Successful Response | [StatusResponse](#statusresponse) | +| 200 | Successful Response | [StatusResponse](#statusresponse) | +
441-443: Use “Username” as one wordAlign terminology with other parts of the spec.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |docs/openapi.json (2)
811-824: AuthorizedResponse: use “Username” as one wordMinor consistency improvement.
- "description": "User name", + "description": "Username",
1152-1163: Grammar in ErrorResponse.detail examplesChange “Error while validation question” to “Error validating question”.
- "response": "Error while validation question" + "response": "Error validating question"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/openapi.json(11 hunks)docs/openapi.md(8 hunks)docs/output.md(16 hunks)src/models/responses.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/models/responses.py
🧰 Additional context used
🪛 LanguageTool
docs/output.md
[style] ~188-~188: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...biddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
[style] ~442-~442: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
docs/openapi.md
[style] ~420-~420: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
🪛 markdownlint-cli2 (0.17.2)
docs/output.md
189-189: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
210-210: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
226-226: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
263-263: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
294-294: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
314-314: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
333-333: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
356-356: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ 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). (1)
- GitHub Check: e2e_tests
🔇 Additional comments (22)
docs/openapi.md (6)
174-179: Clarified feedback status response — LGTMThe added description and explicit StatusResponse return type improve clarity.
211-224: Conversation retrieval flow — clear and actionableGood expansion on behavior and error semantics. Ensure this matches actual error handling in the implementation (400/404/503/500) to avoid drift between docs and runtime.
248-255: DELETE conversation docs — LGTMClear validation and error pathways described with a concrete return type.
736-738: InfoResponse field descriptions — LGTM
818-819: ModelsResponse description — LGTM
902-904: QueryResponse field descriptions — LGTMdocs/output.md (12)
59-68: Expanded /v1/models behavior, raises, returns — LGTM
157-172: Feedback endpoint details — LGTM
196-201: Feedback status clarity — LGTM
225-226: Conversations list response — LGTM
233-246: GET conversation flow and errors — LGTM
260-264: GET conversation responses — LGTM
291-294: DELETE conversation responses — LGTM
332-333: Liveness responses — LGTM
626-627: ErrorResponse.detail description — LGTM
748-750: InfoResponse fields — LGTM
830-831: ModelsResponse models description — LGTM
914-916: QueryResponse fields — LGTMdocs/openapi.json (4)
872-877: Tightened Configuration.authentication (nullable removed) — confirm compatibilitySwitching from anyOf [AuthenticationConfiguration, null] to a direct $ref means if the field is provided it cannot be null anymore (though it remains optional). Verify no configs or clients rely on explicitly setting authentication: null.
1399-1417: InfoResponse field metadata — LGTM
1575-1596: ModelsResponse field metadata and examples — LGTM
1879-1900: QueryResponse field metadata — LGTM
|
@CodeRabbit fullreview |
|
@tisnik I'll conduct a full review of the PR. This appears to be a documentation enhancement that adds field descriptions and examples to response structures in the OpenAPI specifications and Pydantic models. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (11)
docs/openapi.md (1)
482-490: Align ConversationDetails examples with the current schema (last_used_model/provider)The examples still show a “model” field which is no longer part of ConversationDetails. Replace with last_used_model and last_used_provider to avoid confusing clients.
- conversation = ConversationSummary( + conversation = ConversationSummary( conversation_id="123e4567-e89b-12d3-a456-426614174000" created_at="2024-01-01T00:00:00Z", last_message_at="2024-01-01T00:05:00Z", message_count=5, - model="gemini/gemini-2.0-flash" + last_used_model="gemini/gemini-2.0-flash", + last_used_provider="gemini" )ConversationDetails( conversation_id="123e4567-e89b-12d3-a456-426614174000", created_at="2024-01-01T00:00:00Z", last_message_at="2024-01-01T00:05:00Z", message_count=5, - model="gemini/gemini-2.0-flash" + last_used_model="gemini/gemini-2.0-flash", + last_used_provider="gemini" ), ConversationDetails( conversation_id="456e7890-e12b-34d5-a678-901234567890" created_at="2024-01-01T01:00:00Z", message_count=2, - model="gemini/gemini-2.5-flash" + last_used_model="gemini/gemini-2.5-flash", + last_used_provider="gemini" )Also applies to: 553-560
src/models/responses.py (2)
441-456: Fix ConversationDetails docstring to match actual fieldsDocstring mentions “model” and uses ConversationSummary in the example; the model field is gone and last_used_model/last_used_provider were added. Update the docstring and example to prevent client-side confusion.
Attributes: conversation_id: The conversation ID (UUID). created_at: When the conversation was created. last_message_at: When the last message was sent. message_count: Number of user messages in the conversation. - model: The model used for the conversation. + last_used_model: The last model used for the conversation. + last_used_provider: The provider used for the last message. @@ - conversation = ConversationSummary( + conversation = ConversationSummary( conversation_id="123e4567-e89b-12d3-a456-426614174000" created_at="2024-01-01T00:00:00Z", last_message_at="2024-01-01T00:05:00Z", message_count=5, - model="gemini/gemini-2.0-flash" + last_used_model="gemini/gemini-2.0-flash", + last_used_provider="gemini" )
508-515: ConversationsListResponse examples: replace ‘model’ with last_used_model/providerExamples should reflect current schema to avoid consumer drift.
"message_count": 5, - "model": "gemini/gemini-2.0-flash", + "last_used_model": "gemini/gemini-2.0-flash", + "last_used_provider": "gemini", @@ "message_count": 2, - "model": "gemini/gemini-2.5-flash", + "last_used_model": "gemini/gemini-2.5-flash", + "last_used_provider": "gemini",docs/openapi.json (8)
56-68: Invalid keys at response root (OpenAPI violation): move fields under content.example“name” and “version” cannot exist alongside description/content. Place example payloads under content.application/json.example.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/InfoResponse" - } + }, + "example": { + "name": "Lightspeed Stack", + "version": "1.0.0" + } } - }, - "name": "Service name", - "version": "Service version" + } }
80-111: Invalid response root keys for /v1/models 200; move ‘models’ into exampleCurrently violates the spec; attach an inline example to the media type.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ModelsResponse" - } + }, + "example": { + "models": [ + { + "identifier": "all-MiniLM-L6-v2", + "metadata": { "embedding_dimension": 384 }, + "api_model_type": "embedding", + "provider_id": "ollama", + "provider_resource_id": "all-minilm:latest", + "type": "model", + "model_type": "embedding" + }, + { + "identifier": "llama3.2:3b-instruct-fp16", + "metadata": {}, + "api_model_type": "llm", + "provider_id": "ollama", + "provider_resource_id": "llama3.2:3b-instruct-fp16", + "type": "model", + "model_type": "llm" + } + ] + } } - }, - "models": [ - { ... }, - { ... } - ] + } },
137-149: /v1/query responses: move example fields and define 503 content schema200’s conversation_id/response and 503’s detail must be nested under content; 503 should also declare its schema.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/QueryResponse" - } + }, + "example": { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "response": "LLM answer" + } } - }, - "conversation_id": "123e4567-e89b-12d3-a456-426614174000", - "response": "LLM answer" + } }, @@ "503": { - "description": "Service Unavailable", - "detail": { - "response": "Unable to connect to Llama Stack", - "cause": "Connection error." - } + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Unable to connect to Llama Stack", + "cause": "Connection error." + } + } + } + } },Also applies to: 169-175
238-294: /v1/config responses: move the sample payloads into 200 example; structure 503Large inline example fields belong under content.example; 503 should use ErrorResponse.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/Configuration" - } + }, + "example": { + "name": "foo bar baz", + "service": { + "host": "localhost", + "port": 8080, + "auth_enabled": false, + "workers": 1, + "color_log": true, + "access_log": true, + "tls_config": { + "tls_certificate_path": "config/certificate.crt", + "tls_key_path": "config/private.key" + } + }, + "llama_stack": { + "url": "http://localhost:8321", + "api_key": "xyzzy", + "use_as_library_client": false + }, + "user_data_collection": { + "feedback_enabled": true, + "feedback_storage": "/tmp/data/feedback", + "transcripts_enabled": false + }, + "mcp_servers": [ + { "name": "server1", "provider_id": "provider1", "url": "http://url.com:1" }, + { "name": "server2", "provider_id": "provider2", "url": "http://url.com:2" }, + { "name": "server3", "provider_id": "provider3", "url": "http://url.com:3" } + ] + } } - }, - "name": "foo bar baz", - "service": { ... }, - "llama_stack": { ... }, - "user_data_collection": { ... }, - "mcp_servers": [ ... ] + } }, "503": { - "description": "Service Unavailable", - "detail": { - "response": "Configuration is not loaded" - } + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { "detail": { "response": "Configuration is not loaded" } } + } + } }
399-435: /v1/conversations list responses: move list into example; structure 503Same OpenAPI structure fix as above.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ConversationsListResponse" - } + }, + "example": { + "conversations": [ + { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "created_at": "2024-01-01T00:00:00Z", + "last_message_at": "2024-01-01T00:05:00Z", + "last_used_model": "gemini/gemini-1.5-flash", + "last_used_provider": "gemini", + "message_count": 5 + }, + { + "conversation_id": "456e7890-e12b-34d5-a678-901234567890", + "created_at": "2024-01-01T01:00:00Z", + "last_message_at": "2024-01-01T01:02:00Z", + "last_used_model": "gemini/gemini-2.0-flash", + "last_used_provider": "gemini", + "message_count": 2 + } + ] + } } - }, - "conversations": [ ... ] + } }, "503": { - "description": "Service Unavailable", - "detail": { - "response": "Unable to connect to Llama Stack", - "cause": "Connection error." - } + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Unable to connect to Llama Stack", + "cause": "Connection error." + } + } + } + } }
458-510: /v1/conversations/{id} GET: move 200 example and structure 404/503Place example payloads under content; also define ErrorResponse schema for 404/503.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ConversationResponse" - } + }, + "example": { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "chat_history": [ + { + "messages": [ + { "content": "Hi", "type": "user" }, + { "content": "Hello!", "type": "assistant" } + ], + "started_at": "2024-01-01T00:00:00Z", + "completed_at": "2024-01-01T00:00:05Z" + } + ] + } } - }, - "conversation_id": "123e4567-e89b-12d3-a456-426614174000", - "chat_history": [ ... ] + } }, "404": { - "detail": { - "response": "Conversation not found", - "cause": "The specified conversation ID does not exist." - }, - "description": "Not Found" + "description": "Not Found", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Conversation not found", + "cause": "The specified conversation ID does not exist." + } + } + } + } }, "503": { - "detail": { - "response": "Unable to connect to Llama Stack", - "cause": "Connection error." - }, - "description": "Service Unavailable" + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Unable to connect to Llama Stack", + "cause": "Connection error." + } + } + } + } },
530-556: DELETE conversation: move example under content and fix field name (message -> response)“message” is inconsistent with the ConversationDeleteResponse schema (“response”). Align field name and OpenAPI structure.
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ConversationDeleteResponse" - } + }, + "example": { + "conversation_id": "123e4567-e89b-12d3-a456-426614174000", + "success": true, + "response": "Conversation deleted successfully" + } } - }, - "conversation_id": "123e4567-e89b-12d3-a456-426614174000", - "success": true, - "message": "Conversation deleted successfully" + } }, "404": { - "detail": { - "response": "Conversation not found", - "cause": "The specified conversation ID does not exist." - }, - "description": "Not Found" + "description": "Not Found", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Conversation not found", + "cause": "The specified conversation ID does not exist." + } + } + } + } }, "503": { - "detail": { - "response": "Unable to connect to Llama Stack", - "cause": "Connection error." - }, - "description": "Service Unavailable" + "description": "Service Unavailable", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ErrorResponse" }, + "example": { + "detail": { + "response": "Unable to connect to Llama Stack", + "cause": "Connection error." + } + } + } + } },
56-68: Fix invalid OpenAPI response root keys in docs/openapi.jsonAutomated check found response objects with disallowed top-level keys. Move these fields into the response content schema (components/schemas), headers, or convert to x-* vendor extensions.
Files/locations needing attention (from verification):
- docs/openapi.json — /v1/info GET 200 — invalid keys: name, version
- docs/openapi.json — /v1/models GET 200 — invalid keys: models
- docs/openapi.json — /v1/query POST 200 — invalid keys: conversation_id, response
- docs/openapi.json — /v1/query POST 503 — invalid keys: detail
- docs/openapi.json — /v1/config GET 200 — invalid keys: name, service, llama_stack, user_data_collection, mcp_servers
- docs/openapi.json — /v1/config GET 503 — invalid keys: detail
- docs/openapi.json — /v1/conversations GET 200 — invalid keys: conversations
- docs/openapi.json — /v1/conversations GET 503 — invalid keys: detail
- docs/openapi.json — /v1/conversations/{conversation_id} GET 200 — invalid keys: conversation_id, chat_history
- docs/openapi.json — /v1/conversations/{conversation_id} GET 404 — invalid keys: detail
- docs/openapi.json — /v1/conversations/{conversation_id} GET 503 — invalid keys: detail
- docs/openapi.json — /v1/conversations/{conversation_id} DELETE 200 — invalid keys: conversation_id, success, message
- docs/openapi.json — /v1/conversations/{conversation_id} DELETE 404 — invalid keys: detail
- docs/openapi.json — /v1/conversations/{conversation_id} DELETE 503 — invalid keys: detail
Suggested corrected snippet for the /v1/info 200 response (remove name/version from response root):
"200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/InfoResponse" } } } }Action: remove non-standard keys from response objects and ensure the corresponding schema under components/schemas contains those fields (or expose them via headers or x-* if they're intended as extensions). Re-run the jq check after making changes.
🧹 Nitpick comments (10)
docs/openapi.md (1)
419-421: Prefer “Username” (one word) for consistency and styleMinor terminology polish to match common usage and the schema property title.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |src/models/responses.py (3)
526-538: Grammar fix in ErrorResponse.detail examplesUse “Error validating question” instead of “Error while validation question”.
detail: dict[str, str] = Field( description="Error details", examples=[ { - "response": "Error while validation question", + "response": "Error validating question", "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", }, { "response": "Error retrieving conversation history", "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", }, ], )
540-557: Mirror the grammar fix in top-level examplesKeep object-level examples aligned with the property-level wording.
"examples": [ { "detail": { - "response": "Error while validation question", + "response": "Error validating question", "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", }, }, { "detail": { "response": "Error retrieving conversation history", "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", }, }, ]
300-304: Consistency: “Username” vs “User name”Align with common term and the OpenAPI title casing.
username: str = Field( ..., - description="User name", + description="Username", examples=["John Doe", "Adam Smith"], )docs/openapi.json (2)
817-824: Consistency: “Username” vs “User name” in AuthorizedResponseMatch terminology with docs/openapi.md and common usage.
"username": { "type": "string", - "title": "Username", - "description": "User name", + "title": "Username", + "description": "Username", "examples": [ "John Doe", "Adam Smith" ] }
1146-1186: Grammar fix in ErrorResponse.detail examples (schema and object-level)Use “Error validating question” for clarity/grammar.
"examples": [ { - "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", - "response": "Error while validation question" + "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", + "response": "Error validating question" }, { "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", "response": "Error retrieving conversation history" } ] @@ "examples": [ { "detail": { "cause": "Failed to handle request to https://bam-api.res.ibm.com/v2/text", - "response": "Error while validation question" + "response": "Error validating question" } }, { "detail": { "cause": "Invalid conversation ID 1237-e89b-12d3-a456-426614174000", "response": "Error retrieving conversation history" } } ]docs/output.md (4)
185-189: Typo: “can not” → “cannot”Conform to standard usage.
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | +| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |
441-443: Prefer “Username” (one word) in field descriptionsKeep terminology consistent across docs/spec/code.
-| user_id | string | User ID, for example UUID | -| username | string | User name | +| user_id | string | User ID, for example UUID | +| username | string | Username |
378-379: Table pipe/style: add a trailing pipe for consistency and markdownlintMinor formatting tidy-up.
-| 200 | Successful Response | string - | +| 200 | Successful Response | string | +|
183-189: Markdownlint: add blank lines around tablesSeveral tables are missing surrounding blank lines (MD058). Add a blank line before and after each table block to silence lint and improve readability.
If you want, I can push a formatting-only commit to normalize all table blocks across this file.
Also applies to: 223-226, 260-264, 291-295, 313-315, 332-333, 354-356, 378-379
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/openapi.json(11 hunks)docs/openapi.md(8 hunks)docs/output.md(16 hunks)src/models/responses.py(4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/openapi.md
[style] ~420-~420: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
docs/output.md
[style] ~188-~188: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...biddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
[style] ~442-~442: It’s more common nowadays to write this noun as one word.
Context: ...or example UUID | | username | string | User name | ## Configuration Global service c...
(RECOMMENDED_COMPOUNDS)
🪛 markdownlint-cli2 (0.17.2)
docs/output.md
188-188: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
241-241: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
272-272: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
292-292: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
356-356: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
🔇 Additional comments (9)
docs/openapi.md (3)
174-179: Good clarity on feedback status semanticsThe added description and explicit return type improve API usability and DX.
211-224: Expanded GET conversation description, parameters, and error surface look solidClear guidance on behavior and error modes; explicit parameter doc is appreciated.
248-255: DELETE conversation: clear validation and error semanticsGood, concrete language about ID validation and backend error conditions.
src/models/responses.py (3)
11-34: ModelsResponse: field metadata and examples are aptThe description and realistic examples improve schema discoverability.
56-68: QueryResponse: added conversation_id/response metadata is appropriateDescriptions and examples are consistent across docs.
98-106: InfoResponse: concise and aligned metadataLooks good; matches the OpenAPI schema updates.
docs/openapi.json (1)
1568-1596: ModelsResponse schema metadata and examples are well-formedThe additions improve API consumers’ understanding; no issues spotted here.
docs/output.md (2)
59-68: Models endpoint: expanded description is helpfulClear return/raises section aligns with OpenAPI narrative.
157-172: Feedback endpoint narrative: good addition of Raises and processing stepsAdds necessary context without overloading the doc.
Description
LCORE-390: fields in response structures
Type of change
Related Tickets & Documents
Summary by CodeRabbit
New Features
Documentation