Skip to content

fix(ai): simplify resume-stream status flash fix#12679

Closed
gr2m wants to merge 2 commits intomainfrom
gr2m/12102-simpler-fix
Closed

fix(ai): simplify resume-stream status flash fix#12679
gr2m wants to merge 2 commits intomainfrom
gr2m/12102-simpler-fix

Conversation

@gr2m
Copy link
Copy Markdown
Collaborator

@gr2m gr2m commented Feb 18, 2026

Background

PR #12102 fixed a bug where useChat with resume: true would briefly flash status to submitted on page load when there is no active stream to resume (ready → submitted → ready).

However, the fix was more complex than necessary: it moved reconnectToStream out of the existing try-finally block, introduced a separate try-catch, a resumeStream closure variable with a non-null assertion (resumeStream!), and changed onFinish behavior (no longer called when there's nothing to resume).

Summary

This PR replaces the #12102 implementation with a simpler approach (net +3 lines changed in chat.ts vs +28/-13):

  1. Skip setStatus('submitted') for resume-stream triggers — defer it until after reconnectToStream confirms there's an active stream
  2. Set submitted only after reconnect succeeds — inside the existing try block, right after stream = reconnect

This keeps reconnectToStream inside the existing try-finally block, so:

All tests from #12102 are preserved and pass.

Checklist

  • Tests have been added / updated (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)
  • A patch changeset for relevant packages has been added (for bug fixes / features - run pnpm changeset in the project root)
  • I have reviewed this pull request (self-review)

Related Issues

Simplifies #12102

@tigent tigent bot added ai/ui anything UI related bug Something isn't working as documented maintenance CI, internal documentation, automations, etc resumability labels Feb 18, 2026
body,
});

if (reconnect == null) {
Copy link
Copy Markdown
Contributor

@vercel vercel bot Feb 18, 2026

Choose a reason for hiding this comment

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

When reconnectToStream returns null (no active stream to resume), the early return inside the try block triggers the finally block, causing a spurious onFinish callback with an empty/initial message state and unnecessary activeResponse creation/teardown.

Fix on Vercel

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤔 hm this might be legit

@nwalters512
Copy link
Copy Markdown
Contributor

Responding here to #12102 (comment)

Candidly: I want the behavior change where onFinish is no longer called so I can remove the code I currently have to work around this unintuitive (IMO) behavior:

https://github.com/PrairieLearn/PrairieLearn/blob/2c180ff33a4220480de0efd791a153e8c85a9336/apps/prairielearn/src/ee/pages/instructorAiGenerateDraftEditor/components/AiQuestionGenerationChat.tsx#L552-L553

@gr2m
Copy link
Copy Markdown
Collaborator Author

gr2m commented Mar 3, 2026

I want the behavior change where onFinish is no longer called so I can remove the code I currently have to work around this unintuitive (IMO) behavior

Sounds good, and haven't seen any complaints, we should be good.

@gr2m gr2m closed this Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai/ui anything UI related bug Something isn't working as documented maintenance CI, internal documentation, automations, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants