From 88f9f81877dee8d3d9705caf183a54c894207e4e Mon Sep 17 00:00:00 2001 From: qin-chenghan Date: Tue, 28 Jul 2026 19:43:56 +0800 Subject: [PATCH] fix(clarification): normalize dict options (#4527) --- backend/AGENTS.md | 2 +- .../middlewares/clarification_middleware.py | 27 ++++++++++++++-- .../tests/test_clarification_middleware.py | 32 +++++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index f3213c558..1c32c47bc 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -367,7 +367,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc 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. **ModelLengthFinishReasonMiddleware** - Records `stop_reason=model_length_capped` when provider-specific length detectors match a terminal `AIMessage` without tool-call intent (`finish_reason=length` / `MAX_TOKENS`, or `stop_reason=max_tokens`), preserving the original assistant content and never reparsing textual tool-call-like envelopes 34. **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 -35. **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). Payloads are versioned: legacy modes (`free_text` / `choice_with_other`) keep `version: 1` unchanged, while the v2 `form` mode (from `fields`) carries `version: 2` so older frontends reject the payload and degrade to the plain-text fallback. Field normalization is deterministic and lives in the middleware, not the tool schema — the middleware short-circuits before tool execution, so tool-arg typing alone provides no runtime validation. Validation is atomic: any structurally broken entry (non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member like `__proto__`/`constructor`, exceeding the caps of 16 fields / 24 options per field / 200 chars per text, or the whole normalized definition exceeding `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8 — the per-item caps alone admit forms whose IM text fallback would blow channel delivery limits and truncate away trailing fields) degrades the whole form to the legacy option/free-text modes, so a card can never render "complete" while silently missing a business field; benign issues keep local degradation (unknown types — including unhashable JSON like `type: []`, which must never raise from the membership probe — and option-less selects become `text`), and options are trimmed/deduped with blanks dropped (both form-level and top-level) because the frontend parser rejects blank option labels. Checkbox fields are booleans that default to an explicit "no"; `required` on a checkbox means must-agree/consent semantics. The response protocol is deliberately unchanged (v1 `text`/`option` only): form cards submit a readable text summary as `response_kind: "text"`, so journal persistence and answered-card recovery need no new allowlist entries. 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. +35. **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). Payloads are versioned: legacy modes (`free_text` / `choice_with_other`) keep `version: 1` unchanged, while the v2 `form` mode (from `fields`) carries `version: 2` so older frontends reject the payload and degrade to the plain-text fallback. Field normalization is deterministic and lives in the middleware, not the tool schema — the middleware short-circuits before tool execution, so tool-arg typing alone provides no runtime validation. Validation is atomic: any structurally broken entry (non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member like `__proto__`/`constructor`, exceeding the caps of 16 fields / 24 options per field / 200 chars per text, or the whole normalized definition exceeding `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8 — the per-item caps alone admit forms whose IM text fallback would blow channel delivery limits and truncate away trailing fields) degrades the whole form to the legacy option/free-text modes, so a card can never render "complete" while silently missing a business field; benign issues keep local degradation (unknown types — including unhashable JSON like `type: []`, which must never raise from the membership probe — and option-less selects become `text`), and options are trimmed/deduped with blanks dropped (both form-level and top-level) because the frontend parser rejects blank option labels. Model-produced XML-to-dict option payloads are recursively flattened from dict/list containers in source order, scalar string/number leaves are retained, and residual XML tags are removed before the same trimming and deduplication. Checkbox fields are booleans that default to an explicit "no"; `required` on a checkbox means must-agree/consent semantics. The response protocol is deliberately unchanged (v1 `text`/`option` only): form cards submit a readable text summary as `response_kind: "text"`, so journal persistence and answered-card recovery need no new allowlist entries. 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 diff --git a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py index 488740331..6e89f12cc 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py @@ -2,6 +2,7 @@ import json import logging +import re from collections.abc import Callable from hashlib import sha256 from typing import Any, override @@ -56,6 +57,8 @@ MAX_FIELD_TEXT_CHARS = 200 # leaving headroom for question/context. MAX_FORM_SERIALIZED_BYTES = 16_384 +_XML_TAG_RE = re.compile(r"]*?)?\s*/?>") + class ClarificationMiddlewareState(AgentState): """Compatible with the `ThreadState` schema.""" @@ -100,7 +103,9 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): if options is None: return [] - if not isinstance(options, list): + if isinstance(options, dict): + options = self._flatten_dict_option_values(options) + elif not isinstance(options, list): options = [options] # Trim, drop blanks, and dedupe (order-preserving): the frontend parser @@ -109,13 +114,31 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): normalized: list[str] = [] seen: set[str] = set() for option in options: - text = str(option).strip() + text = _XML_TAG_RE.sub("", str(option)).strip() if not text or text in seen: continue seen.add(text) normalized.append(text) return normalized + @staticmethod + def _flatten_dict_option_values(value: dict[str, Any]) -> list[str | int | float]: + """Flatten scalar leaves from XML-to-dict option payloads in source order.""" + flattened: list[str | int | float] = [] + + def collect(nested: Any) -> None: + if isinstance(nested, dict): + for item in nested.values(): + collect(item) + elif isinstance(nested, list): + for item in nested: + collect(item) + elif isinstance(nested, str | int | float): + flattened.append(nested) + + collect(value) + return flattened + @staticmethod def _normalize_bool(raw: Any) -> bool: """Coerce a model-provided boolean; some models serialize booleans as strings or 1/0.""" diff --git a/backend/tests/test_clarification_middleware.py b/backend/tests/test_clarification_middleware.py index 8155443f5..a824e7c2a 100644 --- a/backend/tests/test_clarification_middleware.py +++ b/backend/tests/test_clarification_middleware.py @@ -173,6 +173,38 @@ class TestHumanInputPayload: {"id": "option-4", "label": "None", "value": "None"}, ] + def test_payload_flattens_xml_parsed_dict_options(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "How should the document structure change?", + "clarification_type": "approach_choice", + "options": { + "item": { + "item": "Move the system section earlier", + "$text": "Merge it into the shared patterns section", + }, + "$text": "Add a standalone section", + }, + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["options"] == [ + {"id": "option-1", "label": "Move the system section earlier", "value": "Move the system section earlier"}, + { + "id": "option-2", + "label": "Merge it into the shared patterns section", + "value": "Merge it into the shared patterns section", + }, + {"id": "option-3", "label": "Add a standalone section", "value": "Add a standalone section"}, + ] + + def test_dict_options_recursively_flatten_string_and_number_values(self, middleware): + options = {"item": [["First"], {"$text": 2}], "$text": None} + + assert middleware._normalize_options(options) == ["First", "2"] + def test_payload_with_plain_string_option(self, middleware): payload = middleware._build_human_input_payload( {