Skip to content

Commit 5540e18

Browse files
committed
1. Self return type on validate_image_attachment
2. XOR-style consistency check merging two if-blocks 3. image_attachments field removed from ResponsesApiParams 4. Image attachments threaded as explicit parameters through handlers instead of via ResponsesApiParams Signed-off-by: Lucas <lyoon@redhat.com>
1 parent 6f9d644 commit 5540e18

11 files changed

Lines changed: 57 additions & 49 deletions

File tree

src/app/endpoints/query.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from authorization.middleware import authorize
2424
from client import AsyncLlamaStackClientHolder
2525
from configuration import configuration
26-
from constants import ENDPOINT_PATH_QUERY
26+
from constants import ENDPOINT_PATH_QUERY, IMAGE_CONTENT_TYPES
2727
from log import get_logger
2828
from models.api.requests import QueryRequest
2929
from models.api.responses.constants import UNAUTHORIZED_OPENAPI_EXAMPLES_WITH_MCP_OAUTH
@@ -227,6 +227,13 @@ async def query_endpoint_handler(
227227
):
228228
client = await AsyncLlamaStackClientHolder().update_azure_token()
229229

230+
# Extract image attachments for multimodal support
231+
image_attachments = [
232+
a
233+
for a in (query_request.attachments or [])
234+
if a.content_type in IMAGE_CONTENT_TYPES
235+
] or None
236+
230237
# Retrieve response using Responses API
231238
turn_summary = await retrieve_agent_response(
232239
client,
@@ -235,6 +242,7 @@ async def query_endpoint_handler(
235242
endpoint_path,
236243
compaction.original_input if compaction.compacted else None,
237244
no_tools=bool(query_request.no_tools),
245+
image_attachments=image_attachments,
238246
)
239247

240248
if moderation_result.decision == "passed":

src/app/endpoints/streaming_query.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from configuration import configuration
4747
from constants import (
4848
ENDPOINT_PATH_STREAMING_QUERY,
49+
IMAGE_CONTENT_TYPES,
4950
LLM_TOKEN_EVENT,
5051
LLM_TOOL_CALL_EVENT,
5152
LLM_TOOL_RESULT_EVENT,
@@ -69,6 +70,7 @@
6970
UnprocessableEntityResponse,
7071
)
7172
from models.api.responses.successful import StreamingQueryResponse
73+
from models.common.query import Attachment
7274
from models.common.responses.contexts import ResponseGeneratorContext
7375
from models.common.responses.responses_api_params import ResponsesApiParams
7476
from models.common.responses.types import ResponseInput
@@ -307,6 +309,13 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals
307309
)
308310
recording.record_llm_call(provider_id, model_id, endpoint_path)
309311

312+
# Extract image attachments for multimodal support
313+
image_attachments = [
314+
a
315+
for a in (query_request.attachments or [])
316+
if a.content_type in IMAGE_CONTENT_TYPES
317+
] or None
318+
310319
response_media_type = (
311320
MEDIA_TYPE_TEXT
312321
if query_request.media_type == MEDIA_TYPE_TEXT
@@ -330,6 +339,7 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals
330339
context=context,
331340
responses_params=responses_params,
332341
endpoint_path=endpoint_path,
342+
image_attachments=image_attachments,
333343
),
334344
media_type=response_media_type,
335345
)
@@ -339,6 +349,7 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals
339349
context=context,
340350
endpoint_path=endpoint_path,
341351
no_tools=bool(query_request.no_tools),
352+
image_attachments=image_attachments,
342353
)
343354

