Skip to content

fix(nova): keep user message when stopping generation early#1055

Open
MaheshtheDev wants to merge 1 commit into
mainfrom
mahesh/eng-732-stopping-generation-deletes-the-user-message
Open

fix(nova): keep user message when stopping generation early#1055
MaheshtheDev wants to merge 1 commit into
mainfrom
mahesh/eng-732-stopping-generation-deletes-the-user-message

Conversation

@MaheshtheDev
Copy link
Copy Markdown
Member

@MaheshtheDev MaheshtheDev commented Jun 5, 2026

Clicking stop before the first assistant token streamed in deleted the triggering user message: handleStop unconditionally sliced off a trailing user message, which during the submitted/thinking window is the user's own
message. Drop the slice so the message is preserved on stop.

ENG-732

Clicking stop before the first assistant token streamed in deleted the
triggering user message: handleStop unconditionally sliced off a trailing
user message, which during the submitted/thinking window is the user's own
message. Drop the slice so the message is preserved on stop.

ENG-732
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 5, 2026

ENG-732

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jun 5, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
supermemory-app c591e82 Commit Preview URL

Branch Preview URL
Jun 05 2026, 08:59 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jun 5, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
supermemory-mcp c591e82 Jun 05 2026, 08:59 PM

Copy link
Copy Markdown
Member Author


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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MaheshtheDev MaheshtheDev marked this pull request as ready for review June 5, 2026 20:58
@graphite-app graphite-app Bot requested a review from Dhravya June 5, 2026 20:58
@claude
Copy link
Copy Markdown

claude Bot commented Jun 5, 2026

Claude finished @MaheshtheDev's task in 1m 28s —— View job


Review Complete

  • Fetching PR diff and context
  • Analyzing changes for bugs and security issues
  • Checking surrounding code
  • Submitting review

Summary: This is a clean, correct fix. The original code incorrectly removed the user's message when stopping generation before any assistant response arrived. The fix simply calls stop() without modifying the messages array, preserving the user's input as expected. No bugs, security issues, or regressions found.

Score: 10/10 — Good to ship.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview: Removes incorrect message cleanup logic in handleStop that was deleting the user's message when stopping generation before an assistant response arrived.

Issues found: None — this looks good to ship.

The fix is correct. The original code assumed the last user message was a "dangling" artifact that needed cleanup, but in the submitted/thinking window (before any assistant token streams), that last user message is the user's own input. Removing it lost the user's message.

The Vercel AI SDK's useChat hook manages the messages array internally — when sendMessage is called, it adds a user message immediately. Calling stop() aborts the request but correctly preserves that message. The old comment's concern about duplication "on the next send" was unfounded: each sendMessage call creates a new message with a unique ID, so no deduplication logic is needed.

Score: 10/10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant