fix(web): persist pending plan input drafts and stabilize preset answers#1137
Open
jamesx0416 wants to merge 8 commits into
Open
fix(web): persist pending plan input drafts and stabilize preset answers#1137jamesx0416 wants to merge 8 commits into
jamesx0416 wants to merge 8 commits into
Conversation
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)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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
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
- 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
- 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
- Pass explicit isResponding (from activePendingIsResponding) to the pending- user-input panel so presets disable during thread.user-input.respond, instead of the generic approval respondingRequestIds that left them clickable. - Guard clearInactivePendingUserInputDraftRequests to skip pruning while thread activities have not hydrated, preventing restored drafts from being deleted during a brief empty-activities window on thread switch. - Collapse whitespace-only custom drafts to empty string in setPendingUserInputCustomAnswer so pure-whitespace text is never persisted. - Update ChatComposer to the panel's new onSelectOption prop surface.
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
f70ba78 to
74ee23c
Compare
github-actions
Bot
added
size:XL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new persistence store and complex timing-based state synchronization for draft handling, going beyond a simple bug fix. Additionally, an open review comment identifies a potential issue where user input could be dropped in multi-question flows, warranting human verification. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: macroscopeapp[bot] <170038800+macroscopeapp[bot]@users.noreply.github.com>
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
95ef6c2 to
e64daf7
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
05c65cb to
0a4e02c
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
0a4e02c to
67380ce
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
2 times, most recently
from
38bd4a9 to
06014b1
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
06014b1 to
36d7fb7
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
36d7fb7 to
b01557a
Compare
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
b01557a to
1978f3f
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1978f3f. Configure here.
jamesx0416
force-pushed
the
t3code/fix-plan-input-cursor-draft
branch
from
1978f3f to
34f182c
Compare
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 stabilize preset answer selection
answerSourcediscriminator ('option' | 'custom') in pendingUserInput.ts so answer resolution is deterministic when both a custom answer and selected options exist.answerSourcenow takes precedence over the previous heuristic that always preferred non-empty custom text.Macroscope summarized 34f182c.
Summary by CodeRabbit
Note
Medium Risk
Touches chat submit paths and persisted client state for orchestration user-input; logic is localized but incorrect answer resolution could still mis-submit plan prompts.
Overview
Fixes plan-mode pending user-input bugs by persisting drafts in a new thread/request-scoped Zustand store (localStorage) instead of ephemeral
ChatViewstate, so in-progress answers survive thread switches and reloads.Draft answers now carry an
answerSource(option|custom) so resolution and UI treat preset vs custom explicitly—stale composer text cannot win over a clicked preset at submit. Single-select presets advance or submit immediately viaonSelectOptionoptions; the 200ms timer auto-advance inComposerPendingUserInputPanelis removed.ChatViewadds composer↔draft sync (with a short post-preset suppression window), duplicate-submit guarding on user-input responses, and cleanup of drafts for inactive requests.onAdvanceActivePendingUserInputon the last question with incomplete answers now jumps to the first unanswered question. Tests inpendingUserInput.test.tscover the new resolution and progress behavior.Reviewed by Cursor Bugbot for commit 34f182c. Bugbot is set up for automated code reviews on this repo. Configure here.