fix(task-chat): selecting an option no longer jumps to the next question (#13602)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - An agent that needs a decision asks in the task composer, which
renders a question set one question per page
> - Single-choice questions use radio options, and the composer also
ships a "Next" button and pagination arrows
> - Selecting a radio option also set a pending advance, waited for a
short confirm animation, and turned the page on its own
> - That takes the page away from the reader while they are still
reading it, and a misclick costs them the question
> - Nothing on the option says that clicking it will navigate
> - This pull request makes selection answer the question and nothing
else
> - The benefit is that moving between questions is always something the
reader chose to do

## Linked Issues or Issue Description

**What happened?**

In the task composer, selecting an option with a radio button jumps to
the next question. `QuestionForm.toggleOption()` set `pendingAdvance`
for any single-select question that was not on the last page, and an
effect then read `--motion-question-confirm`, waited that long, and
called `setPage(page + 1)`. With reduced motion it advanced immediately,
with no pause at all.

The result is that a click meant to answer a question also navigates
away from it. There is no way to read the rest of the page after
choosing, and no way to undo the jump other than pressing the back
arrow.

**Expected behavior**

Selecting an option records the answer and stays on the question. Moving
to the next question stays an explicit act.

**Steps to reproduce**

1. Get an agent to ask a question set with two or more single-choice
questions.
2. Open the question in the task composer.
3. Click one of the radio options.
4. Before this change the composer moves to the next question on its
own.

**Paperclip version or commit**

`45586170e` on `master`.

**Deployment mode**

Local.

**Agent adapter(s) involved**

Not adapter-specific (core bug).

**Additional context**

Every route forward already shipped beside the options, so removing the
shortcut traps no one:

- a footer button that reads "Next" on any page but the last, and the
submit label on the last;
- previous and next pagination arrows;
- "Skip" on questions that are not required.

The same question sets render outside the composer in
`IssueThreadInteractionCard`, which never had the auto-advance. This
change brings the two surfaces to the same behavior.

## What Changed

- `QuestionForm` no longer sets a pending advance when a single-choice
option is selected. The effect that turned the page is removed with it.
- Selecting an option still clears a submit error about a missing
answer, which is the case that error is about.
- The confirm-before-advance animation existed only to soften the jump.
Its `--motion-question-confirm` token, its keyframes, its class, and the
now-unused `confirming` prop on the option button are removed.
- Tests that asserted the jump now assert the opposite: selection leaves
the page number unchanged, "Next" advances, and number-key selection
stays on the same question.

## Verification

- `npx vitest run ui/src/components/task-chat` — the composer,
interaction card, protocol card, and motion token suites pass.
- `npx vitest run ui/src/components/task-chat/TaskChatComposer.test.tsx`
— 87 tests pass, including four new tests for the stationary behavior.
- `npx tsc -b ui` — clean.
- Keyboard: options keep `role="radio"` inside a `radiogroup`, and a
test asserts that number-key selection selects without navigating.

## Risks

Low. One deliberate behavior is removed. It costs a reader one extra
click per question on multi-question sets, and the explicit control for
that click already existed and is already tested.

## Model Used

Claude Opus 5 (`claude-opus-5`), 1M context, extended thinking, with
tool use and code execution.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
scotttongandClaude Opus 5 authored and GitHub committed 2026-09-18 14:18:39 -07:00
1 parent 378f11994b
commit 34355e5109
6 files changed
+166 -117

No files matched your search

+36 -64
View File
@@ -24,12 +24,21 @@ import {
useTaskChatComposerTakeoverActions,
} from "./TaskChatComposerTakeoverContext";
import { TaskChatRichInput } from "./TaskChatRichInput";
import { parseCssTimeMs } from "./motion-tokens";
import { matchSafeQuestionValidationPattern } from "./question-validation-pattern";
type Question = PaperclipQuestionSet["questions"][number];
type Answer = PaperclipQuestionResponse["answers"][string];
/**
* A form-level message, tagged with whether answering a question resolves it.
*
* Only a missing-answer complaint is something a selection can settle. A send
* that failed is not: the answers are still unsent, so the message has to
* outlive the next click rather than disappear the moment the reader touches
* an option.
*/
type FormError = { message: string; fromMissingAnswer?: boolean };
export interface QuestionFormProps {
id: string;
questionSet: PaperclipQuestionSet;
@@ -109,7 +118,6 @@ function SelectOption({
recommended,
selected,
multiple,
confirming = false,
disabled,
onClick,
}: {
@@ -119,7 +127,6 @@ function SelectOption({
recommended?: boolean;
selected: boolean;
multiple: boolean;
confirming?: boolean;
disabled: boolean;
onClick: () => void;
}) {
@@ -147,7 +154,6 @@ function SelectOption({
className={cn(
"mt-0.5 flex h-4 w-4 shrink-0 items-center justify-center border",
multiple ? "rounded-sm" : "rounded-full",
confirming && "tc-question-choice-confirm",
selected
? "border-primary bg-primary text-primary-foreground"
: "border-muted-foreground/50",
@@ -267,12 +273,7 @@ export function QuestionForm({
);
const [working, setWorking] = useState<"submit" | "cancel" | null>(null);
const [inputUploading, setInputUploading] = useState(false);
const [error, setError] = useState<string | null>(null);
const [pendingAdvance, setPendingAdvance] = useState<{
page: number;
questionId: string;
optionId: string;
} | null>(null);
const [error, setError] = useState<FormError | null>(null);
const promptRef = useRef<HTMLParagraphElement>(null);
const previousPage = useRef(page);
@@ -296,38 +297,6 @@ export function QuestionForm({
}, [answers, customActive, draftKey, page]);
const question = questionSet.questions[page];
useEffect(() => {
if (!pendingAdvance) return;
if (
disabled ||
working ||
inputUploading ||
pendingAdvance.page !== page ||
pendingAdvance.questionId !== question?.id ||
page >= questionSet.questions.length - 1
) {
setPendingAdvance(null);
return;
}
const duration = getComputedStyle(document.documentElement)
.getPropertyValue("--motion-question-confirm")
.trim();
const durationMs = parseCssTimeMs(duration) || 0;
const advance = () => {
setPendingAdvance(null);
setPage(page + 1);
};
// With reduced motion (or without CSS), no visual hold is needed.
if (durationMs <= 0) {
advance();
return;
}
const timer = window.setTimeout(advance, durationMs);
return () => window.clearTimeout(timer);
}, [
pendingAdvance, page, question?.id, questionSet.questions.length,
disabled, working, inputUploading,
]);
const validationErrors = useMemo(
() =>
Object.fromEntries(
@@ -381,17 +350,18 @@ export function QuestionForm({
setAnswers({ ...answers, [question.id]: nextAnswer });
if (!multiple) {
setCustomActive((current) => ({ ...current, [question.id]: false }));
// Briefly confirm the selected choice before moving on. The last page needs an
// explicit submit, and custom answers stay open for typing.
if (page < questionSet.questions.length - 1) {
setError(null);
setPendingAdvance({ page, questionId: question.id, optionId });
}
// Picking an option answers the question; it does not navigate. Moving on
// stays an explicit act — Next, the pagination arrows, or Submit — so a
// misclick never costs the reader the page they were still reading.
//
// Clear only the complaint this selection actually answers. A failed send
// has to survive it, or the last page quietly loses the one sign that the
// answers never left.
setError((current) => (current?.fromMissingAnswer ? null : current));
}
}
function toggleCustom() {
setPendingAdvance(null);
const active = !isCustomActive;
setCustomActive((current) => ({ ...current, [question.id]: active }));
updateAnswer({
@@ -414,9 +384,10 @@ export function QuestionForm({
// validating, and a restored draft can land past it. Go back to that
// question and say so rather than dropping the send.
setPage(invalidIndex);
setError(
`Question ${invalidIndex + 1} needs an answer before you can send.`,
);
setError({
message: `Question ${invalidIndex + 1} needs an answer before you can send.`,
fromMissingAnswer: true,
});
return;
}
setWorking("submit");
@@ -428,11 +399,12 @@ export function QuestionForm({
});
if (draftKey) clearDraft(draftKey);
} catch (cause) {
setError(
cause instanceof Error
? cause.message
: "The answers could not be submitted.",
);
setError({
message:
cause instanceof Error
? cause.message
: "The answers could not be submitted.",
});
} finally {
setWorking(null);
}
@@ -446,11 +418,12 @@ export function QuestionForm({
await onCancel();
if (draftKey) clearDraft(draftKey);
} catch (cause) {
setError(
cause instanceof Error
? cause.message
: "The questions could not be cancelled.",
);
setError({
message:
cause instanceof Error
? cause.message
: "The questions could not be cancelled.",
});
} finally {
setWorking(null);
}
@@ -635,7 +608,6 @@ export function QuestionForm({
description={option.description}
recommended={option.recommended}
selected={selected.includes(option.id)}
confirming={pendingAdvance?.questionId === question.id && pendingAdvance.optionId === option.id}
multiple={multiple}
disabled={disabled || working != null}
onClick={() => toggleOption(option.id)}
@@ -684,7 +656,7 @@ export function QuestionForm({
<div aria-live="assertive">
{error ? (
<div className="mt-2 rounded-sm border border-destructive/60 bg-destructive/10 px-2.5 py-2 text-sm text-destructive">
{error}
{error.message}
</div>
) : null}
</div>
@@ -2165,7 +2165,7 @@ describe("TaskChatComposer", () => {
});
});
it("advances single selections, preserves answers when going back, and skips optional answers", async () => {
it("advances on Next, preserves answers when going back, and skips optional answers", async () => {
const onSubmit = vi.fn();
render(
<TaskChatComposer
@@ -2219,18 +2219,25 @@ describe("TaskChatComposer", () => {
const byLabel = (label: string) =>
buttons().find((button) => button.textContent?.trim() === label);
// Page 1: required, so no Skip; Next waits for an answer.
// Page 1: required, so no Skip; Next waits for an answer. Answering
// enables Next but stays put — only Next moves on.
expect(byLabel("Skip")).toBeUndefined();
expect(byLabel("Next")?.disabled).toBe(true);
flushSync(() => byLabel("Staging")?.click());
await flushAsync();
expect(container.textContent).toContain("Where?");
expect(byLabel("Next")?.disabled).toBe(false);
flushSync(() => byLabel("Next")?.click());
await flushAsync();
expect(onSubmit).not.toHaveBeenCalled();
expect(container.textContent).toContain("When?");
expect(document.activeElement?.textContent).toBe("When?");
// Page 2: pick advances. Go back to confirm it is saved, then skip.
// Page 2: answer, then Next. Go back to confirm it is saved, then skip.
flushSync(() => byLabel("Today")?.click());
await flushAsync();
flushSync(() => byLabel("Next")?.click());
await flushAsync();
expect(container.textContent).toContain("Who?");
flushSync(() => container.querySelector<HTMLButtonElement>('button[aria-label="Previous question"]')?.click());
await flushAsync();
@@ -2253,7 +2260,7 @@ describe("TaskChatComposer", () => {
expect(response.answers.who).toEqual({ selectedOptionIds: ["me"] });
});
it.each(["click", "keyboard"])("advances a single choice by %s, while Other and multi-select stay put", async (input) => {
it.each(["click", "keyboard"])("records a single choice by %s without leaving the question", async (input) => {
const onSubmit = vi.fn();
render(<QuestionForm
id="selection-modes"
@@ -2284,6 +2291,13 @@ describe("TaskChatComposer", () => {
else byLabel("SQLite").dispatchEvent(new KeyboardEvent("keydown", { key: "1", bubbles: true }));
});
await flushAsync();
// Answering re-enables Next and closes Other, but the reader stays here.
expect(container.textContent).toContain("1 of 3");
expect(container.querySelector('[data-testid="question-other-answer-composer"]')).toBeNull();
expect(byLabel("SQLite").getAttribute("aria-checked")).toBe("true");
expect(byLabel("Next").disabled).toBe(false);
flushSync(() => byLabel("Next").click());
await flushAsync();
expect(container.textContent).toContain("2 of 3");
expect(document.activeElement?.textContent).toBe("Features?");
flushSync(() => document.activeElement?.dispatchEvent(new KeyboardEvent("keydown", { key: "1", repeat: true, bubbles: true })));
@@ -2305,71 +2319,130 @@ describe("TaskChatComposer", () => {
});
});
describe("single-choice confirmation animation", () => {
describe("single-choice selection stays on the page", () => {
const questionSet = {
schema: "paperclip.question_set.v1" as const,
questions: ["First", "Second", "Third"].map((prompt) => ({
id: prompt, prompt, required: true, answerMode: "single_select" as const,
options: [{ id: "yes", label: "Yes" }], customAnswer: { enabled: true as const },
options: [{ id: "yes", label: "Yes" }, { id: "no", label: "No" }],
customAnswer: { enabled: true as const },
})),
};
const form = (disabled = false) => (
<QuestionForm id="animated" questionSet={questionSet} disabled={disabled} onSubmit={vi.fn()} />
const form = () => (
<QuestionForm id="stationary" questionSet={questionSet} onSubmit={vi.fn()} />
);
const click = (selector: string) => act(() => {
flushSync(() => container.querySelector<HTMLButtonElement>(selector)!.click());
});
beforeEach(() => {
vi.useFakeTimers();
document.documentElement.style.setProperty("--motion-question-confirm", "160ms");
});
afterEach(() => {
render(<div />);
vi.useRealTimers();
document.documentElement.style.removeProperty("--motion-question-confirm");
});
const buttonByText = (text: string) =>
Array.from(container.querySelectorAll("button")).find(
(button) => button.textContent?.trim() === text,
);
it("shows the selected radio before advancing exactly one page", () => {
it("records the answer without navigating", () => {
render(form());
click('[role="radio"]');
expect(container.querySelector('[role="radio"]')?.getAttribute("aria-checked")).toBe("true");
expect(container.querySelector(".tc-question-choice-confirm")).not.toBeNull();
expect(container.textContent).toContain("1 of 3");
act(() => vi.advanceTimersByTime(159));
expect(container.textContent).toContain("First");
});
it("lets the reader change their mind before moving on", () => {
render(form());
click('[role="radio"]');
const radios = () => Array.from(container.querySelectorAll('[role="radio"]'));
act(() => { flushSync(() => (radios()[1] as HTMLButtonElement).click()); });
expect(radios()[0]?.getAttribute("aria-checked")).toBe("false");
expect(radios()[1]?.getAttribute("aria-checked")).toBe("true");
expect(container.textContent).toContain("1 of 3");
act(() => vi.advanceTimersByTime(1));
});
it("advances only on an explicit Next", () => {
render(form());
click('[role="radio"]');
expect(container.textContent).toContain("1 of 3");
act(() => { flushSync(() => buttonByText("Next")!.click()); });
expect(container.textContent).toContain("2 of 3");
expect(document.activeElement?.textContent).toBe("Second");
act(() => vi.advanceTimersByTime(160));
expect(container.textContent).toContain("2 of 3");
});
it.each(["navigation", "custom answer", "disabled", "unmount"])("cancels the pending advance on %s", (reason) => {
it("keeps number-key selection on the same question", () => {
render(form());
click('[role="radio"]');
if (reason === "navigation") {
click('[aria-label="Next question"]');
click('[aria-label="Next question"]');
} else if (reason === "custom answer") {
click('#animated-First-custom');
} else if (reason === "disabled") {
render(form(true));
render(form(false));
} else {
render(<div>Closed</div>);
}
act(() => vi.advanceTimersByTime(160));
expect(container.textContent).toContain(
reason === "navigation" ? "3 of 3" : reason === "unmount" ? "Closed" : "1 of 3",
const page = container.querySelector<HTMLElement>(".tc-question-page")!;
act(() => {
flushSync(() => page.dispatchEvent(
new KeyboardEvent("keydown", { key: "2", bubbles: true }),
));
});
expect(Array.from(container.querySelectorAll('[role="radio"]'))[1]?.getAttribute("aria-checked"))
.toBe("true");
expect(container.textContent).toContain("1 of 3");
});
// Selection now clears the form error, which it did not do on the last
// page before. A failed send has to outlive it: the answers are still
// unsent, so wiping the message would leave no sign of that at all.
it("keeps a failed send visible while the reader changes their answer", async () => {
const onSubmit = vi.fn().mockRejectedValue(new Error("Network unreachable"));
render(
<QuestionForm
id="failed-send"
questionSet={{
schema: "paperclip.question_set.v1" as const,
questions: [{
id: "only", prompt: "Only", required: true,
answerMode: "single_select" as const,
options: [{ id: "yes", label: "Yes" }, { id: "no", label: "No" }],
}],
}}
onSubmit={onSubmit}
/>,
);
const radios = () => Array.from(container.querySelectorAll<HTMLButtonElement>('[role="radio"]'));
act(() => { flushSync(() => radios()[0]!.click()); });
act(() => { flushSync(() => buttonByText("Submit answers")!.click()); });
await flushAsync();
expect(onSubmit).toHaveBeenCalledOnce();
expect(container.textContent).toContain("Network unreachable");
act(() => { flushSync(() => radios()[1]!.click()); });
expect(radios()[1]?.getAttribute("aria-checked")).toBe("true");
expect(container.textContent).toContain("Network unreachable");
});
it("advances immediately when the motion token is zero", () => {
document.documentElement.style.setProperty("--motion-question-confirm", "0ms");
render(form());
click('[role="radio"]');
expect(container.textContent).toContain("2 of 3");
expect(container.querySelector(".tc-question-choice-confirm")).toBeNull();
// The other half of the same rule: the one complaint a selection does
// answer still goes away when it is answered.
it("clears a missing-answer complaint once that question is answered", async () => {
render(
<QuestionForm
id="missing-answer"
questionSet={{
schema: "paperclip.question_set.v1" as const,
questions: [
{
id: "First", prompt: "First", required: true,
answerMode: "single_select" as const,
options: [{ id: "yes", label: "Yes" }, { id: "no", label: "No" }],
},
{
id: "Second", prompt: "Second", required: false,
answerMode: "single_select" as const,
options: [{ id: "yes", label: "Yes" }, { id: "no", label: "No" }],
},
],
}}
onSubmit={vi.fn()}
/>,
);
const radios = () => Array.from(container.querySelectorAll<HTMLButtonElement>('[role="radio"]'));
// The pagination arrow browses without validating, so the reader can
// reach the end with the first question still blank. Skip on the last
// question then sends, which is where the complaint comes from.
click('[aria-label="Next question"]');
act(() => { flushSync(() => buttonByText("Skip")!.click()); });
await flushAsync();
expect(container.textContent).toContain("Question 1 needs an answer");
act(() => { flushSync(() => radios()[0]!.click()); });
expect(container.textContent).not.toContain("Question 1 needs an answer");
});
});
@@ -2420,6 +2493,8 @@ describe("TaskChatComposer", () => {
).find((button) => button.textContent?.trim() === label);
flushSync(() => byLabel("Staging")?.click());
await flushAsync();
flushSync(() => byLabel("Next")?.click());
await flushAsync();
expect(container.textContent).toContain("Anything else?");
flushSync(() => byLabel("Skip")?.click());
await flushAsync();
@@ -438,6 +438,13 @@ describe("TaskChatInteractionCard", () => {
);
await act(async () => firstAnswer?.click());
expect(submit).not.toHaveBeenCalled();
// Answering stays on the question; Next is what moves on.
expect(container.textContent).toContain("1 of 2");
await act(async () =>
Array.from(container.querySelectorAll<HTMLButtonElement>("button"))
.find((button) => button.textContent?.trim() === "Next")
?.click(),
);
expect(container.textContent).toContain("2 of 2");
expect(container.textContent).toContain(
@@ -640,11 +640,13 @@ describe("TaskChatProtocolCard", () => {
(button) => button.textContent?.includes("Production"),
);
await act(async () => production?.click());
// Single selection advances; multi-selection waits for Next.
// Selecting answers the question; every page waits for Next.
const nextButton = () =>
Array.from(container.querySelectorAll<HTMLButtonElement>("button")).find(
(button) => button.textContent?.trim() === "Next",
);
expect(container.textContent).toContain("1 of 3");
await act(async () => nextButton()?.click());
expect(container.textContent).toContain("2 of 3");
expect(onDecision).not.toHaveBeenCalled();
expect(container.textContent).toContain(
@@ -55,7 +55,6 @@ export const MOTION_TOKENS: MotionTokenDef[] = [
{ name: "--motion-approval-pulse", group: "States", kind: "time", min: 0, max: 3000, step: 20 },
{ name: "--motion-plan-entry-stagger", group: "States", kind: "time", min: 0, max: 300, step: 5 },
{ name: "--motion-plan-check", group: "States", kind: "time", min: 0, max: 1500, step: 10 },
{ name: "--motion-question-confirm", group: "States", kind: "time", min: 0, max: 1000, step: 10 },
{ name: "--motion-question-page-enter", group: "States", kind: "time", min: 0, max: 1000, step: 10 },
{ name: "--motion-count-tween", group: "States", kind: "time", min: 0, max: 1500, step: 10 },
{ name: "--motion-streaming-cursor-blink", group: "States", kind: "time", min: 0, max: 3000, step: 20 },
-6
View File
@@ -269,7 +269,6 @@
--motion-approval-pulse: 1.6s;
--motion-plan-entry-stagger: 40ms;
--motion-plan-check: var(--motion-duration-fast);
--motion-question-confirm: var(--motion-duration-fast);
--motion-question-page-enter: var(--motion-duration-instant);
--motion-count-tween: var(--motion-duration-slow);
--motion-streaming-cursor-blink: 1.1s;
@@ -870,11 +869,6 @@
0%, 100% { box-shadow: 0 0 0 0 color-mix(in oklab, var(--primary) 0%, transparent); }
50% { box-shadow: 0 0 0 3px color-mix(in oklab, var(--primary) 18%, transparent); }
}
@keyframes tc-question-choice-confirm {
from { transform: scale(0.65); }
50%, to { transform: scale(1); }
}
.tc-question-choice-confirm { animation: tc-question-choice-confirm var(--motion-question-confirm) var(--motion-ease-out-expo) both; }
.tc-question-option { transition-duration: var(--motion-duration-instant); }
.tc-question-page { animation: tc-fade-in var(--motion-question-page-enter) var(--motion-ease-standard) both; }