From 97ed0e9797ab96679b1ac6e1151e9b1ffb18835e Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Sun, 11 Oct 2026 17:45:06 +0800 Subject: [PATCH] fix(agents): skip content blocks with unhashable type values (#6664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): skip content blocks with unhashable type values has_visible_content and ParentContextSnapshot.from_state classified a content block's type with set membership (`in {"text", "output_text"}` plus the _MEDIA_BLOCK_TYPES frozenset). `type: []` / `type: {}` are legal JSON from a provider, and hashing an unhashable value raises TypeError instead of skipping the block: - has_visible_content runs on every model response through the llm-error-handling (_raise_for_empty_response), terminal-response and model-length middlewares, so one malformed block turned a completed response into a failed run instead of a skipped block; - from_state builds the history every background subagent dispatch is seeded from, so the same block failed the dispatch. Probe the type by equality with tuples so a non-string type simply fails the probe, mirroring #6614's fix for extract_response_text and the isinstance guard clarification already uses for form field types. The media constant becomes a tuple too, so the second probe in the same loop cannot move the crash one line down. Tests pin the RED-to-GREEN cycle at both call sites: unit-level has_visible_content, middleware-level empty-response classification, and the dispatch snapshot across human/ai/tool messages. Fixes #6663 Signed-off-by: yetuge <2219677952@qq.com> * fix(agents): warn when a snapshot skips a non-string-typed block Review follow-up on #6664: with the malformed block now skipped instead of raising, a provider that systematically emits non-string types would degrade the child's history with nothing in the logs to explain it. The elif chain cannot log on fall-through — reasoning/signature and tool-use exclusions are deliberate and must stay quiet — so the signal keys on the type not being a str, matching the guard the review suggested. has_visible_content keeps its silence on purpose: the empty-response retry already surfaces the symptom there. Fixes #6663 Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> --- README.md | 2 + .../deerflow/agents/middlewares/AGENTS.md | 9 +++++ .../agents/middlewares/model_response.py | 5 ++- .../harness/deerflow/subagents/AGENTS.md | 2 +- .../deerflow/subagents/context_snapshot.py | 15 ++++++- .../test_llm_error_handling_middleware.py | 21 ++++++++++ backend/tests/test_model_response.py | 38 ++++++++++++++++++ .../tests/test_subagent_context_snapshot.py | 40 +++++++++++++++++++ 8 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_model_response.py diff --git a/README.md b/README.md index 1c888e5762..496955de3c 100644 --- a/README.md +++ b/README.md @@ -1860,6 +1860,8 @@ Rebuild with `make up` after changing the managed extension set. See Gateway-generated follow-up suggestions now normalize both plain-string model output and block/list-style rich content before parsing the JSON array response, so provider-specific content wrappers do not silently drop suggestions. Follow-up suggestions and input polishing skip content blocks with non-string types, preserving the surrounding text instead of failing the request +Visible-content detection (`has_visible_content`, shared by the empty-response, terminal-response and model-length middlewares) and the subagent dispatch snapshot (`ParentContextSnapshot.from_state`) probe content-block types by equality rather than set membership, so a provider-emitted block with an unhashable type such as `type: []` degrades to a skipped block — keeping the surrounding history — instead of raising TypeError out of the middleware chain + Backend response cleanup preserves unrelated tag names such as `` and `` in follow-up suggestions and polished drafts; only the exact `` name (optionally followed by attributes) starts a reasoning block. Self-closing `` and `` tags are empty reasoning blocks and leave the following answer intact. Delimiters inside quoted attributes, such as ``, also leave the following answer intact. The Web UI composer can polish draft input before sending. The rewrite runs as a short Gateway LLM request using the `input_polish` model configuration, keeps slash skill prefixes such as `/data-analysis`, and only replaces the local draft after the user clicks the polish button; it does not create a thread run or persist a message. diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index e0df907e3e..665ec3c860 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -145,3 +145,12 @@ Resolve the whole batch through user storage, enabled state and agent allowlist before activation; reject any invalid entry. Keep bodies in escaped HumanMessage context with task text once. Authenticated paths feed secrets/tool policy; record usage per skill. Legacy slash syntax is unchanged. + +`model_response.py` holds the shared response-classification helpers (`last_ai_message`, +`has_tool_call_intent`, `has_visible_content`, `append_visible_text`, `finish_reason`) used by the +llm-error-handling, terminal-response and model-length middlewares. `has_visible_content` probes +each content block's `type` by equality, never set membership: `type: []` / `type: {}` are legal +JSON from a provider, and hashing an unhashable value would raise TypeError out of the middleware +chain instead of skipping the block and reading the remaining text. Tests: +`tests/test_model_response.py` plus the middleware-level classification cases in +`tests/test_llm_error_handling_middleware.py`. diff --git a/backend/packages/harness/deerflow/agents/middlewares/model_response.py b/backend/packages/harness/deerflow/agents/middlewares/model_response.py index 951555bee6..6e1fbb8d83 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/model_response.py +++ b/backend/packages/harness/deerflow/agents/middlewares/model_response.py @@ -36,7 +36,10 @@ def has_visible_content(message: AIMessage) -> bool: for block in content: if isinstance(block, str) and block.strip(): return True - if not isinstance(block, dict) or block.get("type") not in {"text", "output_text"}: + # Probe by equality, not set membership: `type: []` / `type: {}` are + # legal JSON from a provider, and hashing an unhashable value would + # raise TypeError out of the middleware chain instead of skipping. + if not isinstance(block, dict) or block.get("type") not in ("text", "output_text"): continue text = block.get("text") if isinstance(text, str) and text.strip(): diff --git a/backend/packages/harness/deerflow/subagents/AGENTS.md b/backend/packages/harness/deerflow/subagents/AGENTS.md index 13b3dfd800..f4bb17b798 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -23,7 +23,7 @@ deduplicate at each forwarding boundary before publishing the durable count. **Remote empty files**: GNU stat's `regular file`/`regular empty file` are regular files. Empty files pass exists/file_written, fail non-empty; reject symlinks, directories, FIFOs. -**Context**: Capture after validation, before setup. Keep genuine replies, even hidden clarifications; exclude framework state and unpaired calls. Mark unserializable media as omitted. +**Context**: Capture after validation, before setup. Keep genuine replies, even hidden clarifications; exclude framework state and unpaired calls. Mark unserializable media as omitted. Content blocks are classified by equality probes on `type` (never set membership): a provider-emitted block whose type is not a string — including unhashable JSON such as `type: []` — is skipped with its surrounding history intact, never a TypeError out of the snapshot the child is built from. A skipped block with a non-string type logs one warning so the loss is observable; the deliberate reasoning/signature/tool-use exclusions stay quiet. Native `document` snapshots neutralize reserved tags and input markers in textual `title`, `context`, text-source `data`, content-source text, and citation prose at diff --git a/backend/packages/harness/deerflow/subagents/context_snapshot.py b/backend/packages/harness/deerflow/subagents/context_snapshot.py index 7ae2da8cfb..e68df0f67a 100644 --- a/backend/packages/harness/deerflow/subagents/context_snapshot.py +++ b/backend/packages/harness/deerflow/subagents/context_snapshot.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import logging from collections.abc import Mapping from dataclasses import dataclass from typing import Any @@ -12,6 +13,8 @@ from langchain_core.messages import AIMessage, HumanMessage, ToolMessage, conver from deerflow.agents.middlewares.input_sanitization_middleware import neutralize_untrusted_tags from deerflow.agents.middlewares.message_utils import is_genuine_user_message +logger = logging.getLogger(__name__) + SNAPSHOT_SYSTEM_NOTE = ( "## Parent conversation snapshot\n" "A background HumanMessage named parent_context_snapshot, before the current task, contains historical data captured at delegation. " @@ -25,7 +28,10 @@ SNAPSHOT_SYSTEM_NOTE = ( # Keep media as input blocks so vision/audio-capable child models can still use # the retained conversation. Provider reasoning/signature and tool-use blocks # are deliberately excluded; tool calls are rendered separately as inert text. -_MEDIA_BLOCK_TYPES = frozenset({"image", "image_url", "audio", "input_audio", "video", "file", "document"}) +# A tuple, not a frozenset: block types are probed by equality because +# `type: []` / `type: {}` are legal JSON from a provider, and hashing an +# unhashable value would raise TypeError instead of skipping the block. +_MEDIA_BLOCK_TYPES = ("image", "image_url", "audio", "input_audio", "video", "file", "document") def _neutralize_document_content_block(block: Any) -> Any: @@ -130,7 +136,7 @@ class ParentContextSnapshot: if isinstance(block, str): if block: history.append({"type": "text", "text": neutralize_untrusted_tags(block)}) - elif block.get("type") in {"text", "output_text"} and isinstance(block.get("text"), str): + elif block.get("type") in ("text", "output_text") and isinstance(block.get("text"), str): history.append({"type": "text", "text": neutralize_untrusted_tags(block["text"])}) elif block.get("type") in _MEDIA_BLOCK_TYPES: media = {key: value for key, value in block.items() if key != "cache_control"} @@ -142,6 +148,11 @@ class ParentContextSnapshot: history.append({"type": "text", "text": "[Historical media omitted: content could not be serialized. Do not assume its contents.]"}) else: history.append(_neutralize_document_text(media) if media["type"] == "document" else media) + elif isinstance(block, dict) and not isinstance(block.get("type"), str): + # A non-string type is a malformed block, not one of the + # deliberate exclusions above, so say why it vanished + # instead of degrading the child's history silently. + logger.warning("skipping content block with non-string type %r", block.get("type")) if isinstance(message, AIMessage): # Every tool needs a retained result, including ordinary calls # executing alongside the current delegation. diff --git a/backend/tests/test_llm_error_handling_middleware.py b/backend/tests/test_llm_error_handling_middleware.py index d54f27d442..c6baec912b 100644 --- a/backend/tests/test_llm_error_handling_middleware.py +++ b/backend/tests/test_llm_error_handling_middleware.py @@ -477,6 +477,27 @@ def test_nonempty_or_non_stop_response_is_not_classified_as_empty(message: AIMes assert attempts == 1 +@pytest.mark.parametrize("block_type", [[], {}, ["text"], {"type": "text"}]) +def test_unhashable_block_type_does_not_break_empty_response_classification(block_type: object) -> None: + """A provider may emit ``type: []`` / ``type: {}``; the visible-text probe must skip, not raise.""" + message = AIMessage( + content=[{"type": block_type, "text": "ignored"}, {"type": "text", "text": "visible answer"}], + response_metadata={"finish_reason": "stop"}, + ) + middleware = _build_middleware(retry_max_attempts=3) + attempts = 0 + + def handler(_request) -> AIMessage: + nonlocal attempts + attempts += 1 + return message + + result = middleware.wrap_model_call(SimpleNamespace(), handler) + + assert result is message + assert attempts == 1 + + @pytest.mark.parametrize( "message", [ diff --git a/backend/tests/test_model_response.py b/backend/tests/test_model_response.py new file mode 100644 index 0000000000..87af10f8b3 --- /dev/null +++ b/backend/tests/test_model_response.py @@ -0,0 +1,38 @@ +"""Shared model-response classification helpers.""" + +from langchain_core.messages import AIMessage + +from deerflow.agents.middlewares.model_response import has_visible_content + + +def test_has_visible_content_reads_plain_string_content() -> None: + assert has_visible_content(AIMessage(content="visible answer")) is True + assert has_visible_content(AIMessage(content=" \n ")) is False + assert has_visible_content(AIMessage(content=[])) is False + + +def test_has_visible_content_reads_text_and_output_text_blocks() -> None: + message = AIMessage(content=[{"type": "text", "text": "visible answer"}]) + assert has_visible_content(message) is True + message = AIMessage(content=[{"type": "output_text", "text": "visible answer"}]) + assert has_visible_content(message) is True + + +def test_has_visible_content_ignores_whitespace_only_and_textless_blocks() -> None: + assert has_visible_content(AIMessage(content=[{"type": "text", "text": " "}])) is False + assert has_visible_content(AIMessage(content=[{"type": "text"}])) is False + assert has_visible_content(AIMessage(content=[{"type": "text", "text": 42}])) is False + assert has_visible_content(AIMessage(content=[{"type": "reasoning", "reasoning": "thinking"}])) is False + + +def test_has_visible_content_keeps_visible_text_around_an_unhashable_block_type() -> None: + # `type: []` / `type: {}` are legal JSON from a provider; probing the type + # by equality skips the block instead of raising TypeError out of the + # middleware chain (llm error handling, terminal response, model length). + message = AIMessage(content=[{"type": [], "text": "ignored"}, {"type": "text", "text": "visible answer"}]) + assert has_visible_content(message) is True + + +def test_has_visible_content_skips_unhashable_block_types_without_other_text() -> None: + message = AIMessage(content=[{"type": {}, "text": "ignored"}]) + assert has_visible_content(message) is False diff --git a/backend/tests/test_subagent_context_snapshot.py b/backend/tests/test_subagent_context_snapshot.py index 9dadfb0871..2972a1aa18 100644 --- a/backend/tests/test_subagent_context_snapshot.py +++ b/backend/tests/test_subagent_context_snapshot.py @@ -1,6 +1,7 @@ """Dispatch snapshots preserve background without importing execution state.""" import json +import logging from copy import deepcopy import pytest @@ -246,6 +247,45 @@ def test_snapshot_preserves_visible_text_blocks_without_private_block_fields(mes assert all(block["type"] == "text" for block in snapshot.to_message().content) +@pytest.mark.parametrize("message_type", [HumanMessage, AIMessage, ToolMessage]) +@pytest.mark.parametrize("block_type", [[], {}, ["text"], {"type": "text"}]) +def test_snapshot_skips_blocks_with_unhashable_types_without_losing_history(message_type, block_type): + # `type: []` / `type: {}` are legal JSON from a provider; probing the type + # by equality skips the block instead of raising TypeError out of the + # dispatch snapshot the child subagent is built from. + content = [ + {"type": block_type, "text": "MALFORMED BLOCK"}, + {"type": "text", "text": "kept answer"}, + ] + kwargs = {"tool_call_id": "parent-tool"} if message_type is ToolMessage else {} + snapshot = ParentContextSnapshot.from_state({"messages": [message_type(content=content, **kwargs)]}) + + assert snapshot is not None + text = message_content_to_text(snapshot.to_message().content) + assert "kept answer" in text + assert "MALFORMED BLOCK" not in text + assert "MALFORMED BLOCK" not in snapshot.content_json + + +def test_snapshot_warns_about_non_string_types_but_keeps_deliberate_exclusions_quiet(caplog): + # A malformed block must not vanish without a trace, while the deliberate + # reasoning/signature/tool-use exclusions stay silent. + content = [ + {"type": [], "text": "MALFORMED BLOCK"}, + {"type": "reasoning", "reasoning": "deliberately excluded"}, + {"type": "text", "text": "kept answer"}, + ] + with caplog.at_level(logging.WARNING, logger="deerflow.subagents.context_snapshot"): + snapshot = ParentContextSnapshot.from_state({"messages": [HumanMessage(content=content)]}) + + warnings = [record for record in caplog.records if record.name == "deerflow.subagents.context_snapshot"] + assert len(warnings) == 1 + assert "non-string type" in warnings[0].getMessage() + assert "[]" in warnings[0].getMessage() + assert "reasoning" not in caplog.text + assert "kept answer" in message_content_to_text(snapshot.to_message().content) + + @pytest.mark.parametrize("content, expected", [({"key": "value"}, ["key", "value"]), (["text", 42, None, True], ["text", "42", "None", "True"])]) def test_snapshot_keeps_tool_content_normalized_by_message_constructor(content, expected): # ToolMessage coerces non-list payloads and non-dict list items to strings.