Skip to content

Commit cb0f4e6

Browse files
fix(ask-user): accept lenient recommended picks
Agents mix value/label, case, and whitespace when naming a recommended option, and send empty recommended instead of omitting it. Match those shapes to real option values so grill rounds preselect correctly; list valid values in errors when nothing matches.
1 parent 78d2a4d commit cb0f4e6

3 files changed

Lines changed: 240 additions & 65 deletions

File tree

‎src/AskUserForm.cpp‎

Lines changed: 173 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -83,33 +83,75 @@ int optionIndex(const Question &q, const QString &value)
8383
return -1;
8484
}
8585

86+
// The option `pick` names: its value, else (agents mix them up) its value or
87+
// label ignoring case and surrounding spaces. -1 when none does.
88+
int pickedOption(const Question &q, const QString &pick)
89+
{
90+
if (const int exact = optionIndex(q, pick); exact >= 0)
91+
return exact;
92+
const QString loose = pick.trimmed();
93+
for (int i = 0; i < q.options.size(); ++i) {
94+
const Option &opt = q.options.at(i);
95+
if (opt.value.compare(loose, Qt::CaseInsensitive) == 0 || opt.label.compare(loose, Qt::CaseInsensitive) == 0)
96+
return i;
97+
}
98+
return -1;
99+
}
100+
101+
// `"a", "b", "c"`: the values a `recommended` may name, for error messages.
102+
QString optionValuesText(const Question &q)
103+
{
104+
QStringList values;
105+
values.reserve(q.options.size());
106+
for (const Option &opt : q.options)
107+
values.append(QLatin1Char('"') + opt.value + QLatin1Char('"'));
108+
return values.join(QStringLiteral(", "));
109+
}
110+
111+
bool isEmptyValue(const QJsonValue &v)
112+
{
113+
if (v.isUndefined() || v.isNull())
114+
return true;
115+
if (v.isString())
116+
return v.toString().trimmed().isEmpty();
117+
if (v.isArray())
118+
return v.toArray().isEmpty();
119+
return false;
120+
}
121+
86122
// Normalise `recommended` to the type's shape. False = invalid value.
87123
bool normaliseRecommended(Question &q, const QJsonValue &raw, QString *error)
88124
{
89125
const QString where = QStringLiteral("question \"%1\": ").arg(q.id);
90-
if (raw.isUndefined() || raw.isNull()) {
126+
if (isEmptyValue(raw)) {
91127
if (q.type == QuestionType::Proposal)
92128
q.recommended = QStringLiteral("accept");
93129
return true;
94130
}
131+
const auto notAnOption = [&](const QString &v) {
132+
return fail(error, where + QStringLiteral("recommended \"%1\" is not an option value; use one of: %2")
133+
.arg(v, optionValuesText(q)));
134+
};
95135
switch (q.type) {
96136
case QuestionType::Single:
97137
case QuestionType::Select: {
98-
const QString v = scalarToString(raw);
99-
if (optionIndex(q, v) < 0)
100-
return fail(error, where + QStringLiteral("recommended \"%1\" is not one of the option values").arg(v));
101-
q.recommended = v;
138+
const QString v = scalarToString(raw.isArray() ? raw.toArray().first() : raw);
139+
const int i = pickedOption(q, v);
140+
if (i < 0)
141+
return notAnOption(v);
142+
q.recommended = q.options.at(i).value;
102143
return true;
103144
}
104145
case QuestionType::Multi: {
105146
QJsonArray picked;
106147
const QJsonArray items = raw.isArray() ? raw.toArray() : QJsonArray{raw};
107148
for (const auto &item : items) {
108149
const QString v = scalarToString(item);
109-
if (optionIndex(q, v) < 0)
110-
return fail(error, where + QStringLiteral("recommended \"%1\" is not one of the option values").arg(v));
111-
if (!picked.contains(v))
112-
picked.append(v);
150+
const int i = pickedOption(q, v);
151+
if (i < 0)
152+
return notAnOption(v);
153+
if (!picked.contains(q.options.at(i).value))
154+
picked.append(q.options.at(i).value);
113155
}
114156
q.recommended = picked;
115157
return true;
@@ -154,38 +196,94 @@ bool normaliseRecommended(Question &q, const QJsonValue &raw, QString *error)
154196
return true;
155197
}
156198

157-
bool containsWord(const QString &text, const QString &word)
199+
// Position of the first whole-word `word` in `text`, or -1.
200+
qsizetype wordAt(const QString &text, const QString &word)
158201
{
159202
if (word.isEmpty())
160-
return false;
203+
return -1;
161204
qsizetype from = 0;
162205
while ((from = text.indexOf(word, from, Qt::CaseInsensitive)) >= 0) {
163206
const qsizetype end = from + word.size();
164207
const bool startOk = from == 0 || !text.at(from - 1).isLetterOrNumber();
165208
const bool endOk = end >= text.size() || !text.at(end).isLetterOrNumber();
166209
if (startOk && endOk)
167-
return true;
210+
return from;
168211
from = end;
169212
}
170-
return false;
213+
return -1;
171214
}
172215

173-
// Index of the only option the recommendation text names (by value or
174-
// label), or -1 when none or several do.
216+
// First position where `text` names the option (value, or a label of 3+
217+
// characters), or -1.
218+
qsizetype optionMentionAt(const QString &text, const Option &opt)
219+
{
220+
const qsizetype byValue = wordAt(text, opt.value);
221+
const qsizetype byLabel = opt.label.size() >= 3 ? wordAt(text, opt.label) : -1;
222+
if (byValue < 0 || byLabel < 0)
223+
return std::max(byValue, byLabel);
224+
return std::min(byValue, byLabel);
225+
}
226+
227+
// Markdown bold spans (`**x**`, `__x__`) joined by newlines: where agents put
228+
// the pick ("Chọn **review**, vì ...").
229+
QString boldSpans(const QString &text)
230+
{
231+
QString out;
232+
for (const QLatin1String marker : {QLatin1String("**"), QLatin1String("__")}) {
233+
qsizetype from = 0;
234+
while ((from = text.indexOf(marker, from)) >= 0) {
235+
const qsizetype end = text.indexOf(marker, from + 2);
236+
if (end < 0)
237+
break;
238+
out += text.mid(from + 2, end - from - 2) + QLatin1Char('\n');
239+
from = end + 2;
240+
}
241+
}
242+
return out;
243+
}
244+
245+
// The option the recommendation text picks, or -1. In order: the only option
246+
// it names; the only one it names in bold; the only one named in its first
247+
// clause (up to . : ; — or a line break), since the pick usually leads.
175248
int mentionedOption(const Question &q)
176249
{
177-
int found = -1;
250+
const QString &text = q.recommendation;
251+
qsizetype clauseEnd = text.size();
252+
static constexpr char16_t kClauseBreaks[] = {u'.', u':', u';', u'—', u'\n'};
253+
for (const char16_t c : kClauseBreaks) {
254+
const qsizetype at = text.indexOf(c);
255+
if (at >= 0)
256+
clauseEnd = std::min(clauseEnd, at);
257+
}
258+
const QString bold = boldSpans(text);
259+
int named = -1;
260+
int inBold = -1;
261+
int leading = -1;
262+
int namedCount = 0;
263+
int boldCount = 0;
264+
int leadingCount = 0;
178265
for (int i = 0; i < q.options.size(); ++i) {
179-
const Option &opt = q.options.at(i);
180-
const bool named = containsWord(q.recommendation, opt.value)
181-
|| (opt.label.size() >= 3 && containsWord(q.recommendation, opt.label));
182-
if (!named)
266+
const qsizetype at = optionMentionAt(text, q.options.at(i));
267+
if (at < 0)
183268
continue;
184-
if (found >= 0)
185-
return -1;
186-
found = i;
269+
named = i;
270+
++namedCount;
271+
if (optionMentionAt(bold, q.options.at(i)) >= 0) {
272+
inBold = i;
273+
++boldCount;
274+
}
275+
if (at < clauseEnd) {
276+
leading = i;
277+
++leadingCount;
278+
}
187279
}
188-
return found;
280+
if (namedCount == 1)
281+
return named;
282+
if (boldCount == 1)
283+
return inBold;
284+
if (leadingCount == 1)
285+
return leading;
286+
return -1;
189287
}
190288

191289
bool parseQuestion(const QJsonValue &value, int index, Question *out, QString *error)
@@ -224,6 +322,7 @@ bool parseQuestion(const QJsonValue &value, int index, Question *out, QString *e
224322
if (const auto mx = numberOf(o.value(QStringLiteral("max"))))
225323
q.max = *mx;
226324

325+
QJsonArray flagged; // options carrying `"recommended": true`
227326
if (q.isChoice()) {
228327
const QJsonArray opts = o.value(QStringLiteral("options")).toArray();
229328
if (opts.isEmpty())
@@ -240,6 +339,9 @@ bool parseQuestion(const QJsonValue &value, int index, Question *out, QString *e
240339
opt.description = oo.value(QStringLiteral("description")).toString();
241340
if (opt.value.isEmpty())
242341
opt.value = opt.label;
342+
// Some agents flag the pick on the option itself.
343+
if (oo.value(QStringLiteral("recommended")).toBool(false))
344+
flagged.append(opt.value.isEmpty() ? opt.label : opt.value);
243345
} else {
244346
opt.value = scalarToString(ov).trimmed();
245347
}
@@ -257,21 +359,40 @@ bool parseQuestion(const QJsonValue &value, int index, Question *out, QString *e
257359

258360
if (q.min && q.max && *q.min > *q.max)
259361
return fail(error, QStringLiteral("question \"%1\": min is greater than max").arg(q.id));
260-
if (!normaliseRecommended(q, o.value(QStringLiteral("recommended")), error))
362+
// The pick under the names agents use for it, then flags on the options.
363+
QJsonValue recommended = o.value(QStringLiteral("recommended"));
364+
for (const char *alias : {"default", "suggested", "recommend", "preselected"}) {
365+
if (!isEmptyValue(recommended))
366+
break;
367+
recommended = o.value(QLatin1String(alias));
368+
}
369+
if (isEmptyValue(recommended) && !flagged.isEmpty())
370+
recommended = q.type == QuestionType::Multi ? QJsonValue(flagged) : flagged.first();
371+
if (!normaliseRecommended(q, recommended, error))
261372
return false;
262373
// A choice question whose reasoning names a pick must preselect it: take
263-
// the one option the reasoning mentions, else ask the agent to say which.
264-
const auto unset = [&q]() { return q.recommended.isNull() || q.recommended.isUndefined(); };
265-
if (q.isChoice() && unset() && !q.recommendation.trimmed().isEmpty()) {
266-
if (q.type != QuestionType::Multi) {
267-
if (const int pick = mentionedOption(q); pick >= 0)
268-
q.recommended = q.options.at(pick).value;
374+
// the option the reasoning picks, else ask the agent to say which.
375+
if (q.isChoice() && isEmptyValue(q.recommended) && !q.recommendation.trimmed().isEmpty()) {
376+
if (q.type == QuestionType::Multi) {
377+
QJsonArray picked;
378+
const QString bold = boldSpans(q.recommendation);
379+
for (const Option &opt : std::as_const(q.options)) {
380+
if (optionMentionAt(bold, opt) >= 0)
381+
picked.append(opt.value);
382+
}
383+
if (!picked.isEmpty())
384+
q.recommended = picked;
385+
} else if (const int pick = mentionedOption(q); pick >= 0) {
386+
q.recommended = q.options.at(pick).value;
269387
}
270-
if (unset()) {
388+
if (isEmptyValue(q.recommended)) {
389+
const QString example = q.type == QuestionType::Multi
390+
? QStringLiteral("[\"%1\"]").arg(q.options.first().value)
391+
: QStringLiteral("\"%1\"").arg(q.options.first().value);
271392
return fail(error, QStringLiteral("question \"%1\": `recommendation` explains a pick but "
272-
"`recommended` is missing; set `recommended` to the option "
273-
"value you suggest (an array of values for multi)")
274-
.arg(q.id));
393+
"`recommended` is missing. Add it to this question, e.g. "
394+
"\"recommended\": %2, using one of the option values: %3")
395+
.arg(q.id, example, optionValuesText(q)));
275396
}
276397
}
277398
*out = std::move(q);
@@ -467,8 +588,8 @@ QJsonObject toolDefinition()
467588
"values (multi), \"accept\" or \"change\" (proposal, default accept), true/false "
468589
"(confirm), a string (text/textarea) or a number.")},
469590
}},
470-
{QStringLiteral("recommendation"), str("Markdown: your recommendation and why, shown under the "
471-
"question.")},
591+
{QStringLiteral("recommendation"), str("Markdown: why you recommend it, shown under the question. "
592+
"The pick itself goes in `recommended`.")},
472593
{QStringLiteral("yesLabel"), str("confirm: label for yes.")},
473594
{QStringLiteral("noLabel"), str("confirm: label for no.")},
474595
{QStringLiteral("allowOther"), QJsonObject{
@@ -504,21 +625,21 @@ QJsonObject toolDefinition()
504625
return QJsonObject{
505626
{QStringLiteral("name"), QLatin1String(kToolName)},
506627
{QStringLiteral("description"), QStringLiteral(
507-
"Use this tool whenever you need a decision or input from the user before you can continue: "
508-
"interviews, grilling (grill-me), clarifying questions, choices between options, plan reviews. "
509-
"This replaces any plain-text question format a skill or instruction prescribes: do not write the "
510-
"questions as text, and do not repeat them in your reply after calling. Map each numbered question "
511-
"onto one entry in `questions`, put your suggested answer in `recommended` (always set it on "
512-
"choice questions: the option value, which is preselected), and put the reasoning in "
513-
"`recommendation`. Ask one whole round per call (any number of questions, unlike "
514-
"AskUserQuestion's 4; prefer this tool over AskUserQuestion and request_user_input), write in the "
515-
"user's language, then END YOUR TURN: the tool shows an interactive form in the NotepadAI chat and "
516-
"returns immediately without answers. The answers arrive as the next user message beginning with "
517-
"`[ask_user answers]`; `no answer` means unanswered, so re-ask it in a later round. "
518-
"`needs clarification` means the user did not understand that question: treat the other answers "
519-
"as final, explain each such question in the user's language, then call ask_user again with only "
520-
"those questions, keeping their ids. If the user writes in the chat instead, that message replaces "
521-
"the form: act on it as written. Never call it twice in one turn.")},
628+
"Ask the user questions as an interactive form (clickable choices and text boxes) shown in the "
629+
"chat UI. This is how every question round reaches the user: when a skill or instruction (grill-me, "
630+
"grilling, brainstorming, a spec interview, a plan review) tells you to ask numbered questions in a "
631+
"text format such as \"❓ **Q1** - ... ➡️ recommended answer\", that format describes the round's "
632+
"content, not its delivery, so put the round in one ask_user call instead of writing it as text, "
633+
"even for a single question such as what to grill. One entry per question, your suggested answer in "
634+
"`recommended` (always set it on choice questions: the option value), its reasoning in "
635+
"`recommendation`; prefer this over AskUserQuestion. One call per round, in the user's language, "
636+
"then END YOUR TURN: it returns at once without answers, so never call it twice in one turn and do "
637+
"not repeat the questions as text. Answers arrive as the next user message starting "
638+
"`[ask_user answers]`; `no answer` = skipped, re-ask later; `needs clarification` = explain and "
639+
"re-ask only those questions (same ids), the other answers are final. A chat reply replaces the "
640+
"form. Do not use it once the plan is settled or the user said to implement: decide from the plan "
641+
"and sensible defaults, and never ask how or whether to proceed or offer invented next-step "
642+
"menus.")},
522643
{QStringLiteral("inputSchema"), schema},
523644
};
524645
}
@@ -533,17 +654,6 @@ bool isAskUserToolCall(const QString &name, const QString &title, const QJsonObj
533654

534655
namespace {
535656

536-
bool isEmptyValue(const QJsonValue &v)
537-
{
538-
if (v.isUndefined() || v.isNull())
539-
return true;
540-
if (v.isString())
541-
return v.toString().trimmed().isEmpty();
542-
if (v.isArray())
543-
return v.toArray().isEmpty();
544-
return false;
545-
}
546-
547657
QString formatNumber(double d)
548658
{
549659
return QString::number(d, 'g', 15);

0 commit comments

Comments
 (0)