-
Notifications
You must be signed in to change notification settings - Fork 52
LCORE-433: Updated OpenAPI doc #384
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
Conversation
WalkthroughUpdates OpenAPI documentation for POST /v1/feedback: revises response codes/descriptions, replaces 400 with 401, updates 403 text, adds 500 referencing a new ErrorResponse model, and adjusts FeedbackRequest.categories type. Mirrors these changes in generated output docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as /v1/feedback
participant Auth
participant Store as Storage
Client->>API: POST feedback (FeedbackRequest)
API->>Auth: Validate credentials
alt Missing/invalid creds
Auth-->>API: Unauthorized
API-->>Client: 401 Unauthorized
else Authorized
API->>Store: Persist feedback
alt Success
Store-->>API: OK
API-->>Client: 200 Feedback received and stored
else Permission issue
Store-->>API: Forbidden
API-->>Client: 403 Forbidden
else Internal error
Store-->>API: Error
API-->>Client: 500 ErrorResponse{detail}
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ 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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/openapi.md(3 hunks)docs/output.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/output.md
[style] ~174-~174: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...iddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
docs/openapi.md
[style] ~174-~174: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...iddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
docs/output.md
168-168: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
169-169: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
170-170: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
171-171: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
172-172: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
173-173: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
174-174: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
docs/openapi.md
168-168: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
169-169: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
170-170: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
171-171: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
172-172: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
173-173: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
174-174: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
⏰ 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
| | 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) | ||
| | | ||
| | 400 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | ||
| | 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | ||
| | | ||
| | 403 | User is not authorized | [ForbiddenResponse](#forbiddenresponse) | ||
| | 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) | ||
| | | ||
| | 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | ||
| | |
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
Fix response table formatting and "cannot" wording; verify 401 standardization.
- Rows need trailing pipes; remove stray “ |” lines (markdownlint MD055/MD056).
- Prefer “cannot” over “can not” (LanguageTool).
- If moving to 401 for credential issues is intentional across the API, consider following up on other endpoints in a separate PR.
-| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse)
- |
-| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse)
- |
-| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse)
- |
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse)
- |
+| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) |
+| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) |
+| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) |
+| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |Optional verification:
🏁 Script executed:
#!/bin/bash
# Style/wording
rg -n "can not be stored|can not" docs
# Response code consistency for auth errors across docs
rg -n $'\\| 400 \\| Missing or invalid credentials provided by client' docs
rg -n $'\\| 401 \\| Missing or invalid credentials provided by client' docsLength of output: 1657
Update table formatting, wording, and verify 401 usage
- In docs/openapi.md (lines 168–175), add trailing
|on each row and remove stray blank-pipe lines. - Change “can not” → “cannot” for consistency (also appears in docs/output.md:174, docs/openapi.json:335, docs/deployment_guide.md:1125).
- This endpoint now returns 401 for auth errors, but other endpoints still list 400 (docs/openapi.md:89 & 329; docs/output.md:89 & 329). If you intend to standardize on 401, update those in this PR or file a follow-up.
-| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse)
-|
-| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse)
-|
-| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse)
-|
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse)
-|
+| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) |
+| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) |
+| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) |
+| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |📝 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.
| | 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) | |
| | | |
| | 400 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | |
| | 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | |
| | | |
| | 403 | User is not authorized | [ForbiddenResponse](#forbiddenresponse) | |
| | 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) | |
| | | |
| | 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | |
| | | |
| | 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) | | |
| | 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | | |
| | 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) | | |
| | 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) | |
🧰 Tools
🪛 LanguageTool
[style] ~174-~174: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...iddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
168-168: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
169-169: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
170-170: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
171-171: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
172-172: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
173-173: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
174-174: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
175-175: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
🤖 Prompt for AI Agents
In docs/openapi.md around lines 168–175, the response table rows are missing
trailing pipe characters and contain stray blank-pipe lines; add a trailing '|'
to each row and remove any empty pipe-only lines, change "can not" to "cannot"
in this file (and also update docs/output.md:174, docs/openapi.json:335,
docs/deployment_guide.md:1125 to maintain consistency), and reconcile the
status-code usage by either updating other endpoints that list 400 to 401 in
this PR for consistency or file a follow-up task documenting that those
occurrences need to be changed.
| ## ErrorResponse | ||
|
|
||
|
|
||
| Model representing error response for query endpoint. | ||
|
|
||
|
|
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.
🛠️ Refactor suggestion
Make ErrorResponse description endpoint-agnostic.
It’s used by POST /v1/feedback (500). Current wording suggests it’s query-specific.
-Model representing error response for query endpoint.
+Model representing a generic error response payload used by multiple 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.
| ## ErrorResponse | |
| Model representing error response for query endpoint. | |
| ## ErrorResponse | |
| Model representing a generic error response payload used by multiple endpoints. |
🤖 Prompt for AI Agents
In docs/openapi.md around lines 611 to 616, the ErrorResponse description is
currently phrased as "Model representing error response for query endpoint"
which implies it is query-specific; update the text to be endpoint-agnostic
(e.g., "Model representing a generic error response returned by API endpoints")
and ensure any parameter/examples or following sentences refer to generic errors
rather than a specific query endpoint so the same model clearly applies to POST
/v1/feedback (500) and other endpoints.
| | sentiment | | User sentiment, if provided must be -1 or 1 | | ||
| | user_feedback | | Feedback on the LLM response. | | ||
| | categories | array[FeedbackCategory] | List of feedback categories that describe issues with the LLM response (for negative feedback). | | ||
| | categories | | List of feedback categories that describe issues with the LLM response (for negative feedback). | |
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
Re-type FeedbackRequest.categories or document allowed values.
The type is now unspecified while examples use an array of strings. If the enum was intentionally dropped:
- Set type to array and clarify allowed string values in description; or
- Keep enum by referencing FeedbackCategory.
-| categories | | List of feedback categories that describe issues with the LLM response (for negative feedback). |
+| categories | array | List of feedback categories (strings) describing issues with the LLM response (for negative feedback). Allowed values: "incomplete", "not_relevant", ... |Follow-up: If you intend to retain FeedbackCategory as a component, consider restoring:
- categories: array[FeedbackCategory], and
- populate FeedbackCategory with the allowed enum values.
🏁 Script executed:
#!/bin/bash
# Locate FeedbackRequest definition in docs/openapi.md
rg -C3 "FeedbackRequest" docs/openapi.md || echo "FeedbackRequest not found"
# Find occurrences of FeedbackCategory
rg -C3 "FeedbackCategory" docs/openapi.md || echo "FeedbackCategory not found"Length of output: 855
🏁 Script executed:
#!/bin/bash
# Display the enum values defined in FeedbackCategory
rg -A10 "## FeedbackCategory" docs/openapi.mdLength of output: 362
Document FeedbackRequest.categories type and allowed values
The categories field currently lacks a type and doesn’t reference the FeedbackCategory enum or list allowed strings. Please update the schema so clients know what to send. For example:
• In docs/openapi.md under FeedbackRequest:
-| categories | | List of feedback categories that describe issues with the LLM response (for negative feedback). |
+| categories | array[string] | List of feedback categories describing issues with the LLM response (for negative feedback). Refer to `FeedbackCategory` for allowed values. |• Or inline the enum values if you’ve dropped the component, e.g.:
-| categories | | … |
+| categories | array[string] | … Allowed values: `"incomplete"`, `"not_relevant"`, `"incorrect"`, … |If you intend to keep the FeedbackCategory component, restore the reference:
categories:
type: array
items:
$ref: '#/components/schemas/FeedbackCategory'🤖 Prompt for AI Agents
In docs/openapi.md around line 677, the FeedbackRequest.categories entry is
missing a type and allowed values; update the schema to declare categories as an
array whose items reference the existing FeedbackCategory component or inline
the allowed enum strings. Specifically, replace the current empty cell with a
YAML schema block that sets type: array and items: $ref:
'#/components/schemas/FeedbackCategory' (or items: { type: string, enum: [list,
of, allowed, values] } if you removed the component), so clients know the
expected type and permitted values.
| | 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) | ||
| | | ||
| | 400 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | ||
| | 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) | ||
| | | ||
| | 403 | User is not authorized | [ForbiddenResponse](#forbiddenresponse) | ||
| | 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) | ||
| | | ||
| | 500 | User feedback can not be stored | [ErrorResponse](#errorresponse) | ||
| | |
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
Fix response table formatting and "cannot" wording; consider 401 consistency.
- The table rows are missing trailing pipes and have an extra standalone " |" line, triggering markdownlint MD055/MD056.
- Prefer “cannot” over “can not” (LanguageTool hint).
- Optional: You’ve standardized this endpoint to use 401; if that’s a global policy, consider aligning other endpoints later.
Apply:
-| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse)
- |
-| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse)
- |
-| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse)
- |
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse)
- |
+| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) |
+| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) |
+| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) |
+| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |If you want to check for other occurrences:
🏁 Script executed:
#!/bin/bash
# Style/wording
rg -n "can not be stored|can not" docs
# Response code consistency for auth errors
rg -n $'\\| 400 \\| Missing or invalid credentials provided by client' docs
rg -n $'\\| 401 \\| Missing or invalid credentials provided by client' docsLength of output: 1657
Fix table formatting and wording in feedback endpoint docs
- Add missing trailing pipes to each row in docs/output.md (lines 168–175) and remove the extra standalone
|lines to satisfy MD055/MD056. - Replace “can not” with “cannot” in docs/output.md (line 174). This wording also appears in:
- docs/openapi.md (line 174)
- docs/openapi.json (description at line 335)
- Optional: “Missing or invalid credentials provided by client” is still documented under both 400 and 401 across the docs (output.md: 89, 170, 329; openapi.md: 89, 170, 329). If you’re standardizing on 401 globally, consider updating those 400 entries.
Apply to docs/output.md:
-| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse)
- |
-| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse)
- |
-| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse)
- |
-| 500 | User feedback can not be stored | [ErrorResponse](#errorresponse)
- |
+| 200 | Feedback received and stored | [FeedbackResponse](#feedbackresponse) |
+| 401 | Missing or invalid credentials provided by client | [UnauthorizedResponse](#unauthorizedresponse) |
+| 403 | Client does not have permission to access resource | [ForbiddenResponse](#forbiddenresponse) |
+| 500 | User feedback cannot be stored | [ErrorResponse](#errorresponse) |And update the “can not” occurrences elsewhere:
rg -n "can not be stored" docs/openapi.md docs/openapi.json🧰 Tools
🪛 LanguageTool
[style] ~174-~174: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...iddenresponse) | | 500 | User feedback can not be stored | [ErrorResponse](#errorrespo...
(CAN_NOT_PREMIUM)
🪛 markdownlint-cli2 (0.17.2)
168-168: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
169-169: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
170-170: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
171-171: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
172-172: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
173-173: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
174-174: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
175-175: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
🤖 Prompt for AI Agents
In docs/output.md around lines 168–175, fix the markdown table by adding the
missing trailing pipe character to each row and remove any stray standalone "|"
lines so every row is properly pipe-delimited; on the same lines replace the
phrase "can not" with "cannot". Also update the identical "can not be stored"
occurrences in docs/openapi.md (line ~174) and docs/openapi.json (description
around line ~335) to "cannot be stored". Optionally review and standardize the
status-message duplication where "Missing or invalid credentials provided by
client" appears under both 400 and 401 (files/lines noted in the comment) and,
if standardizing to 401, change those 400 entries accordingly.
| ## ErrorResponse | ||
|
|
||
|
|
||
| Model representing error response for query endpoint. | ||
|
|
||
|
|
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.
🛠️ Refactor suggestion
Broaden ErrorResponse description beyond “query endpoint.”
ErrorResponse is referenced by the feedback endpoint (500). The current description reads as query-specific and may confuse readers.
-Model representing error response for query endpoint.
+Model representing a generic error response payload used by multiple 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.
| ## ErrorResponse | |
| Model representing error response for query endpoint. | |
| ## ErrorResponse | |
| Model representing a generic error response payload used by multiple endpoints. |
🤖 Prompt for AI Agents
In docs/output.md around lines 611 to 616, the ErrorResponse description is
narrowly labeled as "Model representing error response for query endpoint."
Update the text to a broader, endpoint-agnostic description (e.g., "Model
representing a generic error response returned by API endpoints") so it reflects
usage by other endpoints like feedback (500); ensure the wording indicates it is
used for server/error responses across endpoints rather than only the query
endpoint.
Description
Updated OpenAPI doc
Type of change
Summary by CodeRabbit