From b7c7301f974c81eb82070810304a9267f62d7699 Mon Sep 17 00:00:00 2001 From: Erik Kotsiuba Date: Fri, 24 Apr 2026 19:12:54 +0200 Subject: [PATCH] feat: improve ask flow elaboration and validation --- README.md | 92 ++++++++++++++- docs/contract.md | 111 +++++++++++++++++- src/ask-tool.ts | 70 ++++++++--- src/constants.ts | 6 +- src/result-format.ts | 23 ++++ src/result.ts | 15 ++- src/schema.ts | 59 +++++----- src/state/normalize.ts | 168 ++++++++++++++++++++------ src/state/result.ts | 181 +++++++++++++++++++++++++++- src/state/transitions.ts | 13 +- src/types.ts | 77 +++++++++++- src/ui/render-submit.ts | 123 +++++++++++++++---- tests/ask-tool.test.ts | 173 ++++++++++++++++++++++++++- tests/render-submit.test.ts | 52 +++++++- tests/result.test.ts | 228 ++++++++++++++++++++++++++++++++++++ tests/state.test.ts | 13 ++ 16 files changed, 1270 insertions(+), 134 deletions(-) diff --git a/README.md b/README.md index 149cbd4..5af8943 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ That is enough for the agent to gain this clarification capability. The extensio You can still add your own agent instruction if you want to further reinforce when and how the tool should be used. -Use `type: "preview"` when an option needs a dedicated preview pane. +Use `type: "preview"` only when every option includes `preview` text for a dedicated preview pane. Descriptions alone are not enough. ### Optional extra agent instruction @@ -58,7 +58,8 @@ Prefer 2-4 options per question. Include descriptions for each option. Use `type: "single"` unless multiple options can genuinely apply. Use `type: "multi"` only when the user may need to select several answers. -Use `type: "preview"` when an option needs a preview panel. +Always include a non-empty `value` for every option. +Use `type: "preview"` only when every option includes non-empty `preview` text for the preview panel. After answers are returned, continue the task using those answers explicitly. ``` @@ -135,6 +136,75 @@ After answers are returned, continue the task using those answers explicitly. ## Returned result +The returned result now also includes: + +```ts +{ + error?: { + kind: "invalid_input" + issues: Array<{ path: string; message: string }> + } + mode: "submit" | "elaborate" + continuation?: { + strategy: "refine_only" | "resume" + affectedQuestionIds: string[] + preservedAnswers: Record + questionStates: Record + } + elaboration?: { + instruction: string + nextAction: "clarify" | "clarify_then_reask" + items: Array< + | { + target: { kind: "question" } + question: { + id: string + label: string + prompt: string + type: "single" | "multi" | "preview" + options: Array<{ + value: string + label: string + description?: string + preview?: string + }> + } + answered: boolean + answer?: AskResultAnswer + note: string + } + | { + target: { kind: "option"; optionValue: string } + question: { + id: string + label: string + prompt: string + type: "single" | "multi" | "preview" + options: Array<{ + value: string + label: string + description?: string + preview?: string + }> + } + option: { + value: string + label: string + description?: string + preview?: string + } + selected: boolean + answered: boolean + answer?: AskResultAnswer + note: string + } + > + } +} +``` + The returned `details.answers[questionId]` object may include: ```ts @@ -151,7 +221,7 @@ The returned `details.answers[questionId]` object may include: Behavior details: - payloads are validated before the UI opens: question ids must be unique, option values must be unique within a question, and required text fields must not be blank -- preview questions require preview text for every option +- preview questions require preview text for every option; descriptions alone are not enough, and invalid preview payloads suggest adding preview text or switching to `type: "single"` - `required` defaults to `false` and remains advisory only - question-level notes are submitted whenever authored - option notes can be authored for any active option during the UI flow @@ -161,7 +231,23 @@ Behavior details: - on single-select questions, saving a free-form answer replaces selected options for that question - on multi-select questions, saving a free-form answer keeps other selected options intact - on multi-select questions, submitted `values` and `labels` keep selected options first and append the free-form answer last when both exist +- invalid payloads return `details.error.kind === "invalid_input"` with structured `issues` and a transcript-friendly `Invalid ask_user payload:` message - unanswered questions are omitted from `details.answers` +- in elaborate mode, `details.answers` contains only committed answers; note-only entries move into `details.elaboration.items` +- `details.continuation.strategy === "refine_only"` means the next ask should refine the current flow instead of restarting it +- `details.continuation.preservedAnswers` contains prior answers that should be carried forward without re-asking +- `details.continuation.affectedQuestionIds` lists the only questions that should be revisited +- `details.continuation.questionStates` marks each question as `answered`, `needs_clarification`, or `unanswered` +- submit review tab now offers `Submit`, `Elaborate`, and `Cancel` +- while `Submit` or `Cancel` is highlighted on the review tab, notes are previewed only for questions with selected answers +- while `Elaborate` is highlighted on the review tab, the preview expands to show all question notes and all option notes, including notes on unselected options +- choosing `Elaborate` finishes immediately and returns `mode: "elaborate"` +- `details.elaboration.items` includes all question notes and all option notes, even for unselected options +- every elaboration item carries the full normalized question and option list for that question, so the agent can resolve referential notes like `above` without relying on chat memory alone +- `details.elaboration.instruction` explicitly tells the agent to answer the clarification directly first, then re-ask only the affected questions if needed +- after clarification, agents should prefer another structured follow-up over plain-text multiple choice if a decision is still unresolved +- once prior answers narrow the branch, agents should bundle the next 2-3 related unresolved decisions into one follow-up ask when possible instead of asking a long chain of one-question follow-ups +- elaborate summary/result text now emits direct note-specific lines such as `User asked to elaborate on question "Which option would you like to select?" option "Option A" with note "why this one?"` - when the free-form option is selected, it becomes an inline input row with the selected-tab background style spanning the full width - while editing a note or free-form answer, arrow keys and `Tab` stay inside the editor while text is present so the typing cursor can move naturally - when a note or free-form editor is empty, `Up`/`Down` navigate options and `Tab`/`Shift+Tab`/`Left`/`Right` navigate tabs without closing the editor first diff --git a/docs/contract.md b/docs/contract.md index 7345b1f..b83f0ad 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -37,7 +37,7 @@ This document defines the stable external behavior. It does not explain internal - `type` defaults to `single` - `required` defaults to `false` - `required` is metadata only; it never blocks submission -- preview questions require preview text for every option +- preview questions require preview text for every option; option descriptions do not satisfy this requirement, and invalid preview payloads report a fix hint to add preview text or switch to `type: "single"` - non-`preview` questions get an internal `Type your own` option ## Output @@ -48,6 +48,14 @@ This document defines the stable external behavior. It does not explain internal details: { title?: string; cancelled: boolean; + error?: { + kind: "invalid_input"; + issues: Array<{ + path: string; + message: string; + }>; + }; + mode: "submit" | "elaborate"; questions: Array<{ id: string; label: string; @@ -65,6 +73,84 @@ This document defines the stable external behavior. It does not explain internal optionNotes?: Record; } >; + continuation?: { + strategy: "refine_only" | "resume"; + affectedQuestionIds: string[]; + preservedAnswers: Record; + }>; + questionStates: Record; + }; + elaboration?: { + instruction: string; + nextAction: "clarify" | "clarify_then_reask"; + items: Array< + | { + target: { kind: "question" }; + question: { + id: string; + label: string; + prompt: string; + type: "single" | "multi" | "preview"; + options: Array<{ + value: string; + label: string; + description?: string; + preview?: string; + }>; + }; + answered: boolean; + answer?: { + values: string[]; + labels: string[]; + indices: number[]; + customText?: string; + note?: string; + optionNotes?: Record; + }; + note: string; + } + | { + target: { kind: "option"; optionValue: string }; + question: { + id: string; + label: string; + prompt: string; + type: "single" | "multi" | "preview"; + options: Array<{ + value: string; + label: string; + description?: string; + preview?: string; + }>; + }; + option: { + value: string; + label: string; + description?: string; + preview?: string; + }; + selected: boolean; + answered: boolean; + answer?: { + values: string[]; + labels: string[]; + indices: number[]; + customText?: string; + note?: string; + optionNotes?: Record; + }; + note: string; + } + >; + }; }; } ``` @@ -72,7 +158,14 @@ This document defines the stable external behavior. It does not explain internal ## Output rules - `cancelled: true` means the user dismissed the flow, UI was unavailable, or the payload was invalid before UI opened +- invalid payloads return `error.kind === "invalid_input"` with structured `issues` and a transcript-friendly `Invalid ask_user payload:` message +- `mode: "submit"` is normal completion; `mode: "elaborate"` means the user asked the agent to continue with follow-up clarification based on notes - unanswered questions are omitted from `answers` +- in `mode: "elaborate"`, `answers` contains only committed answers; note-only entries move to `elaboration.items` +- `continuation.strategy === "refine_only"` means the next ask should refine the current flow rather than restart it +- `continuation.preservedAnswers` contains previously committed answers that should be kept as context and not re-asked +- `continuation.affectedQuestionIds` lists the only questions that should be revisited +- `continuation.questionStates` marks each question as `answered`, `needs_clarification`, or `unanswered` - single-select answers still use arrays - `indices` are 1-based rendered option positions - `customText` stores the free-form answer @@ -83,6 +176,15 @@ This document defines the stable external behavior. It does not explain internal - `note` stores a question-level note - `optionNotes` includes only notes for selected options - question notes may exist without a selected answer +- `elaboration.items` includes all question notes and all option notes, even for unselected options +- every elaboration item includes the full normalized question and option list for that question so referential notes like `above` remain understandable to the agent +- option-targeted elaboration items include the specific noted option plus whether it is currently selected +- question-targeted elaboration items include whether the question already has a committed answer +- `elaboration.instruction` tells the agent to answer the clarification directly first, then re-ask only the affected questions if a choice is still needed +- after clarification, agents should prefer another structured follow-up over plain-text multiple choice when a decision is still unresolved +- once prior answers narrow the branch, agents should bundle the next 2-3 related unresolved questions into one follow-up ask when possible, instead of using a long sequence of single-question asks +- `elaboration` is only present when `mode === "elaborate"` +- elaborate `content` text and transcript rendering describe each note directly using the full question prompt and option label, instead of a generic elaboration banner ## Supported UX @@ -93,8 +195,11 @@ This document defines the stable external behavior. It does not explain internal - question notes via `Shift+N` - option notes via `n` - number-key quick selection -- submit/cancel review tab +- submit/elaborate/cancel review tab +- on the review tab, `Submit` and `Cancel` preview notes only for answered questions +- on the review tab, `Elaborate` preview expands to all question notes and all option notes, including notes on unselected options - transcript-friendly call and result rendering +- elaborate results are phrased as direct follow-up instructions, for example: `User asked to elaborate on question "Which option would you like to select?" option "Option A" with note "why this one?"` ## Keyboard behavior @@ -123,6 +228,8 @@ Editing flow: If `ctx.hasUI === false`, the tool returns a `Needs user input` message in `content` and a cancelled result in `details`. +Validation is handled inside the tool so malformed calls produce the same structured error shape as other invalid payloads instead of relying on pre-execution schema failures. + The fallback message includes normalized pending questions and options so the caller can re-ask them manually. `details.questions` still contains normalized question metadata, while `details.answers` stays empty until a user responds. ## Source of truth diff --git a/src/ask-tool.ts b/src/ask-tool.ts index 8f4bb32..bcf48c3 100644 --- a/src/ask-tool.ts +++ b/src/ask-tool.ts @@ -3,8 +3,14 @@ import { Text, truncateToWidth } from "@mariozechner/pi-tui"; import { renderResultText } from "./result.ts"; import { AskParamsSchema } from "./schema.ts"; import { createInitialState } from "./state/create.ts"; +import { collectValidationIssues } from "./state/normalize.ts"; import { summarizeResult, toAskResult } from "./state/result.ts"; -import type { AskParams, AskQuestionInput, AskResult } from "./types.ts"; +import type { + AskParams, + AskQuestionInput, + AskResult, + AskValidationIssue, +} from "./types.ts"; import { UI_DIMENSIONS } from "./ui/constants.ts"; import { runAskFlow } from "./ui/controller.ts"; @@ -13,14 +19,19 @@ export function registerAskTool(pi: ExtensionAPI) { name: "ask_user", label: "Ask User", description: - "Interactive clarification tool for cases where the next step depends on user preferences, missing requirements, or choosing between multiple valid directions. Ask a short structured interview, collect normalized answers, and continue using those answers explicitly instead of guessing. Supports single-select, multi-select, and preview questions with a dedicated preview pane.", + "Interactive clarification tool for cases where the next step depends on user preferences, missing requirements, or choosing between multiple valid directions. Ask a short structured interview, collect normalized answers, and continue using those answers explicitly instead of guessing. Supports single-select, multi-select, and preview-pane questions. Always include a machine-readable `value` for every option. Use `preview` only when every option includes `preview` text; descriptions alone are not enough.", promptSnippet: "Clarify ambiguous or preference-sensitive decisions with a short interactive interview before proceeding", promptGuidelines: [ "Use this tool before making preference-sensitive decisions about scope, tone, UX, naming, architecture, docs, or implementation direction.", "When multiple valid directions exist, ask 1-3 concise questions instead of committing to one path on your own.", "Prefer one focused decision per question and use short labels with 2-4 clear options.", - 'Use `type: "single"` by default, `type: "multi"` only when several answers can genuinely apply, and `type: "preview"` when options need a dedicated preview pane.', + "Always include a non-empty `value` for every option.", + 'Use `type: "single"` by default and `type: "multi"` only when several answers can genuinely apply.', + 'Use `type: "preview"` only when every option includes non-empty `preview` text for the dedicated preview pane. Option descriptions do not satisfy this requirement.', + "After clarifying a note or follow-up question, prefer another structured ask_user follow-up if a choice is still needed instead of switching to plain-text multiple choice in chat.", + "When prior answers already narrow the branch, bundle the next 2-3 related unresolved decisions into one follow-up ask instead of issuing a long sequence of single-question asks.", + "Use one-at-a-time follow-up asks only when the next question materially depends on the previous answer.", ], parameters: AskParamsSchema, @@ -28,8 +39,10 @@ export function registerAskTool(pi: ExtensionAPI) { const validation = validateParams(params); if (!validation.ok) { return { - content: [{ type: "text", text: `Error: ${validation.error}` }], - details: errorResultDetails(params, true), + content: [ + { type: "text", text: formatValidationError(validation.issues) }, + ], + details: errorResultDetails(params, validation.issues), }; } @@ -83,6 +96,9 @@ export function registerAskTool(pi: ExtensionAPI) { return new Text(text?.type === "text" ? text.text : "", 0, 0); } + if (details.error) { + return new Text(theme.fg("warning", "Invalid input"), 0, 0); + } if (details.cancelled) { return new Text(theme.fg("warning", "Cancelled"), 0, 0); } @@ -92,19 +108,31 @@ export function registerAskTool(pi: ExtensionAPI) { }); } -function errorResultDetails(params: AskParams, cancelled: boolean): AskResult { +function errorResultDetails( + params: AskParams, + issues: AskValidationIssue[] +): AskResult { try { const state = createInitialState(params); return { ...toAskResult(state), - cancelled, + cancelled: true, + error: { + kind: "invalid_input", + issues, + }, }; } catch { return { title: params.title, - cancelled, + cancelled: true, + mode: "submit", questions: [], answers: {}, + error: { + kind: "invalid_input", + issues, + }, }; } } @@ -113,19 +141,27 @@ function validateParams( params: AskParams ): | { ok: true; state: ReturnType } - | { ok: false; error: string } { - try { - return { - ok: true, - state: createInitialState(params), - }; - } catch (error) { + | { ok: false; issues: AskValidationIssue[] } { + const issues = collectValidationIssues(params); + if (issues.length > 0) { return { ok: false, - error: - error instanceof Error ? error.message : "Invalid ask_user payload", + issues, }; } + + return { + ok: true, + state: createInitialState(params), + }; +} + +function formatValidationError(issues: AskValidationIssue[]): string { + const lines = [ + "Invalid ask_user payload:", + ...issues.map((issue) => `- ${issue.path}: ${issue.message}`), + ]; + return lines.join("\n"); } function formatNonInteractiveMessage( diff --git a/src/constants.ts b/src/constants.ts index 590a12e..95e15ec 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,8 +1,12 @@ export const OTHER_OPTION_VALUE = "__other__"; export const OTHER_OPTION_LABEL = "Type your own"; -export const SUBMIT_CHOICES = ["Submit", "Cancel"] as const; +export const SUBMIT_CHOICES = ["Submit", "Elaborate", "Cancel"] as const; export const NO_PREVIEW_TEXT = "No preview available"; +export const 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."; export const CANCELLED_SUMMARY = "User cancelled the ask flow"; export const SUBMITTED_SUMMARY = "User submitted the ask flow"; +export const ELABORATED_SUMMARY = "User asked for elaboration based on notes"; export const CANCELLED_RESULT_TEXT = "Cancelled"; export const SUBMITTED_RESULT_TEXT = "Submitted"; +export const ELABORATED_RESULT_TEXT = "Elaboration requested"; diff --git a/src/result-format.ts b/src/result-format.ts index 609ac8d..192826d 100644 --- a/src/result-format.ts +++ b/src/result-format.ts @@ -1,3 +1,4 @@ +import { ELABORATED_SUMMARY } from "./constants.ts"; import type { AskResult } from "./types.ts"; export function formatResultLines( @@ -67,6 +68,28 @@ function formatQuestionNoteLine( : ` note: ${note}`; } +export function formatElaborationLines( + result: AskResult, + _options: { mode: "summary" | "render" } +): string[] { + const items = result.elaboration?.items ?? []; + const lines = items.map((item) => { + if (item.target.kind === "question") { + return `User asked to elaborate on question ${quote(item.question.prompt)} with note ${quote(item.note)}`; + } + if (!("option" in item)) { + return `User asked to elaborate on question ${quote(item.question.prompt)} with note ${quote(item.note)}`; + } + return `User asked to elaborate on question ${quote(item.question.prompt)} option ${quote(item.option.label)} with note ${quote(item.note)}`; + }); + + return lines.length > 0 ? lines : [ELABORATED_SUMMARY]; +} + +function quote(value: string): string { + return JSON.stringify(value); +} + function formatOptionNoteLines( questionLabel: string, answer: AskResult["answers"][string], diff --git a/src/result.ts b/src/result.ts index ea775cb..494121a 100644 --- a/src/result.ts +++ b/src/result.ts @@ -1,11 +1,22 @@ -import { CANCELLED_RESULT_TEXT, SUBMITTED_RESULT_TEXT } from "./constants.ts"; -import { formatResultLines } from "./result-format.ts"; +import { + CANCELLED_RESULT_TEXT, + ELABORATED_RESULT_TEXT, + SUBMITTED_RESULT_TEXT, +} from "./constants.ts"; +import { formatElaborationLines, formatResultLines } from "./result-format.ts"; import type { AskResult } from "./types.ts"; export function renderResultText(result: AskResult): string { + if (result.error) { + return "Invalid input"; + } if (result.cancelled) { return CANCELLED_RESULT_TEXT; } + if (result.mode === "elaborate") { + const lines = formatElaborationLines(result, { mode: "render" }); + return lines.join("\n") || ELABORATED_RESULT_TEXT; + } const lines = formatResultLines(result, { mode: "render" }); return lines.join("\n") || SUBMITTED_RESULT_TEXT; diff --git a/src/schema.ts b/src/schema.ts index a6f5a1d..7e63dec 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -1,24 +1,24 @@ import { Type } from "typebox"; export const AskOptionSchema = Type.Object({ - value: Type.String({ - minLength: 1, - description: - "Machine-readable value returned for this option in the result", - }), - label: Type.String({ - minLength: 1, - description: "Short visible option label shown in the list", - }), + value: Type.Optional( + Type.String({ + description: + "Required machine-readable value returned for this option in the result", + }) + ), + label: Type.Optional( + Type.String({ + description: "Required short visible option label shown in the list", + }) + ), description: Type.Optional( Type.String({ - minLength: 1, description: "Optional one-line explanation to help the user choose", }) ), preview: Type.Optional( Type.String({ - minLength: 1, description: "Optional preview content shown in the dedicated preview pane for preview questions", }) @@ -26,30 +26,28 @@ export const AskOptionSchema = Type.Object({ }); export const AskQuestionSchema = Type.Object({ - id: Type.String({ - minLength: 1, - description: - "Stable question identifier used as the key in returned answers", - }), + id: Type.Optional( + Type.String({ + description: + "Required stable question identifier used as the key in returned answers", + }) + ), label: Type.Optional( Type.String({ - minLength: 1, description: "Short tab label, e.g. Goal, Audience, Tone, Scope", }) ), - prompt: Type.String({ - minLength: 1, - description: - "Direct question shown to the user; ask about one decision at a time", - }), + prompt: Type.Optional( + Type.String({ + description: + "Required direct question shown to the user; ask about one decision at a time", + }) + ), type: Type.Optional( - Type.Union( - [Type.Literal("single"), Type.Literal("multi"), Type.Literal("preview")], - { - description: - "Question type: use `single` by default, `multi` when several answers may apply, and `preview` when each option needs a preview pane", - } - ) + Type.String({ + description: + "Question type: use `single` by default. Use `preview` only when every option includes `preview` text for the dedicated preview pane; descriptions alone are not enough.", + }) ), required: Type.Optional( Type.Boolean({ @@ -58,7 +56,6 @@ export const AskQuestionSchema = Type.Object({ }) ), options: Type.Array(AskOptionSchema, { - minItems: 1, description: "Answer options; prefer 2-4 clear choices when possible", }), }); @@ -66,13 +63,11 @@ export const AskQuestionSchema = Type.Object({ export const AskParamsSchema = Type.Object({ title: Type.Optional( Type.String({ - minLength: 1, description: "Optional short title shown above the clarification flow, e.g. README direction", }) ), questions: Type.Array(AskQuestionSchema, { - minItems: 1, description: "Questions to ask in the interactive clarification flow", }), }); diff --git a/src/state/normalize.ts b/src/state/normalize.ts index 9238dd1..1883255 100644 --- a/src/state/normalize.ts +++ b/src/state/normalize.ts @@ -3,15 +3,27 @@ import type { AskParams, AskQuestion, AskQuestionInput, + AskValidationIssue, } from "../types.ts"; export function normalizeQuestions(params: AskParams): AskQuestion[] { - validateQuestions(params.questions); + const issues = collectValidationIssues(params); + if (issues.length > 0) { + throw new Error(issues[0]?.message ?? "Invalid ask_user payload"); + } return params.questions.map((question, index) => normalizeQuestion(question, index) ); } +export function collectValidationIssues( + params: AskParams +): AskValidationIssue[] { + const issues: AskValidationIssue[] = []; + validateQuestions(params.questions, issues); + return issues; +} + function normalizeQuestion( question: AskQuestionInput, index: number @@ -20,7 +32,7 @@ function normalizeQuestion( id: question.id.trim(), label: question.label?.trim() || `Q${index + 1}`, prompt: question.prompt.trim(), - type: question.type ?? "single", + type: normalizeQuestionType(question.type), required: question.required ?? false, options: question.options.map(normalizeOption), }; @@ -35,43 +47,71 @@ function normalizeOption(option: AskOption): AskOption { }; } -function validateQuestions(questions: AskParams["questions"]): void { +function validateQuestions( + questions: AskParams["questions"], + issues: AskValidationIssue[] +): void { if (questions.length === 0) { - throw new Error("At least one question is required"); + issues.push({ + path: "questions", + message: "At least one question is required", + }); + return; } const questionIds = new Set(); for (const [questionIndex, question] of questions.entries()) { - validateQuestion(question, questionIndex, questionIds); + validateQuestion(question, questionIndex, questionIds, issues); } } function validateQuestion( question: AskQuestionInput, questionIndex: number, - questionIds: Set + questionIds: Set, + issues: AskValidationIssue[] ): void { const questionNumber = questionIndex + 1; - const questionId = question.id.trim(); - const questionType = question.type ?? "single"; + const questionPath = `questions[${questionIndex}]`; + const questionId = question.id?.trim(); + const questionType = normalizeQuestionType(question.type); + assertValidQuestionType( + question.type, + `Question ${questionNumber}: invalid type ${JSON.stringify(question.type)}; expected "single", "multi", or "preview"`, + issues, + `${questionPath}.type` + ); - assertNonEmpty(questionId, `Question ${questionNumber}: id is required`); + assertNonEmpty( + questionId, + `Question ${questionNumber}: id is required`, + issues, + `${questionPath}.id` + ); assertUnique( questionIds, questionId, - `Question ${questionNumber}: duplicate question id "${questionId}"` + `Question ${questionNumber}: duplicate question id "${questionId}"`, + issues, + `${questionPath}.id` ); assertOptionalNonEmpty( question.label, - `Question ${questionNumber}: label must not be empty` + `Question ${questionNumber}: label must not be empty`, + issues, + `${questionPath}.label` ); assertNonEmpty( - question.prompt.trim(), - `Question ${questionNumber}: prompt is required` + question.prompt?.trim(), + `Question ${questionNumber}: prompt is required`, + issues, + `${questionPath}.prompt` ); assertHasItems( question.options, - `Question ${questionNumber}: at least one option is required` + `Question ${questionNumber}: at least one option is required`, + issues, + `${questionPath}.options` ); const optionValues = new Set(); @@ -81,7 +121,9 @@ function validateQuestion( optionIndex, optionValues, questionNumber, - questionType + questionType, + issues, + `${questionPath}.options[${optionIndex}]` ); } } @@ -91,63 +133,123 @@ function validateOption( optionIndex: number, optionValues: Set, questionNumber: number, - questionType: AskQuestion["type"] + questionType: AskQuestion["type"], + issues: AskValidationIssue[], + optionPath: string ): void { const optionNumber = optionIndex + 1; const prefix = `Question ${questionNumber}, option ${optionNumber}`; - const optionValue = option.value.trim(); + const optionValue = option.value?.trim(); - assertNonEmpty(optionValue, `${prefix}: value is required`); + assertNonEmpty( + optionValue, + `${prefix}: value is required`, + issues, + `${optionPath}.value` + ); assertUnique( optionValues, optionValue, - `${prefix}: duplicate option value "${optionValue}"` + `${prefix}: duplicate option value "${optionValue}"`, + issues, + `${optionPath}.value` + ); + assertNonEmpty( + option.label?.trim(), + `${prefix}: label is required`, + issues, + `${optionPath}.label` ); - assertNonEmpty(option.label.trim(), `${prefix}: label is required`); assertOptionalNonEmpty( option.description, - `${prefix}: description must not be empty` + `${prefix}: description must not be empty`, + issues, + `${optionPath}.description` ); assertOptionalNonEmpty( option.preview, - `${prefix}: preview must not be empty` + `${prefix}: preview must not be empty`, + issues, + `${optionPath}.preview` ); if (questionType === "preview") { assertNonEmpty( option.preview?.trim(), - `${prefix}: preview questions require preview text for every option` + `${prefix}: preview questions require preview text for every option; add preview text or use type "single" instead`, + issues, + `${optionPath}.preview` ); } } -function assertHasItems(items: unknown[], errorMessage: string): void { - if (items.length === 0) { - throw new Error(errorMessage); +function normalizeQuestionType( + value: AskQuestionInput["type"] +): AskQuestion["type"] { + return value === "multi" || value === "preview" ? value : "single"; +} + +function assertValidQuestionType( + value: AskQuestionInput["type"], + errorMessage: string, + issues: AskValidationIssue[], + path: string +): void { + if ( + value !== undefined && + value !== "single" && + value !== "multi" && + value !== "preview" + ) { + issues.push({ path, message: errorMessage }); } } -function assertNonEmpty(value: string | undefined, errorMessage: string): void { +function assertHasItems( + items: unknown[], + errorMessage: string, + issues: AskValidationIssue[], + path: string +): void { + if (items.length === 0) { + issues.push({ path, message: errorMessage }); + } +} + +function assertNonEmpty( + value: string | undefined, + errorMessage: string, + issues: AskValidationIssue[], + path: string +): void { if (!value) { - throw new Error(errorMessage); + issues.push({ path, message: errorMessage }); } } function assertOptionalNonEmpty( value: string | undefined, - errorMessage: string + errorMessage: string, + issues: AskValidationIssue[], + path: string ): void { if (value !== undefined && !value.trim()) { - throw new Error(errorMessage); + issues.push({ path, message: errorMessage }); } } function assertUnique( seen: Set, - value: string, - errorMessage: string + value: string | undefined, + errorMessage: string, + issues: AskValidationIssue[], + path: string ): void { + if (!value) { + return; + } if (seen.has(value)) { - throw new Error(errorMessage); + issues.push({ path, message: errorMessage }); + return; } seen.add(value); } diff --git a/src/state/result.ts b/src/state/result.ts index a5d6f4c..eb47afe 100644 --- a/src/state/result.ts +++ b/src/state/result.ts @@ -1,7 +1,25 @@ -import { CANCELLED_SUMMARY, SUBMITTED_SUMMARY } from "../constants.ts"; -import { formatResultLines } from "../result-format.ts"; -import type { AskResult, AskState } from "../types.ts"; -import { isAnswerEmpty, serializeAnswer } from "./answers.ts"; +import { + CANCELLED_SUMMARY, + ELABORATED_SUMMARY, + ELABORATION_INSTRUCTION, + SUBMITTED_SUMMARY, +} from "../constants.ts"; +import { formatElaborationLines, formatResultLines } from "../result-format.ts"; +import type { + AskContinuationPayload, + AskElaborationPayload, + AskResult, + AskResultAnswer, + AskState, + AskStateAnswer, +} from "../types.ts"; +import { + cloneResultAnswer, + isAnswerAnswered, + isAnswerEmpty, + isOptionSelected, + serializeAnswer, +} from "./answers.ts"; export function toAskResult(state: AskState): AskResult { const answers = Object.fromEntries( @@ -9,12 +27,17 @@ export function toAskResult(state: AskState): AskResult { .map( ([questionId, answer]) => [questionId, serializeAnswer(answer)] as const ) - .filter(([, answer]) => !isSerializedAnswerEmpty(answer)) + .filter(([, answer]) => + state.mode === "elaborate" + ? isSerializedAnswerCommitted(answer) + : !isSerializedAnswerEmpty(answer) + ) ); return { title: state.title, cancelled: state.cancelled, + mode: state.mode, questions: state.questions.map((question) => ({ id: question.id, label: question.label, @@ -22,6 +45,12 @@ export function toAskResult(state: AskState): AskResult { type: question.type, })), answers, + continuation: + state.mode === "elaborate" + ? serializeContinuation(state, answers) + : undefined, + elaboration: + state.mode === "elaborate" ? serializeElaboration(state) : undefined, }; } @@ -29,6 +58,10 @@ export function summarizeResult(result: AskResult): string { if (result.cancelled) { return CANCELLED_SUMMARY; } + if (result.mode === "elaborate") { + const lines = formatElaborationLines(result, { mode: "summary" }); + return lines.join("\n") || ELABORATED_SUMMARY; + } const lines = formatResultLines(result, { mode: "summary" }); return lines.join("\n") || SUBMITTED_SUMMARY; @@ -51,3 +84,141 @@ function isSerializedAnswerEmpty( (!answer.optionNotes || Object.keys(answer.optionNotes).length === 0) ); } + +function serializeContinuation( + state: AskState, + answers: AskResult["answers"] +): AskContinuationPayload { + const affectedQuestionIds: string[] = []; + const preservedAnswers: AskContinuationPayload["preservedAnswers"] = {}; + const questionStates: AskContinuationPayload["questionStates"] = {}; + + for (const question of state.questions) { + const answer = state.answers[question.id]; + const hasClarificationNeed = !!(answer?.note || answer?.optionNotes); + const committedAnswer = answers[question.id]; + const answered = !!committedAnswer; + + if (hasClarificationNeed) { + affectedQuestionIds.push(question.id); + questionStates[question.id] = { status: "needs_clarification" }; + continue; + } + + if (answered) { + preservedAnswers[question.id] = cloneResultAnswer(committedAnswer); + questionStates[question.id] = { status: "answered" }; + continue; + } + + questionStates[question.id] = { status: "unanswered" }; + } + + return { + affectedQuestionIds, + preservedAnswers, + questionStates, + strategy: "refine_only", + }; +} + +function serializeElaboration(state: AskState): AskElaborationPayload { + const items = state.questions.flatMap((question) => + serializeElaborationItemsForQuestion(question, state.answers[question.id]) + ); + + return { + instruction: ELABORATION_INSTRUCTION, + nextAction: "clarify_then_reask", + items, + }; +} + +function serializeElaborationItemsForQuestion( + question: AskState["questions"][number], + answer: AskStateAnswer | undefined +): AskElaborationPayload["items"] { + if (!(answer?.note || answer?.optionNotes)) { + return []; + } + + const questionContext = createElaborationQuestionContext(question); + const serializedAnswer = toCommittedResultAnswer(answer); + const answered = isAnswerAnswered(answer); + const items: AskElaborationPayload["items"] = []; + + if (answer.note) { + items.push({ + target: { kind: "question" }, + question: questionContext, + answered, + answer: serializedAnswer, + note: answer.note, + }); + } + + for (const [value, note] of Object.entries(answer.optionNotes ?? {})) { + const option = question.options.find( + (candidate) => candidate.value === value + ); + if (!(option && note)) { + continue; + } + + items.push({ + target: { + kind: "option", + optionValue: value, + }, + question: questionContext, + option: cloneOption(option), + selected: isOptionSelected(answer, value), + answered, + answer: serializedAnswer, + note, + }); + } + + return items; +} + +function createElaborationQuestionContext( + question: AskState["questions"][number] +) { + return { + id: question.id, + label: question.label, + prompt: question.prompt, + type: question.type, + options: question.options.map(cloneOption), + }; +} + +function cloneOption(option: AskState["questions"][number]["options"][number]) { + return { + value: option.value, + label: option.label, + ...(option.description ? { description: option.description } : {}), + ...(option.preview ? { preview: option.preview } : {}), + }; +} + +function toCommittedResultAnswer( + answer: AskStateAnswer | undefined +): AskResultAnswer | undefined { + if (!(answer && isAnswerAnswered(answer))) { + return; + } + return cloneResultAnswer(serializeAnswer(answer)); +} + +function isSerializedAnswerCommitted( + answer: AskResult["answers"][string] +): boolean { + return ( + answer.values.length > 0 || + answer.labels.length > 0 || + answer.indices.length > 0 || + !!answer.customText + ); +} diff --git a/src/state/transitions.ts b/src/state/transitions.ts index d1fe295..69605f9 100644 --- a/src/state/transitions.ts +++ b/src/state/transitions.ts @@ -26,7 +26,7 @@ import { submitView, } from "./view.ts"; -const SUBMIT_ACTION_COUNT = 2; +const SUBMIT_ACTION_COUNT = 3; export function createInitialState(params: { title?: string; @@ -42,6 +42,7 @@ export function createInitialState(params: { answers: {}, completed: false, cancelled: false, + mode: "submit", }; } @@ -154,9 +155,13 @@ export function toggleCurrentMultiOption(state: AskState): AskState { export function confirmCurrentSelection(state: AskState): AskState { if (isSubmitTab(state)) { - return state.activeSubmitActionIndex === 1 - ? { ...state, cancelled: true, completed: true } - : { ...state, completed: true }; + if (state.activeSubmitActionIndex === 2) { + return { ...state, cancelled: true, completed: true }; + } + if (state.activeSubmitActionIndex === 1) { + return { ...state, mode: "elaborate", completed: true }; + } + return { ...state, mode: "submit", completed: true }; } const question = getCurrentQuestion(state); diff --git a/src/types.ts b/src/types.ts index a14bb4d..8ff46de 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,6 +21,16 @@ export interface AskParams { title?: string; } +export interface AskValidationIssue { + message: string; + path: string; +} + +export interface AskValidationError { + issues: AskValidationIssue[]; + kind: "invalid_input"; +} + export interface AskQuestion extends Omit { label: string; @@ -50,15 +60,69 @@ export interface AskResultAnswer { values: string[]; } +export interface AskQuestionSummary { + id: string; + label: string; + prompt: string; + type: AskQuestionType; +} + +export interface AskElaborationQuestionContext extends AskQuestionSummary { + options: AskOption[]; +} + +export interface AskElaborationQuestionItem { + answer?: AskResultAnswer; + answered: boolean; + note: string; + question: AskElaborationQuestionContext; + target: { + kind: "question"; + }; +} + +export interface AskElaborationOptionItem { + answer?: AskResultAnswer; + answered: boolean; + note: string; + option: AskOption; + question: AskElaborationQuestionContext; + selected: boolean; + target: { + kind: "option"; + optionValue: string; + }; +} + +export type AskElaborationItem = + | AskElaborationQuestionItem + | AskElaborationOptionItem; + +export interface AskElaborationPayload { + instruction: string; + items: AskElaborationItem[]; + nextAction: "clarify" | "clarify_then_reask"; +} + +export interface AskContinuationQuestionState { + status: "answered" | "needs_clarification" | "unanswered"; +} + +export interface AskContinuationPayload { + affectedQuestionIds: string[]; + preservedAnswers: Record; + questionStates: Record; + strategy: "refine_only" | "resume"; +} + export interface AskResult { answers: Record; cancelled: boolean; - questions: Array<{ - id: string; - label: string; - prompt: string; - type: AskQuestionType; - }>; + continuation?: AskContinuationPayload; + elaboration?: AskElaborationPayload; + error?: AskValidationError; + mode: "submit" | "elaborate"; + questions: AskQuestionSummary[]; title?: string; } @@ -75,6 +139,7 @@ export interface AskState { answers: Record; cancelled: boolean; completed: boolean; + mode: "submit" | "elaborate"; questions: AskQuestion[]; title?: string; view: ViewState; diff --git a/src/ui/render-submit.ts b/src/ui/render-submit.ts index f3f6a9e..175ef98 100644 --- a/src/ui/render-submit.ts +++ b/src/ui/render-submit.ts @@ -1,11 +1,18 @@ import { truncateToWidth, visibleWidth } from "@mariozechner/pi-tui"; import { SUBMIT_CHOICES } from "../constants.ts"; -import { toAskResult } from "../state/result.ts"; -import type { AskResult, AskState } from "../types.ts"; +import { serializeAnswer } from "../state/answers.ts"; +import type { AskResult, AskState, AskStateAnswer } from "../types.ts"; import { UI_TEXT } from "./constants.ts"; import { pushWrappedText } from "./render-helpers.ts"; import type { Theme } from "./render-types.ts"; +type ReviewAnswer = AskResult["answers"][string] & { + extraOptionNotes?: Array<{ + label: string; + note: string; + }>; +}; + export function renderSubmitScreen( lines: string[], state: AskState, @@ -15,13 +22,13 @@ export function renderSubmitScreen( pushWrappedText(lines, UI_TEXT.reviewTitle, width, theme, "accent", " ", " "); lines.push(""); - const submittedAnswers = toAskResult(state).answers; + const showAllNotes = state.activeSubmitActionIndex === 1; for (let index = 0; index < state.questions.length; index++) { const question = state.questions[index]; renderSubmittedQuestion( lines, question, - submittedAnswers[question.id], + toReviewAnswer(question, state.answers[question.id], showAllNotes), theme, width ); @@ -37,7 +44,7 @@ export function renderSubmitScreen( function renderSubmittedQuestion( lines: string[], question: AskState["questions"][number], - answer: AskResult["answers"][string] | undefined, + answer: ReviewAnswer | undefined, theme: Theme, width: number ) { @@ -54,7 +61,7 @@ function renderSubmittedQuestion( function renderSubmittedAnswer( lines: string[], - answer: AskResult["answers"][string], + answer: ReviewAnswer, theme: Theme, width: number ) { @@ -64,28 +71,29 @@ function renderSubmittedAnswer( if (shouldRenderAnswersIndividually(answer)) { renderSubmittedSelections(lines, answer, theme, width); + renderExtraOptionNotes(lines, answer, theme, width); return; } const answerText = answer.labels.join(", "); - if (!answerText) { - return; + if (answerText) { + pushWrappedText( + lines, + `→ ${answerText}`, + width, + theme, + isCustomOnlyAnswer(answer) ? "text" : "success", + " ", + " " + ); } - pushWrappedText( - lines, - `→ ${answerText}`, - width, - theme, - isCustomOnlyAnswer(answer) ? "text" : "success", - " ", - " " - ); + renderExtraOptionNotes(lines, answer, theme, width); } function renderSubmittedSelections( lines: string[], - answer: AskResult["answers"][string], + answer: ReviewAnswer, theme: Theme, width: number ) { @@ -109,9 +117,7 @@ function renderSubmittedSelections( } } -function shouldRenderAnswersIndividually( - answer: AskResult["answers"][string] -): boolean { +function shouldRenderAnswersIndividually(answer: ReviewAnswer): boolean { if (!answer.labels.length) { return false; } @@ -142,10 +148,85 @@ function renderSubmittedNote( ); } +function renderLabeledSubmittedNote( + lines: string[], + label: string, + note: string, + theme: Theme, + width: number +) { + const indent = " "; + const title = `${label} ${UI_TEXT.questionNoteTitle}`; + const notePrefix = `${indent}${theme.fg("syntaxString", title)} `; + const continuationPrefix = `${indent}${" ".repeat(visibleWidth(title) + 1)}`; + pushWrappedText( + lines, + note, + width, + theme, + "muted", + notePrefix, + continuationPrefix + ); +} + +function renderExtraOptionNotes( + lines: string[], + answer: ReviewAnswer, + theme: Theme, + width: number +) { + for (const option of answer.extraOptionNotes ?? []) { + renderLabeledSubmittedNote(lines, option.label, option.note, theme, width); + } +} + function isCustomOnlyAnswer(answer: AskResult["answers"][string]): boolean { return answer.indices.length === 0 && !!answer.customText; } +function toReviewAnswer( + question: AskState["questions"][number], + answer: AskStateAnswer | undefined, + showAllNotes: boolean +): ReviewAnswer | undefined { + if (!answer) { + return; + } + + const serialized = serializeAnswer(answer); + const hasSelectedAnswer = serialized.labels.length > 0; + if (!showAllNotes) { + return hasSelectedAnswer ? serialized : undefined; + } + + const selectedValues = new Set(serialized.values); + const extraOptionNotes = Object.entries(answer.optionNotes ?? {}) + .filter(([value, note]) => !selectedValues.has(value) && Boolean(note)) + .map(([value, note]) => { + const option = question.options.find( + (candidate) => candidate.value === value + ); + return option ? { label: option.label, note } : undefined; + }) + .filter((entry): entry is { label: string; note: string } => + Boolean(entry) + ); + + if ( + !(hasSelectedAnswer || serialized.note) && + extraOptionNotes.length === 0 + ) { + return; + } + + return { + ...serialized, + extraOptionNotes: + extraOptionNotes.length > 0 ? extraOptionNotes : undefined, + }; +} + function renderSubmitActions( lines: string[], state: AskState, diff --git a/tests/ask-tool.test.ts b/tests/ask-tool.test.ts index 3e2c7d3..32268df 100644 --- a/tests/ask-tool.test.ts +++ b/tests/ask-tool.test.ts @@ -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"); }); diff --git a/tests/render-submit.test.ts b/tests/render-submit.test.ts index 720852f..37abf90 100644 --- a/tests/render-submit.test.ts +++ b/tests/render-submit.test.ts @@ -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("Multi") ); - 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("Note:")) ); + 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: [ diff --git a/tests/result.test.ts b/tests/result.test.ts index 9454530..6d42f99 100644 --- a/tests/result.test.ts +++ b/tests/result.test.ts @@ -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: {}, }; diff --git a/tests/state.test.ts b/tests/state.test.ts index 6bf32cc..99b7650 100644 --- a/tests/state.test.ts +++ b/tests/state.test.ts @@ -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", () => {