feat: improve ask flow elaboration and validation
This commit is contained in:
+171
-2
@@ -8,7 +8,15 @@ const NON_INTERACTIVE_MESSAGE_RE =
|
||||
const FIRST_QUESTION_RE = /1\. Goal: What should I optimize for\?/;
|
||||
const SPEED_OPTION_RE = /- Speed \[speed\]/;
|
||||
const CUSTOM_OPTION_RE = /- Type your own \[custom\]/;
|
||||
const DUPLICATE_ID_RE = /duplicate question id "scope"/;
|
||||
const DUPLICATE_ID_RE =
|
||||
/questions\[1\]\.id: Question 2: duplicate question id "scope"/;
|
||||
const PREVIEW_RULE_RE =
|
||||
/questions\[0\]\.options\[0\]\.preview: Question 1, option 1: preview questions require preview text for every option; add preview text or use type "single" instead/;
|
||||
const MISSING_OPTION_VALUE_RE =
|
||||
/questions\[0\]\.options\[0\]\.value: Question 1, option 1: value is required/;
|
||||
const EMPTY_QUESTIONS_RE = /questions: At least one question is required/;
|
||||
const INVALID_TYPE_RE =
|
||||
/questions\[0\]\.type: Question 1: invalid type "grid"; expected "single", "multi", or "preview"/;
|
||||
const HAS_UI = "hasUI";
|
||||
const noop = () => {
|
||||
// intentional test callback
|
||||
@@ -65,6 +73,7 @@ test("ask tool returns pending questions in non-interactive mode", async () => {
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.equal(result.details.mode, "submit");
|
||||
assert.deepEqual(result.details.questions, [
|
||||
{
|
||||
id: "goal",
|
||||
@@ -80,7 +89,7 @@ test("ask tool returns pending questions in non-interactive mode", async () => {
|
||||
assert.match(result.content[0].text, CUSTOM_OPTION_RE);
|
||||
});
|
||||
|
||||
test("ask tool rejects invalid payloads before UI opens", async () => {
|
||||
test("ask tool rejects invalid payloads before UI opens with structured issues", async () => {
|
||||
const tool = registerMockTool();
|
||||
|
||||
const result = await tool.execute(
|
||||
@@ -106,10 +115,150 @@ test("ask tool rejects invalid payloads before UI opens", async () => {
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.equal(result.details.mode, "submit");
|
||||
assert.equal(result.details.questions.length, 0);
|
||||
assert.deepEqual(result.details.error, {
|
||||
kind: "invalid_input",
|
||||
issues: [
|
||||
{
|
||||
path: "questions[1].id",
|
||||
message: 'Question 2: duplicate question id "scope"',
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.match(result.content[0].text, DUPLICATE_ID_RE);
|
||||
});
|
||||
|
||||
test("ask tool reports missing option values with structured issues", async () => {
|
||||
const tool = registerMockTool();
|
||||
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{
|
||||
title: "Clarify",
|
||||
questions: [
|
||||
{
|
||||
id: "api_style",
|
||||
prompt: "Pick API style",
|
||||
options: [{ label: "REST" } as never],
|
||||
},
|
||||
],
|
||||
},
|
||||
undefined,
|
||||
noop,
|
||||
makeCtx(true)
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.deepEqual(result.details.error, {
|
||||
kind: "invalid_input",
|
||||
issues: [
|
||||
{
|
||||
path: "questions[0].options[0].value",
|
||||
message: "Question 1, option 1: value is required",
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.match(result.content[0].text, MISSING_OPTION_VALUE_RE);
|
||||
});
|
||||
|
||||
test("ask tool reports empty questions with structured issues", async () => {
|
||||
const tool = registerMockTool();
|
||||
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{
|
||||
title: "Clarify",
|
||||
questions: [],
|
||||
},
|
||||
undefined,
|
||||
noop,
|
||||
makeCtx(true)
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.deepEqual(result.details.error, {
|
||||
kind: "invalid_input",
|
||||
issues: [
|
||||
{
|
||||
path: "questions",
|
||||
message: "At least one question is required",
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.match(result.content[0].text, EMPTY_QUESTIONS_RE);
|
||||
});
|
||||
|
||||
test("ask tool reports invalid question types with structured issues", async () => {
|
||||
const tool = registerMockTool();
|
||||
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{
|
||||
title: "Clarify",
|
||||
questions: [
|
||||
{
|
||||
id: "layout",
|
||||
prompt: "Pick layout",
|
||||
type: "grid" as never,
|
||||
options: [{ value: "compact", label: "Compact" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
undefined,
|
||||
noop,
|
||||
makeCtx(true)
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.deepEqual(result.details.error, {
|
||||
kind: "invalid_input",
|
||||
issues: [
|
||||
{
|
||||
path: "questions[0].type",
|
||||
message:
|
||||
'Question 1: invalid type "grid"; expected "single", "multi", or "preview"',
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.match(result.content[0].text, INVALID_TYPE_RE);
|
||||
});
|
||||
|
||||
test("ask tool reports preview validation with structured issues", async () => {
|
||||
const tool = registerMockTool();
|
||||
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{
|
||||
title: "Clarify",
|
||||
questions: [
|
||||
{
|
||||
id: "layout",
|
||||
prompt: "Pick layout",
|
||||
type: "preview",
|
||||
options: [{ value: "compact", label: "Compact" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
undefined,
|
||||
noop,
|
||||
makeCtx(true)
|
||||
);
|
||||
|
||||
assert.equal(result.details.cancelled, true);
|
||||
assert.deepEqual(result.details.error, {
|
||||
kind: "invalid_input",
|
||||
issues: [
|
||||
{
|
||||
path: "questions[0].options[0].preview",
|
||||
message:
|
||||
'Question 1, option 1: preview questions require preview text for every option; add preview text or use type "single" instead',
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.match(result.content[0].text, PREVIEW_RULE_RE);
|
||||
});
|
||||
|
||||
test("ask tool transcript renderers summarize call and cancelled result", () => {
|
||||
const tool = registerMockTool();
|
||||
const theme = {
|
||||
@@ -133,6 +282,7 @@ test("ask tool transcript renderers summarize call and cancelled result", () =>
|
||||
content: [{ type: "text", text: "ignored" }],
|
||||
details: {
|
||||
cancelled: true,
|
||||
mode: "submit",
|
||||
questions: [],
|
||||
answers: {},
|
||||
},
|
||||
@@ -141,4 +291,23 @@ test("ask tool transcript renderers summarize call and cancelled result", () =>
|
||||
theme
|
||||
).text;
|
||||
assert.equal(resultText, "Cancelled");
|
||||
|
||||
const invalidText = tool.renderResult(
|
||||
{
|
||||
content: [{ type: "text", text: "ignored" }],
|
||||
details: {
|
||||
cancelled: true,
|
||||
mode: "submit",
|
||||
questions: [],
|
||||
answers: {},
|
||||
error: {
|
||||
kind: "invalid_input",
|
||||
issues: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
undefined,
|
||||
theme
|
||||
).text;
|
||||
assert.equal(invalidText, "Invalid input");
|
||||
});
|
||||
|
||||
@@ -40,11 +40,12 @@ test("submit screen uses shorter action labels without extra prompt text", () =>
|
||||
|
||||
assert(!lines.some((line) => line.includes("Submit answers?")));
|
||||
assert(lines.some((line) => line.includes("1. Submit")));
|
||||
assert(lines.some((line) => line.includes("2. Cancel")));
|
||||
assert(lines.some((line) => line.includes("2. Elaborate")));
|
||||
assert(lines.some((line) => line.includes("3. Cancel")));
|
||||
assert(!lines.some((line) => line.includes("1. Submit answers")));
|
||||
});
|
||||
|
||||
test("submit screen renders indented notes before answer and separates questions", () => {
|
||||
test("submit screen shows notes only for answered questions in submit mode", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
{
|
||||
@@ -88,16 +89,12 @@ test("submit screen renders indented notes before answer and separates questions
|
||||
const secondQuestionIndex = lines.findIndex((line) =>
|
||||
line.includes("<text>Multi</text>")
|
||||
);
|
||||
const secondQuestionNoteIndex = lines.findIndex((line) =>
|
||||
line.includes("Second note")
|
||||
);
|
||||
|
||||
assert.notEqual(firstQuestionIndex, -1);
|
||||
assert.notEqual(firstQuestionNoteIndex, -1);
|
||||
assert.notEqual(firstAnswerIndex, -1);
|
||||
assert.notEqual(optionNoteIndex, -1);
|
||||
assert.notEqual(secondQuestionIndex, -1);
|
||||
assert.notEqual(secondQuestionNoteIndex, -1);
|
||||
assert(firstQuestionNoteIndex < firstAnswerIndex);
|
||||
assert(optionNoteIndex > firstAnswerIndex);
|
||||
assert(lines[firstQuestionNoteIndex]?.startsWith(" "));
|
||||
@@ -106,6 +103,7 @@ test("submit screen renders indented notes before answer and separates questions
|
||||
assert(
|
||||
lines.some((line) => line.includes("<syntaxString>Note:</syntaxString>"))
|
||||
);
|
||||
assert(!lines.some((line) => line.includes("Second note")));
|
||||
assert(!lines.some((line) => line.includes("Question note:")));
|
||||
assert(!lines.some((line) => line.includes("Code demo note:")));
|
||||
assert(!lines.some((line) => line.includes("Pick one primary demo style.")));
|
||||
@@ -114,6 +112,48 @@ test("submit screen renders indented notes before answer and separates questions
|
||||
);
|
||||
});
|
||||
|
||||
test("submit screen shows all notes when elaborate action is selected", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
{
|
||||
id: "q1",
|
||||
label: "Single",
|
||||
prompt: "Pick one primary demo style.",
|
||||
options: [
|
||||
{ value: "code", label: "Code demo" },
|
||||
{ value: "slides", label: "Slides demo" },
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "q2",
|
||||
label: "Multi",
|
||||
prompt: "Pick any extra things to include.",
|
||||
options: [{ value: "extra", label: "Extra" }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
state = enterQuestionNoteMode(state, "q1");
|
||||
state = saveNote(state, "Question note");
|
||||
state = applyNumberShortcut(state, 1);
|
||||
state = enterOptionNoteMode(state, "q1", "code");
|
||||
state = saveNote(state, "Selected option note");
|
||||
state = enterOptionNoteMode(state, "q1", "slides");
|
||||
state = saveNote(state, "Unselected option note");
|
||||
state = enterQuestionNoteMode(state, "q2");
|
||||
state = saveNote(state, "Second note");
|
||||
state = { ...state, activeSubmitActionIndex: 1 };
|
||||
|
||||
const lines: string[] = [];
|
||||
renderSubmitScreen(lines, state, mockTheme(), 120);
|
||||
|
||||
assert(lines.some((line) => line.includes("Question note")));
|
||||
assert(lines.some((line) => line.includes("Selected option note")));
|
||||
assert(lines.some((line) => line.includes("Unselected option note")));
|
||||
assert(lines.some((line) => line.includes("Slides demo Note:")));
|
||||
assert(lines.some((line) => line.includes("Second note")));
|
||||
});
|
||||
|
||||
test("submit screen renders multi-select option notes under their related answers", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
applyNumberShortcut,
|
||||
enterOptionNoteMode,
|
||||
enterQuestionNoteMode,
|
||||
moveTab,
|
||||
saveNote,
|
||||
submitCustomAnswer,
|
||||
} from "../src/state/transitions.ts";
|
||||
@@ -157,6 +158,7 @@ test("question-note-only results still render note content", () => {
|
||||
const result = {
|
||||
title: "Interview",
|
||||
cancelled: false,
|
||||
mode: "submit" as const,
|
||||
questions: [
|
||||
{
|
||||
id: "scope",
|
||||
@@ -178,10 +180,236 @@ test("question-note-only results still render note content", () => {
|
||||
assert.equal(renderResultText(result), " note: Prefer incremental changes");
|
||||
});
|
||||
|
||||
test("elaborate results include unselected notes in summary and render output", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
{
|
||||
id: "framework",
|
||||
label: "Framework",
|
||||
prompt: "Pick a framework",
|
||||
options: [
|
||||
{ value: "react", label: "React" },
|
||||
{ value: "vue", label: "Vue" },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
state = enterQuestionNoteMode(state, "framework");
|
||||
state = saveNote(state, "Need SSR support");
|
||||
state = enterOptionNoteMode(state, "framework", "react");
|
||||
state = saveNote(state, "Legacy codebase");
|
||||
state = { ...state, mode: "elaborate" };
|
||||
const result = toAskResult(state);
|
||||
|
||||
assert.equal(result.mode, "elaborate");
|
||||
assert.deepEqual(result.answers, {});
|
||||
assert.deepEqual(result.continuation, {
|
||||
affectedQuestionIds: ["framework"],
|
||||
preservedAnswers: {},
|
||||
questionStates: {
|
||||
framework: { status: "needs_clarification" },
|
||||
},
|
||||
strategy: "refine_only",
|
||||
});
|
||||
assert.deepEqual(result.elaboration, {
|
||||
instruction:
|
||||
"First answer the user's noted clarification directly and concisely using the provided question and option context. Do not treat these notes as final answers. Then re-ask only the affected questions if a choice is still needed afterward. Do not jump straight to a follow-up question unless the note is already resolved.",
|
||||
nextAction: "clarify_then_reask",
|
||||
items: [
|
||||
{
|
||||
target: { kind: "question" },
|
||||
question: {
|
||||
id: "framework",
|
||||
label: "Framework",
|
||||
prompt: "Pick a framework",
|
||||
type: "single",
|
||||
options: [
|
||||
{ value: "react", label: "React" },
|
||||
{ value: "vue", label: "Vue" },
|
||||
],
|
||||
},
|
||||
answered: false,
|
||||
answer: undefined,
|
||||
note: "Need SSR support",
|
||||
},
|
||||
{
|
||||
target: {
|
||||
kind: "option",
|
||||
optionValue: "react",
|
||||
},
|
||||
question: {
|
||||
id: "framework",
|
||||
label: "Framework",
|
||||
prompt: "Pick a framework",
|
||||
type: "single",
|
||||
options: [
|
||||
{ value: "react", label: "React" },
|
||||
{ value: "vue", label: "Vue" },
|
||||
],
|
||||
},
|
||||
option: { value: "react", label: "React" },
|
||||
selected: false,
|
||||
answered: false,
|
||||
answer: undefined,
|
||||
note: "Legacy codebase",
|
||||
},
|
||||
],
|
||||
});
|
||||
assert.equal(
|
||||
summarizeResult(result),
|
||||
[
|
||||
'User asked to elaborate on question "Pick a framework" with note "Need SSR support"',
|
||||
'User asked to elaborate on question "Pick a framework" option "React" with note "Legacy codebase"',
|
||||
].join("\n")
|
||||
);
|
||||
assert.equal(
|
||||
renderResultText(result),
|
||||
[
|
||||
'User asked to elaborate on question "Pick a framework" with note "Need SSR support"',
|
||||
'User asked to elaborate on question "Pick a framework" option "React" with note "Legacy codebase"',
|
||||
].join("\n")
|
||||
);
|
||||
});
|
||||
|
||||
test("elaborate results keep committed answers alongside elaboration context", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
{
|
||||
id: "framework",
|
||||
label: "Framework",
|
||||
prompt: "Pick a framework",
|
||||
options: [
|
||||
{ value: "react", label: "React" },
|
||||
{ value: "vue", label: "Vue" },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
state = applyNumberShortcut(state, 2);
|
||||
state = enterQuestionNoteMode(state, "framework");
|
||||
state = saveNote(state, "Compare tradeoffs");
|
||||
state = { ...state, mode: "elaborate" };
|
||||
const result = toAskResult(state);
|
||||
|
||||
assert.deepEqual(result.continuation, {
|
||||
affectedQuestionIds: ["framework"],
|
||||
preservedAnswers: {},
|
||||
questionStates: {
|
||||
framework: { status: "needs_clarification" },
|
||||
},
|
||||
strategy: "refine_only",
|
||||
});
|
||||
assert.deepEqual(result.answers, {
|
||||
framework: {
|
||||
values: ["vue"],
|
||||
labels: ["Vue"],
|
||||
indices: [2],
|
||||
customText: undefined,
|
||||
note: "Compare tradeoffs",
|
||||
optionNotes: undefined,
|
||||
},
|
||||
});
|
||||
assert.deepEqual(result.elaboration?.items[0], {
|
||||
target: { kind: "question" },
|
||||
question: {
|
||||
id: "framework",
|
||||
label: "Framework",
|
||||
prompt: "Pick a framework",
|
||||
type: "single",
|
||||
options: [
|
||||
{ value: "react", label: "React" },
|
||||
{ value: "vue", label: "Vue" },
|
||||
],
|
||||
},
|
||||
answered: true,
|
||||
answer: {
|
||||
values: ["vue"],
|
||||
labels: ["Vue"],
|
||||
indices: [2],
|
||||
customText: undefined,
|
||||
note: "Compare tradeoffs",
|
||||
optionNotes: undefined,
|
||||
},
|
||||
note: "Compare tradeoffs",
|
||||
});
|
||||
});
|
||||
|
||||
test("elaborate continuation preserves unrelated committed answers", () => {
|
||||
let state = createInitialState({
|
||||
questions: [
|
||||
{
|
||||
id: "goal",
|
||||
label: "Goal",
|
||||
prompt: "Pick goal",
|
||||
options: [
|
||||
{ value: "fast", label: "Fast" },
|
||||
{ value: "safe", label: "Safe" },
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "constraints",
|
||||
label: "Constraints",
|
||||
prompt: "Pick constraints",
|
||||
type: "multi",
|
||||
options: [
|
||||
{ value: "small", label: "Small diff" },
|
||||
{ value: "tests", label: "Add tests" },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
state = enterQuestionNoteMode(state, "goal");
|
||||
state = saveNote(state, "Explain tradeoff");
|
||||
state = moveTab(state, 1);
|
||||
state = applyNumberShortcut(state, 1);
|
||||
state = applyNumberShortcut(state, 2);
|
||||
state = { ...state, mode: "elaborate" };
|
||||
const result = toAskResult(state);
|
||||
|
||||
assert.deepEqual(result.continuation, {
|
||||
affectedQuestionIds: ["goal"],
|
||||
preservedAnswers: {
|
||||
constraints: {
|
||||
values: ["small", "tests"],
|
||||
labels: ["Small diff", "Add tests"],
|
||||
indices: [1, 2],
|
||||
customText: undefined,
|
||||
note: undefined,
|
||||
optionNotes: undefined,
|
||||
},
|
||||
},
|
||||
questionStates: {
|
||||
goal: { status: "needs_clarification" },
|
||||
constraints: { status: "answered" },
|
||||
},
|
||||
strategy: "refine_only",
|
||||
});
|
||||
});
|
||||
|
||||
test("invalid input result renders as invalid input", () => {
|
||||
const result = {
|
||||
title: "Interview",
|
||||
cancelled: true,
|
||||
mode: "submit" as const,
|
||||
questions: [],
|
||||
answers: {},
|
||||
error: {
|
||||
kind: "invalid_input" as const,
|
||||
issues: [],
|
||||
},
|
||||
};
|
||||
|
||||
assert.equal(renderResultText(result), "Invalid input");
|
||||
});
|
||||
|
||||
test("cancelled result renders as cancelled", () => {
|
||||
const result = {
|
||||
title: "Interview",
|
||||
cancelled: true,
|
||||
mode: "submit" as const,
|
||||
questions: [],
|
||||
answers: {},
|
||||
};
|
||||
|
||||
@@ -237,6 +237,19 @@ test("submit can complete even when some questions are unanswered", () => {
|
||||
|
||||
assert.equal(state.completed, true);
|
||||
assert.equal(state.cancelled, false);
|
||||
assert.equal(state.mode, "submit");
|
||||
});
|
||||
|
||||
test("submit tab can complete in elaborate mode", () => {
|
||||
let state = createInitialState(sampleParams());
|
||||
state = moveTab(state, -1);
|
||||
state = moveOption(state, 1);
|
||||
assert.equal(state.activeSubmitActionIndex, 1);
|
||||
state = confirmCurrentSelection(state);
|
||||
|
||||
assert.equal(state.completed, true);
|
||||
assert.equal(state.cancelled, false);
|
||||
assert.equal(state.mode, "elaborate");
|
||||
});
|
||||
|
||||
test("selecting other enters input mode and custom submit stores typed answer", () => {
|
||||
|
||||
Reference in New Issue
Block a user