344355
# Combine inline RAG results (BYOK + Solr) with tool-based results
@@ -461,6 +472,7 @@ async def generate_response_with_compaction(
461472
context: ResponseGeneratorContext,
462473
responses_params: ResponsesApiParams,
463474
endpoint_path: str,
475+
image_attachments: Optional[list[Attachment]] = None,
464476
) -> AsyncIterator[str]:
465477
"""Stream a response for a conversation that requires compaction.
466478
@@ -475,6 +487,7 @@ async def generate_response_with_compaction(
475487
context: The response generator context.
476488
responses_params: The base Responses API parameters.
477489
endpoint_path: API endpoint path used for metric labeling.
490+
image_attachments: Image attachments for multimodal prompt construction.
478491
479492
Yields:
480493
SSE-formatted strings.
@@ -507,6 +520,7 @@ async def generate_response_with_compaction(
507520
responses_params=responses_params,
508521
context=context,
509522
endpoint_path=endpoint_path,
523+
image_attachments=image_attachments,
510524
)
511525
except HTTPException as e:
512526
yield http_exception_stream_event(e)

src/models/common/query.py

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import base64
44
import binascii
5-
from typing import Any, Literal, Optional
5+
from typing import Any, Literal, Optional, Self
66

77
from pydantic import BaseModel, ConfigDict, Field, model_validator
88

@@ -41,30 +41,25 @@ class Attachment(BaseModel):
4141
)
4242

4343
@model_validator(mode="after")
44-
def validate_image_attachment(self) -> "Attachment":
44+
def validate_image_attachment(self) -> Self:
4545
"""Validate consistency between attachment_type and content_type for images.
4646
4747
Returns:
4848
Self: The validated Attachment instance.
4949
5050
Raises:
51-
ValueError: If image content_type is used without attachment_type='image',
52-
if attachment_type='image' is used without an image content_type,
51+
ValueError: If attachment_type and content_type are inconsistent
52+
(one indicates an image while the other does not),
5353
if image content is not valid base64, or if decoded size exceeds the limit.
5454
"""
5555
is_image_content_type = self.content_type in IMAGE_CONTENT_TYPES
5656
is_image_attachment_type = self.attachment_type == "image"
5757

58-
if is_image_content_type and not is_image_attachment_type:
58+
if is_image_content_type != is_image_attachment_type:
5959
raise ValueError(
60-
f"attachment_type must be 'image' when content_type is "
61-
f"'{self.content_type}'"
62-
)
63-
64-
if is_image_attachment_type and not is_image_content_type:
65-
raise ValueError(
66-
f"content_type must be 'image/jpeg' or 'image/png' when "
67-
f"attachment_type is 'image', got '{self.content_type}'"
60+
f"attachment_type and content_type are inconsistent: "
61+
f"attachment_type='{self.attachment_type}', "
62+
f"content_type='{self.content_type}'"
6863
)
6964

7065
if is_image_content_type:

src/models/common/responses/responses_api_params.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
)
2121
from pydantic import BaseModel, Field
2222

23-
from models.common.query import Attachment
2423
from models.common.responses.types import IncludeParameter, InputTool, ResponseInput
2524
from utils.tool_formatter import translate_vector_store_ids_to_user_facing
2625

@@ -131,12 +130,6 @@ class ResponsesApiParams(BaseModel):
131130
"compacted, lightspeed-stack supplies explicit input and must not let "
132131
"Llama Stack reload the full history via the conversation parameter.",
133132
)
134-
image_attachments: Optional[list[Attachment]] = Field(
135-
default=None,
136-
exclude=True,
137-
description="Image attachments from the query request. Excluded from the "
138-
"API request body — used by agent runners to build multimodal prompts.",
139-
)
140133

141134
def model_dump(self, *args: Any, **kwargs: Any) -> dict[str, Any]:
142135
"""Serialize to a request body dict.

