supermemory icon indicating copy to clipboard operation
supermemory copied to clipboard

Enhancement: Multiple prompts handling when a prompt is under submit/streaming stage

Open ryujan404 opened this issue 2 months ago • 8 comments

Fixes #538

ryujan404 avatar Oct 28 '25 12:10 ryujan404

How to use the Graphite Merge Queue

Add the label Main to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

graphite-app[bot] avatar Oct 28 '25 12:10 graphite-app[bot]

Code Review: Multiple Prompts Handling Enhancement

Thank you for working on this improvement! This PR addresses an important UX issue where users could send multiple prompts while AI is generating responses.

Positive Aspects

  1. Solves Real Problem: Directly addresses issue #538 of overlapping responses
  2. User Experience: Adding a stop button improves user control
  3. Visual Feedback: Red stop button with Square icon provides clear visual indication

Critical Bug: Missing Absolute Positioning Wrapper

The stop button is missing the absolute positioning wrapper that the submit button has. This causes the button layout to shift when switching between states, creating a jarring visual jump.

The old code had: <div className="absolute bottom-2 right-2"><Button /></div>

The new code removes this wrapper entirely, putting buttons in document flow instead of absolute positioning.

Fix needed: Wrap the conditional button rendering in the positioning div.

Other Issues

Event Handling: Missing type annotation on handleStop parameter. Should be React.MouseEvent<HTMLButtonElement>

Enter Key Behavior: When stopping via Enter key, input remains in textarea. Is this intentional?

Code Quality:

  • Missing newline at end of global-constants.ts
  • STATUS.READY is defined but never used
  • Inconsistent indentation (run bun run format-lint)

Test Coverage: No tests included for the new functionality

Accessibility: Consider adding aria-label to stop button for screen readers

Recommendations

  1. Fix the absolute positioning wrapper (CRITICAL - causes layout shifts)
  2. Add proper TypeScript types
  3. Add newline at end of global-constants.ts
  4. Run format-lint per CLAUDE.md guidelines
  5. Add tests for stop button behavior and status handling
  6. Consider extracting button rendering to separate component

Overall Assessment

Good improvement addressing a real user pain point. The critical positioning issue must be fixed before merging to avoid negative UX impact.

Recommendation: Request changes for the positioning fix.

claude[bot] avatar Oct 30 '25 09:10 claude[bot]

@ryujan404 can you resolve the conflicts?

MaheshtheDev avatar Oct 30 '25 16:10 MaheshtheDev

@MaheshtheDev made the changes...pls re-review

ryujan404 avatar Oct 30 '25 18:10 ryujan404

Code Review for PR 537

Thank you for implementing this feature! The PR addresses issue 538 by preventing multiple prompts from being sent while a response is streaming and adding a stop button.

Positive Aspects:

  1. Good problem identification - Correctly addresses the overlapping responses issue
  2. User experience improvement - Stop button provides better control
  3. Constants extraction - Good practice moving status strings to constants
  4. Visual feedback - Red stop button clearly communicates cancel ability

Critical Issues:

  1. Inconsistent Stop Behavior (chat-messages.tsx:399, :649) The handleKeyDown function calls stop() when streaming, but the form onSubmit returns early WITHOUT calling stop(). Pressing Enter while streaming stops generation, but clicking submit does nothing. Either remove stop logic from both or keep in both for consistency.

  2. Type Safety Issue (chat-messages.tsx:408) Missing type annotation: handleStop = (e) should be handleStop = (e: React.MouseEvent<HTMLButtonElement>)

  3. Unnecessary stopPropagation (chat-messages.tsx:409) The stop button is not nested, so e.stopPropagation() is unnecessary.

Code Quality Issues:

  1. Inconsistent Formatting Mixed indentation and trailing semicolon after STATUS export. Run: bun run format-lint

Testing Checklist:

  • Stop button appears only when streaming/submitted
  • Stop button cancels generation
  • Cannot send messages while streaming
  • Enter key behavior matches button behavior
  • UI state resets after stopping
  • No message queuing on rapid Enter presses
  • Dark mode visual feedback works

Summary:

Must Fix:

  1. Fix inconsistent stop behavior between Enter and submit
  2. Add TypeScript type to handleStop
  3. Run bun run format-lint

Nice to Have: 4. Remove stopPropagation() 5. Add aria-label for accessibility

Good first implementation! Main issue is Enter vs submit inconsistency. Estimated fix time: 10 minutes.

claude[bot] avatar Oct 31 '25 17:10 claude[bot]

@ryujan404 , looks like streaming response is not working properly after you changes, can you please confirm?

image

MaheshtheDev avatar Nov 02 '25 22:11 MaheshtheDev

@MaheshtheDev, i was not able to reproduce the issue....can you pls explain me what exactly is the issue and how to reproduce it?

ryujan404 avatar Nov 04 '25 14:11 ryujan404

@MaheshtheDev, i was not able to reproduce the issue....can you pls explain me what exactly is the issue and how to reproduce it?

you do same the prompt like i did and please share if you are able to get the output properly or not

MaheshtheDev avatar Nov 07 '25 06:11 MaheshtheDev

@ryujan404 any update on this?

MaheshtheDev avatar Nov 15 '25 20:11 MaheshtheDev

closed #583

MaheshtheDev avatar Nov 18 '25 23:11 MaheshtheDev