From f692ba23aef110b41347249619f95efb969ba652 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Mon, 17 Aug 2026 11:49:54 +0200 Subject: [PATCH 1/4] LCORE-3582: fix compacted-mode 500s in the agent pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a conversation compacted, every subsequent request on it failed with HTTP 500 on both /v1/query and /v1/streaming_query, permanently bricking the conversation. Root cause: in compacted mode (LCORE-1572) CompactionResult.params.input is an explicit item list (summaries + recent verbatim turns + new query) with the conversation parameter omitted, but the pydantic-ai agent pipeline did prompt = cast(str, responses_params.input) and handed the list to agent.run(), which dies client-side before any request reaches Llama Stack. The A2A executor had a quieter variant of the same gap: it passes the raw user text as the prompt, so compacted A2A turns silently lost all conversation context. The fix makes the explicit input reach the wire through the existing params-to-model-settings seam: - _model_settings_from_responses_params (pydantic_ai_lightspeed/ llamastack/_model.py): when omit_conversation is set and input is an item list, the dumped list is added to extra_body. The OpenAI SDK merges extra_body into the request body with precedence (_merge_mappings: "the second mapping takes precedence"), so the explicit list replaces the prompt-derived input — the request body matches what the non-agent /v1/responses path sends. This also fixes the A2A context loss with no a2a.py changes, since build_agent already routes params through this seam. - OgxResponsesModel gains _prepare_compacted_input, applied in both request() and request_stream() after the conversation-continuation trim: once a ModelResponse exists in the message history (a client-side tool-loop continuation), the input override is dropped so pydantic-ai's mapped messages — which carry the tool results — win. - New agent_prompt_text() helper (utils/conversation_compaction.py) replaces the cast(str, ...) at all four call sites (non-streaming and streaming, plain and multimodal): returns input unchanged when it is a string, else the text of the trailing message item of the explicit list. The prompt still drives capabilities and multimodal input construction; the wire input comes from the override. - The blocked-moderation path in retrieve_agent_response now skips append_turn_items_to_conversation when omit_conversation is set, mirroring the streaming path — appending the full explicit list would duplicate summaries and history into the conversation. - map_agent_inference_error now logs the original exception at error level with the traceback before mapping. Previously the mapped HTTPException discarded it and callers raised without logging, so these failures produced a generic 500 with nothing in the logs — which is what made this bug expensive to diagnose. Verified live against llama-stack 0.6.0: on a compacted conversation, turn after turn returns 200 on both endpoints (previously 500), the streaming path emits compaction/token/end events, and the model correctly answers questions about pre-compaction turns, proving the summary context reaches the model. Known limitation: image attachments are not folded into the overridden explicit input, so a compacted turn with images sends the text-only list (compaction+images previously hard -failed; noted in LCORE-3582). Unit tests cover the extra_body override (present in compacted mode, absent otherwise), the tool-loop guard, agent_prompt_text, the prompt threading through both retrieve paths, the moderation-append guard, and the error logging. --- .../llamastack/_model.py | 40 ++++++++ src/utils/agents/error_handler.py | 3 + src/utils/agents/query.py | 20 ++-- src/utils/agents/streaming.py | 7 +- src/utils/conversation_compaction.py | 28 ++++++ .../llamastack/test_model.py | 81 ++++++++++++++++ tests/unit/utils/agents/test_query.py | 92 +++++++++++++++++++ tests/unit/utils/agents/test_streaming.py | 52 +++++++++++ .../utils/test_conversation_compaction.py | 21 +++++ 9 files changed, 332 insertions(+), 12 deletions(-) diff --git a/src/pydantic_ai_lightspeed/llamastack/_model.py b/src/pydantic_ai_lightspeed/llamastack/_model.py index 330077e6a..80a608a3f 100644 --- a/src/pydantic_ai_lightspeed/llamastack/_model.py +++ b/src/pydantic_ai_lightspeed/llamastack/_model.py @@ -74,6 +74,16 @@ def _model_settings_from_responses_params( """Map ``ResponsesApiParams`` into Pydantic AI OpenAI Responses model settings.""" payload = responses_params.model_dump(exclude_none=True) extra_body = {k: v for k, v in payload.items() if k in _LLS_RESPONSES_EXTRA_FIELDS} + if responses_params.omit_conversation and not isinstance( + responses_params.input, str + ): + # Compacted mode (LCORE-3582): the request must carry the explicit item + # list (summaries + recent turns + new query), but pydantic-ai builds + # the wire ``input`` from the prompt alone. Overriding via extra_body + # replaces it with the explicit list, exactly as the non-agent + # /v1/responses path sends it. Dropped again on tool-loop + # continuations — see ``_prepare_compacted_input``. + extra_body["input"] = payload["input"] settings_dict: dict[str, Any] = {} if extra_body: settings_dict["extra_body"] = extra_body @@ -291,6 +301,7 @@ async def request( # pylint: disable=unused-argument messages, model_settings = self._prepare_conversation_continuation( messages, model_settings ) + model_settings = self._prepare_compacted_input(messages, model_settings) return await super().request(messages, model_settings, model_request_parameters) def _prepare_conversation_continuation( @@ -334,6 +345,34 @@ def _prepare_conversation_continuation( new_settings.pop("openai_previous_response_id", None) return trimmed_messages, cast(ModelSettings, new_settings) + def _prepare_compacted_input( + self, + messages: list[ModelMessage], + model_settings: Optional[ModelSettings], + ) -> Optional[ModelSettings]: + """Drop the compacted ``input`` override on tool-loop continuations. + + In compacted mode (LCORE-3582) the request body ``input`` is overridden + via ``extra_body`` with the explicit item list. That override is only + valid for the first request of an agent run: on client-side tool-loop + iterations pydantic-ai's mapped messages carry the tool results and + must win, so the override is removed once a ``ModelResponse`` exists in + the message history. + """ + if not model_settings or not isinstance(model_settings, dict): + return model_settings + extra_body = model_settings.get("extra_body") + if not isinstance(extra_body, dict) or "input" not in extra_body: + return model_settings + if not any(isinstance(message, ModelResponse) for message in messages): + return model_settings + + new_extra_body = dict(extra_body) + new_extra_body.pop("input") + new_settings = dict(model_settings) + new_settings["extra_body"] = new_extra_body + return cast(ModelSettings, new_settings) + @asynccontextmanager async def request_stream( # pylint: disable=unused-argument self, @@ -360,6 +399,7 @@ async def request_stream( # pylint: disable=unused-argument messages, model_settings = self._prepare_conversation_continuation( messages, model_settings ) + model_settings = self._prepare_compacted_input(messages, model_settings) model_settings_cast = cast(OpenAIResponsesModelSettings, model_settings or {}) response = await self._responses_create( diff --git a/src/utils/agents/error_handler.py b/src/utils/agents/error_handler.py index 15c28ba03..3259c32ba 100644 --- a/src/utils/agents/error_handler.py +++ b/src/utils/agents/error_handler.py @@ -49,6 +49,9 @@ def map_agent_inference_error( RuntimeError: Re-raised when ``exc`` is a non-agent ``RuntimeError`` that is not a recognized context-length failure. """ + # The mapped HTTPException loses the original exception, and callers raise + # it without logging — log here so failures are diagnosable (LCORE-3582). + logger.error("Agent inference failed: %s", exc, exc_info=exc) match exc: case AgentRunError() as agent_exc: return map_pydantic_agent_run_error(agent_exc, model_id) diff --git a/src/utils/agents/query.py b/src/utils/agents/query.py index f0660faf1..eb4cbd5af 100644 --- a/src/utils/agents/query.py +++ b/src/utils/agents/query.py @@ -3,7 +3,7 @@ from __future__ import annotations from enum import Enum -from typing import Optional, cast +from typing import Optional from fastapi import HTTPException from ogx_client import APIConnectionError, APIStatusError, AsyncOgxClient @@ -36,6 +36,7 @@ process_native_tool_call, process_native_tool_result, ) +from utils.conversation_compaction import agent_prompt_text from utils.conversations import append_turn_items_to_conversation from utils.otel_tracing import ( SpanAttributes, @@ -276,12 +277,13 @@ async def retrieve_agent_response( ) if moderation_result.decision == "blocked": - await append_turn_items_to_conversation( - client, - responses_params.conversation, - responses_params.input, - [moderation_result.refusal_response], - ) + if not responses_params.omit_conversation: + await append_turn_items_to_conversation( + client, + responses_params.conversation, + responses_params.input, + [moderation_result.refusal_response], + ) return TurnSummary( id=moderation_result.moderation_id, llm_response=moderation_result.message, @@ -301,11 +303,11 @@ async def retrieve_agent_response( logger.debug("Starting agent non-streaming response processing") if image_attachments: prompt = build_multimodal_input( - cast(str, responses_params.input), + agent_prompt_text(responses_params), image_attachments, ) else: - prompt = cast(str, responses_params.input) + prompt = agent_prompt_text(responses_params) run_result = await agent.run(prompt) except ( AgentRunError, diff --git a/src/utils/agents/streaming.py b/src/utils/agents/streaming.py index 8bbc19cce..053b81f69 100644 --- a/src/utils/agents/streaming.py +++ b/src/utils/agents/streaming.py @@ -8,7 +8,7 @@ import datetime from collections.abc import AsyncIterator from functools import singledispatch -from typing import Any, Final, Optional, cast +from typing import Any, Final, Optional from fastapi import HTTPException from ogx_client import APIConnectionError, APIStatusError @@ -60,6 +60,7 @@ process_native_tool_call, process_native_tool_result, ) +from utils.conversation_compaction import agent_prompt_text from utils.conversations import append_turn_items_to_conversation from utils.otel_tracing import ( SpanAttributes, @@ -388,11 +389,11 @@ async def agent_response_generator( ) if image_attachments: prompt = build_multimodal_input( - cast(str, responses_params.input), + agent_prompt_text(responses_params), image_attachments, ) else: - prompt = cast(str, responses_params.input) + prompt = agent_prompt_text(responses_params) logger.debug("Starting agent streaming response processing") async with agent.run_stream_events(prompt) as stream: diff --git a/src/utils/conversation_compaction.py b/src/utils/conversation_compaction.py index 479ed8263..3bd4c32fb 100644 --- a/src/utils/conversation_compaction.py +++ b/src/utils/conversation_compaction.py @@ -249,6 +249,34 @@ def _verbatim_input_message(item: Any) -> Optional[OpenAIResponseMessage]: return OpenAIResponseMessage(role=cast(Any, role), content=text) +def agent_prompt_text(params: ResponsesApiParams) -> str: + """Return the textual user prompt for a pydantic-ai agent run. + + In compacted mode ``params.input`` is the explicit item list built by + :func:`_build_explicit_input` (summaries + recent turns + new query), so + the new user query is the trailing message item. The agent pipeline still + needs a plain string prompt (capabilities and multimodal input operate on + it); the full explicit list reaches the request body separately via the + ``extra_body`` input override (LCORE-3582). + + Args: + params: Prepared (possibly compaction-rewritten) request parameters. + + Returns: + ``params.input`` unchanged when it is a string; otherwise the text of + the last message item in the explicit list, or ``""`` when there is + none. + """ + if isinstance(params.input, str): + return params.input + for item in reversed(list(params.input)): + if is_message_item(item): + text = extract_message_text(item) + if text: + return text + return "" + + def _query_input_message(original_input: ResponseInput) -> list[Any]: """Render the new user query as explicit input items. diff --git a/tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py b/tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py index 9a059cd72..3773c37d9 100644 --- a/tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py +++ b/tests/unit/pydantic_ai_lightspeed/llamastack/test_model.py @@ -6,6 +6,7 @@ from typing import Any import pytest +from ogx_api.openai_responses import OpenAIResponseMessage from openai.types import responses from pydantic_ai import ModelMessage, UnexpectedModelBehavior from pydantic_ai.messages import ModelResponse @@ -120,6 +121,86 @@ def test_none_fields_excluded(self) -> None: assert "extra_headers" not in settings assert "openai_previous_response_id" not in settings + def test_compacted_input_overrides_via_extra_body(self) -> None: + """Test compacted params carry the explicit input list in extra_body.""" + items = [ + OpenAIResponseMessage( + role="user", content="Summary of earlier conversation:\nS1" + ), + OpenAIResponseMessage(role="user", content="new question"), + ] + params = _make_params(input=items, omit_conversation=True) + settings = _model_settings_from_responses_params(params) + extra_body = settings["extra_body"] + assert "conversation" not in extra_body + assert extra_body["input"] == [ + { + "role": "user", + "content": "Summary of earlier conversation:\nS1", + "type": "message", + }, + {"role": "user", "content": "new question", "type": "message"}, + ] + + def test_string_input_never_lands_in_extra_body(self) -> None: + """Test that a plain string input is not duplicated into extra_body.""" + params = _make_params(input="hello", omit_conversation=True) + settings = _model_settings_from_responses_params(params) + assert "input" not in settings.get("extra_body", {}) + + def test_non_compacted_list_input_not_in_extra_body(self) -> None: + """Test that without omit_conversation the input stays out of extra_body.""" + items = [OpenAIResponseMessage(role="user", content="q")] + params = _make_params(input=items, omit_conversation=False) + settings = _model_settings_from_responses_params(params) + assert "input" not in settings.get("extra_body", {}) + + +class TestPrepareCompactedInput: + """Tests for the compacted-input tool-loop guard.""" + + @pytest.fixture(name="model") + def model_fixture(self, mocker: MockerFixture) -> OgxResponsesModel: + """Create a OgxResponsesModel with mocked __init__.""" + mocker.patch.object(OgxResponsesModel, "__init__", return_value=None) + return OgxResponsesModel("test-model") + + def test_no_input_override_returns_unchanged( + self, model: OgxResponsesModel, mocker: MockerFixture + ) -> None: + """Test settings without an input override pass through untouched.""" + messages = [mocker.Mock()] + settings: ModelSettings = {"extra_body": {"max_infer_iters": 5}} + assert model._prepare_compacted_input(messages, settings) is settings + + def test_first_request_keeps_input_override( + self, model: OgxResponsesModel, mocker: MockerFixture + ) -> None: + """Test the override survives when no ModelResponse is in messages.""" + messages = [mocker.Mock(spec=[])] + settings: ModelSettings = { + "extra_body": {"input": [{"role": "user", "content": "q"}]} + } + assert model._prepare_compacted_input(messages, settings) is settings + + def test_tool_loop_continuation_drops_input_override( + self, model: OgxResponsesModel + ) -> None: + """Test the override is dropped once a ModelResponse exists.""" + messages: list[ModelMessage] = [ModelResponse(parts=[])] + settings: ModelSettings = { + "extra_body": { + "input": [{"role": "user", "content": "q"}], + "max_infer_iters": 5, + } + } + result = model._prepare_compacted_input(messages, settings) + assert result is not settings + assert "input" not in result["extra_body"] + assert result["extra_body"]["max_infer_iters"] == 5 + # original settings untouched + assert "input" in settings["extra_body"] + class TestFromOgxClient: """Tests for OgxResponsesModel.from_ogx_client factory.""" diff --git a/tests/unit/utils/agents/test_query.py b/tests/unit/utils/agents/test_query.py index ea630159b..bcfdabd0c 100644 --- a/tests/unit/utils/agents/test_query.py +++ b/tests/unit/utils/agents/test_query.py @@ -6,6 +6,7 @@ import pytest from fastapi import HTTPException +from ogx_api.openai_responses import OpenAIResponseMessage from ogx_client import APIConnectionError, APIStatusError from pydantic_ai.messages import ( FinishReason, @@ -427,6 +428,97 @@ async def test_success_returns_turn_summary( assert summary.llm_response == "Hello!" assert summary.id == "resp-success" + @pytest.mark.asyncio + async def test_compacted_input_runs_agent_with_prompt_text( + self, + mocker: MockerFixture, + make_agent_run_result: Callable[..., Any], + make_responses_params: Callable[..., ResponsesApiParams], + patch_recording_metrics: None, + ) -> None: + """Test compacted explicit input is reduced to the query text for agent.run.""" + explicit = [ + OpenAIResponseMessage( + role="user", content="Summary of earlier conversation:\nS1" + ), + OpenAIResponseMessage(role="user", content="new question"), + ] + params = make_responses_params(input_text="ignored").model_copy( + update={"input": explicit, "omit_conversation": True} + ) + run_result = make_agent_run_result(content="Answer") + mock_agent = mocker.AsyncMock() + mock_agent.run = mocker.AsyncMock(return_value=run_result) + mocker.patch("utils.agents.query.build_agent", return_value=mock_agent) + + summary = await retrieve_agent_response( + client=mocker.AsyncMock(), + responses_params=params, + moderation_result=ShieldModerationPassed(), + endpoint_path=ENDPOINT_PATH_QUERY, + ) + + mock_agent.run.assert_awaited_once_with("new question") + assert summary.llm_response == "Answer" + + @pytest.mark.asyncio + async def test_blocked_moderation_compacted_skips_append( + self, + mocker: MockerFixture, + make_responses_params: Callable[..., ResponsesApiParams], + blocked_moderation: ShieldModerationBlocked, + ) -> None: + """Test blocked moderation does not append explicit input in compacted mode.""" + params = make_responses_params().model_copy( + update={ + "input": [OpenAIResponseMessage(role="user", content="q")], + "omit_conversation": True, + } + ) + mock_append = mocker.patch( + "utils.agents.query.append_turn_items_to_conversation", + new=mocker.AsyncMock(), + ) + + summary = await retrieve_agent_response( + client=mocker.AsyncMock(), + responses_params=params, + moderation_result=blocked_moderation, + endpoint_path=ENDPOINT_PATH_QUERY, + ) + + mock_append.assert_not_awaited() + assert summary.llm_response == "Content blocked by shield." + + @pytest.mark.asyncio + async def test_inference_error_is_logged( + self, + mocker: MockerFixture, + make_responses_params: Callable[..., ResponsesApiParams], + patch_recording_metrics: None, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Test mapped agent inference errors are logged before raising.""" + mock_agent = mocker.AsyncMock() + mock_agent.run = mocker.AsyncMock( + side_effect=APIConnectionError(request=mocker.Mock()) + ) + mocker.patch("utils.agents.query.build_agent", return_value=mock_agent) + + with caplog.at_level("ERROR"): + with pytest.raises(HTTPException): + await retrieve_agent_response( + client=mocker.AsyncMock(), + responses_params=make_responses_params(), + moderation_result=ShieldModerationPassed(), + endpoint_path=ENDPOINT_PATH_QUERY, + ) + + assert any( + record.levelname == "ERROR" and "Agent inference failed" in record.message + for record in caplog.records + ) + @pytest.mark.asyncio async def test_success_with_image_attachments_sends_multimodal_prompt( self, diff --git a/tests/unit/utils/agents/test_streaming.py b/tests/unit/utils/agents/test_streaming.py index ff7f58ffc..9d2c2d172 100644 --- a/tests/unit/utils/agents/test_streaming.py +++ b/tests/unit/utils/agents/test_streaming.py @@ -10,6 +10,7 @@ import pytest from fastapi import HTTPException +from ogx_api.openai_responses import OpenAIResponseMessage from ogx_client import APIStatusError from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, @@ -1284,6 +1285,57 @@ async def test_streams_token_events_and_updates_summary( assert turn_summary.token_usage.input_tokens == 4 assert turn_summary.token_usage.output_tokens == 2 + @pytest.mark.asyncio + async def test_compacted_input_streams_with_prompt_text( + self, + mocker: MockerFixture, + make_generator_context: Callable[..., ResponseGeneratorContext], + make_responses_params: Callable[..., ResponsesApiParams], + make_agent_run_result: Callable[..., Any], + patch_recording_metrics: None, + ) -> None: + """Test compacted explicit input is reduced to the query text for streaming.""" + context = make_generator_context() + turn_summary = TurnSummary() + run_result = make_agent_run_result(content="Answer", response_id="resp-c1") + events = [ + PartStartEvent(index=0, part=TextPart(content="Answer")), + AgentRunResultEvent(result=run_result), + ] + mock_agent = mocker.Mock() + mock_agent.run_stream_events.return_value = _mock_run_stream(events) + mocker.patch( + "utils.agents.streaming.get_agent_finish_reason", + return_value=AgentFinishReason.SUCCESS, + ) + mocker.patch( + "utils.agents.streaming.deduplicate_referenced_documents", + side_effect=lambda docs: docs, + ) + + explicit = [ + OpenAIResponseMessage( + role="user", content="Summary of earlier conversation:\nS1" + ), + OpenAIResponseMessage(role="user", content="new question"), + ] + params = make_responses_params(input_text="ignored").model_copy( + update={"input": explicit, "omit_conversation": True} + ) + + _ = [ + event + async for event in agent_response_generator( + mock_agent, + params, + context, + turn_summary, + ENDPOINT_PATH_STREAMING_QUERY, + ) + ] + + assert mock_agent.run_stream_events.call_args[0][0] == "new question" + @pytest.mark.asyncio async def test_streams_with_image_attachments_passes_multimodal_prompt( self, diff --git a/tests/unit/utils/test_conversation_compaction.py b/tests/unit/utils/test_conversation_compaction.py index 4499a4a16..3e5cc35da 100644 --- a/tests/unit/utils/test_conversation_compaction.py +++ b/tests/unit/utils/test_conversation_compaction.py @@ -128,6 +128,27 @@ def test_compaction_result_context_status() -> None: assert cc.CompactionResult(params, compacted=True).context_status == "summarized" +def test_agent_prompt_text_string_input() -> None: + """A plain string input is returned unchanged.""" + assert cc.agent_prompt_text(_params("what is a pod?")) == "what is a pod?" + + +def test_agent_prompt_text_explicit_list_returns_last_message_text() -> None: + """For compacted explicit input, the trailing user query text is returned.""" + params = _params() + explicit = cc._build_explicit_input( + ["earlier summary"], [_msg("assistant", "prior answer")], "new question" + ) + compacted = params.model_copy(update={"input": explicit, "omit_conversation": True}) + assert cc.agent_prompt_text(compacted) == "new question" + + +def test_agent_prompt_text_empty_list_returns_empty() -> None: + """An empty explicit list yields an empty prompt rather than crashing.""" + params = _params().model_copy(update={"input": [], "omit_conversation": True}) + assert cc.agent_prompt_text(params) == "" + + # --- apply_compaction --- From 538e4cdc9cf4007a028ca93a158b5bc3782a37c1 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 27 Aug 2026 17:25:37 +0200 Subject: [PATCH 2/4] LCORE-3582: log mapped inference failures at a level matching their cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit map_agent_inference_error logged every failure at error level with a full traceback. The function also maps conditions that are not service faults: a context-length failure becomes HTTP 413 (the caller sent too much), an upstream 429 becomes a quota response, and a connection failure becomes 503 (the backend is down). It is called from the shield path too, so shield-model rate limits took the same treatment. Logging those at error level with stack traces buries the unclassified 500s this logging was added to surface, which is the opposite of the intent — in a service with quotas, 429s are routine traffic, not incidents. Classify from the mapped response instead: anything below 500, plus 503, logs a single warning line without a traceback; everything else keeps the error level and the traceback. A response whose status cannot be read as an int is treated as unclassified and logged loudly, so an unexpected mapping fails towards more diagnostics rather than fewer. Update the connection-error test to assert the warning and the absence of an error record, and add the unclassified-failure case it never covered, asserting both the error level and that the traceback is attached. --- src/utils/agents/error_handler.py | 31 ++++++++++++----- tests/unit/utils/agents/test_query.py | 50 +++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/src/utils/agents/error_handler.py b/src/utils/agents/error_handler.py index 3259c32ba..8b16efb70 100644 --- a/src/utils/agents/error_handler.py +++ b/src/utils/agents/error_handler.py @@ -1,5 +1,6 @@ """Error mapping for agent inference failures to structured API error responses.""" +from fastapi import status from ogx_client import APIConnectionError, APIStatusError from pydantic_ai.exceptions import ( AgentRunError, @@ -49,23 +50,37 @@ def map_agent_inference_error( RuntimeError: Re-raised when ``exc`` is a non-agent ``RuntimeError`` that is not a recognized context-length failure. """ - # The mapped HTTPException loses the original exception, and callers raise - # it without logging — log here so failures are diagnosable (LCORE-3582). - logger.error("Agent inference failed: %s", exc, exc_info=exc) match exc: case AgentRunError() as agent_exc: - return map_pydantic_agent_run_error(agent_exc, model_id) + response = map_pydantic_agent_run_error(agent_exc, model_id) case APIStatusError() as status_exc: - return handle_known_apistatus_errors(status_exc, model_id) + response = handle_known_apistatus_errors(status_exc, model_id) case APIConnectionError() as connection_exc: - return ServiceUnavailableResponse( + response = ServiceUnavailableResponse( backend_name="OGX", cause=str(connection_exc), ) case RuntimeError() as runtime_exc if is_context_length_error(str(runtime_exc)): - return PromptTooLongResponse(model=model_id) + response = PromptTooLongResponse(model=model_id) case _: - return InternalServerErrorResponse.generic() + response = InternalServerErrorResponse.generic() + + # The mapped HTTPException loses the original exception, and callers raise + # it without logging — log here so failures are diagnosable (LCORE-3582). + # Only unclassified failures are genuine service faults worth a traceback: + # a mapped 4xx is caller-caused (413 over-long prompt, 429 rate limit) and + # a 503 is an upstream outage, so logging those at error level with a stack + # trace would bury the 500s this logging exists to surface. + status_code = getattr(response, "status_code", None) + caller_or_upstream = isinstance(status_code, int) and ( + status_code < status.HTTP_500_INTERNAL_SERVER_ERROR + or status_code == status.HTTP_503_SERVICE_UNAVAILABLE + ) + if caller_or_upstream: + logger.warning("Agent inference returned %s: %s", status_code, exc) + else: + logger.error("Agent inference failed: %s", exc, exc_info=exc) + return response def map_pydantic_agent_run_error( # pylint: disable=too-many-return-statements diff --git a/tests/unit/utils/agents/test_query.py b/tests/unit/utils/agents/test_query.py index bcfdabd0c..b31253aac 100644 --- a/tests/unit/utils/agents/test_query.py +++ b/tests/unit/utils/agents/test_query.py @@ -498,14 +498,20 @@ async def test_inference_error_is_logged( patch_recording_metrics: None, caplog: pytest.LogCaptureFixture, ) -> None: - """Test mapped agent inference errors are logged before raising.""" + """Test an upstream connection failure logs a warning, not an error. + + APIConnectionError maps to 503: the backend is down, which is not a + service fault of ours, so it must not be logged at error level with a + traceback — that noise would bury the genuine 500s this logging exists + to surface (LCORE-3582). + """ mock_agent = mocker.AsyncMock() mock_agent.run = mocker.AsyncMock( side_effect=APIConnectionError(request=mocker.Mock()) ) mocker.patch("utils.agents.query.build_agent", return_value=mock_agent) - with caplog.at_level("ERROR"): + with caplog.at_level("WARNING"): with pytest.raises(HTTPException): await retrieve_agent_response( client=mocker.AsyncMock(), @@ -515,9 +521,47 @@ async def test_inference_error_is_logged( ) assert any( - record.levelname == "ERROR" and "Agent inference failed" in record.message + record.levelname == "WARNING" + and "Agent inference returned 503" in record.message for record in caplog.records ) + assert not any(record.levelname == "ERROR" for record in caplog.records) + + @pytest.mark.asyncio + async def test_unclassified_inference_error_is_logged_with_traceback( + self, + mocker: MockerFixture, + make_responses_params: Callable[..., ResponsesApiParams], + patch_recording_metrics: None, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Test an unclassified failure is logged at error level with the traceback. + + This is the case the logging was added for: it maps to a generic 500, + the mapped HTTPException discards the original exception, and callers + raise without logging, so without this the failure left nothing behind. + """ + mock_agent = mocker.AsyncMock() + mock_agent.run = mocker.AsyncMock(side_effect=RuntimeError("kaboom")) + mocker.patch("utils.agents.query.build_agent", return_value=mock_agent) + + with caplog.at_level("ERROR"): + with pytest.raises(HTTPException): + await retrieve_agent_response( + client=mocker.AsyncMock(), + responses_params=make_responses_params(), + moderation_result=ShieldModerationPassed(), + endpoint_path=ENDPOINT_PATH_QUERY, + ) + + matching = [ + record + for record in caplog.records + if record.levelname == "ERROR" + and "Agent inference failed" in record.message + ] + assert matching, "unclassified failure was not logged at error level" + assert matching[0].exc_info is not None, "traceback was not attached" @pytest.mark.asyncio async def test_success_with_image_attachments_sends_multimodal_prompt( From 274d838fa43434370240d8ca22c78e6bf75cdc53 Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 27 Aug 2026 17:25:37 +0200 Subject: [PATCH 3/4] LCORE-3582: warn when the compacted input yields no agent prompt text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agent_prompt_text falls back to an empty string when the explicit compacted input carries no textual message item. The request itself still succeeds — the real input reaches the wire through the extra_body override — but the prompt drives capability selection and multimodal input construction, so an empty one silently degrades those while the turn appears to work. That fallback should never be reached: compaction always appends the new user query as the trailing message item. Reaching it means compaction produced something unexpected, so log a warning rather than absorbing it. --- src/utils/conversation_compaction.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/utils/conversation_compaction.py b/src/utils/conversation_compaction.py index 3bd4c32fb..ac5d387ba 100644 --- a/src/utils/conversation_compaction.py +++ b/src/utils/conversation_compaction.py @@ -274,6 +274,15 @@ def agent_prompt_text(params: ResponsesApiParams) -> str: text = extract_message_text(item) if text: return text + # The wire input still carries the explicit list via the extra_body + # override, so the request itself is well-formed — but capabilities and + # multimodal construction operate on this prompt, and an explicit input + # with no textual message item means compaction produced something + # unexpected. Surface it rather than degrading silently. + logger.warning( + "Explicit compacted input carries no textual message item; " + "agent prompt falls back to an empty string" + ) return "" From 0e8d545b35e8fbef4037c5853e75e1b00ee9d6af Mon Sep 17 00:00:00 2001 From: Maxim Svistunov Date: Thu, 27 Aug 2026 18:29:39 +0200 Subject: [PATCH 4/4] LCORE-3582: reject compacted turns carrying image attachments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In compacted mode the wire input is overridden with the explicit item list, which is text-only. Image attachments are converted into pydantic-ai ImageUrl parts on the prompt, and the override replaces the prompt-derived input wholesale, so those parts never reach the request body: the model answers having never seen the image. Before this branch that combination failed outright, because every compacted turn died client-side. Restoring the turn therefore changed a loud failure into a silent one — the caller gets a confident answer about an image the model was never sent, with nothing indicating the attachment was dropped. A wrong answer presented as correct is worse than the error it replaced. Reject the combination with 422 until the explicit input can carry input_image content parts of its own (LCORE-3789), telling the caller why and what to do instead. The guard lives beside the explicit-input builders that create the constraint, and both the blocking and streaming paths call it, so the two cannot drift. Every other combination is unaffected: images without compaction, compaction without images, and neither. --- src/utils/agents/query.py | 8 +++- src/utils/agents/streaming.py | 6 ++- src/utils/conversation_compaction.py | 44 +++++++++++++++++++ .../utils/test_conversation_compaction.py | 33 ++++++++++++++ 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/utils/agents/query.py b/src/utils/agents/query.py index eb4cbd5af..1c3d1a80d 100644 --- a/src/utils/agents/query.py +++ b/src/utils/agents/query.py @@ -36,7 +36,10 @@ process_native_tool_call, process_native_tool_result, ) -from utils.conversation_compaction import agent_prompt_text +from utils.conversation_compaction import ( + agent_prompt_text, + reject_image_attachments_in_compacted_mode, +) from utils.conversations import append_turn_items_to_conversation from utils.otel_tracing import ( SpanAttributes, @@ -301,6 +304,9 @@ async def retrieve_agent_response( no_tools=no_tools, ) logger.debug("Starting agent non-streaming response processing") + reject_image_attachments_in_compacted_mode( + responses_params, image_attachments + ) if image_attachments: prompt = build_multimodal_input( agent_prompt_text(responses_params), diff --git a/src/utils/agents/streaming.py b/src/utils/agents/streaming.py index 053b81f69..7ecf2d603 100644 --- a/src/utils/agents/streaming.py +++ b/src/utils/agents/streaming.py @@ -60,7 +60,10 @@ process_native_tool_call, process_native_tool_result, ) -from utils.conversation_compaction import agent_prompt_text +from utils.conversation_compaction import ( + agent_prompt_text, + reject_image_attachments_in_compacted_mode, +) from utils.conversations import append_turn_items_to_conversation from utils.otel_tracing import ( SpanAttributes, @@ -387,6 +390,7 @@ async def agent_response_generator( rag_id_mapping=context.rag_id_mapping, turn_summary=turn_summary, ) + reject_image_attachments_in_compacted_mode(responses_params, image_attachments) if image_attachments: prompt = build_multimodal_input( agent_prompt_text(responses_params), diff --git a/src/utils/conversation_compaction.py b/src/utils/conversation_compaction.py index ac5d387ba..f826bab84 100644 --- a/src/utils/conversation_compaction.py +++ b/src/utils/conversation_compaction.py @@ -47,6 +47,7 @@ from dataclasses import dataclass from typing import Any, Optional, cast +from fastapi import HTTPException from ogx_api.openai_responses import OpenAIResponseMessage from ogx_client import AsyncOgxClient from ogx_client.types.conversations.item_create_params import Item @@ -55,6 +56,7 @@ from cache.cache_error import CacheError from configuration import configuration from log import get_logger +from models.api.responses.error import UnprocessableEntityResponse from models.common.responses.responses_api_params import ResponsesApiParams from models.common.responses.types import ResponseInput from models.common.turn_summary import ContextStatus @@ -286,6 +288,48 @@ def agent_prompt_text(params: ResponsesApiParams) -> str: return "" +def reject_image_attachments_in_compacted_mode( + params: ResponsesApiParams, + image_attachments: Optional[Sequence[Any]], +) -> None: + """Reject a compacted turn that carries image attachments (LCORE-3582). + + In compacted mode the wire ``input`` is overridden with the explicit item + list built by :func:`_build_explicit_input`, which is text-only: image + attachments are converted to pydantic-ai ``ImageUrl`` parts on the prompt, + and the override replaces the prompt-derived input wholesale, so those + parts never reach the request body. + + Answering anyway would return a confident response that never saw the + image, with nothing to tell the caller their attachment was ignored. Fail + explicitly instead until the explicit input can carry ``input_image`` + content parts of its own (LCORE-3789). + + Args: + params: Prepared (possibly compaction-rewritten) request parameters. + image_attachments: Image attachments for this turn, if any. + + Raises: + HTTPException: 422 when the turn is compacted and carries images. + """ + if not image_attachments or not params.omit_conversation: + return + logger.warning( + "Rejecting compacted turn with %d image attachment(s): the explicit " + "input override cannot carry image content parts (LCORE-3789)", + len(image_attachments), + ) + response = UnprocessableEntityResponse( + response="Image attachments are not supported on this conversation", + cause=( + "This conversation has been compacted to fit the model's context " + "window, and compacted turns cannot carry image attachments yet. " + "Send the image in a new conversation, or retry without it." + ), + ) + raise HTTPException(**response.model_dump()) + + def _query_input_message(original_input: ResponseInput) -> list[Any]: """Render the new user query as explicit input items. diff --git a/tests/unit/utils/test_conversation_compaction.py b/tests/unit/utils/test_conversation_compaction.py index 3e5cc35da..f7b3a4af2 100644 --- a/tests/unit/utils/test_conversation_compaction.py +++ b/tests/unit/utils/test_conversation_compaction.py @@ -8,6 +8,7 @@ from typing import Any, Optional, cast import pytest +from fastapi import HTTPException from ogx_api.openai_responses import OpenAIResponseMessage from pytest_mock import MockerFixture @@ -143,6 +144,38 @@ def test_agent_prompt_text_explicit_list_returns_last_message_text() -> None: assert cc.agent_prompt_text(compacted) == "new question" +@pytest.mark.parametrize( + ("omit_conversation", "has_images"), + [(True, False), (False, True), (False, False)], +) +def test_reject_image_attachments_allows_supported_combinations( + omit_conversation: bool, has_images: bool +) -> None: + """Only a compacted turn carrying images is rejected; the rest pass through.""" + params = _params().model_copy(update={"omit_conversation": omit_conversation}) + attachments = [object()] if has_images else None + cc.reject_image_attachments_in_compacted_mode(params, attachments) + + +def test_reject_image_attachments_in_compacted_mode_raises_422() -> None: + """A compacted turn with images fails explicitly instead of ignoring them. + + The explicit-input override replaces the prompt-derived request input, and + the explicit list is text-only, so the images would never reach the model. + Answering anyway would return a confident response that never saw the + image, with nothing telling the caller it was dropped (LCORE-3582). + """ + params = _params().model_copy(update={"omit_conversation": True}) + + with pytest.raises(HTTPException) as exc_info: + cc.reject_image_attachments_in_compacted_mode(params, [object()]) + + assert exc_info.value.status_code == 422 + detail = exc_info.value.detail + assert "Image attachments are not supported" in detail["response"] + assert "compacted" in detail["cause"] + + def test_agent_prompt_text_empty_list_returns_empty() -> None: """An empty explicit list yields an empty prompt rather than crashing.""" params = _params().model_copy(update={"input": [], "omit_conversation": True})