LCORE-1880: Refactor 400 response#1554
Conversation
WalkthroughThe changes standardize error response documentation across endpoints by replacing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
|
|
||
| file_responses: dict[int | str, dict[str, Any]] = { | ||
| 200: FileResponse.openapi_response(), | ||
| 400: BadRequestResponse.openapi_response(examples=["file_upload"]), |
There was a problem hiding this comment.
This is not raised in the endpoint anyway.
File operation errors are raised under 413.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/endpoints/prompts.py (1)
188-219:⚠️ Potential issue | 🟡 MinorRefresh curl examples to use valid prompt IDs.
The docstrings still show
pmpt_abc123, but Lines 216, 281, and 342 now reject anything other thanpmpt_plus 48 hex characters. These examples will return 400 if copied.📝 Proposed documentation fix
- curl http://localhost:8080/v1/prompts/pmpt_abc123?version=1 + curl http://localhost:8080/v1/prompts/pmpt_0123456789abcdef0123456789abcdef0123456789abcdef?version=1- curl -X PUT http://localhost:8080/v1/prompts/pmpt_abc123 \\ + curl -X PUT http://localhost:8080/v1/prompts/pmpt_0123456789abcdef0123456789abcdef0123456789abcdef \\- curl -X DELETE http://localhost:8080/v1/prompts/pmpt_abc123 + curl -X DELETE http://localhost:8080/v1/prompts/pmpt_0123456789abcdef0123456789abcdef0123456789abcdefAlso applies to: 250-285, 312-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/endpoints/prompts.py` around lines 188 - 219, The docstring curl examples in the GET /prompts/{prompt_id} handler use invalid IDs (e.g., "pmpt_abc123") which will be rejected by the check_suid_prompt validation; update all examples in the prompt retrieval docstring (and the other occurrences noted) to use a valid prompt identifier format such as "pmpt_" followed by 48 hexadecimal characters (match the validator used by check_suid_prompt) so that copying the example won’t produce a 400; locate the examples in the prompts handler docstring and any other docstrings around the GET/related prompt endpoints (references: check_suid_prompt and the GET /prompts/{prompt_id} docstring) and replace the placeholder IDs with a conforming example.src/models/responses.py (1)
2200-2206:⚠️ Potential issue | 🟡 MinorUpdate prompt examples to the new 48-hex ID format.
These OpenAPI examples still use a 40-hex
pmpt_...suffix. With the new validator, copied IDs from the success/not-found examples are rejected as 400 before they can represent valid success or 404 flows.📝 Proposed documentation fix
- "pmpt_0123456789abcdef0123456789abcdef01234567 does not exist" + "pmpt_0123456789abcdef0123456789abcdef0123456789abcdef does not exist"- "prompt_id": "pmpt_0123456789abcdef0123456789abcdef01234567", + "prompt_id": "pmpt_0123456789abcdef0123456789abcdef0123456789abcdef",- "prompt_id": "pmpt_0123456789abcdef0123456789abcdef01234567", + "prompt_id": "pmpt_0123456789abcdef0123456789abcdef0123456789abcdef",- examples=["pmpt_0123456789abcdef0123456789abcdef01234567"], + examples=["pmpt_0123456789abcdef0123456789abcdef0123456789abcdef"],- "prompt_id": "pmpt_0123456789abcdef0123456789abcdef01234567", + "prompt_id": "pmpt_0123456789abcdef0123456789abcdef0123456789abcdef",Also applies to: 3007-3009, 3033-3035, 3050-3053, 3091-3100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/responses.py` around lines 2200 - 2206, Replace the old 40-hex example prompt IDs with the new 48-hex format wherever the sample string "pmpt_0123456789abcdef0123456789abcdef01234567" (and its siblings in the example response/cause fields) appears; update each example value to start with "pmpt_" followed by 48 hex characters (e.g., extend the hex suffix to 48 characters) so the examples pass the new validator—apply this change to all example blocks that reference prompt IDs (including the other example groups noted in the comment).docs/openapi.json (1)
9941-9959:⚠️ Potential issue | 🟠 MajorKeep the POST A2A
operationIddistinct from GET.Line 9959 duplicates the GET operation ID from line 9941. Both GET and POST methods on the
/a2aendpoint share the sameoperationId("handle_a2a_jsonrpc_a2a_get"), which violates OpenAPI specifications and causes OpenAPI code generators to overwrite or collide on generated client methods.🛠️ Proposed fix
- "operationId": "handle_a2a_jsonrpc_a2a_post", + "operationId": "handle_a2a_jsonrpc_a2a_post",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi.json` around lines 9941 - 9959, The POST operation on the /a2a endpoint reuses the GET operationId "handle_a2a_jsonrpc_a2a_get"; update the POST operationId to a distinct identifier (for example "handle_a2a_jsonrpc_a2a_post") so GET and POST have unique operationIds; locate the POST block in the OpenAPI fragment where "operationId": "handle_a2a_jsonrpc_a2a_get" appears and replace it with the new unique operationId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/suid.py`:
- Around line 130-140: The current validation accepts leading '+', '-', or
whitespace because int(hex_part, 16) is permissive; change the check in the SUID
validation (the block using prefix, hex_part, and int(hex_part, 16)) to ensure
hex_part is exactly 48 hex characters by validating against a strict hex pattern
(e.g., match against /^[0-9a-fA-F]{48}$/) or using str checks (all characters in
string.hexdigits and len == 48) before calling int(); return False for any
non-matching hex_part so only pure hexadecimal characters are accepted.
In `@tests/unit/app/endpoints/test_prompts.py`:
- Around line 26-29: Annotate the module-level constants VALID_PMPT_ID,
VALID_PMPT_ID_B, and VALID_PMPT_ID_NOT_FOUND with Final[str] to satisfy the
project's typing rule; import Final from typing if not already imported and
update the declarations to include the type hint (e.g., VALID_PMPT_ID:
Final[str] = "...") so these constants are explicitly typed as final strings.
- Around line 337-385: The tests call handlers (get_prompt_handler,
update_prompt_handler, delete_prompt_handler) which invoke
check_configuration_loaded(configuration) first, so patch the app configuration
as loaded before invoking these handlers to avoid the 500 branch; update each
test to use the existing fixture that marks configuration as loaded (e.g.,
patch_configuration_loaded) or explicitly set the configuration object used by
check_configuration_loaded to a loaded state (configuration.loaded = True) so
the handlers proceed to prompt-id validation and the 400 assertions become
deterministic.
---
Outside diff comments:
In `@docs/openapi.json`:
- Around line 9941-9959: The POST operation on the /a2a endpoint reuses the GET
operationId "handle_a2a_jsonrpc_a2a_get"; update the POST operationId to a
distinct identifier (for example "handle_a2a_jsonrpc_a2a_post") so GET and POST
have unique operationIds; locate the POST block in the OpenAPI fragment where
"operationId": "handle_a2a_jsonrpc_a2a_get" appears and replace it with the new
unique operationId.
In `@src/app/endpoints/prompts.py`:
- Around line 188-219: The docstring curl examples in the GET
/prompts/{prompt_id} handler use invalid IDs (e.g., "pmpt_abc123") which will be
rejected by the check_suid_prompt validation; update all examples in the prompt
retrieval docstring (and the other occurrences noted) to use a valid prompt
identifier format such as "pmpt_" followed by 48 hexadecimal characters (match
the validator used by check_suid_prompt) so that copying the example won’t
produce a 400; locate the examples in the prompts handler docstring and any
other docstrings around the GET/related prompt endpoints (references:
check_suid_prompt and the GET /prompts/{prompt_id} docstring) and replace the
placeholder IDs with a conforming example.
In `@src/models/responses.py`:
- Around line 2200-2206: Replace the old 40-hex example prompt IDs with the new
48-hex format wherever the sample string
"pmpt_0123456789abcdef0123456789abcdef01234567" (and its siblings in the example
response/cause fields) appears; update each example value to start with "pmpt_"
followed by 48 hex characters (e.g., extend the hex suffix to 48 characters) so
the examples pass the new validator—apply this change to all example blocks that
reference prompt IDs (including the other example groups noted in the comment).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e650af1-a5f7-4f42-a73e-35e9f63c6652
📒 Files selected for processing (9)
docs/openapi.jsonsrc/app/endpoints/conversations_v1.pysrc/app/endpoints/conversations_v2.pysrc/app/endpoints/prompts.pysrc/app/endpoints/vector_stores.pysrc/models/responses.pysrc/utils/suid.pytests/unit/app/endpoints/test_prompts.pytests/unit/models/responses/test_error_responses.py
💤 Files with no reviewable changes (1)
- src/app/endpoints/vector_stores.py
📜 Review details
⏰ 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). (15)
- GitHub Check: unit_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: unit_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: black
- GitHub Check: Pylinter
- GitHub Check: build-pr
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Import FastAPI dependencies with:from fastapi import APIRouter, HTTPException, Request, status, Depends
Import Llama Stack client with:from llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
Type aliases defined at module level for clarity
Use Final[type] as type hint for all constants
All functions require docstrings with brief descriptions
Complete type annotations for parameters and return types in functions
Usetyping_extensions.Selffor model validators in Pydantic models
Use modern union type syntaxstr | intinstead ofUnion[str, int]
UseOptional[Type]for optional type hints
Use snake_case with descriptive, action-oriented function names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead
Useasync deffor I/O operations and external API calls
HandleAPIConnectionErrorfrom Llama Stack in error handling
Use standard log levels with clear purposes: debug, info, warning, error
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use ABC for abstract base classes with@abstractmethoddecorators
Use@model_validatorand@field_validatorfor Pydantic model validation
Complete type annotations for all class attributes; use specific types, notAny
Follow Google Python docstring conventions with Parameters, Returns, Raises, and Attributes sections
Files:
tests/unit/models/responses/test_error_responses.pysrc/app/endpoints/conversations_v2.pysrc/utils/suid.pysrc/models/responses.pytests/unit/app/endpoints/test_prompts.pysrc/app/endpoints/prompts.pysrc/app/endpoints/conversations_v1.py
tests/{unit,integration}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests
Do not use unittest; pytest is the standard for this project
Usepytest-mockfor AsyncMock objects in tests
Use markerpytest.mark.asynciofor async tests
Unit tests require 60% coverage, integration tests 10%
Files:
tests/unit/models/responses/test_error_responses.pytests/unit/app/endpoints/test_prompts.py
src/app/endpoints/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use FastAPI
HTTPExceptionwith appropriate status codes for API endpoints
Files:
src/app/endpoints/conversations_v2.pysrc/app/endpoints/prompts.pysrc/app/endpoints/conversations_v1.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Pydantic models extend
ConfigurationBasefor config,BaseModelfor data models
Files:
src/app/endpoints/conversations_v2.pysrc/utils/suid.pysrc/models/responses.pysrc/app/endpoints/prompts.pysrc/app/endpoints/conversations_v1.py
🧠 Learnings (3)
📚 Learning: 2026-04-06T20:18:07.852Z
Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.
Applied to files:
src/app/endpoints/conversations_v2.pysrc/app/endpoints/prompts.pysrc/app/endpoints/conversations_v1.py
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/responses.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/responses.py
🔇 Additional comments (7)
tests/unit/models/responses/test_error_responses.py (1)
87-98: LGTM — the 400 example assertions now coverprompt_id.The test verifies both the new example label and the expected
"Invalid prompt ID format"response.src/app/endpoints/conversations_v2.py (1)
35-68: LGTM — 400 examples are correctly scoped to conversation IDs.This keeps the OpenAPI docs aligned with the actual invalid-conversation-ID validation path.
src/app/endpoints/conversations_v1.py (1)
59-95: LGTM — conversation v1 now documents only the relevant 400 example.Using
examples=["conversation_id"]prevents prompt-specific 400 examples from appearing on conversation endpoints.src/models/responses.py (1)
1833-1836: LGTM — the 400 example now targets invalid prompt IDs.The renamed
prompt_idexample matches the new prompt endpoint 400 responses.src/app/endpoints/prompts.py (1)
18-32: LGTM — prompt endpoints now document the new 400 response.The
prompt_idbad-request example is correctly wired into GET, PUT, and DELETE prompt response maps.Also applies to: 55-77
tests/unit/app/endpoints/test_prompts.py (1)
112-249: LGTM — existing tests now use validator-compatible prompt IDs.The updated success, not-found, list, and backend-error tests align with the new
pmpt_plus 48-hex format.Also applies to: 286-333
docs/openapi.json (1)
2436-2455: Prompt ID 400 examples look consistent.The new
prompt_idBadRequest examples consistently document the invalid prompt ID format and align with the PR objective to remove the misplacedfile_upload400 example.Also applies to: 2680-2699, 2921-2940, 11023-11026
2 participants
Description
This PR refactors 400 error responses:
file_uploadexample that should be only under 413 status codeType of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
Tests