mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
feat(clarify): extend multi-select to gateway text fallback and TUI bridge
Cross-surface coverage #23768 missed (per review feedback): - tools/clarify_gateway.py: _ClarifyEntry carries a multi_select flag (register() accepts it; signature() exposes it to adapters). _coerce_text_response now parses multi-select replies — comma- or space-separated numbers ('1,3' / '1 3'), exact labels, dedup — into a JSON array string that _parse_multi_select_response decodes into a list. Out-of-range/unknown tokens reject the reply (native button UI) or fall back to custom text (awaiting_text/'Other' mode). - gateway/run.py: _clarify_callback_sync accepts multi_select and registers it on the pending entry. - gateway/platforms/base.py: default numbered-list text fallback tells the user multiple selections are allowed and how to reply. - tui_gateway/server.py: clarify_callback passes multi_select through the clarify.request payload as a hint; renderers without checkbox support ignore the field and remain single-select-compatible. - tests: 13 new gateway tests (flag storage, comma/space/single-number parsing, label matching, out-of-range rejection, dedup, end-to-end resolve, single-select regressions).
This commit is contained in:
parent
0b670f0539
commit
10b7ab5cb6
5 changed files with 215 additions and 4 deletions
|
|
@ -3512,11 +3512,28 @@ class BasePlatformAdapter(ABC):
|
|||
override this for a richer UX.
|
||||
"""
|
||||
if choices:
|
||||
# Multi-select clarifies register their flag on the pending entry;
|
||||
# look it up by id so the signature stays adapter-compatible.
|
||||
_is_multi = False
|
||||
try:
|
||||
from tools import clarify_gateway as _cg
|
||||
with _cg._lock:
|
||||
_entry = _cg._entries.get(clarify_id)
|
||||
_is_multi = bool(_entry and getattr(_entry, "multi_select", False))
|
||||
except Exception:
|
||||
_is_multi = False
|
||||
lines = [f"❓ {question}", ""]
|
||||
for i, choice in enumerate(choices, start=1):
|
||||
lines.append(f" {i}. {choice}")
|
||||
lines.append("")
|
||||
lines.append("Reply with the number, the option text, or your own answer.")
|
||||
if _is_multi:
|
||||
lines.append(
|
||||
"Multiple selections allowed — reply with the numbers "
|
||||
"separated by commas or spaces (e.g. \"1, 3\"), the option "
|
||||
"text, or your own answer."
|
||||
)
|
||||
else:
|
||||
lines.append("Reply with the number, the option text, or your own answer.")
|
||||
text = "\n".join(lines)
|
||||
# Text fallback: enable text-capture so the gateway intercept
|
||||
# picks up the user's typed reply (e.g. "2" or choice text).
|
||||
|
|
|
|||
|
|
@ -21877,7 +21877,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
# explaining that no response arrived (so the agent can adapt
|
||||
# rather than hang forever).
|
||||
# ------------------------------------------------------------------
|
||||
def _clarify_callback_sync(question: str, choices) -> str:
|
||||
def _clarify_callback_sync(question: str, choices, multi_select: bool = False) -> str:
|
||||
from tools import clarify_gateway as _clarify_mod
|
||||
import uuid as _uuid
|
||||
|
||||
|
|
@ -21890,6 +21890,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
session_key=session_key or "",
|
||||
question=question,
|
||||
choices=list(choices) if choices else None,
|
||||
multi_select=bool(multi_select),
|
||||
)
|
||||
|
||||
# Pause typing — like approval, we don't want a "thinking..."
|
||||
|
|
|
|||
|
|
@ -439,3 +439,120 @@ class TestUnlimitedWait:
|
|||
t.join(timeout=5.0)
|
||||
assert not t.is_alive()
|
||||
assert result_box["r"] == "B"
|
||||
|
||||
|
||||
class TestMultiSelectTextFallback:
|
||||
"""Multi-select clarifies via the gateway text fallback.
|
||||
|
||||
The adapter's numbered-list fallback asks the user to reply with
|
||||
comma/space-separated numbers; _coerce_text_response must map those to a
|
||||
JSON array of choice labels (which _parse_multi_select_response on the
|
||||
tool side decodes into a list).
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
_clear_clarify_state()
|
||||
|
||||
def _register_multi(self, cid="m1", choices=("A", "B", "C")):
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register(cid, "sk", "Pick some", list(choices), multi_select=True)
|
||||
# Text fallback path always flips awaiting_text on.
|
||||
cm.mark_awaiting_text(cid)
|
||||
return entry
|
||||
|
||||
def test_register_stores_multi_select_flag(self):
|
||||
entry = self._register_multi()
|
||||
assert entry.multi_select is True
|
||||
assert entry.signature()["multi_select"] is True
|
||||
|
||||
def test_register_default_multi_select_false(self):
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register("s1", "sk", "Q?", ["A"])
|
||||
assert entry.multi_select is False
|
||||
assert entry.signature()["multi_select"] is False
|
||||
|
||||
def test_multi_select_without_choices_is_ignored(self):
|
||||
"""multi_select on an open-ended clarify is meaningless — dropped."""
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register("s2", "sk", "Q?", None, multi_select=True)
|
||||
assert entry.multi_select is False
|
||||
|
||||
def test_comma_separated_numbers(self):
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
coerced = cm._coerce_text_response(entry, "1, 3")
|
||||
assert json.loads(coerced) == ["A", "C"]
|
||||
|
||||
def test_space_separated_numbers(self):
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
coerced = cm._coerce_text_response(entry, "1 3")
|
||||
assert json.loads(coerced) == ["A", "C"]
|
||||
|
||||
def test_single_number(self):
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
coerced = cm._coerce_text_response(entry, "2")
|
||||
assert json.loads(coerced) == ["B"]
|
||||
|
||||
def test_choice_labels_comma_separated(self):
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
coerced = cm._coerce_text_response(entry, "a, C")
|
||||
assert json.loads(coerced) == ["A", "C"]
|
||||
|
||||
def test_out_of_range_number_rejected_but_custom_text_kept(self):
|
||||
"""Out-of-range numbers don't parse as a selection; awaiting_text
|
||||
mode falls back to accepting the raw text as a custom answer."""
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
assert cm._coerce_multi_select_text(entry, "1, 9") is None
|
||||
# awaiting_text (text fallback) keeps the raw reply as custom text
|
||||
assert cm._coerce_text_response(entry, "1, 9") == "1, 9"
|
||||
|
||||
def test_out_of_range_rejected_for_native_button_ui(self):
|
||||
"""Without awaiting_text (button UI), a bad selection rejects the
|
||||
reply entirely so it flows through as a normal message."""
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register("m2", "sk", "Pick some", ["A", "B"], multi_select=True)
|
||||
assert cm._coerce_text_response(entry, "5") is None
|
||||
assert cm._coerce_text_response(entry, "random prose") is None
|
||||
|
||||
def test_duplicate_selections_deduped(self):
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
entry = self._register_multi()
|
||||
coerced = cm._coerce_text_response(entry, "1, 1, 2")
|
||||
assert json.loads(coerced) == ["A", "B"]
|
||||
|
||||
def test_resolve_text_response_end_to_end(self):
|
||||
"""resolve_text_response_for_session delivers the JSON array to the waiter."""
|
||||
import json
|
||||
from tools import clarify_gateway as cm
|
||||
self._register_multi(cid="m3")
|
||||
result_box = {}
|
||||
|
||||
def waiter():
|
||||
result_box["r"] = cm.wait_for_response("m3", timeout=5)
|
||||
|
||||
t = threading.Thread(target=waiter)
|
||||
t.start()
|
||||
time.sleep(0.05)
|
||||
assert cm.resolve_text_response_for_session("sk", "1,2") is True
|
||||
t.join(timeout=5)
|
||||
assert json.loads(result_box["r"]) == ["A", "B"]
|
||||
|
||||
def test_single_select_regression_numeric(self):
|
||||
"""Single-select coercion unchanged: '2' maps to the choice label string."""
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register("s3", "sk", "Q?", ["A", "B", "C"])
|
||||
assert cm._coerce_text_response(entry, "2") == "B"
|
||||
|
||||
def test_single_select_regression_label(self):
|
||||
from tools import clarify_gateway as cm
|
||||
entry = cm.register("s4", "sk", "Q?", ["A", "B"])
|
||||
assert cm._coerce_text_response(entry, "b") == "B"
|
||||
|
|
|
|||
|
|
@ -51,6 +51,7 @@ class _ClarifyEntry:
|
|||
session_key: str
|
||||
question: str
|
||||
choices: Optional[List[str]]
|
||||
multi_select: bool = False
|
||||
event: threading.Event = field(default_factory=threading.Event)
|
||||
response: Optional[str] = None
|
||||
awaiting_text: bool = False # set when user picked "Other" or clarify is open-ended
|
||||
|
|
@ -61,6 +62,7 @@ class _ClarifyEntry:
|
|||
"session_key": self.session_key,
|
||||
"question": self.question,
|
||||
"choices": list(self.choices) if self.choices else None,
|
||||
"multi_select": bool(self.multi_select),
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -80,6 +82,7 @@ def register(
|
|||
session_key: str,
|
||||
question: str,
|
||||
choices: Optional[List[str]],
|
||||
multi_select: bool = False,
|
||||
) -> _ClarifyEntry:
|
||||
"""Register a pending clarify request and return the entry.
|
||||
|
||||
|
|
@ -91,6 +94,7 @@ def register(
|
|||
session_key=session_key,
|
||||
question=question,
|
||||
choices=list(choices) if choices else None,
|
||||
multi_select=bool(multi_select) and bool(choices),
|
||||
# Open-ended (no choices) → next message IS the response, no buttons needed.
|
||||
awaiting_text=not bool(choices),
|
||||
)
|
||||
|
|
@ -205,6 +209,14 @@ def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
|||
- Accept exact choice label matches (case-insensitive)
|
||||
- Reject arbitrary prose (return None) so the message continues as a normal turn
|
||||
|
||||
For multi-select clarifies (entry.multi_select=True):
|
||||
- Accept several numbers separated by commas and/or spaces ("1,3" / "1 3")
|
||||
- Accept exact choice label matches (single or comma-separated)
|
||||
- Out-of-range numbers reject the whole reply (return None) so the user
|
||||
can retry instead of silently getting a partial selection
|
||||
- Selections are returned as a JSON array string, which the clarify
|
||||
tool's ``_parse_multi_select_response`` decodes back into a list
|
||||
|
||||
For text fallback or awaiting_text mode:
|
||||
- Accept any text (numeric/label/custom) after passing through coercion
|
||||
|
||||
|
|
@ -219,6 +231,14 @@ def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
|||
# Open-ended: accept any text
|
||||
return text
|
||||
|
||||
if entry.multi_select:
|
||||
coerced = _coerce_multi_select_text(entry, text)
|
||||
if coerced is not None:
|
||||
return coerced
|
||||
# Not a parseable selection — accept as custom text only in
|
||||
# awaiting_text mode (the "Other" path); otherwise reject.
|
||||
return text if entry.awaiting_text else None
|
||||
|
||||
# Try numeric selection first (always valid for multi-choice)
|
||||
try:
|
||||
idx = int(text) - 1
|
||||
|
|
@ -241,6 +261,58 @@ def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def _coerce_multi_select_text(entry: _ClarifyEntry, text: str) -> Optional[str]:
|
||||
"""Parse a typed multi-select reply into a JSON array of choice labels.
|
||||
|
||||
Accepts numbers and/or exact labels separated by commas (and, for
|
||||
all-numeric replies, bare spaces): "1,3", "1 3", "staging, prod".
|
||||
Returns ``None`` when any token is out of range or unrecognised so the
|
||||
caller can reject the reply cleanly instead of resolving a partial or
|
||||
wrong selection.
|
||||
"""
|
||||
import json as _json
|
||||
|
||||
if not text:
|
||||
return None
|
||||
choices = entry.choices or []
|
||||
|
||||
# Split on commas first; if no commas and every whitespace-separated
|
||||
# token is numeric, treat spaces as separators too ("1 3").
|
||||
if "," in text:
|
||||
tokens = [t.strip() for t in text.split(",") if t.strip()]
|
||||
else:
|
||||
parts = text.split()
|
||||
if len(parts) > 1 and all(p.strip().isdigit() for p in parts):
|
||||
tokens = [p.strip() for p in parts]
|
||||
else:
|
||||
tokens = [text]
|
||||
|
||||
selected: List[str] = []
|
||||
for token in tokens:
|
||||
if token.isdigit():
|
||||
idx = int(token) - 1
|
||||
if 0 <= idx < len(choices):
|
||||
label = str(choices[idx]).strip()
|
||||
if label not in selected:
|
||||
selected.append(label)
|
||||
continue
|
||||
return None # out-of-range number → reject whole reply
|
||||
# Exact label match (case-insensitive)
|
||||
matched = None
|
||||
for choice in choices:
|
||||
if token.casefold() == str(choice).strip().casefold():
|
||||
matched = str(choice).strip()
|
||||
break
|
||||
if matched is None:
|
||||
return None
|
||||
if matched not in selected:
|
||||
selected.append(matched)
|
||||
|
||||
if not selected:
|
||||
return None
|
||||
return _json.dumps(selected, ensure_ascii=False)
|
||||
|
||||
|
||||
def resolve_text_response_for_session(session_key: str, response: str) -> bool:
|
||||
"""Resolve the oldest pending clarify in ``session_key`` from typed text.
|
||||
|
||||
|
|
|
|||
|
|
@ -5034,10 +5034,14 @@ def _agent_cbs(sid: str) -> dict:
|
|||
"notice_clear_callback": lambda key: _emit(
|
||||
"notification.clear", sid, {"key": key}
|
||||
),
|
||||
"clarify_callback": lambda q, c: _block(
|
||||
"clarify_callback": lambda q, c, multi_select=False: _block(
|
||||
"clarify.request",
|
||||
sid,
|
||||
{"question": q, "choices": c},
|
||||
# multi_select is a pass-through hint: renderers with checkbox
|
||||
# support can honor it; older renderers ignore the extra field
|
||||
# and stay single-select (a single answer still parses as a
|
||||
# one-element list on the tool side).
|
||||
{"question": q, "choices": c, "multi_select": bool(multi_select)},
|
||||
timeout=_clarify_timeout_seconds(),
|
||||
),
|
||||
# read_terminal tool (desktop GUI): same blocking bridge as clarify — the
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue