From 2f4d7263457728b08d088c160724a16c9b9718be Mon Sep 17 00:00:00 2001 From: simonyang08 Date: Sun, 6 Sep 2026 00:43:57 +0800 Subject: [PATCH] fix(backends): reject empty user content before sending chat request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SimpleContext.view_for_generation() returns [] by design (stateless context), so a caller who chains .add(...) and then passes an empty or whitespace-only action to generate_from_context used to ship an empty user-role conversation to the model. Some chat models — Granite 4.2 in particular, see issue #1587 — spin on an empty prompt and burn tokens silently, which is hard to diagnose in CI. Raise ValueError in three concrete chat assembly paths: * OpenAIBackend._generate_from_chat_context_standard * LiteLLMBackend._generate_from_chat_context_standard * OllamaModelBackend.generate_from_chat_context The check ignores whitespace-only strings but still accepts user messages that carry images, audio, or documents (vision / RAG paths must continue to work). Each guard raises a ValueError before any HTTP or SDK call is issued. WatsonxAIBackend (mellea/backends/watsonx.py) and LocalHFBackend (mellea/backends/huggingface.py via mellea/backends/utils.py:to_chat) also assemble user-role conversations and would benefit from the same guard, but they are intentionally left untouched to keep this change focused on the backends named in the issue. Happy to mirror the same guard block in those two backends in a follow-up if preferred. SimpleContext.add's docstring is updated to make the stateless semantics explicit (recorded turns are never forwarded; the action argument is the only thing that reaches the model) and to call out which backends now enforce the invariant. Closes #1597 Assisted-by: ZCode (GLM) Signed-off-by: simonyang08 --- mellea/backends/litellm.py | 24 ++ mellea/backends/ollama.py | 24 ++ mellea/backends/openai.py | 24 ++ mellea/stdlib/context/simple.py | 24 +- test/backends/test_simple_context_guard.py | 367 +++++++++++++++++++++ 5 files changed, 461 insertions(+), 2 deletions(-) create mode 100644 test/backends/test_simple_context_guard.py diff --git a/mellea/backends/litellm.py b/mellea/backends/litellm.py index cd63301496..360ca33c0b 100644 --- a/mellea/backends/litellm.py +++ b/mellea/backends/litellm.py @@ -369,6 +369,30 @@ async def _generate_from_chat_context_standard( case _: messages.extend(self.formatter.to_chat_messages([action])) + # Issue #1597: refuse to send an empty user prompt. `SimpleContext` + # intentionally discards recorded turns from `view_for_generation()`, + # so a caller who chains `.add(...)` and then passes an empty action + # would otherwise hit the model with no user-role content at all. + # Some chat models (e.g. Granite 4.2, see #1587) spin on empty prompts + # and burn tokens silently. Fail fast instead. + if not any( + m.role == "user" + and ( + (m.content and m.content.strip()) + or m.images + or m.audio + or getattr(m, "_docs", None) + ) + for m in messages + ): + raise ValueError( + "Refusing to call the model: no user-role content in the assembled " + "conversation. This usually means a stateless context (e.g. " + "SimpleContext) was combined with an empty or whitespace-only " + "action; recorded turns are not forwarded to the model. See " + "issue #1597." + ) + # TODO: the supports_vision function is not reliably predicting if models support vision. E.g., ollama/llava is not a vision model? # if any(m.images is not None for m in messages): # # check if model can handle images diff --git a/mellea/backends/ollama.py b/mellea/backends/ollama.py index d555706ca1..2696fccc01 100644 --- a/mellea/backends/ollama.py +++ b/mellea/backends/ollama.py @@ -494,6 +494,30 @@ async def generate_from_chat_context( ) case _: messages.extend(self.formatter.to_chat_messages([action])) + + # Issue #1597: refuse to send an empty user prompt. `SimpleContext` + # intentionally discards recorded turns from `view_for_generation()`, + # so a caller who chains `.add(...)` and then passes an empty action + # would otherwise hit the model with no user-role content at all. + # Some chat models (e.g. Granite 4.2, see #1587) spin on empty prompts + # and burn tokens silently. Fail fast instead. + if not any( + m.role == "user" + and ( + (m.content and m.content.strip()) + or m.images + or m.audio + or getattr(m, "_docs", None) + ) + for m in messages + ): + raise ValueError( + "Refusing to call the model: no user-role content in the assembled " + "conversation. This usually means a stateless context (e.g. " + "SimpleContext) was combined with an empty or whitespace-only " + "action; recorded turns are not forwarded to the model. See " + "issue #1597." + ) # construct the conversation from our messages, adding a system prompt at the first message if one was provided. conversation: list[dict] = [] # We use system prompt None/empty-string semantics in a way that is consistent with Hugging Face and other libraries. diff --git a/mellea/backends/openai.py b/mellea/backends/openai.py index be73f513a5..3e12df6fa1 100644 --- a/mellea/backends/openai.py +++ b/mellea/backends/openai.py @@ -1095,6 +1095,30 @@ async def _generate_from_chat_context_standard( # ALoraRequirement may arrive here when no adapter is registered; # _generate is responsible for logging a warning in that case. + # Issue #1597: refuse to send an empty user prompt. `SimpleContext` + # intentionally discards recorded turns from `view_for_generation()`, + # so a caller who chains `.add(...)` and then passes an empty action + # would otherwise hit the model with no user-role content at all. + # Some chat models (e.g. Granite 4.2, see #1587) spin on empty prompts + # and burn tokens silently. Fail fast instead. + if not any( + m.role == "user" + and ( + (m.content and m.content.strip()) + or m.images + or m.audio + or getattr(m, "_docs", None) + ) + for m in messages + ): + raise ValueError( + "Refusing to call the model: no user-role content in the assembled " + "conversation. This usually means a stateless context (e.g. " + "SimpleContext) was combined with an empty or whitespace-only " + "action; recorded turns are not forwarded to the model. See " + "issue #1597." + ) + conversation: list[dict] = [] system_prompt = model_opts.get(ModelOption.SYSTEM_PROMPT, "") diff --git a/mellea/stdlib/context/simple.py b/mellea/stdlib/context/simple.py index 6803caa841..a24dc97414 100644 --- a/mellea/stdlib/context/simple.py +++ b/mellea/stdlib/context/simple.py @@ -9,11 +9,29 @@ class SimpleContext(Context): - """A `SimpleContext` is a context in which each interaction is a separate and independent turn. The history of all previous turns is NOT saved..""" + """A `SimpleContext` is a context in which each interaction is a separate and independent turn. The history of all previous turns is NOT saved.. + + Note: + Because `view_for_generation` always returns an empty list, anything + passed to `SimpleContext.add` is **never forwarded to the model** — it + is recorded only on the in-memory context chain. The action passed to + `generate_from_context` (or `MelleaSession.chat`) is the *only* thing + that reaches the model. Combining `.add(...)` with an empty/whitespace + action therefore produces an empty user prompt; the OpenAI, LiteLLM, + and Ollama backends now reject such calls with a `ValueError` (see + issue #1597) rather than sending an empty conversation to the model. + """ def add(self, c: Span) -> SimpleContext: """Add a new component or CBlock to the context and return the updated context. + The added span is stored on the context chain but is **not forwarded + to the model on subsequent generations** — `SimpleContext.view_for_generation` + always returns an empty list, so each generation is treated as a + stateless, independent turn. To actually talk to the model, pass the + prompt as the `action` argument to `MelleaSession.chat` / + `Backend.generate_from_context`, not via `add`. + Args: c (Span): The component, content block, or model output to record. @@ -28,7 +46,9 @@ def view_for_generation(self) -> list[Span] | None: """Return an empty list, since `SimpleContext` does not pass history to the model. Each call to the model is treated as a stateless, independent exchange. - No prior turns are forwarded. + No prior turns are forwarded. Spans recorded via `add` are kept on + the in-memory chain (`as_list`) for inspection but discarded for + generation. Returns: list[Span] | None: Always an empty list. diff --git a/test/backends/test_simple_context_guard.py b/test/backends/test_simple_context_guard.py new file mode 100644 index 0000000000..72f730e12b --- /dev/null +++ b/test/backends/test_simple_context_guard.py @@ -0,0 +1,367 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Regression tests for issue #1597: empty user content must not silently reach the model. + +`SimpleContext.view_for_generation()` returns `[]` by design (stateless context). +A caller can still `.add()` a user message and chain that to a generation; if the +action they pass is empty/whitespace, the assembled conversation sent to the +model contains no user-role content at all. The OpenAI/LiteLLM/Ollama chat +backends used to ship this empty payload, which makes some models (e.g. Granite +4.2, see issue #1587) spin on an empty prompt and burn tokens. + +The expected behaviour is a `ValueError` raised before any HTTP/SDK request is +issued. These tests pin that contract for the three concrete backends covered +by the fix; WatsonxAIBackend and LocalHFBackend are tracked as remaining work +in the commit message. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch + +import pytest +from openai.types.chat import ChatCompletion, ChatCompletionMessage +from openai.types.chat.chat_completion import Choice + + +def _ok_chat_completion(model: str = "gpt-4o") -> ChatCompletion: + return ChatCompletion( + id="test", + choices=[ + Choice( + finish_reason="stop", + index=0, + message=ChatCompletionMessage(role="assistant", content="ok"), + ) + ], + created=0, + model=model, + object="chat.completion", + ) + + +def _ok_litellm_response(): + """Return a non-streaming litellm ModelResponse that survives post_processing.""" + pytest.importorskip("litellm", reason="litellm not installed") + from litellm.types.utils import Choices, Message, ModelResponse + + msg = Message(content="ok", role="assistant") + choice = Choices(finish_reason="stop", index=0, message=msg) + return ModelResponse( + id="test", + choices=[choice], + created=0, + model="hosted_vllm/qwen3", + object="chat.completion", + usage={"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + ) + + +def _ok_ollama_response(content: str = "ok") -> "ollama.ChatResponse": + import ollama + + return ollama.ChatResponse( + model="granite4.2:3b", + created_at=None, + message=ollama.Message(role="assistant", content=content), + done=True, + ) + + +def _make_openai_backend(): + from mellea.backends.openai import OpenAIBackend + + return OpenAIBackend( + model_id="gpt-4o", + api_key="test-key", + base_url="http://localhost:9999/v1", + ) + + +def _make_litellm_backend(): + pytest.importorskip("litellm", reason="litellm not installed") + from mellea.backends.litellm import LiteLLMBackend + + return LiteLLMBackend( + model_id="hosted_vllm/qwen3", + base_url="http://localhost:9997", + ) + + +def _make_ollama_backend(): + from mellea.backends.ollama import OllamaModelBackend + + with ( + patch.object(OllamaModelBackend, "_check_ollama_server", return_value=True), + patch.object(OllamaModelBackend, "_pull_ollama_model", return_value=True), + patch("mellea.backends.ollama.ollama.Client", return_value=MagicMock()), + patch("mellea.backends.ollama.ollama.AsyncClient", return_value=MagicMock()), + ): + return OllamaModelBackend( + model_id="granite4.2:3b", + model_options=None, + ) + + +# --------------------------------------------------------------------------- +# OpenAI backend +# --------------------------------------------------------------------------- + + +async def test_simple_context_with_empty_action_raises_on_openai(): + """OpenAI: SimpleContext + empty CBlock must raise before HTTP request.""" + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_openai_backend() + ctx = SimpleContext().add(CBlock("recorded-only")) + + with patch.object( + backend._async_client.chat.completions, "create", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = _ok_chat_completion() + + with pytest.raises(ValueError, match="user"): + await backend.generate_from_chat_context(CBlock(value=""), ctx) + + assert not mock_create.called, ( + "Empty-user-content request must be rejected before any HTTP call " + "is issued. See issue #1597." + ) + + +async def test_simple_context_with_whitespace_action_raises_on_openai(): + """OpenAI: whitespace-only user content must also raise (issue #1587).""" + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_openai_backend() + ctx = SimpleContext() + + with patch.object( + backend._async_client.chat.completions, "create", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = _ok_chat_completion() + + with pytest.raises(ValueError, match="user"): + await backend.generate_from_chat_context(CBlock(value=" \n\t"), ctx) + + assert not mock_create.called + + +async def test_nonempty_user_content_still_sends_request_on_openai(): + """Sanity check: a non-empty user action must still hit the network.""" + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_openai_backend() + ctx = SimpleContext() + + with patch.object( + backend._async_client.chat.completions, "create", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = _ok_chat_completion() + + mot, _ = await backend.generate_from_chat_context( + CBlock(value="real question"), ctx + ) + await mot.avalue() + + assert mock_create.called, "Real user content must reach the model." + + +async def test_user_message_with_image_passes_guard_on_openai(): + """P3 passthrough: a user message with empty text but images must NOT raise. + + Vision/RAG workflows rely on attaching image or document blocks to a + message whose text content is short (e.g. "Caption this:"). The guard + must accept these as valid user-role content. + """ + from mellea.core import CBlock, ImageBlock + from mellea.stdlib.components import Message as MelleaMessage + from mellea.stdlib.context import SimpleContext + + backend = _make_openai_backend() + # SimpleContext discards prior turns, but the image-bearing action is the + # one that drives the request. + ctx = SimpleContext().add(CBlock("recorded-only")) + # 1x1 transparent PNG, base64-encoded as a string. + image = ImageBlock( + value="iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=" + ) + action = MelleaMessage("user", "", images=[image]) + + with patch.object( + backend._async_client.chat.completions, "create", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = _ok_chat_completion() + + mot, _ = await backend.generate_from_chat_context(action, ctx) + await mot.avalue() + + assert mock_create.called, ( + "User message with images must reach the model even when its text " + "content is empty." + ) + + +# --------------------------------------------------------------------------- +# LiteLLM backend +# --------------------------------------------------------------------------- + + +async def test_simple_context_with_empty_action_raises_on_litellm(): + """LiteLLM: SimpleContext + empty CBlock must raise before litellm.acompletion.""" + pytest.importorskip("litellm", reason="litellm not installed") + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_litellm_backend() + ctx = SimpleContext().add(CBlock("recorded-only")) + + with patch("litellm.acompletion", new_callable=AsyncMock) as mock_acomplete: + mock_acomplete.return_value = _ok_litellm_response() + + with pytest.raises(ValueError, match="user"): + await backend.generate_from_context(CBlock(value=""), ctx) + + assert not mock_acomplete.called + + +async def test_nonempty_user_content_still_sends_request_on_litellm(): + """LiteLLM counterpart to the OpenAI sanity check above.""" + pytest.importorskip("litellm", reason="litellm not installed") + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_litellm_backend() + ctx = SimpleContext() + + with patch("litellm.acompletion", new_callable=AsyncMock) as mock_acomplete: + mock_acomplete.return_value = _ok_litellm_response() + + mot, _ = await backend.generate_from_context( + CBlock(value="real question"), ctx + ) + await mot.avalue() + + assert mock_acomplete.called + + +# --------------------------------------------------------------------------- +# Ollama backend (issue #1587's real culprit) +# --------------------------------------------------------------------------- + + +def _patch_ollama_chat(backend, canned): + """Return `(context_manager, mock_chat_call)` for `_async_client.chat`. + + `_async_client` is an event-loop-keyed property, so we patch it at the + class level. The returned mock object's `.chat` is the AsyncMock + asserting whether the request was issued. + """ + mock_async = MagicMock() + mock_chat = AsyncMock(return_value=canned) + mock_async.chat = mock_chat + cm = patch.object( + type(backend), + "_async_client", + new_callable=PropertyMock, + return_value=mock_async, + ) + return cm, mock_chat + + +async def test_simple_context_with_empty_action_raises_on_ollama(): + """Ollama: SimpleContext + empty CBlock must raise before client.chat. + + Regression for the Granite 4.2 case in #1587. Without the guard the + empty conversation reaches the local Ollama server, which burns tokens + while the model spins on an empty prompt. + """ + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_ollama_backend() + ctx = SimpleContext().add(CBlock("recorded-only")) + + cm, mock_chat = _patch_ollama_chat(backend, _ok_ollama_response()) + with cm: + with pytest.raises(ValueError, match="user"): + await backend.generate_from_chat_context(CBlock(value=""), ctx) + + assert not mock_chat.called, ( + "Empty-user-content request must be rejected before any SDK call " + "is issued. See issue #1597." + ) + + +async def test_simple_context_with_whitespace_action_raises_on_ollama(): + """Ollama: whitespace-only user content must also raise.""" + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_ollama_backend() + ctx = SimpleContext() + + cm, mock_chat = _patch_ollama_chat(backend, _ok_ollama_response()) + with cm: + with pytest.raises(ValueError, match="user"): + await backend.generate_from_chat_context(CBlock(value=" \n\t"), ctx) + + assert not mock_chat.called + + +async def test_nonempty_user_content_still_sends_request_on_ollama(): + """Ollama sanity check: non-empty user action reaches client.chat.""" + from mellea.core import CBlock + from mellea.stdlib.context import SimpleContext + + backend = _make_ollama_backend() + ctx = SimpleContext() + + cm, mock_chat = _patch_ollama_chat(backend, _ok_ollama_response()) + with cm: + mot = await backend.generate_from_chat_context( + CBlock(value="real question"), ctx + ) + await mot.avalue() + + assert mock_chat.called + + +async def test_ollama_guard_does_not_trigger_when_ctx_has_real_user_message(): + """Guardian-style regression: a chat context with real history is left alone. + + Mirrors the call pattern in `mellea/stdlib/components/guardian.py`, where + the guardian calls the LLM with the *current* chat context (which + already contains real user turns) plus an empty/instructional action. + The guard must NOT fire here — the conversation has a non-empty user + message even if the action itself is empty. + """ + from mellea.core import CBlock + from mellea.stdlib.components import Message as MelleaMessage + from mellea.stdlib.context import ChatContext + + backend = _make_ollama_backend() + gctx = ChatContext() + gctx = gctx.add(MelleaMessage("user", "Is this code safe?")) + gctx = gctx.add(MelleaMessage("assistant", "It looks fine.")) + # The "action" is an empty CBlock (a structural placeholder), but the + # existing context already carries a real user turn, so the guard must + # NOT fire. + + cm, mock_chat = _patch_ollama_chat(backend, _ok_ollama_response("looks safe")) + with cm: + mot = await backend.generate_from_chat_context(CBlock(value=""), gctx) + await mot.avalue() + + assert mock_chat.called, ( + "ChatContext with real user history must reach the model even when " + "the action is empty — see guardian-style usage pattern." + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])