mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-28 00:48:07 +00:00
fix(safety): backfill empty content-filter responses so they don't poison the thread (#4394)
An empty assistant message from a provider safety filter (content_filter with
no content, no tool calls) was persisted into thread history and replayed to
strict OpenAI-compatible providers, which reject it with HTTP 400 ("message ...
with role 'assistant' must not be empty") — breaking every later turn until a
new chat is started.
SafetyFinishReasonMiddleware only handled the tool-call case (#3028) and
TerminalResponseMiddleware only the post-tool case (#4027), so a plain empty
content-filter response fell through both. Extend the safety middleware to
backfill a user-facing explanation when a safety-terminated message is
otherwise blank, so the persisted turn is non-empty (and the user sees why it
was blocked).
Fixes #4393
This commit is contained in:
parent
d4fdc2758e
commit
b7933d18e4
@ -346,7 +346,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc
|
||||
30. **Custom middlewares** - *(optional)* Any `custom_middlewares` passed to `build_middlewares` are injected here, before config-declared extensions and the terminal-response/safety/clarification tail
|
||||
31. **Configured extension middlewares** - *(optional, if `extensions.middlewares` is set in `config.yaml` or `extensions_config.json`)* Zero-argument `AgentMiddleware` classes loaded from `module.path:ClassName` entries via `deerflow.reflection.resolve_class`. Missing packages, invalid classes, and broken modules fail loudly at agent creation. These run after built-ins/programmatic custom middleware and after the lead/subagent loop/token guards, but before the terminal-response/safety/clarification tail; subagents receive the same configured extension middleware class list before their safety tail. Treat these files as trusted operator config because middleware paths instantiate arbitrary code. Gateway skill/MCP toggle endpoints preserve this field through `to_file_dict()` but must not add a write path for `extensions.middlewares` without an explicit trust-boundary review. Lead-only vs subagent-only middleware lists and per-context constructor parameters are not expressible in this MVP.
|
||||
32. **TerminalResponseMiddleware** - When a provider returns an empty terminal `AIMessage` after tool execution, injects a hidden recovery prompt and retries the model once; a second empty response is replaced in checkpoint state by a visible error fallback marked for the run worker, so the run finishes as an error instead of a silent success
|
||||
33. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Suppresses tool execution when the provider safety-terminated the response (e.g. `finish_reason=content_filter`); registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first
|
||||
33. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Repairs AIMessages the provider safety-terminated (e.g. `finish_reason=content_filter`): strips truncated tool calls so they are not executed (#3028), and — when the response is otherwise blank (no tool calls, no visible content) — backfills a user-facing explanation so the empty message is not persisted and then rejected by strict OpenAI-compatible providers on the next request (`message ... with role 'assistant' must not be empty`), which would otherwise strand the whole thread (#4393). A safety-terminated response that still carries visible text is left untouched. Registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first
|
||||
34. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context.
|
||||
|
||||
### Configuration System
|
||||
|
||||
@ -1,6 +1,8 @@
|
||||
"""Suppress tool execution when the provider safety-terminated the response.
|
||||
"""Repair AIMessages the provider safety-terminated so they are neither
|
||||
executed nor persisted empty.
|
||||
|
||||
Background — see issue bytedance/deer-flow#3028.
|
||||
Background — see issues bytedance/deer-flow#3028 (truncated tool calls) and
|
||||
#4393 (empty response poisons the thread).
|
||||
|
||||
Some providers (OpenAI ``finish_reason='content_filter'``, Anthropic
|
||||
``stop_reason='refusal'``, Gemini ``finish_reason='SAFETY'`` ...) can stop
|
||||
@ -12,11 +14,22 @@ they were complete. The agent then sees the truncated file, tries to fix it,
|
||||
gets filtered again, and loops.
|
||||
|
||||
This middleware sits at ``after_model`` and gates that behaviour: when a
|
||||
configured ``SafetyTerminationDetector`` fires *and* the AIMessage carries
|
||||
tool calls, we strip the tool calls (both structured and raw provider
|
||||
payloads), append a user-facing explanation, and stash observability fields
|
||||
in ``additional_kwargs.safety_termination`` so logs, traces, and SSE
|
||||
consumers can see what happened.
|
||||
configured ``SafetyTerminationDetector`` fires it either
|
||||
|
||||
* strips the AIMessage's tool calls (both structured and raw provider
|
||||
payloads) when it carries any — the truncated-tool-call case (#3028), or
|
||||
* backfills a user-facing explanation when the message is otherwise blank
|
||||
(no tool calls, no visible content) — the empty-response case (#4393),
|
||||
where the empty assistant message would otherwise be persisted and then
|
||||
rejected by strict OpenAI-compatible providers on every following request
|
||||
("message ... with role 'assistant' must not be empty"), stranding the
|
||||
whole thread until a new chat is started.
|
||||
|
||||
A safety-terminated message that carries visible text but no tool calls is
|
||||
left untouched so its partial answer still reaches the user. Either way we
|
||||
append the explanation and stash observability fields in
|
||||
``additional_kwargs.safety_termination`` so logs, traces, and SSE consumers
|
||||
can see what happened.
|
||||
|
||||
Hook choice: ``after_model`` (not ``wrap_model_call``) because the response
|
||||
is a *normal* return — not an exception — and we want to participate in the
|
||||
@ -48,6 +61,7 @@ from deerflow.agents.middlewares.safety_termination_detectors import (
|
||||
default_detectors,
|
||||
)
|
||||
from deerflow.agents.middlewares.tool_call_metadata import clone_ai_message_with_tool_calls
|
||||
from deerflow.utils.messages import message_content_to_text
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from deerflow.config.safety_finish_reason_config import SafetyFinishReasonConfig
|
||||
@ -63,9 +77,15 @@ _USER_FACING_MESSAGE = (
|
||||
"or ask for a narrower output."
|
||||
)
|
||||
|
||||
# Used when the safety termination produced no tool calls *and* no content:
|
||||
# the message is rewritten only so it is not persisted empty (see #4393), so
|
||||
# it must not claim any tool calls were suppressed.
|
||||
_USER_FACING_EMPTY_MESSAGE = "The model provider stopped this response with a safety-related signal ({reason_field}={reason_value!r}, detector={detector!r}) and returned no content. Please rephrase your request or start a new conversation."
|
||||
|
||||
|
||||
class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]):
|
||||
"""Strip tool_calls from AIMessages flagged by a SafetyTerminationDetector."""
|
||||
"""Repair AIMessages flagged by a SafetyTerminationDetector: strip tool
|
||||
calls, or backfill an explanation when the message is otherwise empty."""
|
||||
|
||||
def __init__(self, detectors: list[SafetyTerminationDetector] | None = None) -> None:
|
||||
super().__init__()
|
||||
@ -135,8 +155,10 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]):
|
||||
message: AIMessage,
|
||||
termination: SafetyTermination,
|
||||
) -> AIMessage:
|
||||
suppressed_names = [tc.get("name") or "unknown" for tc in (message.tool_calls or [])]
|
||||
explanation = _USER_FACING_MESSAGE.format(
|
||||
tool_calls = message.tool_calls or []
|
||||
suppressed_names = [tc.get("name") or "unknown" for tc in tool_calls]
|
||||
template = _USER_FACING_MESSAGE if tool_calls else _USER_FACING_EMPTY_MESSAGE
|
||||
explanation = template.format(
|
||||
reason_field=termination.reason_field,
|
||||
reason_value=termination.reason_value,
|
||||
detector=termination.detector,
|
||||
@ -272,17 +294,34 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]):
|
||||
if not isinstance(last, AIMessage):
|
||||
return None
|
||||
|
||||
# Issue scope: only intervene when there's something to suppress.
|
||||
# ``content_filter`` without tool_calls is allowed through unchanged
|
||||
# so the partial text response (if any) reaches the user naturally.
|
||||
tool_calls = last.tool_calls
|
||||
if not tool_calls:
|
||||
# Two provider-safety failure modes are worth rewriting; a safety
|
||||
# termination that produced visible text with no tool calls is left
|
||||
# untouched so the partial answer still reaches the user naturally.
|
||||
# 1. tool_calls present: they may be truncated/unsafe (#3028), so
|
||||
# suppress them.
|
||||
# 2. blank content and no tool_calls: an empty assistant message
|
||||
# that strict OpenAI-compatible providers (Moonshot/Kimi, ...)
|
||||
# reject on the *next* request ("message ... with role
|
||||
# 'assistant' must not be empty", #4393), which poisons the whole
|
||||
# thread until a new chat is started. Backfill an explanation so
|
||||
# the persisted message is non-empty.
|
||||
tool_calls = list(last.tool_calls or [])
|
||||
# ``or ""`` normalizes every "no visible content" shape to blank:
|
||||
# None, "", [] and whitespace all count. None is reachable via
|
||||
# ``model_copy(update={"content": None})`` (a rewrite path that skips
|
||||
# validation); without the guard message_content_to_text stringifies
|
||||
# it to "None" and the backfill would be skipped, re-poisoning the
|
||||
# thread this fix is meant to protect.
|
||||
content_is_blank = not message_content_to_text(last.content or "").strip()
|
||||
if not tool_calls and not content_is_blank:
|
||||
return None
|
||||
|
||||
termination = self._detect(last)
|
||||
if termination is None:
|
||||
return None
|
||||
|
||||
backfilled_empty = content_is_blank and not tool_calls
|
||||
|
||||
# Stamp stop_reason so the worker can surface this capped completion
|
||||
# alongside loop_capped / token_capped (#4176).
|
||||
ctx = getattr(runtime, "context", None)
|
||||
@ -295,14 +334,16 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]):
|
||||
thread_id = runtime.context.get("thread_id") if isinstance(runtime.context, dict) else None
|
||||
|
||||
logger.warning(
|
||||
"Provider safety termination detected — suppressed %d tool call(s)",
|
||||
"Provider safety termination detected — suppressed %d tool call(s), backfilled_empty_content=%s",
|
||||
len(tool_calls),
|
||||
backfilled_empty,
|
||||
extra={
|
||||
"thread_id": thread_id,
|
||||
"detector": termination.detector,
|
||||
"reason_field": termination.reason_field,
|
||||
"reason_value": termination.reason_value,
|
||||
"suppressed_tool_call_names": [tc.get("name") for tc in tool_calls],
|
||||
"backfilled_empty_content": backfilled_empty,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@ -179,6 +179,56 @@ def test_content_filter_without_tool_calls_passes_through_unchanged():
|
||||
assert _TOOL_INVOCATIONS == []
|
||||
|
||||
|
||||
def test_content_filter_empty_no_tool_calls_is_backfilled_not_persisted_empty():
|
||||
"""#4393: a content_filter response with empty content and no tool calls
|
||||
must not survive in graph state as an empty AIMessage — strict
|
||||
OpenAI-compatible providers reject an empty assistant message on the next
|
||||
request, poisoning the whole thread. The middleware backfills a
|
||||
user-facing explanation so the persisted message is non-empty."""
|
||||
_TOOL_INVOCATIONS.clear()
|
||||
|
||||
class _EmptyContentFilterModel(BaseChatModel):
|
||||
"""Mimics Kimi/Moonshot refusing a sensitive question: an empty
|
||||
assistant message flagged finish_reason='content_filter'."""
|
||||
|
||||
@property
|
||||
def _llm_type(self) -> str:
|
||||
return "fake-empty-content-filter"
|
||||
|
||||
def bind_tools(self, tools, **kwargs):
|
||||
return self
|
||||
|
||||
def _generate(self, messages, stop=None, run_manager=None, **kwargs):
|
||||
msg = AIMessage(content="", response_metadata={"finish_reason": "content_filter"})
|
||||
return ChatResult(generations=[ChatGeneration(message=msg)])
|
||||
|
||||
async def _agenerate(self, messages, stop=None, run_manager=None, **kwargs):
|
||||
return self._generate(messages, stop=stop, run_manager=run_manager, **kwargs)
|
||||
|
||||
agent = create_agent(
|
||||
model=_EmptyContentFilterModel(),
|
||||
tools=[write_file],
|
||||
middleware=[SafetyFinishReasonMiddleware()],
|
||||
)
|
||||
result = agent.invoke({"messages": [HumanMessage(content="a sensitive question")]})
|
||||
final_ai = next(m for m in reversed(result["messages"]) if isinstance(m, AIMessage))
|
||||
|
||||
# The poison condition: the persisted assistant message must not be empty.
|
||||
assert isinstance(final_ai.content, str)
|
||||
assert final_ai.content.strip(), "empty assistant message would be rejected by strict providers on the next turn"
|
||||
assert "safety-related signal" in final_ai.content
|
||||
assert "returned no content" in final_ai.content
|
||||
|
||||
# Observability stamp present with zero suppressed tool calls.
|
||||
record = final_ai.additional_kwargs.get("safety_termination")
|
||||
assert record is not None
|
||||
assert record["suppressed_tool_call_count"] == 0
|
||||
|
||||
# Real provider reason preserved for downstream SSE / converters.
|
||||
assert final_ai.response_metadata.get("finish_reason") == "content_filter"
|
||||
assert _TOOL_INVOCATIONS == []
|
||||
|
||||
|
||||
def test_normal_tool_call_round_trip_is_not_affected():
|
||||
"""Regression: a healthy finish_reason='tool_calls' response must still
|
||||
execute the tool. The middleware must not over-fire."""
|
||||
|
||||
@ -113,6 +113,93 @@ class TestTriggerCriteria:
|
||||
}
|
||||
assert mw._apply(state, _runtime()) is None
|
||||
|
||||
def test_content_filter_blank_content_no_tool_calls_backfills(self):
|
||||
"""#4393: an empty content_filter response with no tool calls would be
|
||||
persisted empty and rejected by strict providers on the next request.
|
||||
Backfill an explanation so the persisted message is non-empty."""
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
state = {
|
||||
"messages": [
|
||||
_ai(
|
||||
content="",
|
||||
response_metadata={"finish_reason": "content_filter"},
|
||||
)
|
||||
]
|
||||
}
|
||||
result = mw._apply(state, _runtime())
|
||||
assert result is not None
|
||||
patched = result["messages"][0]
|
||||
assert patched.tool_calls == []
|
||||
assert isinstance(patched.content, str)
|
||||
assert patched.content.strip() # never persisted empty
|
||||
assert "safety-related signal" in patched.content
|
||||
assert "returned no content" in patched.content
|
||||
# It must not claim tool calls were suppressed — none existed.
|
||||
assert "were suppressed" not in patched.content
|
||||
record = patched.additional_kwargs["safety_termination"]
|
||||
assert record["suppressed_tool_call_count"] == 0
|
||||
assert record["suppressed_tool_call_names"] == []
|
||||
|
||||
def test_content_filter_whitespace_content_no_tool_calls_backfills(self):
|
||||
"""Whitespace-only content is still blank to a strict provider."""
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
state = {
|
||||
"messages": [
|
||||
_ai(
|
||||
content=" \n ",
|
||||
response_metadata={"finish_reason": "content_filter"},
|
||||
)
|
||||
]
|
||||
}
|
||||
result = mw._apply(state, _runtime())
|
||||
assert result is not None
|
||||
patched = result["messages"][0]
|
||||
assert patched.tool_calls == []
|
||||
assert "returned no content" in patched.content
|
||||
|
||||
def test_content_filter_none_content_no_tool_calls_backfills(self):
|
||||
"""content=None is reachable via model_copy rewrites (which skip
|
||||
validation) and must be treated as blank, not stringified to 'None'."""
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
none_content = _ai(response_metadata={"finish_reason": "content_filter"}).model_copy(update={"content": None})
|
||||
assert none_content.content is None # precondition for the regression
|
||||
result = mw._apply({"messages": [none_content]}, _runtime())
|
||||
assert result is not None
|
||||
patched = result["messages"][0]
|
||||
assert patched.tool_calls == []
|
||||
assert isinstance(patched.content, str)
|
||||
assert patched.content.strip()
|
||||
assert "returned no content" in patched.content
|
||||
|
||||
def test_anthropic_refusal_blank_content_no_tool_calls_backfills(self):
|
||||
"""The empty-content backfill is detector-agnostic (#4393)."""
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
state = {
|
||||
"messages": [
|
||||
_ai(
|
||||
content="",
|
||||
response_metadata={"stop_reason": "refusal"},
|
||||
)
|
||||
]
|
||||
}
|
||||
result = mw._apply(state, _runtime())
|
||||
assert result is not None
|
||||
assert result["messages"][0].content.strip()
|
||||
|
||||
def test_blank_content_no_tool_calls_without_safety_signal_passes_through(self):
|
||||
"""A blank response with no safety signal is out of scope: only a
|
||||
detected safety termination triggers the backfill."""
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
state = {
|
||||
"messages": [
|
||||
_ai(
|
||||
content="",
|
||||
response_metadata={"finish_reason": "stop"},
|
||||
)
|
||||
]
|
||||
}
|
||||
assert mw._apply(state, _runtime()) is None
|
||||
|
||||
def test_normal_tool_calls_pass_through(self):
|
||||
mw = SafetyFinishReasonMiddleware()
|
||||
state = {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user