Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Dec 17, 2025

Description

LCORE-1032: REST API doc update

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

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1032

Summary by CodeRabbit

  • Documentation
    • Reorganized and expanded REST API diagrams and sections.
    • Added Conversation endpoints: list, retrieve, update, delete (GET/PUT/DELETE).
    • Added Feedback (POST /v1/feedback) and Status endpoints (GET /v1/status, PUT /v1/status).
    • Added Providers/Models/Tools/RAGs/Shields lookup endpoints (GET).
    • Updated Query endpoints: POST /v1/query, POST /v1/streaming_query, and new POST /infer flow with corresponding responses.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Walkthrough

Replaces the single PlantUML REST API diagram file to reorganize and expand the public API surface: adds styling, groups endpoints into Root/Metrics/Status/Info/Query/Conversations/Feedback/Providers+Models+Tools+RAGs, and introduces new endpoint paths and request/response types.

Changes

Cohort / File(s) Summary
REST API diagram
docs/rest_api.uml
Replaces/updates the PlantUML source: adds skin rose, aliases API components, reorganizes endpoints into named sections (Root, Metrics, Readiness/Liveness/Status, Info, Query, Streaming Query, Infer, Conversations, Feedback, Providers/Models/Tools/RAGs, Shields), introduces new paths (e.g., POST /infer, GET/PUT /v1/status, conversation CRUD), and expands request/response type declarations across the diagram.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check PlantUML syntax and render of the updated diagram.
  • Verify endpoint paths/methods and associated request/response names against implementation or API spec.
  • Confirm newly introduced endpoints (e.g., /infer, /v1/status, conversation CRUD, /v1/shields) are intended and correctly represented.

Possibly related PRs

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 and specifically summarizes the main change: updating REST API documentation with a ticket reference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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: 2

🧹 Nitpick comments (2)
docs/rest_api.uml (2)

147-150: Inconsistent path parameter notation.

Use {RAG_ID} placeholder syntax to match the convention used elsewhere (e.g., {conversation_id} on lines 91, 96, 101).

-Client->>API_server: GET /v1/rags/RAG_ID
+Client->>API_server: GET /v1/rags/{RAG_ID}

50-53: Consider moving authorized endpoint to its own section.

The /v1/authorized endpoint is grouped under "Readiness and liveness endpoints" but is semantically different—it's an authorization check rather than a health probe. Consider creating a separate section header (e.g., == Authorization endpoint ==) for clarity, or adding a brief comment explaining why it's grouped with health endpoints.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b49a58 and 22d820b.

⛔ Files ignored due to path filters (2)
  • docs/rest_api.png is excluded by !**/*.png
  • docs/rest_api.svg is excluded by !**/*.svg
📒 Files selected for processing (1)
  • docs/rest_api.uml (1 hunks)
⏰ 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). (8)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / azure
  • GitHub Check: E2E: server mode / ci
  • GitHub Check: E2E: library mode / azure
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: library mode / vertexai
  • GitHub Check: E2E: server mode / vertexai

@tisnik tisnik force-pushed the rest-api-doc-update branch from 22d820b to 5efd11e Compare December 17, 2025 16:28
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: 2

🧹 Nitpick comments (1)
docs/rest_api.uml (1)

39-54: Reorganize section to separate authorization from health checks.

The /v1/authorized endpoint (lines 50-53) is grouped under "Readiness and liveness endpoints" but serves a different purpose—it checks authorization status, not service health. Consider either:

  • Renaming the section to "Health and authorization endpoints"
  • Creating a separate section for the authorization endpoint

This improves documentation clarity by grouping endpoints by their functional purpose.

Apply this diff to split into separate sections:

 == Readiness and liveness endpoints ==
 Client->>API_server: GET /readiness
 API_server->> health_py: Request
 health_py-->>API_server: ReadinessResponse
 API_server-->>Client: ReadinessResponse
 
 Client->>API_server: GET /liveness
 API_server->> health_py: Request
 health_py-->>API_server: LivenessResponse
 API_server-->>Client: LivenessResponse
 
+
+== Authorization endpoint ==
 Client->>API_server: GET /v1/authorized
 API_server->> authorized_py: Request
 authorized_py-->>API_server: AuthorizedResponse
 API_server-->>Client: AuthorizedResponse
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22d820b and 5efd11e.

⛔ Files ignored due to path filters (2)
  • docs/rest_api.png is excluded by !**/*.png
  • docs/rest_api.svg is excluded by !**/*.svg
📒 Files selected for processing (1)
  • docs/rest_api.uml (1 hunks)
⏰ 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). (8)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: library mode / vertexai
  • GitHub Check: E2E: library mode / azure
  • GitHub Check: E2E: server mode / vertexai
  • GitHub Check: E2E: server mode / azure
  • GitHub Check: E2E: server mode / ci
