Skip to content

LCORE-1583: Allowed reasoning and max_output_tokens attributes #1483

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.

Sign up for GitHub

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

Draft
asimurka wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

LCORE-1583: Allowed reasoning and max_output_tokens attributes #1483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lightspeed-stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ llama_stack:
# The instance would have already been started with a llama-stack-run.yaml file
use_as_library_client: false
# Alternative for "as library use"
# use_as_library_client: true
# library_client_config_path: <path-to-llama-stack-run.yaml-file>
url: http://llama-stack:8321
api_key: xyzzy
use_as_library_client: true
Comment on lines 13 to +15
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove duplicated use_as_library_client key (conflicting values).

This map defines use_as_library_client twice (false then true). It is ambiguous, fails lint, and parser behavior can differ. Keep only one value.

Suggested fix 
 llama_stack:
-  # Uses a remote llama-stack service
-  # The instance would have already been started with a llama-stack-run.yaml file
-  use_as_library_client: false
-  # Alternative for "as library use"
+  # Use llama-stack in library-client mode
   use_as_library_client: true
   library_client_config_path: run.yaml
   # url: http://llama-stack:8321
   # api_key: xyzzy
📝 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
use_as_library_client: false
# Alternative for "as library use"
# use_as_library_client: true
# library_client_config_path: <path-to-llama-stack-run.yaml-file>
url: http://llama-stack:8321
api_key: xyzzy
use_as_library_client: true
llama_stack:
# Use llama-stack in library-client mode
use_as_library_client: true
library_client_config_path: run.yaml
# url: http://llama-stack:8321
# api_key: xyzzy
🧰 Tools
🪛 YAMLlint (1.38.0)

[error] 15-15: duplication of key "use_as_library_client" in mapping

(key-duplicates)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lightspeed-stack.yaml` around lines 13 - 15, The YAML contains a duplicated
key use_as_library_client with conflicting values (false and true); remove one
of the entries so the map contains a single use_as_library_client key with the
intended boolean value, ensuring the remaining value reflects the desired
behavior and resolving the parser/lint error.

library_client_config_path: run.yaml
# url: http://llama-stack:8321
# api_key: xyzzy
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
Expand Down
1 change: 0 additions & 1 deletion run.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ apis:
benchmarks: []
datasets: []
image_name: starter
external_providers_dir: ${env.EXTERNAL_PROVIDERS_DIR}

providers:
inference:
Expand Down
6 changes: 1 addition & 5 deletions src/app/endpoints/responses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,9 @@ async def responses_endpoint_handler(
if responses_request.reasoning is not None:
logger.warning("reasoning is not yet supported in LCORE and will be ignored")
responses_request.reasoning = None
if responses_request.max_output_tokens is not None:
logger.warning(
"max_output_tokens is not yet supported in LCORE and will be ignored"
)
responses_request.max_output_tokens = None

responses_request = responses_request.model_copy(deep=True)

Comment on lines 165 to +170
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not strip reasoning; this currently breaks the advertised passthrough behavior.

Line 165-Line 167 still nullifies responses_request.reasoning, so the field can never reach ResponsesApiParams.model_validate(...) (Line 302/Line 657) or client.responses.create(...). This contradicts LCORE-1583 and also mutates the request object in place before the copy on Line 169.

✅ Suggested fix 
-    # Known LLS bug: https://redhat.atlassian.net/browse/LCORE-1583
-    if responses_request.reasoning is not None:
-        logger.warning("reasoning is not yet supported in LCORE and will be ignored")
-        responses_request.reasoning = None
-
-    responses_request = responses_request.model_copy(deep=True)
+    responses_request = responses_request.model_copy(deep=True)

As per coding guidelines "Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/app/endpoints/responses.py` around lines 165 - 170, The code currently
nulls out responses_request.reasoning (responses_request.reasoning = None) and
mutates the input before copying, preventing the field from reaching
ResponsesApiParams.model_validate(...) and client.responses.create(...); remove
the in-place assignment so reasoning is preserved, log a warning if present but
do not modify responses_request, and ensure you perform the deep copy
(responses_request = responses_request.model_copy(deep=True)) before any
non-mutating checks or logs so the original object is not altered and the
reasoning value is passed through to downstream validation and
client.responses.create.

check_configuration_loaded(configuration)
responses_request.instructions = get_system_prompt(
responses_request.instructions, field_name="instructions"
Expand Down
4 changes: 4 additions & 0 deletions src/utils/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ class ResponsesApiParams(BaseModel):
default=None,
description="Reasoning configuration for the response",
)
safety_identifier: Optional[str] = Field(
default=None,
description="Stable identifier for safety monitoring and abuse detection",
)
store: bool = Field(description="Whether to store the response")
stream: bool = Field(description="Whether to stream the response")
temperature: Optional[float] = Field(
Expand Down
6 changes: 0 additions & 6 deletions tests/e2e/features/responses.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ Feature: Responses endpoint API tests
Then The status code of the response is 200
And The body of the response contains hello

# https://redhat.atlassian.net/browse/LCORE-1583
@skip
Scenario: Responses accepts passthrough parameters with valid types
Given The system is in default state
And I set the Authorization header to Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6Ikpva
Expand All @@ -28,8 +26,6 @@ Feature: Responses endpoint API tests
"model": "{PROVIDER}/{MODEL}",
"stream": false,
"instructions": "You are a helpful assistant.",
"prompt": {"id": "e2e_responses_passthrough_prompt"},
"reasoning": {"effort": "low"},
"safety_identifier": "e2e-responses-passthrough",
"text": {"format": {"type": "text"}},
"tool_choice": "auto",
Expand All @@ -51,8 +47,6 @@ Feature: Responses endpoint API tests
"status": "completed",
"model": "{PROVIDER}/{MODEL}",
"instructions": "You are a helpful assistant.",
"prompt": {"id": "e2e_responses_passthrough_prompt"},
"reasoning": {"effort": "low"},
"safety_identifier": "e2e-responses-passthrough",
"text": {"format": {"type": "text"}},
"tool_choice": "auto",
Expand Down
22 changes: 0 additions & 22 deletions tests/e2e/test_list.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1 @@
features/faiss.feature
features/inline_rag.feature
features/smoketests.feature
features/authorized_noop.feature
features/authorized_noop_token.feature
features/authorized_rh_identity.feature
features/rbac.feature
features/conversations.feature
features/conversation_cache_v2.feature
features/feedback.feature
features/health.feature
features/info.feature
features/responses.feature
features/responses_streaming.feature
features/query.feature
features/rlsapi_v1.feature
features/rlsapi_v1_errors.feature
features/streaming_query.feature
features/rest_api.feature
features/models.feature
features/proxy.feature
features/tls.feature
features/mcp.feature
Loading