src/utils/agents/query.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
)
3333
from models.common.agents import AgentTurnAccumulator
3434
from models.common.moderation import ShieldModerationResult
35+
from models.common.query import Attachment
3536
from models.common.responses.responses_api_params import ResponsesApiParams
3637
from models.common.responses.types import ResponseInput
3738
from models.common.turn_summary import TurnSummary
@@ -285,6 +286,7 @@ async def retrieve_agent_response(
285286
endpoint_path: str,
286287
_original_input: Optional[ResponseInput] = None,
287288
no_tools: bool = False,
289+
image_attachments: Optional[list[Attachment]] = None,
288290
) -> TurnSummary:
289291
"""Retrieve a turn summary from a blocking agent run.
290292
@@ -297,6 +299,7 @@ async def retrieve_agent_response(
297299
endpoint_path: Endpoint path used for metric labeling.
298300
_original_input: Original user input before the explicit-input rewrite.
299301
no_tools: Whether to skip tool processing.
302+
image_attachments: Image attachments for multimodal prompt construction.
300303
Returns:
301304
Turn summary for the completed agent run.
302305
@@ -319,10 +322,10 @@ async def retrieve_agent_response(
319322
client, responses_params, configuration.skills, no_tools=no_tools
320323
)
321324
logger.debug("Starting agent non-streaming response processing")
322-
if responses_params.image_attachments:
325+
if image_attachments:
323326
prompt = build_multimodal_input(
324327
cast(str, responses_params.input),
325-
responses_params.image_attachments,
328+
image_attachments,
326329
)
327330
else:
328331
prompt = cast(str, responses_params.input)

src/utils/agents/streaming.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
ToolResultStreamPayload,
4040
TurnCompleteStreamPayload,
4141
)
42+
from models.common.query import Attachment
4243
from models.common.responses import ResponseInput
4344
from models.common.responses.contexts import ResponseGeneratorContext
4445
from models.common.responses.responses_api_params import ResponsesApiParams
@@ -90,6 +91,7 @@ async def retrieve_agent_response_generator(
9091
context: ResponseGeneratorContext,
9192
endpoint_path: str,
9293
no_tools: bool = False,
94+
image_attachments: Optional[list[Attachment]] = None,
9395
) -> tuple[AsyncIterator[str], TurnSummary]:
9496
"""Return the SSE generator and mutable turn summary for an agent run.
9597
@@ -98,6 +100,7 @@ async def retrieve_agent_response_generator(
98100
context: Streaming request context and moderation result.
99101
endpoint_path: Endpoint path used for metric labeling.
100102
no_tools: Whether to skip tool processing.
103+
image_attachments: Image attachments for multimodal prompt construction.
101104
102105
Returns:
103106
Tuple of SSE async iterator and mutable turn summary.
@@ -135,6 +138,7 @@ async def retrieve_agent_response_generator(
135138
context,
136139
turn_summary,
137140
endpoint_path,
141+
image_attachments=image_attachments,
138142
),
139143
turn_summary,
140144
)
@@ -301,6 +305,7 @@ async def agent_response_generator(
301305
context: ResponseGeneratorContext,
302306
turn_summary: TurnSummary,
303307
endpoint_path: str,
308+
image_attachments: Optional[list[Attachment]] = None,
304309
) -> AsyncIterator[str]:
305310
"""Stream SSE events from an agent run and update the turn summary.
306311
@@ -310,6 +315,7 @@ async def agent_response_generator(
310315
context: Streaming request context.
311316
turn_summary: Mutable summary to fill while streaming.
312317
endpoint_path: Endpoint path used for metric labeling.
318+
image_attachments: Image attachments for multimodal prompt construction.
313319
314320
Yields:
315321
Serialized SSE event strings.
@@ -320,10 +326,10 @@ async def agent_response_generator(
320326
rag_id_mapping=context.rag_id_mapping,
321327
turn_summary=turn_summary,
322328
)
323-
if responses_params.image_attachments:
329+
if image_attachments:
324330
prompt = build_multimodal_input(
325331
cast(str, responses_params.input),
326-
responses_params.image_attachments,
332+
image_attachments,
327333
)
328334
else:
329335
prompt = cast(str, responses_params.input)

src/utils/responses.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@
9393
NotFoundResponse,
9494
ServiceUnavailableResponse,
9595
)
96-
from models.common.query import Attachment
9796
from models.common.responses.responses_api_params import ResponsesApiParams
9897
from models.common.responses.types import (
9998
InputTool,
@@ -388,17 +387,6 @@ async def prepare_responses_params( # pylint: disable=too-many-arguments,too-ma
388387
# Adds inline RAG context and text attachments (images are excluded)
389388
input_text = prepare_input(query_request, inline_rag_context)
390389

391-
# Extract image attachments for multimodal support
392-
image_attachments: Optional[list[Attachment]] = None
393-
if query_request.attachments:
394-
images = [
395-
a
396-
for a in query_request.attachments
397-
if a.content_type in constants.IMAGE_CONTENT_TYPES
398-
]
399-
if images:
400-
image_attachments = images
401-
402390
# Handle conversation ID for Responses API
403391
conversation_id = query_request.conversation_id
404392
if conversation_id:
@@ -443,7 +431,6 @@ async def prepare_responses_params( # pylint: disable=too-many-arguments,too-ma
443431
extra_headers=extra_headers,
444432
max_infer_iters=configuration.inference.max_infer_iters,
445433
max_tool_calls=configuration.inference.max_tool_calls,
446-
image_attachments=image_attachments,
447434
)
448435

449436

tests/unit/models/requests/test_attachment.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ def test_valid_image_attachment_png(self) -> None:
6767
def test_image_content_type_requires_image_attachment_type(self) -> None:
6868
"""Test that image content_type requires attachment_type='image'."""
6969
image_data = base64.b64encode(b"\xff\xd8\xff\xe0").decode()
70-
with pytest.raises(ValidationError, match="attachment_type must be 'image'"):
70+
with pytest.raises(
71+
ValidationError, match="attachment_type and content_type are inconsistent"
72+
):
7173
Attachment(
7274
attachment_type="log",
7375
content_type="image/jpeg",
@@ -77,7 +79,7 @@ def test_image_content_type_requires_image_attachment_type(self) -> None:
7779
def test_image_attachment_type_requires_image_content_type(self) -> None:
7880
"""Test that attachment_type='image' requires an image content_type."""
7981
with pytest.raises(
80-
ValidationError, match="content_type must be 'image/jpeg' or 'image/png'"
82+
ValidationError, match="attachment_type and content_type are inconsistent"
8183
):
8284
Attachment(
8385
attachment_type="image",

tests/unit/utils/agents/test_query.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,13 +461,13 @@ async def test_success_with_image_attachments_sends_multimodal_prompt(
461461
content_type="image/jpeg",
462462
)
463463
params = make_responses_params(input_text="describe this")
464-
params.image_attachments = [image_attachment]
465464

466465
summary = await retrieve_agent_response(
467466
client=mocker.AsyncMock(),
468467
responses_params=params,
469468
moderation_result=ShieldModerationPassed(),
470469
endpoint_path=ENDPOINT_PATH_QUERY,
470+
image_attachments=[image_attachment],
471471
)
472472

473473
prompt_arg = mock_agent.run.call_args[0][0]

tests/unit/utils/agents/test_streaming.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,7 @@ async def test_success_returns_agent_generator(
594594
context,
595595
turn_summary,
596596
ENDPOINT_PATH_STREAMING_QUERY,
597+
image_attachments=None,
597598
)
598599

599600
@pytest.mark.asyncio
@@ -935,7 +936,6 @@ async def test_streams_with_image_attachments_passes_multimodal_prompt(
935936
content_type="image/jpeg",
936937
)
937938
params = make_responses_params(input_text="describe this")
938-
params.image_attachments = [image_attachment]
939939

940940
_ = [
941941
event
@@ -945,6 +945,7 @@ async def test_streams_with_image_attachments_passes_multimodal_prompt(
945945
context,
946946
turn_summary,
947947
ENDPOINT_PATH_STREAMING_QUERY,
948+
image_attachments=[image_attachment],
948949
)
949950
]
950951

0 commit comments

Comments
 (0)