🔇 Additional comments (3)
docs/rest_api.uml (3)

68-82: Query endpoints section looks good.

The query endpoints are properly structured with consistent request/response types. The previously flagged typo ("QueryRequst") and response type inconsistency for the /infer endpoint have been corrected.


126-156: Providers, models, tools, and RAGs section is well-structured.

The endpoint sequences in this section are properly organized with consistent patterns. The only issue is the path parameter notation on line 147, which has been flagged separately.


114-123: Verify that status endpoints should route to feedback module.

Lines 115 and 120 show status-related endpoints (/v1/status) being routed to feedback_py. This couples status management with feedback functionality, which may affect modularity and separation of concerns. Please confirm this architectural decision is intentional.

If these endpoints are implemented in separate modules, update the diagram accordingly. For example:

#!/bin/bash
# Description: Check which module actually handles the /v1/status endpoints

# Search for the status endpoint definitions
rg -n "GET.*['\"]\/v1\/status['\"]|PUT.*['\"]\/v1\/status['\"]" --type py -C3

Comment on lines +108 to +123
== Feedback endpoint ==
Client->>API_server: POST /v1/feedback (FeedbackRequest)
API_server->> feedback_py: FeedbackRequest
feedback_py-->>API_server: FeedbackResponse
API_server-->>Client: FeedbackResponse

Client->>API_server: GET /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: StatusResponse
API_server-->>Client: StatusResponse

Client->>API_server: PUT /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: FeedbackStatusUpdateResponse
API_server-->>Client: FeedbackStatusUpdateResponse

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 | 🟡 Minor

Section title doesn't match content.

The section is titled "Feedback endpoint" (singular) but contains three different endpoints: /v1/feedback, /v1/status (GET), and /v1/status (PUT). Update the title to reflect the actual content.

Apply this diff:

-== Feedback endpoint ==
+== Feedback and Status endpoints ==
📝 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
== Feedback endpoint ==
Client->>API_server: POST /v1/feedback (FeedbackRequest)
API_server->> feedback_py: FeedbackRequest
feedback_py-->>API_server: FeedbackResponse
API_server-->>Client: FeedbackResponse
Client->>API_server: GET /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: StatusResponse
API_server-->>Client: StatusResponse
Client->>API_server: PUT /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: FeedbackStatusUpdateResponse
API_server-->>Client: FeedbackStatusUpdateResponse
== Feedback and Status endpoints ==
Client->>API_server: POST /v1/feedback (FeedbackRequest)
API_server->> feedback_py: FeedbackRequest
feedback_py-->>API_server: FeedbackResponse
API_server-->>Client: FeedbackResponse
Client->>API_server: GET /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: StatusResponse
API_server-->>Client: StatusResponse
Client->>API_server: PUT /v1/status
API_server->> feedback_py: Request
feedback_py-->>API_server: FeedbackStatusUpdateResponse
API_server-->>Client: FeedbackStatusUpdateResponse
🤖 Prompt for AI Agents
In docs/rest_api.uml around lines 108 to 123, the section title "Feedback
endpoint" is inaccurate because the block documents three endpoints
(/v1/feedback, GET /v1/status, PUT /v1/status); update the section title to a
plural or broader heading such as "Feedback endpoints" or "Feedback and Status
endpoints" so it correctly reflects the multiple routes, leaving the sequence
diagram body unchanged.

Comment on lines +147 to +150
Client->>API_server: GET /v1/rags/RAG_ID
API_server->> rags_py: Request
rags_py-->>API_server: RAGInfoResponse
API_server-->>Client: RAGInfoResponse
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 | 🟡 Minor

Inconsistent path parameter notation.

Line 147 uses RAG_ID as a literal path component, which is inconsistent with other path parameters in the diagram (e.g., lines 91, 96, 101 use {conversation_id} with braces). Use braces to indicate it's a variable path parameter.

Apply this diff:

-Client->>API_server: GET /v1/rags/RAG_ID
+Client->>API_server: GET /v1/rags/{rag_id}
📝 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
Client->>API_server: GET /v1/rags/RAG_ID
API_server->> rags_py: Request
rags_py-->>API_server: RAGInfoResponse
API_server-->>Client: RAGInfoResponse
Client->>API_server: GET /v1/rags/{rag_id}
API_server->> rags_py: Request
rags_py-->>API_server: RAGInfoResponse
API_server-->>Client: RAGInfoResponse
🤖 Prompt for AI Agents
In docs/rest_api.uml around lines 147 to 150, the path parameter is written as a
literal RAG_ID which is inconsistent with other endpoints; change the diagram to
use brace notation for variable path parameters (e.g., replace RAG_ID with
{rag_id}) so the GET line and any related arrows reflect the parameter in braces
to match the rest of the file.

@tisnik tisnik merged commit 96888ca into lightspeed-core:main Dec 17, 2025
19 of 27 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