fix(web): persist pending plan input drafts and stabilize preset answers#1137
Open
jamesx0416 wants to merge 1 commit intopingdotgg:mainfrom
Open
fix(web): persist pending plan input drafts and stabilize preset answers#1137jamesx0416 wants to merge 1 commit intopingdotgg:mainfrom
jamesx0416 wants to merge 1 commit intopingdotgg:mainfrom
Conversation
- Move pending user-input draft state into a per-thread draft store - Track explicit answer source (`option` vs `custom`) to avoid stale custom text overriding preset picks - Preserve pending input edits/cursor behavior across caret moves and thread navigation - Update pending-input panel flow to auto-advance/submit on option select and add coverage
github-actions
bot
added
size:XXL
📝 WalkthroughWalkthroughA new persistent draft store manages pending user input answers per thread, replacing local component state. This enables answer preservation across thread navigation and improves user experience during multi-question workflows. Component signatures are updated to reflect new auto-advance options, and extensive test coverage validates the draft preservation and cursor interaction flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatView
participant ComposerPanel
participant DraftStore
participant Storage["localStorage"]
User->>ComposerPanel: Edit answer or select option
ComposerPanel->>ChatView: onSelectOption(questionId, label, options)
ChatView->>DraftStore: setPendingUserInputDraftAnswer(threadId, requestId, questionId, answer)
DraftStore->>Storage: persist (debounced)
Storage-->>DraftStore: sync complete
Note over User,ChatView: User navigates to different thread
User->>ChatView: Switch back to original thread
ChatView->>DraftStore: usePendingUserInputThreadDraft(threadId)
DraftStore->>Storage: hydrate from localStorage
Storage-->>DraftStore: draft data
DraftStore-->>ChatView: return draft (answersByRequestId, questionIndexByRequestId)
ChatView->>ComposerPanel: Pass restored answers and question index
ComposerPanel-->>User: Display restored draft state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/ChatView.tsx (1)
3485-3491:⚠️ Potential issue | 🟠 MajorPass the user-input responding state into the pending-input panel.
This still passes
respondingRequestIdsfrom approval responses. Whilethread.user-input.respondis in flight, the preset buttons and digit shortcuts remain enabled, so the new auto-submit path can dispatch duplicate user-input responses.💡 Suggested fix
<ComposerPendingUserInputPanel pendingUserInputs={pendingUserInputs} - respondingRequestIds={respondingRequestIds} + respondingRequestIds={respondingUserInputRequestIds} answers={activePendingDraftAnswers} questionIndex={activePendingQuestionIndex} onSelectOption={onSelectActivePendingUserInputOption} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 3485 - 3491, Replace the generic approval-response `respondingRequestIds` prop with the user-input responding state so the pending-input panel disables presets while a `thread.user-input.respond` is in flight: locate the ComposerPendingUserInputPanel usage and either pass an existing variable that tracks user-input responses (e.g., userInputRespondingRequestIds or threadUserInputRespondingIds) or add a selector/prop that returns the request IDs for `thread.user-input.respond`, and pass that prop (name it clearly like userInputRespondingRequestIds) into ComposerPendingUserInputPanel instead of the approval `respondingRequestIds` so the panel can correctly disable buttons/shortcuts during user-input respond.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/ChatView.browser.tsx`:
- Around line 406-419: The secondary thread fixture copies
baseSnapshot.threads[0]!.session so its session.threadId still equals THREAD_ID;
update the copied thread to have a session whose threadId is SECOND_THREAD_ID
(and if the session object contains an id that encodes the thread, update that
too) so the thread’s session uniquely references SECOND_THREAD_ID instead of
THREAD_ID (locate the object with SECOND_THREAD_ID in the diff and
replace/override its session.threadId accordingly).
In `@apps/web/src/components/ChatView.tsx`:
- Around line 666-668: The effect currently calls
clearInactivePendingUserInputDraftRequests(threadId,
activePendingUserInputRequestIds) immediately, which can delete restored drafts
before pending requests hydrate; change the effect to guard so it only runs
after pendingUserInputs have been hydrated (e.g., add pendingUserInputs to the
dependency array and check that pendingUserInputs is not undefined/null and/or
that pendingUserInputs.length > 0 OR activePendingUserInputRequestIds.length >
0) before invoking clearInactivePendingUserInputDraftRequests; this ensures you
only prune drafts once the thread's pending inputs have loaded.
In `@apps/web/src/pendingUserInput.ts`:
- Around line 56-68: Normalize the stored customAnswer to collapse
whitespace-only drafts: keep calling normalizeDraftAnswer(customAnswer) into
normalizedCustomAnswer, then compute a storedCustomAnswer variable that is
customAnswer when normalizedCustomAnswer is truthy, but if
normalizedCustomAnswer is falsy and customAnswer is only whitespace
(customAnswer?.trim() === ""), set storedCustomAnswer to "" (otherwise keep the
original customAnswer); return storedCustomAnswer instead of the raw
customAnswer and leave the existing logic around selectedOptionLabel and
answerSource (references: normalizeDraftAnswer, customAnswer,
selectedOptionLabel, normalizedCustomAnswer).
---
Outside diff comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 3485-3491: Replace the generic approval-response
`respondingRequestIds` prop with the user-input responding state so the
pending-input panel disables presets while a `thread.user-input.respond` is in
flight: locate the ComposerPendingUserInputPanel usage and either pass an
existing variable that tracks user-input responses (e.g.,
userInputRespondingRequestIds or threadUserInputRespondingIds) or add a
selector/prop that returns the request IDs for `thread.user-input.respond`, and
pass that prop (name it clearly like userInputRespondingRequestIds) into
ComposerPendingUserInputPanel instead of the approval `respondingRequestIds` so
the panel can correctly disable buttons/shortcuts during user-input respond.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1358f907-5b9b-4fab-b8ee-b7a93d0a5f6b
📒 Files selected for processing (6)
apps/web/src/components/ChatView.browser.tsxapps/web/src/components/ChatView.tsxapps/web/src/components/chat/ComposerPendingUserInputPanel.tsxapps/web/src/pendingUserInput.test.tsapps/web/src/pendingUserInput.tsapps/web/src/pendingUserInputDraftStore.ts
Comment on lines
+406
to
+419
| { | ||
| ...baseSnapshot.threads[0]!, | ||
| id: SECOND_THREAD_ID, | ||
| title: "Secondary thread", | ||
| activities: [], | ||
| messages: [ | ||
| createUserMessage({ | ||
| id: "msg-user-second-thread" as MessageId, | ||
| text: "secondary thread", | ||
| offsetSeconds: 700, | ||
| }), | ||
| ], | ||
| updatedAt: isoAt(701), | ||
| }, |
There was a problem hiding this comment.
Fix the secondary thread fixture’s session ID.
This block copies baseSnapshot.threads[0]!.session unchanged, so the secondary thread still reports session.threadId === THREAD_ID. Any code that keys off the session object will see an impossible snapshot, which can mask navigation regressions in these browser tests.
💡 Suggested fix
{
...baseSnapshot.threads[0]!,
id: SECOND_THREAD_ID,
title: "Secondary thread",
+ session: {
+ ...baseSnapshot.threads[0]!.session,
+ threadId: SECOND_THREAD_ID,
+ },
activities: [],
messages: [
createUserMessage({
id: "msg-user-second-thread" as MessageId,
text: "secondary thread",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| ...baseSnapshot.threads[0]!, | |
| id: SECOND_THREAD_ID, | |
| title: "Secondary thread", | |
| activities: [], | |
| messages: [ | |
| createUserMessage({ | |
| id: "msg-user-second-thread" as MessageId, | |
| text: "secondary thread", | |
| offsetSeconds: 700, | |
| }), | |
| ], | |
| updatedAt: isoAt(701), | |
| }, | |
| { | |
| ...baseSnapshot.threads[0]!, | |
| id: SECOND_THREAD_ID, | |
| title: "Secondary thread", | |
| session: { | |
| ...baseSnapshot.threads[0]!.session, | |
| threadId: SECOND_THREAD_ID, | |
| }, | |
| activities: [], | |
| messages: [ | |
| createUserMessage({ | |
| id: "msg-user-second-thread" as MessageId, | |
| text: "secondary thread", | |
| offsetSeconds: 700, | |
| }), | |
| ], | |
| updatedAt: isoAt(701), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ChatView.browser.tsx` around lines 406 - 419, The
secondary thread fixture copies baseSnapshot.threads[0]!.session so its
session.threadId still equals THREAD_ID; update the copied thread to have a
session whose threadId is SECOND_THREAD_ID (and if the session object contains
an id that encodes the thread, update that too) so the thread’s session uniquely
references SECOND_THREAD_ID instead of THREAD_ID (locate the object with
SECOND_THREAD_ID in the diff and replace/override its session.threadId
accordingly).
Comment on lines
+666
to
+668
| useEffect(() => { | ||
| clearInactivePendingUserInputDraftRequests(threadId, activePendingUserInputRequestIds); | ||
| }, [activePendingUserInputRequestIds, clearInactivePendingUserInputDraftRequests, threadId]); |
There was a problem hiding this comment.
Don’t prune persisted pending-input drafts before the thread has loaded.
On a cold load, pendingUserInputs is [] until the snapshot hydrates. This effect then calls clearInactiveRequests(threadId, []) and deletes the restored localStorage draft before the pending request appears, so refresh/startup loses the answers this PR is trying to persist.
💡 Suggested fix
useEffect(() => {
+ if (!activeThread) {
+ return;
+ }
clearInactivePendingUserInputDraftRequests(threadId, activePendingUserInputRequestIds);
- }, [activePendingUserInputRequestIds, clearInactivePendingUserInputDraftRequests, threadId]);
+ }, [
+ activePendingUserInputRequestIds,
+ activeThread,
+ clearInactivePendingUserInputDraftRequests,
+ threadId,
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| clearInactivePendingUserInputDraftRequests(threadId, activePendingUserInputRequestIds); | |
| }, [activePendingUserInputRequestIds, clearInactivePendingUserInputDraftRequests, threadId]); | |
| useEffect(() => { | |
| if (!activeThread) { | |
| return; | |
| } | |
| clearInactivePendingUserInputDraftRequests(threadId, activePendingUserInputRequestIds); | |
| }, [ | |
| activePendingUserInputRequestIds, | |
| activeThread, | |
| clearInactivePendingUserInputDraftRequests, | |
| threadId, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ChatView.tsx` around lines 666 - 668, The effect
currently calls clearInactivePendingUserInputDraftRequests(threadId,
activePendingUserInputRequestIds) immediately, which can delete restored drafts
before pending requests hydrate; change the effect to guard so it only runs
after pendingUserInputs have been hydrated (e.g., add pendingUserInputs to the
dependency array and check that pendingUserInputs is not undefined/null and/or
that pendingUserInputs.length > 0 OR activePendingUserInputRequestIds.length >
0) before invoking clearInactivePendingUserInputDraftRequests; this ensures you
only prune drafts once the thread's pending inputs have loaded.
Comment on lines
+56
to
68
| const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer); | ||
| const selectedOptionLabel = normalizedCustomAnswer ? undefined : draft?.selectedOptionLabel; | ||
| const answerSource = normalizedCustomAnswer | ||
| ? "custom" | ||
| : selectedOptionLabel | ||
| ? "option" | ||
| : undefined; | ||
|
|
||
| return { | ||
| ...(answerSource ? { answerSource } : {}), | ||
| customAnswer, | ||
| ...(selectedOptionLabel ? { selectedOptionLabel } : {}), | ||
| }; |
There was a problem hiding this comment.
Collapse whitespace-only custom drafts to "" before storing them.
normalizeDraftAnswer() already treats whitespace-only input as unanswered, but this still persists the raw spaces in customAnswer. That leaves blank-looking drafts in the store after navigation and makes any caller that inspects customAnswer directly easier to desync from the new source semantics.
💡 Suggested fix
export function setPendingUserInputCustomAnswer(
draft: PendingUserInputDraftAnswer | undefined,
customAnswer: string,
): PendingUserInputDraftAnswer {
const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer);
+ const storedCustomAnswer = normalizedCustomAnswer === null ? "" : customAnswer;
const selectedOptionLabel = normalizedCustomAnswer ? undefined : draft?.selectedOptionLabel;
const answerSource = normalizedCustomAnswer
? "custom"
: selectedOptionLabel
? "option"
: undefined;
return {
...(answerSource ? { answerSource } : {}),
- customAnswer,
+ customAnswer: storedCustomAnswer,
...(selectedOptionLabel ? { selectedOptionLabel } : {}),
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer); | |
| const selectedOptionLabel = normalizedCustomAnswer ? undefined : draft?.selectedOptionLabel; | |
| const answerSource = normalizedCustomAnswer | |
| ? "custom" | |
| : selectedOptionLabel | |
| ? "option" | |
| : undefined; | |
| return { | |
| ...(answerSource ? { answerSource } : {}), | |
| customAnswer, | |
| ...(selectedOptionLabel ? { selectedOptionLabel } : {}), | |
| }; | |
| const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer); | |
| const storedCustomAnswer = normalizedCustomAnswer === null ? "" : customAnswer; | |
| const selectedOptionLabel = normalizedCustomAnswer ? undefined : draft?.selectedOptionLabel; | |
| const answerSource = normalizedCustomAnswer | |
| ? "custom" | |
| : selectedOptionLabel | |
| ? "option" | |
| : undefined; | |
| return { | |
| ...(answerSource ? { answerSource } : {}), | |
| customAnswer: storedCustomAnswer, | |
| ...(selectedOptionLabel ? { selectedOptionLabel } : {}), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/pendingUserInput.ts` around lines 56 - 68, Normalize the stored
customAnswer to collapse whitespace-only drafts: keep calling
normalizeDraftAnswer(customAnswer) into normalizedCustomAnswer, then compute a
storedCustomAnswer variable that is customAnswer when normalizedCustomAnswer is
truthy, but if normalizedCustomAnswer is falsy and customAnswer is only
whitespace (customAnswer?.trim() === ""), set storedCustomAnswer to ""
(otherwise keep the original customAnswer); return storedCustomAnswer instead of
the raw customAnswer and leave the existing logic around selectedOptionLabel and
answerSource (references: normalizeDraftAnswer, customAnswer,
selectedOptionLabel, normalizedCustomAnswer).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #375
Closes #918
Closes #528
Supersedes #921
What Changed
Why
Plan-mode pending user input had three user-facing correctness bugs in the web client:
ChatViewThis keeps the fix scoped to the pending-input flow and makes answer transitions deterministic.
UI Changes
Interaction fix in the pending-input composer/preset flow.
Videos
Before:
https://drive.google.com/file/d/14BZBw1eOdVpHUkeXesZhcPjferbygFmV/view?usp=sharing
After:
https://drive.google.com/file/d/12kl7Yd2Y5cy4UMer7jSU74Uc-CjpLcYS/view?usp=sharing
Checklist
Verification
bun fmtbun lintbun typecheckbun run --cwd apps/web test -- src/pendingUserInput.test.tsbun run --cwd apps/web test:browser -- src/components/ChatView.browser.tsxNote
Persist pending plan input drafts per thread and fix preset answer priority
answerSourcefield ('option'or'custom') toPendingUserInputDraftAnswerso a selected preset stays active even when stale custom text exists in the composer.Macroscope summarized 7bf2a1d.
Summary by CodeRabbit
Release Notes