LCORE-390: description for all QueryRequest fields#352
Conversation
WalkthroughDescriptive metadata and example values were added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant QueryRequest Model
Client->>API: Sends request with QueryRequest payload
API->>QueryRequest Model: Validates and parses fields (with new descriptions/examples)
QueryRequest Model-->>API: Returns validated data
API-->>Client: Responds (no change in logic, improved docs)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/models/requests.py (4)
99-110: Consider narrowingprovider/modelto an Enum or LiteralBoth fields currently accept arbitrary strings, but only a finite list is actually supported.
UsingLiteral["openai", "watsonx", …]or a dedicatedProvider(str, Enum)would give callers instant feedback and improve OpenAPI clarity.
117-137: Attachments list lacks upper bound & per-item size checksAn unbounded list (or huge
contentfield) can easily exhaust memory or hit upstream limits.
At minimum, cap the list length withconlist(Attachment, max_items=N)and consider restrictingcontentlength inAttachment.
139-143:no_toolsshould be a plainboolBecause the default is
False, allowingnulladds an unnecessary third state.
Changing tono_tools: bool = Field(False, …)simplifies validation and generated docs.
147-151: Clarifymedia_typedeprecation statusThe comment says the value is ignored; consider
- Marking it as
Deprecatedin the description, and- Rejecting non-
Nonevalues via validation unless explicit compatibility mode is enabled.This prevents silent acceptance of stale clients.
docs/output.md (1)
722-730: Type column left blank in generated table
conversation_id,provider,model, etc., show an empty “Type” which degrades readability of the Markdown docs.
If the generator cannot infer Optionals, manually forcestringin the schema or addtype: stringoverrides.docs/openapi.json (2)
1405-1540: Generated OpenAPI artefact should not be committed – prefer build-time generation instead
docs/openapi.jsonlooks like a FastAPI/Pydantic autogenerated dump. Committing this 4 000-line artefact makes every doc-only change explode into a huge diff, invites merge conflicts and risks the file getting out-of-sync with the source models.
Consider:# .gitignore (excerpt) +docs/openapi.jsonand regenerate the schema during CI/CD or a
make docsstep (e.g.python -m lightspeed_core.generate_openapi > docs/openapi.json).This keeps the repository slim and the review surface focused on the actual source of truth (the Pydantic model with the new
Field(..., description=..., examples=...)calls).
1424-1428: Nit: use “ID” (all caps) for the abbreviationIn several other schemas the term is written “Conversation ID”, while here it is
Conversation Id. Aligning the capitalization improves consistency across the API:- "title": "Conversation Id", + "title": "Conversation ID",Same for the description string a few lines below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/openapi.json(8 hunks)docs/openapi.md(1 hunks)docs/output.md(1 hunks)src/models/requests.py(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). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (1)
docs/openapi.md (1)
740-747: Same missing type information as inoutput.mdThe OpenAPI-flavoured Markdown lists blank types for several
QueryRequestfields.
Confirm the JSON schema contains correcttype: stringand update the doc generator if necessary.
| query: str = Field( | ||
| description="The query string", | ||
| examples=["What is Kubernetes?"], | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce non-empty query and document the requirement
query is the only mandatory input, yet an empty string would pass validation and yield undefined behaviour downstream.
Add a minimal length (or strip_whitespace=True) to guard against accidental empty submissions:
- query: str = Field(
- description="The query string",
- examples=["What is Kubernetes?"],
- )
+ query: str = Field(
+ ...,
+ min_length=1,
+ description="The query string (must contain at least one character)",
+ examples=["What is Kubernetes?"],
+ )📝 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.
| query: str = Field( | |
| description="The query string", | |
| examples=["What is Kubernetes?"], | |
| ) | |
| query: str = Field( | |
| ..., | |
| min_length=1, | |
| description="The query string (must contain at least one character)", | |
| examples=["What is Kubernetes?"], | |
| ) |
🤖 Prompt for AI Agents
In src/models/requests.py around lines 88 to 92, the query field currently
allows empty strings which can cause undefined behavior downstream. To fix this,
enforce that the query is non-empty by adding a minimal length constraint or
setting strip_whitespace=True in the Field definition. Also update the
description to document that the query must not be empty.
| conversation_id: Optional[str] = Field( | ||
| None, | ||
| description="The optional conversation ID (UUID)", | ||
| examples=["c5260aec-4d82-4370-9fdf-05cf908b3f16"], | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate conversation_id the same way as in FeedbackRequest
You already have suid.check_suid() available and use it in another model. Re-using that validator here prevents malformed UUIDs from leaking into persistence layers and log correlation.
@@
conversation_id: Optional[str] = Field(
None,
description="The optional conversation ID (UUID)",
examples=["c5260aec-4d82-4370-9fdf-05cf908b3f16"],
)
+
+ @field_validator("conversation_id")
+ @classmethod
+ def _validate_conversation_id(cls, v: Optional[str]) -> Optional[str]:
+ if v is None:
+ return v
+ if not suid.check_suid(v):
+ raise ValueError(f"Improper conversation ID {v}")
+ return v📝 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.
| conversation_id: Optional[str] = Field( | |
| None, | |
| description="The optional conversation ID (UUID)", | |
| examples=["c5260aec-4d82-4370-9fdf-05cf908b3f16"], | |
| ) | |
| conversation_id: Optional[str] = Field( | |
| None, | |
| description="The optional conversation ID (UUID)", | |
| examples=["c5260aec-4d82-4370-9fdf-05cf908b3f16"], | |
| ) | |
| @field_validator("conversation_id") | |
| @classmethod | |
| def _validate_conversation_id(cls, v: Optional[str]) -> Optional[str]: | |
| if v is None: | |
| return v | |
| if not suid.check_suid(v): | |
| raise ValueError(f"Improper conversation ID {v}") | |
| return v |
🤖 Prompt for AI Agents
In src/models/requests.py around lines 93 to 98, the conversation_id field lacks
validation to ensure it is a properly formed UUID. To fix this, add a validator
for conversation_id that uses suid.check_suid() just like in the FeedbackRequest
model. This will prevent malformed UUIDs from being accepted and improve data
integrity and log correlation.
| system_prompt: Optional[str] = Field( | ||
| None, | ||
| description="The optional system prompt.", | ||
| examples=["You are OpenShift assistant.", "You are Ansible assistant."], | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add reasonable length cap to system_prompt
system_prompt is free-form text potentially propagated to LLM back-ends. A runaway megabyte-sized prompt could blow up request size or exceed provider limits.
Align with the 4 096-char limit you already apply to user_feedback:
- system_prompt: Optional[str] = Field(
- None,
- description="The optional system prompt.",
- examples=["You are OpenShift assistant.", "You are Ansible assistant."],
- )
+ system_prompt: Optional[str] = Field(
+ None,
+ max_length=4096,
+ description="The optional system prompt.",
+ examples=["You are OpenShift assistant.", "You are Ansible assistant."],
+ )📝 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.
| system_prompt: Optional[str] = Field( | |
| None, | |
| description="The optional system prompt.", | |
| examples=["You are OpenShift assistant.", "You are Ansible assistant."], | |
| ) | |
| system_prompt: Optional[str] = Field( | |
| None, | |
| max_length=4096, | |
| description="The optional system prompt.", | |
| examples=["You are OpenShift assistant.", "You are Ansible assistant."], | |
| ) |
🤖 Prompt for AI Agents
In src/models/requests.py around lines 111 to 116, the system_prompt field lacks
a length limit, which risks excessively large inputs causing issues with LLM
back-ends. Add a max_length constraint of 4096 characters to the system_prompt
Field definition, similar to the existing limit on user_feedback, to prevent
oversized prompts.
1 participant
Description
LCORE-390: description for all QueryRequest fields
Type of change
Related Tickets & Documents
Summary by CodeRabbit