mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-17 14:42:06 +00:00
fix(clarify): unwrap dict choices at the source so every surface gets clean text
The Discord fix (previous commit) handles dict-shaped clarify choices at the
Discord adapter only. The same dict-repr leak originates upstream at
tools/clarify_tool.py's str(c).strip() normalization — the single
platform-agnostic point both the CLI and every gateway adapter flow through.
When an LLM emits [{"description": "..."}] instead of bare strings, str(c)
produced {'description': '...'} which leaked onto the CLI panel
(cli.py:13048/13081), was returned verbatim as the user's answer
(cli.py:11945), and hit Telegram's numbered list too.
Add _flatten_choice (same label->description->text->title unwrap as the
Discord adapter, name/value excluded, keyless dicts dropped) and apply it at
the normalization line. Fixes CLI + Telegram + all platforms at the root;
the Discord smart-truncation now operates on already-clean text.
Adds johnjacobkenny to AUTHOR_MAP for the salvaged commit.
This commit is contained in:
parent
bce1e36b57
commit
2c3aebcadc
3 changed files with 105 additions and 1 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue