Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Nov 13, 2025

Description

LCORE-460: sequence diagram for query endpoint

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-460

Summary by CodeRabbit

  • Documentation
    • Expanded README with three new documentation subsections covering System Prompt configuration options: System Prompt Path, System Prompt Literal, and Custom Profile settings.
    • Added comprehensive architectural sequence diagrams documenting the Query Endpoint handler workflow, including authentication validation, quota checks, model selection, error scenarios (HTTP 500, 429, 400/403/404), and response caching mechanisms.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 13, 2025

Walkthrough

Adds documentation for the Query endpoint architecture and extends README with new system prompt configuration sections and project structure TOC entries, including a new PlantUML sequence diagram illustrating the Query endpoint request-response flow and error handling.

Changes

Cohort / File(s) Summary
Documentation additions
README.md
Introduces three new subsections under System prompt (System Prompt Path, System Prompt Literal, Custom Profile); extends Project structure TOC with Sequence diagrams entry and nested Query endpoint REST API handler reference; adds new Sequence diagrams section with Query endpoint diagram reference
Query endpoint diagram
docs/query_endpoint.puml
New PlantUML diagram defining the Query Endpoint handler flow with participants (Client, Endpoint, Auth, LlamaStack, Cache), full request-response sequence including auth validation, quota checks, conversation retrieval, model selection, and response transformation, plus alternative error paths for connection failures, rate limits, and invalid requests

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • README.md changes are primarily documentation additions and TOC reorganization
  • query_endpoint.puml is a new diagram file with no code logic implications

Possibly related PRs

  • LCORE-304: REST API #264 — Adds REST API sequence diagrams; overlaps with this PR's Query endpoint documentation and diagram additions.

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 accurately describes the main changes: adding a sequence diagram for the query endpoint, which aligns with the documented additions of a new PlantUML diagram and related documentation updates.
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

📜 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 65acb98 and a3dc0ff.

⛔ Files ignored due to path filters (1)
  • docs/query_endpoint.svg is excluded by !**/*.svg
📒 Files selected for processing (2)
  • README.md (3 hunks)
  • docs/query_endpoint.puml (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

37-37: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)


38-38: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)


39-39: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)


81-81: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


82-82: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)

⏰ 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). (4)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (ci)
🔇 Additional comments (5)
docs/query_endpoint.puml (2)

9-27: Main flow sequence is well-structured.

The query endpoint flow clearly captures the key steps: auth validation, quota checks, model selection, capability retrieval, turn creation, and response transformation. The use of self-interactions (lines 16, 19-23) appropriately represents internal processing.


28-41: Error handling branches are comprehensive.

The three alternative scenarios (connection errors, quota exceeded, invalid requests) with appropriate HTTP status codes (500, 429, 400/403/404) are properly documented.

README.md (3)

386-417: System prompt configuration documentation is clear and well-structured.

The three new subsections provide helpful examples for configuring system prompts via path, literal value, and custom profile. The documentation clearly explains the configuration options and includes practical YAML examples.


81-82: Fix list indentation for Sequence diagrams section (markdownlint MD007).

The "Sequence diagrams" entry and its nested "Query endpoint REST API handler" use inconsistent indentation. Line 81 should use 4-space indentation (as a subsection of "Project structure"), and line 82 should use 8 spaces (as a subsection of "Sequence diagrams").

     * [REST API](#rest-api)
-    * [Sequence diagrams](#sequence-diagrams)
-        * [Query endpoint REST API handler](#query-endpoint-rest-api-handler)
+    * [Sequence diagrams](#sequence-diagrams)
+        * [Query endpoint REST API handler](#query-endpoint-rest-api-handler)

Likely an incorrect or invalid review comment.


898-902: The diagram file reference is valid and committed.

The .svg file referenced at line 902 (docs/query_endpoint.svg) is already tracked in the repository's git history. The reference in the README is correct and no action is needed.

Likely an incorrect or invalid review comment.

Comment on lines +1 to +8
@startuml

participant Client
participant Endpoint as "Query Endpoint handler"
participant Auth
participant LlamaStack as "Llama Stack Client"
participant Cache as Cache

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 | 🔴 Critical

Declare the DB participant or remove the interaction.

Lines 14-15 reference a "DB" participant that is never declared at the top of the diagram. PlantUML participant declarations at lines 3-7 do not include DB.

Either declare DB as a participant:

 @startuml

 participant Client
 participant Endpoint as "Query Endpoint handler"
 participant Auth
 participant LlamaStack as "Llama Stack Client"
 participant Cache as Cache
+participant DB as "Database"

 Client->>Endpoint: POST /query + attachments

Or, if the database interaction should remain implicit and not shown in the diagram, remove lines 14-15.

📝 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
@startuml
participant Client
participant Endpoint as "Query Endpoint handler"
participant Auth
participant LlamaStack as "Llama Stack Client"
participant Cache as Cache
@startuml
participant Client
participant Endpoint as "Query Endpoint handler"
participant Auth
participant LlamaStack as "Llama Stack Client"
participant Cache as Cache
participant DB as "Database"
🤖 Prompt for AI Agents
In docs/query_endpoint.puml around lines 1 to 8 (the participant declarations)
the diagram later references a "DB" participant (lines ~14-15) that isn't
declared; either add a DB participant declaration at the top (matching the label
used later, e.g., participant DB or participant "DB" as <alias>) so the later
interactions resolve, or remove the two lines that reference the DB if you
intend the database interaction to be implicit and not shown; ensure the chosen
fix keeps participant naming consistent with the later interaction.

Comment on lines +37 to +39
* [System Prompt Path](#system-prompt-path)
* [System Prompt Literal](#system-prompt-literal)
* [Custom Profile](#custom-profile)
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

Fix list indentation for System prompt subsections (markdownlint MD007).

The three new subsections under "System prompt" use inconsistent indentation. Based on markdownlint feedback, these should use 4-space indentation instead of 8 spaces to align with the expected nested list structure.

     * [System prompt](#system-prompt)
-        * [System Prompt Path](#system-prompt-path)
-        * [System Prompt Literal](#system-prompt-literal)
-        * [Custom Profile](#custom-profile)
+    * [System Prompt Path](#system-prompt-path)
+    * [System Prompt Literal](#system-prompt-literal)
+    * [Custom Profile](#custom-profile)
     * [Control model/provider overrides via authorization](#control-modelprovider-overrides-via-authorization)
📝 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
* [System Prompt Path](#system-prompt-path)
* [System Prompt Literal](#system-prompt-literal)
* [Custom Profile](#custom-profile)
* [System prompt](#system-prompt)
* [System Prompt Path](#system-prompt-path)
* [System Prompt Literal](#system-prompt-literal)
* [Custom Profile](#custom-profile)
* [Control model/provider overrides via authorization](#control-modelprovider-overrides-via-authorization)
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

37-37: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)


38-38: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)


39-39: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)

🤖 Prompt for AI Agents
In README.md around lines 37 to 39, the three subsections under the "System
prompt" list are indented with 8 spaces which violates markdownlint MD007;
change their indentation to 4 spaces so they nest correctly under the parent
list item (i.e., reduce each subsection's leading spaces from 8 to 4) to produce
a proper nested markdown list.

@tisnik tisnik merged commit 552704c into lightspeed-core:main Nov 13, 2025
21 of 23 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