Skip to content

Commit 0023047

Browse files
chuenchen309claude
andcommitted
fix(llm): AttributeError instead of BadRequestError on list-typed instruction content
_validate_messages in openai.py, anthropic.py, and google.py all check `(m.content or "").strip()` to detect empty system/developer content. But `SystemMessage`/`DeveloperMessage.content` is typed `str | Sequence[TextContent]` -- a plain-dict payload (e.g. content as a list of `{"type": "text", "text": ...}` blocks, matching real provider API shapes) validates fine against the Pydantic model, but a non-empty list is truthy, so `.strip()` is called on a `list` and raises AttributeError instead of the intended graceful BadRequestError -- or, for genuinely non-empty content, crashes a request that should have succeeded. Fix: use the existing `.text` property (already defined on these message models for exactly this normalization) instead of raw `.content`. Same one-line fix duplicated across all three providers. Added one regression test per provider constructing a developer message with list-typed content and asserting the completion succeeds instead of raising AttributeError. TDD red->green verified: reverting only the three provider files reproduces `AttributeError: 'list' object has no attribute 'strip'` for all three; reapplying passes. Full libs/giskard-llm unit suite (211 passed, 7 skipped); ruff/ruff-format clean; basedpyright 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9f9ed89 commit 0023047

4 files changed

Lines changed: 90 additions & 3 deletions

File tree

libs/giskard-llm/src/giskard/llm/providers/anthropic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ def _validate_messages(self, messages: Sequence[ChatMessage]) -> None:
224224
raise BadRequestError(
225225
400, "Tool messages must have a tool_call_id.", PROVIDER
226226
)
227-
if m.role in _ANTHROPIC_INSTRUCTION_ROLES and not (m.content or "").strip():
227+
if m.role in _ANTHROPIC_INSTRUCTION_ROLES and not (m.text or "").strip():
228228
raise BadRequestError(
229229
400,
230230
"System and developer messages must have non-empty content.",

libs/giskard-llm/src/giskard/llm/providers/google.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def _validate_messages(self, messages: Sequence[ChatMessage]) -> None:
331331
raise BadRequestError(
332332
400, "Tool messages must have a tool_call_id.", PROVIDER
333333
)
334-
if m.role in _INSTRUCTION_ROLES and not (m.content or "").strip():
334+
if m.role in _INSTRUCTION_ROLES and not (m.text or "").strip():
335335
raise BadRequestError(
336336
400, "System messages must have non-empty content.", PROVIDER
337337
)

libs/giskard-llm/src/giskard/llm/providers/openai.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ def _validate_messages(self, messages: Sequence[ChatMessage]) -> None:
222222
raise BadRequestError(
223223
400, "Tool messages must have a tool_call_id.", self._PROVIDER
224224
)
225-
if m.role in _INSTRUCTION_ROLES and not (m.content or "").strip():
225+
if m.role in _INSTRUCTION_ROLES and not (m.text or "").strip():
226226
raise BadRequestError(
227227
400, "System messages must have non-empty content.", self._PROVIDER
228228
)

libs/giskard-llm/tests/test_providers.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,27 @@ async def test_openai_validate_empty_system_content(mock_import):
596596
)
597597

598598

599+
@patch("giskard.llm.providers.openai._import_openai")
600+
@pytest.mark.openai
601+
async def test_openai_validate_developer_content_as_content_blocks(mock_import):
602+
"""Developer ``content`` typed as a list of ``TextContent`` blocks (the SDK-native
603+
shape, reachable via plain-dict validation) must be normalized through ``.text``,
604+
not crash with ``AttributeError`` from calling ``.strip()`` on a list."""
605+
mock_import.return_value = MagicMock()
606+
provider = _make_openai_provider()
607+
provider._client.chat.completions.create = AsyncMock(
608+
return_value=_make_openai_response("Hello")
609+
)
610+
resp = await provider.complete(
611+
"gpt-4o",
612+
[
613+
{"role": "developer", "content": [{"type": "text", "text": "Be helpful"}]},
614+
{"role": "user", "content": "Hi"},
615+
],
616+
)
617+
assert resp.choices[0].message.content == "Hello"
618+
619+
599620
@patch("giskard.llm.providers.openai._import_openai")
600621
@pytest.mark.openai
601622
async def test_openai_multiple_system_works(mock_import):
@@ -719,6 +740,31 @@ async def test_anthropic_validate_empty_developer_content(mock_import):
719740
)
720741

721742

743+
@patch("giskard.llm.providers.anthropic._import_anthropic")
744+
async def test_anthropic_validate_developer_content_as_content_blocks(mock_import):
745+
"""Developer ``content`` typed as a list of ``TextContent`` blocks must be
746+
normalized through ``.text``, not crash with ``AttributeError`` from calling
747+
``.strip()`` on a list."""
748+
mock_import.return_value = MagicMock()
749+
provider = _make_anthropic_provider()
750+
751+
mock_raw = MagicMock()
752+
mock_raw.content = [SimpleNamespace(type="text", text="Hello")]
753+
mock_raw.stop_reason = "end_turn"
754+
mock_raw.model = "claude-3"
755+
mock_raw.usage = SimpleNamespace(input_tokens=10, output_tokens=5)
756+
provider._client.messages.create = AsyncMock(return_value=mock_raw)
757+
758+
resp = await provider.complete(
759+
"claude-3",
760+
[
761+
{"role": "developer", "content": [{"type": "text", "text": "Be helpful"}]},
762+
{"role": "user", "content": "Hi"},
763+
],
764+
)
765+
assert resp.choices[0].message.content == [TextContent(text="Hello")]
766+
767+
722768
@patch("giskard.llm.providers.anthropic._import_anthropic")
723769
async def test_anthropic_validate_alternation(mock_import):
724770
mock_import.return_value = MagicMock()
@@ -734,6 +780,47 @@ async def test_anthropic_validate_alternation(mock_import):
734780
)
735781

736782

783+
# -- Google message validation --------------------------------------------------
784+
785+
786+
@patch("giskard.llm.providers.google._import_genai_errors")
787+
@pytest.mark.google
788+
async def test_google_validate_developer_content_as_content_blocks(mock_errors):
789+
"""Developer ``content`` typed as a list of ``TextContent`` blocks must be
790+
normalized through ``.text``, not crash with ``AttributeError`` from calling
791+
``.strip()`` on a list."""
792+
genai_types = pytest.importorskip("google.genai.types")
793+
mock_errors.return_value = MagicMock()
794+
provider = _make_google_provider()
795+
provider._client.aio = MagicMock()
796+
provider._client.aio.models = MagicMock()
797+
raw = genai_types.GenerateContentResponse.model_validate(
798+
{
799+
"candidates": [
800+
{
801+
"content": {"parts": [{"text": "Hello"}]},
802+
"finish_reason": "STOP",
803+
}
804+
],
805+
"usage_metadata": {
806+
"prompt_token_count": 3,
807+
"candidates_token_count": 4,
808+
"total_token_count": 7,
809+
},
810+
}
811+
)
812+
provider._client.aio.models.generate_content = AsyncMock(return_value=raw)
813+
814+
resp = await provider.complete(
815+
"gemini-3.5-flash",
816+
[
817+
{"role": "developer", "content": [{"type": "text", "text": "Be helpful"}]},
818+
{"role": "user", "content": "Hi"},
819+
],
820+
)
821+
assert resp.choices[0].message.content == [TextContent(text="Hello")]
822+
823+
737824
# -- OpenAI Responses API (respond) -------------------------------------------
738825

739826

0 commit comments

Comments
 (0)