diff --git a/scripts/release.py b/scripts/release.py index 7e5901fd5682..20c6a6bfa0a1 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -103,6 +103,7 @@ AUTHOR_MAP = { "290859878+synapsesx@users.noreply.github.com": "synapsesx", "157689911+itsflownium@users.noreply.github.com": "itsflownium", "dirtyren@users.noreply.github.com": "dirtyren", + "johnjacobkenny@users.noreply.github.com": "johnjacobkenny", "chanyoung.kim@nota.ai": "channkim", "stevenn.damatoo@gmail.com": "x1erra", "evansrory@gmail.com": "zimigit2020", diff --git a/tests/tools/test_clarify_tool.py b/tests/tools/test_clarify_tool.py index 8659e1f13af5..0c38961dd8d2 100644 --- a/tests/tools/test_clarify_tool.py +++ b/tests/tools/test_clarify_tool.py @@ -9,6 +9,7 @@ from tools.clarify_tool import ( check_clarify_requirements, MAX_CHOICES, CLARIFY_SCHEMA, + _flatten_choice, ) @@ -164,6 +165,70 @@ class TestCheckClarifyRequirements: assert check_clarify_requirements() is True +class TestClarifyDictChoices: + """Dict-shaped choices must be unwrapped to user-facing text at the source. + + LLMs sometimes emit [{"description": "..."}] instead of bare strings. The + naive str(c) coercion leaked the Python dict repr onto every surface (CLI + panel, Discord buttons, Telegram list) AND returned it verbatim as the + user's answer. _flatten_choice normalises at the one platform-agnostic + entry point so the whole class is fixed in one place. + """ + + def test_flatten_unwraps_label_first(self): + assert _flatten_choice({"label": "Short", "description": "Long"}) == "Short" + + def test_flatten_unwraps_description_when_no_label(self): + assert _flatten_choice({"description": "A loose layout"}) == "A loose layout" + + def test_flatten_unwrap_order_label_over_description(self): + assert _flatten_choice({"description": "verbose", "label": "tight"}) == "tight" + + def test_flatten_drops_name_value_only_dict(self): + # name/value are component-shaped fields, not user-facing labels — + # picking them would leak raw enum values / short model ids. + assert _flatten_choice({"name": "tight", "value": "x"}) == "" + + def test_flatten_prefers_canonical_key_over_name(self): + assert _flatten_choice({"name": "tight", "description": "Tight desc"}) == "Tight desc" + + def test_flatten_drops_keyless_dict(self): + assert _flatten_choice({"foo": "bar", "n": 1}) == "" + + def test_flatten_passthrough_string_and_scalar(self): + assert _flatten_choice("plain") == "plain" + assert _flatten_choice(7) == "7" + assert _flatten_choice(None) == "" + + def test_dict_choices_reach_callback_as_clean_text(self): + """The whole point: the UI callback never sees a dict repr.""" + seen = [] + + def cb(question, choices): + seen.extend(choices or []) + return choices[0] + + result = json.loads(clarify_tool( + "Pick a layout", + choices=[ + {"choice": "Tight", "description": "Tight, covers all 3 points"}, + {"description": "Loose layout"}, + {"name": "modelid", "value": "abc"}, # dropped, not leaked + "A plain string choice", + ], + callback=cb, + )) # type: ignore + assert seen == [ + "Tight, covers all 3 points", + "Loose layout", + "A plain string choice", + ] + # and the resolved answer is clean text, not a dict repr + assert result["user_response"] == "Tight, covers all 3 points" + assert "{" not in result["user_response"] + assert all("{" not in c for c in result["choices_offered"]) + + class TestClarifySchema: """Tests for the OpenAI function-calling schema.""" diff --git a/tools/clarify_tool.py b/tools/clarify_tool.py index c44787554cca..3560ccf61268 100644 --- a/tools/clarify_tool.py +++ b/tools/clarify_tool.py @@ -20,6 +20,39 @@ from typing import List, Optional, Callable MAX_CHOICES = 4 +def _flatten_choice(c) -> str: + """Coerce a single choice into its user-facing display string. + + The schema declares choices as bare strings, but LLMs sometimes emit + dict-shaped choices like ``[{"description": "..."}]``. A naive ``str(c)`` + turns the whole dict into its Python repr — ``{'description': '...'}`` — + which then leaks onto every surface that renders the choice (CLI panel, + Discord buttons, Telegram numbered list) AND is returned verbatim as the + user's answer. Normalising here, at the one platform-agnostic entry point, + fixes the whole class in one place instead of per-adapter. + + Dict unwrap order is the canonical LLM tool-call user-facing keys: + ``label`` → ``description`` → ``text`` → ``title``. ``name`` and ``value`` + are deliberately excluded — they're component-shaped fields that could + carry raw enum values or short identifiers, not human-readable labels. A + dict with none of the canonical keys is dropped (returns ""), since a + garbage label is worse than no choice at all. + """ + if c is None: + return "" + if isinstance(c, str): + return c.strip() + if isinstance(c, dict): + for key in ("label", "description", "text", "title"): + v = c.get(key) + if isinstance(v, str) and v.strip(): + return v.strip() + return "" + if isinstance(c, (list, tuple)): + return " ".join(_flatten_choice(x) for x in c).strip() + return str(c).strip() + + def clarify_tool( question: str, choices: Optional[List[str]] = None, @@ -48,7 +81,12 @@ def clarify_tool( if choices is not None: if not isinstance(choices, list): return tool_error("choices must be a list of strings.") - choices = [str(c).strip() for c in choices if str(c).strip()] + # LLMs sometimes emit dict-shaped choices (e.g. [{"description": "..."}]) + # instead of bare strings. _flatten_choice unwraps them to their + # user-facing text here — the single platform-agnostic entry point — + # so the CLI panel, Discord buttons, and Telegram list all render clean + # text and the resolved answer is never a raw Python dict repr. + choices = [s for s in (_flatten_choice(c) for c in choices) if s] if len(choices) > MAX_CHOICES: choices = choices[:MAX_CHOICES] if not choices: