feat(clarify): label the agent's recommended choice on every surface
The clarify schema now tells the model to order choices best-first, and mark_recommended tags element 0 with "(Recommended)" at the tool layer -- the one platform-agnostic entry point -- so CLI, TUI, desktop, and every messaging adapter inherit the label without a copy each. Each surface already defaults its cursor to index 0, so the recommendation is the pre-highlighted row too. The label is presentation only: strip_recommended takes it back off user_response, and choices_offered reports the bare list, so the agent never reasons about (or echoes back) a string it did not write. Typed replies on messaging platforms match with or without the suffix.
This commit is contained in:
parent
08a3b20dff
commit
10cf651484
|
|
@ -62,7 +62,7 @@ class TestClarifyToolChoicesValidation:
|
|||
return "answer"
|
||||
|
||||
clarify_tool("Pick", choices=[1, 2, 3], callback=mock_callback) # type: ignore
|
||||
assert choices_received == ["1", "2", "3"]
|
||||
assert choices_received == ["1 (Recommended)", "2", "3"]
|
||||
|
||||
|
||||
class TestClarifyToolCallbackHandling:
|
||||
|
|
@ -129,7 +129,7 @@ class TestClarifyDictChoices:
|
|||
callback=cb,
|
||||
)) # type: ignore
|
||||
assert seen == [
|
||||
"Tight, covers all 3 points",
|
||||
"Tight, covers all 3 points (Recommended)",
|
||||
"Loose layout",
|
||||
"A plain string choice",
|
||||
]
|
||||
|
|
@ -222,6 +222,78 @@ class TestClarifyToolMultiSelect:
|
|||
assert len(choices_passed) == MAX_CHOICES
|
||||
|
||||
|
||||
class TestClarifyRecommendedLabel:
|
||||
"""The first choice is the agent's pick and is labelled as such.
|
||||
|
||||
The schema tells the model to order choices best-first, so the tool tags
|
||||
element 0 with "(Recommended)" at the one platform-agnostic entry point —
|
||||
CLI, TUI, desktop, and messaging adapters all inherit the same label. The
|
||||
label is presentation only: it never appears in the answer the agent reads.
|
||||
"""
|
||||
|
||||
def test_first_choice_is_labelled(self):
|
||||
seen = []
|
||||
|
||||
def cb(question, choices):
|
||||
seen.extend(choices or [])
|
||||
return choices[1]
|
||||
|
||||
clarify_tool("Pick", choices=["Rebase", "Merge"], callback=cb)
|
||||
assert seen == ["Rebase (Recommended)", "Merge"]
|
||||
|
||||
def test_answer_strips_the_label(self):
|
||||
"""Picking the recommended option returns the bare option text."""
|
||||
def cb(question, choices):
|
||||
return choices[0]
|
||||
|
||||
result = json.loads(clarify_tool("Pick", choices=["Rebase", "Merge"], callback=cb))
|
||||
assert result["user_response"] == "Rebase"
|
||||
assert result["choices_offered"] == ["Rebase", "Merge"]
|
||||
|
||||
def test_multi_select_answers_strip_the_label(self):
|
||||
def cb(question, choices, multi_select=False):
|
||||
return ", ".join(choices[:2])
|
||||
|
||||
result = json.loads(clarify_tool(
|
||||
"Pick some",
|
||||
choices=["Rebase", "Merge", "Squash"],
|
||||
multi_select=True,
|
||||
callback=cb,
|
||||
))
|
||||
assert result["user_response"] == ["Rebase", "Merge"]
|
||||
|
||||
def test_single_choice_is_not_labelled(self):
|
||||
"""One option isn't a recommendation — there's nothing to prefer it over."""
|
||||
seen = []
|
||||
|
||||
def cb(question, choices):
|
||||
seen.extend(choices or [])
|
||||
return choices[0]
|
||||
|
||||
clarify_tool("Confirm", choices=["Ship it"], callback=cb)
|
||||
assert seen == ["Ship it"]
|
||||
|
||||
def test_label_is_not_doubled(self):
|
||||
"""A model that wrote its own label doesn't get a second one."""
|
||||
seen = []
|
||||
|
||||
def cb(question, choices):
|
||||
seen.extend(choices or [])
|
||||
return choices[0]
|
||||
|
||||
clarify_tool("Pick", choices=["Rebase (recommended)", "Merge"], callback=cb)
|
||||
assert seen == ["Rebase (recommended)", "Merge"]
|
||||
|
||||
def test_open_ended_unaffected(self):
|
||||
def cb(question, choices):
|
||||
assert choices is None
|
||||
return "whatever"
|
||||
|
||||
result = json.loads(clarify_tool("Thoughts?", callback=cb))
|
||||
assert result["choices_offered"] is None
|
||||
assert result["user_response"] == "whatever"
|
||||
|
||||
|
||||
class TestInvokeCallbackDispatch:
|
||||
"""_invoke_callback uses signature inspection, never a TypeError retry."""
|
||||
|
||||
|
|
|
|||
|
|
@ -201,6 +201,19 @@ def get_pending_for_session(
|
|||
return None
|
||||
|
||||
|
||||
def _label_matches(text: str, choice: object) -> bool:
|
||||
"""Case-insensitive label match that ignores the '(Recommended)' suffix.
|
||||
|
||||
The first choice reaches adapters already decorated (see
|
||||
``tools.clarify_tool.mark_recommended``), so a user who types the option
|
||||
text as the agent worded it — without the label — must still resolve the
|
||||
prompt.
|
||||
"""
|
||||
from tools.clarify_tool import strip_recommended
|
||||
|
||||
return strip_recommended(text).casefold() == strip_recommended(str(choice)).casefold()
|
||||
|
||||
|
||||
def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
||||
"""Map typed choice replies to canonical choice text, otherwise keep or reject custom text.
|
||||
|
||||
|
|
@ -250,7 +263,7 @@ def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
|||
|
||||
# Try exact choice label match (always valid for multi-choice)
|
||||
for choice in entry.choices:
|
||||
if text.casefold() == str(choice).strip().casefold():
|
||||
if _label_matches(text, choice):
|
||||
return str(choice).strip()
|
||||
|
||||
# For text fallback or awaiting_text mode, accept custom text
|
||||
|
|
@ -300,7 +313,7 @@ def _coerce_multi_select_text(entry: _ClarifyEntry, text: str) -> Optional[str]:
|
|||
# Exact label match (case-insensitive)
|
||||
matched = None
|
||||
for choice in choices:
|
||||
if token.casefold() == str(choice).strip().casefold():
|
||||
if _label_matches(token, choice):
|
||||
matched = str(choice).strip()
|
||||
break
|
||||
if matched is None:
|
||||
|
|
|
|||
|
|
@ -22,6 +22,11 @@ from typing import List, Optional, Callable
|
|||
# A 5th "Other (type your answer)" option is always appended by the UI.
|
||||
MAX_CHOICES = 4
|
||||
|
||||
# Suffix appended to the first choice so the user can see, at a glance, which
|
||||
# option the agent actually recommends. Applied here rather than per-surface so
|
||||
# CLI, TUI, desktop, and messaging adapters all render the same label.
|
||||
RECOMMENDED_LABEL = "(Recommended)"
|
||||
|
||||
|
||||
def _flatten_choice(c) -> str:
|
||||
"""Coerce a single choice into its user-facing display string.
|
||||
|
|
@ -56,6 +61,42 @@ def _flatten_choice(c) -> str:
|
|||
return str(c).strip()
|
||||
|
||||
|
||||
def mark_recommended(choices: List[str]) -> List[str]:
|
||||
"""Label the first choice as the agent's recommendation.
|
||||
|
||||
The schema tells the model to order ``choices`` best-first, so element 0 is
|
||||
always the option it would pick itself. Tagging it here — the one
|
||||
platform-agnostic entry point — means every surface (CLI panel, TUI,
|
||||
desktop card, Telegram buttons) reads the same way without four copies of
|
||||
the same string concatenation, and the label can never drift between them.
|
||||
|
||||
Idempotent: a model that writes its own "(recommended)" into the choice is
|
||||
left alone rather than getting the suffix twice. A lone choice isn't a
|
||||
recommendation — there's nothing to prefer it over — so single-choice lists
|
||||
pass through untouched.
|
||||
"""
|
||||
if len(choices) < 2:
|
||||
return choices
|
||||
first = str(choices[0]).strip()
|
||||
if first != strip_recommended(first):
|
||||
return choices
|
||||
return [f"{first} {RECOMMENDED_LABEL}"] + list(choices[1:])
|
||||
|
||||
|
||||
def strip_recommended(text: str) -> str:
|
||||
"""Remove the recommendation label from a resolved answer.
|
||||
|
||||
The user picks the decorated string, but the agent asked about the bare
|
||||
option — returning "Rebase onto main (Recommended)" as ``user_response``
|
||||
would leak presentation into the answer the model reasons about and into
|
||||
anything it echoes back.
|
||||
"""
|
||||
stripped = str(text).strip()
|
||||
if stripped.casefold().endswith(RECOMMENDED_LABEL.casefold()):
|
||||
return stripped[: -len(RECOMMENDED_LABEL)].strip()
|
||||
return stripped
|
||||
|
||||
|
||||
def _invoke_callback(callback, question, choices, multi_select):
|
||||
"""Invoke the platform callback, passing multi_select if supported.
|
||||
|
||||
|
|
@ -159,19 +200,26 @@ def clarify_tool(
|
|||
if callback is None:
|
||||
return tool_error("Clarify tool is not available in this execution context.")
|
||||
|
||||
# The first choice is the agent's pick (the schema says order best-first),
|
||||
# so it reaches every surface carrying the "(Recommended)" label. The bare
|
||||
# list is what goes back to the agent — the label is presentation only.
|
||||
offered = choices
|
||||
if choices is not None:
|
||||
choices = mark_recommended(choices)
|
||||
|
||||
try:
|
||||
raw_response = _invoke_callback(callback, question, choices, multi_select)
|
||||
except Exception as exc:
|
||||
return tool_error(f"Failed to get user input: {exc}")
|
||||
|
||||
if multi_select and choices is not None:
|
||||
user_response = _parse_multi_select_response(raw_response)
|
||||
user_response = [strip_recommended(r) for r in _parse_multi_select_response(raw_response)]
|
||||
else:
|
||||
user_response = str(raw_response).strip()
|
||||
user_response = strip_recommended(raw_response)
|
||||
|
||||
return json.dumps({
|
||||
"question": question,
|
||||
"choices_offered": choices,
|
||||
"choices_offered": offered,
|
||||
"user_response": user_response,
|
||||
}, ensure_ascii=False)
|
||||
|
||||
|
|
@ -191,7 +239,8 @@ CLARIFY_SCHEMA = {
|
|||
"Ask the user a question when you need clarification, feedback, or a "
|
||||
"decision before proceeding. Supports three modes:\n\n"
|
||||
"1. **Single-select multiple choice** — provide up to 4 choices. The user picks one "
|
||||
"or types their own answer via a 5th 'Other' option.\n"
|
||||
"or types their own answer via a 5th 'Other' option. List the choice you recommend "
|
||||
"FIRST: the UI labels it '(Recommended)' and highlights it by default.\n"
|
||||
"2. **Multi-select multiple choice** — set multi_select=true. The user can select "
|
||||
"multiple options via checkboxes. user_response will be a list of selected choices.\n"
|
||||
"3. **Open-ended** — omit choices entirely. The user types a free-form "
|
||||
|
|
@ -229,6 +278,10 @@ CLARIFY_SCHEMA = {
|
|||
"description": (
|
||||
"REQUIRED whenever you are presenting selectable options: "
|
||||
"each distinct option is its own array element (up to 4). "
|
||||
"ORDER MATTERS: put the option you actually recommend "
|
||||
"FIRST — the UI labels it '(Recommended)' and pre-selects "
|
||||
"it, so a list ordered arbitrarily recommends the wrong "
|
||||
"thing to the user. Do not write '(Recommended)' yourself. "
|
||||
"The UI renders these as pickable rows and auto-appends an "
|
||||
"'Other (type your answer)' option. Omit this parameter "
|
||||
"entirely ONLY for a genuinely open-ended free-text question."
|
||||
|
|
|
|||
Loading…
Reference in New Issue