fix(web): persist pending plan input drafts and stabilize preset answers#1137
Open
jamesx0416 wants to merge 2 commits intopingdotgg:mainfrom
Open
fix(web): persist pending plan input drafts and stabilize preset answers#1137jamesx0416 wants to merge 2 commits intopingdotgg:mainfrom
jamesx0416 wants to merge 2 commits 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
📝 WalkthroughWalkthroughPer-thread persistent drafts for pending user input were added via a new Zustand store; components migrated from local state to this draft store, pending-answer source tracking (option vs custom) was introduced, Composer panel selection API changed to include advance/submit flags, and tests were extended for multi-question and navigation flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Composer
participant ChatView
participant DraftStore
participant Storage
User->>Composer: type or select answer
Composer->>ChatView: onChange/onSelect(questionId, label, {advance/submit?})
ChatView->>DraftStore: setAnswer(threadId, requestId, questionId, draft)
DraftStore->>Storage: persist (debounced)
Storage-->>DraftStore: persisted
Note over User,ChatView: navigate away and return
User->>ChatView: switch to thread
ChatView->>DraftStore: usePendingUserInputThreadDraft(threadId)
DraftStore-->>ChatView: restored answers & questionIndex
ChatView->>Composer: supply restored draft
Composer-->>User: render preserved input
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 CodeRabbit can suggest fixes for GitHub Check annotations.Configure 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
- Keep secondary snapshot thread sessions aligned with their thread IDs - Only clear inactive pending-input drafts when an active thread exists - Pass user-input responding request IDs to the composer pending-input panel
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/components/ChatView.tsx (1)
2648-2648: Extract the suppression TTL into a named constant.Using a named constant improves readability and makes timing adjustments safer.
💡 Suggested refactor
+const PENDING_CUSTOM_RESTORE_SUPPRESSION_MS = 500;- expiresAtMs: performance.now() + 500, + expiresAtMs: performance.now() + PENDING_CUSTOM_RESTORE_SUPPRESSION_MS,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` at line 2648, Replace the magic number 500 used for suppression TTL in the expiresAtMs assignment with a named constant (e.g., SUPPRESSION_TTL_MS) declared near the top of the ChatView component or module scope; update the expiresAtMs: performance.now() + 500 expression in the code that sets suppression (the occurrence around expiresAtMs in ChatView.tsx) to use SUPPRESSION_TTL_MS, and choose a clear name and location so future changes to the suppression duration are centralized and readable.apps/web/src/components/ChatView.browser.tsx (1)
686-700: Use the latest matchingthread.user-input.respondrequest in the helper.This helper currently picks the first matching request, which can become stale if a test emits multiple responses.
💡 Suggested change
- const request = wsRequests.find((wsRequest) => { + const request = [...wsRequests].reverse().find((wsRequest) => { if (wsRequest._tag !== ORCHESTRATION_WS_METHODS.dispatchCommand) { return false; } const command = "command" in wsRequest && wsRequest.command && typeof wsRequest.command === "object" ? (wsRequest.command as Record<string, unknown>) : null; return command?.type === "thread.user-input.respond"; });🤖 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 686 - 700, The helper waitForPendingUserInputResponse currently finds the first matching wsRequest and can return stale data; instead locate the latest matching "thread.user-input.respond" dispatchCommand by scanning wsRequests for all entries where wsRequest._tag === ORCHESTRATION_WS_METHODS.dispatchCommand and the command object has type === "thread.user-input.respond" and pick the last one (e.g., findLast or reverse iterate) before asserting and extracting answers; update the request lookup in waitForPendingUserInputResponse to use that latest match so tests that emit multiple responses use the most recent message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/components/ChatView.browser.tsx`:
- Around line 686-700: The helper waitForPendingUserInputResponse currently
finds the first matching wsRequest and can return stale data; instead locate the
latest matching "thread.user-input.respond" dispatchCommand by scanning
wsRequests for all entries where wsRequest._tag ===
ORCHESTRATION_WS_METHODS.dispatchCommand and the command object has type ===
"thread.user-input.respond" and pick the last one (e.g., findLast or reverse
iterate) before asserting and extracting answers; update the request lookup in
waitForPendingUserInputResponse to use that latest match so tests that emit
multiple responses use the most recent message.
In `@apps/web/src/components/ChatView.tsx`:
- Line 2648: Replace the magic number 500 used for suppression TTL in the
expiresAtMs assignment with a named constant (e.g., SUPPRESSION_TTL_MS) declared
near the top of the ChatView component or module scope; update the expiresAtMs:
performance.now() + 500 expression in the code that sets suppression (the
occurrence around expiresAtMs in ChatView.tsx) to use SUPPRESSION_TTL_MS, and
choose a clear name and location so future changes to the suppression duration
are centralized and readable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a70128a3-606a-47a9-98aa-e05c679d260b
📒 Files selected for processing (2)
apps/web/src/components/ChatView.browser.tsxapps/web/src/components/ChatView.tsx
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 user-input drafts per thread and stabilize preset answer selection
answerSourcetracking ('custom'vs'option') in pendingUserInput.ts so preset selections are not overwritten by stale custom text and vice versa.ComposerPendingUserInputCard's internal timer intoChatView, triggered via updatedonSelectOptionoptions; removes theonAdvanceprop from ComposerPendingUserInputPanel.tsx.Macroscope summarized f70ba78.
Summary by CodeRabbit