Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Oct 9, 2025

Description

LCORE-632: updated OpenAPI Swagger

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-632

Summary by CodeRabbit

  • Documentation
    • Added docs for new endpoints: GET /v1/tools, GET /v1/providers, GET /v1/providers/{provider_id}, and PUT /v2/conversations/{conversation_id}.
    • Introduced models: ToolsResponse, ProviderResponse, ProvidersListResponse, ConversationUpdateRequest, ConversationUpdateResponse.
    • Expanded provider endpoint details with clearer responses and explicit 404/422/500 errors.
    • Added BYOK RAG configuration (ByokRag) and exposed byok_rag in global Configuration.
    • Refined schema structures, examples, and descriptions for providers, tools, and configuration to improve clarity.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

OpenAPI artifacts and docs were updated to add provider, tool, and conversation update endpoints and models. A new ByokRag schema and byok_rag configuration field were introduced. Providers-related schemas were restructured. Documentation pages were synchronized to reflect these endpoints, models, and configuration changes.

Changes

Cohort / File(s) Summary of Changes
OpenAPI JSON spec
docs/openapi.json
Revised Providers endpoints descriptions and error responses; redefined ProviderResponse; added ProvidersListResponse; introduced ByokRag schema; added byok_rag to Configuration; enhanced GET provider by ID responses (404/422/500).
OpenAPI Markdown docs
docs/openapi.md
Documented new endpoints: GET /v1/tools, GET /v1/providers, GET /v1/providers/{provider_id}, PUT /v2/conversations/{conversation_id}; added models: ToolsResponse, ProviderResponse, ProvidersListResponse, ConversationUpdateRequest/Response, ByokRag; added byok_rag in Configuration.
Generated/output docs
docs/output.md
Reflected the new endpoints and models; added BYOK RAG config in global configuration; noted the documentation-only nature of changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant API as API Gateway
  participant ProviderSvc as Providers Service

  Client->>API: GET /v1/providers
  API->>ProviderSvc: Fetch providers grouped by API type
  ProviderSvc-->>API: ProvidersListResponse
  API-->>Client: 200 ProvidersListResponse
  alt Not found/invalid
    API-->>Client: 404/422 error
  else Server error
    API-->>Client: 500 error
  end
Loading
sequenceDiagram
  autonumber
  actor Client
  participant API as API Gateway
  participant ConvSvc as Conversations Service

  Client->>API: PUT /v2/conversations/{id} (ConversationUpdateRequest)
  API->>ConvSvc: Update conversation topic summary
  ConvSvc-->>API: ConversationUpdateResponse
  API-->>Client: 200 ConversationUpdateResponse
  alt Invalid request
    API-->>Client: 422 error
  else Server error
    API-->>Client: 500 error
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

I twitch my whiskers at fresh endpoints bright,
Tools and providers hop into the light.
BYOK trails through clovered RAGs,
Conversations don updated tags.
With schemas tidy, docs in bloom—
I thump approval: bun-ba-dum. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly indicates that the pull request updates the OpenAPI/Swagger definitions, which directly reflects the extensive changes to the API schema and documentation without including unnecessary detail or unrelated information.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
docs/openapi.md (1)

104-109: Add blank lines around new tables

markdownlint (MD058) flags these tables because there’s no blank line separating them from surrounding content. Please insert an empty line after each table (likewise for the other new tables you added) to keep docs lint-clean.
Example:

 | 200 | Successful Response | [ToolsResponse](#toolsresponse) |
 | 500 | Connection to Llama Stack is broken or MCP server error |  |
+
 ## GET `/v1/shields`
docs/output.md (1)

104-109: Add blank lines after the inserted tables

Same MD058 warning here—the tables you just added need a blank line separating them from the next heading. Please add an empty line after each of the new tables (tools/providers/conversation updates) to satisfy markdownlint.

 | 200 | Successful Response | [ToolsResponse](#toolsresponse) |
 | 500 | Connection to Llama Stack is broken or MCP server error |  |
+
 ## GET `/v1/shields`
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8523376 and dd6ef7f.

📒 Files selected for processing (3)
  • docs/openapi.json (6 hunks)
  • docs/openapi.md (8 hunks)
  • docs/output.md (8 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/output.md

109-109: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


162-162: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


195-195: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


559-559: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

docs/openapi.md

109-109: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


162-162: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


195-195: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


559-559: 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). (2)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests (ci)

Comment on lines +299 to +307
"404": {
"response": "Provider with given id not found",
"description": "Not Found"
},
"500": {
"response": "Unable to retrieve list of providers",
"cause": "Connection to Llama Stack is broken"
"cause": "Connection to Llama Stack is broken",
"description": "Internal Server Error"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix invalid error response shape

Response objects may only carry description, headers, content, or links. Adding response/cause keys at the top level breaks OpenAPI validation, so clients and tooling will reject the spec. Please move these details inside the JSON payload (e.g. reuse ErrorResponse) and drop the extraneous fields.

-                    "response": "Provider with given id not found",
-                    "description": "Not Found"
+                    "description": "Not Found",
+                    "content": {
+                        "application/json": {
+                            "schema": {
+                                "$ref": "#/components/schemas/ErrorResponse"
+                            },
+                            "example": {
+                                "detail": {
+                                    "response": "Provider with given id not found"
+                                }
+                            }
+                        }
+                    }
                 },
                 "500": {
-                    "response": "Unable to retrieve list of providers",
-                    "cause": "Connection to Llama Stack is broken",
-                    "description": "Internal Server Error"
+                    "description": "Internal Server Error",
+                    "content": {
+                        "application/json": {
+                            "schema": {
+                                "$ref": "#/components/schemas/ErrorResponse"
+                            },
+                            "example": {
+                                "detail": {
+                                    "response": "Unable to retrieve list of providers",
+                                    "cause": "Connection to Llama Stack is broken"
+                                }
+                            }
+                        }
+                    }
                 },
📝 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.

Suggested change
"404": {
"response": "Provider with given id not found",
"description": "Not Found"
},
"500": {
"response": "Unable to retrieve list of providers",
"cause": "Connection to Llama Stack is broken"
"cause": "Connection to Llama Stack is broken",
"description": "Internal Server Error"
},
"404": {
"description": "Not Found",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
},
"example": {
"detail": {
"response": "Provider with given id not found"
}
}
}
}
},
"500": {
"description": "Internal Server Error",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
},
"example": {
"detail": {
"response": "Unable to retrieve list of providers",
"cause": "Connection to Llama Stack is broken"
}
}
}
}
},
🤖 Prompt for AI Agents
In docs/openapi.json around lines 299 to 307, the 404 and 500 response objects
include invalid top-level keys ("response" and "cause") that break OpenAPI
validation; remove those extraneous keys and replace them with a proper
"content" entry using application/json that references or embeds the existing
ErrorResponse schema (or an equivalent object with message/cause fields) so the
human-readable message and cause are conveyed in the JSON payload instead of as
top-level properties, and keep only allowed response fields (description,
headers, content, links).

@tisnik tisnik merged commit 622b11d into lightspeed-core:main Oct 9, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant