Repository navigation
LCORE-4621: Max response request size customization #2961
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3165,4 +3165,4 @@ | |
| } | ||
| }, | ||
| "paths": {} | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1963,6 +1963,12 @@ class Customization(ConfigurationBase): | |||||||
| agent_card_path: Optional[FilePath] = None | ||||||||
| agent_card_config: Optional[dict[str, Any]] = None | ||||||||
| custom_profile: Optional[CustomProfile] = Field(default=None, init=False) | ||||||||
| responses_request_max_size: int = Field( | ||||||||
| default=constants.RESPONSES_REQUEST_MAX_SIZE, | ||||||||
| title="Maximum character length for the serialized responses request body", | ||||||||
| description="Maximum character length for the serialized /v1/response responses " | ||||||||
| "request body", | ||||||||
|
Comment on lines
+1969
to
+1970
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Correct the documented endpoint path. The description says The shared constant uses Proposed fix- description="Maximum character length for the serialized /v1/response responses "
- "request body",
+ description="Maximum character length for the serialized /v1/responses request body",📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| ) | ||||||||
|
|
||||||||
| @model_validator(mode="after") | ||||||||
| def check_customization_model(self) -> Self: | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pydantic import ValidationError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pytest_subtests import SubTests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from constants import RESPONSES_REQUEST_MAX_SIZE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from models.config import Customization | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -58,6 +59,21 @@ def test_service_customization(subtests: SubTests) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert c.disable_query_system_prompt is True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_service_customization_max_response_request_size(subtests: SubTests) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Check the service customization class.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with subtests.test(msg="Default response request size should be used"): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| c = Customization() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert c is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # default value should not be used now | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert c.responses_request_max_size == RESPONSES_REQUEST_MAX_SIZE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with subtests.test(msg="Max response request size is specified"): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| c = Customization(responses_request_max_size=42) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert c is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # default value should not be used now | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert c.responses_request_max_size == 42 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
+75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Fix the misleading comment and add a validation test. The comment on Line 73 says "default value should not be used now". This comment is correct for the explicit-value subtest. The comment on Line 67 repeats it, but that subtest checks that the default is used. Change Line 67 to say the default value is expected. - # default value should not be used now
+ # default value should be used when no value is specified
assert c.responses_request_max_size == RESPONSES_REQUEST_MAX_SIZEAlso, the field in 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_service_customization_wrong_system_prompt_path() -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Check the service customization class. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lightspeed-core/lightspeed-stack
Length of output: 10344
🏁 Script executed:
Repository: lightspeed-core/lightspeed-stack
Length of output: 32445
🏁 Script executed:
Repository: lightspeed-core/lightspeed-stack
Length of output: 3359
Pass the configured limit into
ResponsesRequestvalidation.responses_endpoint_handlerreceivesResponsesRequestas a direct FastAPI body parameter. It does not callmodel_validatewith a context containingconfiguration.customization.responses_request_max_size. The validator therefore uses its 65,536-character fallback, so the customization has no effect on HTTP requests. Use a request-validation dependency that passes the configured value throughValidationInfo.context.🤖 Prompt for AI Agents