Skip to content

fix(cli): defer update notifications until model response completes#3321

Merged
tanzhenxin merged 1 commit intoQwenLM:mainfrom
pic4xiu:fix/defer-update-notification-until-idle
Apr 16, 2026
Merged

fix(cli): defer update notifications until model response completes#3321
tanzhenxin merged 1 commit intoQwenLM:mainfrom
pic4xiu:fix/defer-update-notification-until-idle

Conversation

@pic4xiu
Copy link
Copy Markdown
Contributor

@pic4xiu pic4xiu commented Apr 16, 2026

When a background auto-update finished while the model was streaming,
the success/failure notification was inserted mid-conversation via
addItem(), disrupting the user's reading flow.

Introduce a "defer until idle" mechanism in setUpdateHandler():

  • Accept an isIdleRef param that tracks whether StreamingState is Idle
  • Queue notifications in a pendingNotifications array when not idle
  • Expose a flush() method that drains the queue once idle
  • AppContainer keeps isIdleRef.current in sync with streamingState
    and calls flush() via a useEffect when transitioning back to Idle

All four event types (update-received, update-success, update-failed,
update-info) are routed through the same addItemOrDefer() helper.
The third parameter defaults to { current: true } for backward
compatibility.

TLDR

When a background auto-update completed while the model was streaming a response, the success/failure notification was inserted mid-conversation via addItem(), visually disrupting the user's reading flow. This PR introduces a "defer until idle" mechanism that queues notifications and flushes them only after the model finishes responding.

Screenshots / Video Demo

Before:

User: What does this command mean?
AI: This is a command using Homebrew to install...
● Update successful! The new version will be used on your next run.   ← interrupts mid-stream
AI: ...continuing the explanation...

After:

User: What does this command mean?
AI: This is a command using Homebrew to install... (complete response)
● Update successful! The new version will be used on your next run.   ← appears after response completes

Dive Deeper

The fix adds an isIdleRef parameter to setUpdateHandler() that tracks whether StreamingState is Idle. All four event types (update-received, update-success, update-failed, update-info) are routed through a unified addItemOrDefer() helper:

  • Idle → notification is delivered immediately via addItem()
  • Not idle (streaming) → notification is pushed into a pendingNotifications queue
  • Transition back to idleAppContainer calls flush() via a useEffect to drain the queue

The third parameter defaults to { current: true } for backward compatibility, so existing callers are unaffected.

Reviewer Test Plan

Unit tests (primary validation):

cd packages/cli && npx vitest run src/utils/handleAutoUpdate.test.ts

18 tests cover the core scenarios: immediate delivery when idle, deferral when streaming, flush on idle transition, multi-notification ordering, cleanup clearing the queue, and empty-queue no-ops.

Code review checklist:

  • addItemOrDefer() correctly checks isIdleRef.current before deciding to deliver or queue
  • flush() drains pendingNotifications in FIFO order via shift()
  • cleanup() removes all event listeners and clears the pending queue
  • AppContainer keeps isIdleRef.current in sync with streamingState on every render
  • useEffect on streamingState calls flush() only when transitioning to Idle
  • Default parameter isIdleRef = { current: true } preserves backward compatibility for any other callers

Smoke test (optional):

Normal auto-update behavior (idle delivery, no mid-stream interruption) is fully covered by unit tests. Manual E2E verification is not required.

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

Fixes #3046

…wenLM#3046)

When a background auto-update finished while the model was streaming,
the success/failure notification was inserted mid-conversation via
addItem(), disrupting the user's reading flow.

Introduce a "defer until idle" mechanism in setUpdateHandler():
- Accept an `isIdleRef` param that tracks whether StreamingState is Idle
- Queue notifications in a `pendingNotifications` array when not idle
- Expose a `flush()` method that drains the queue once idle
- AppContainer keeps `isIdleRef.current` in sync with `streamingState`
  and calls `flush()` via a useEffect when transitioning back to Idle

All four event types (update-received, update-success, update-failed,
update-info) are routed through the same addItemOrDefer() helper.
The third parameter defaults to `{ current: true }` for backward
compatibility.
@pic4xiu pic4xiu marked this pull request as ready for review April 16, 2026 05:05
@tanzhenxin tanzhenxin added the type/bug Something isn't working as expected label Apr 16, 2026
Copy link
Copy Markdown
Collaborator

@tanzhenxin tanzhenxin left a comment

Choose a reason for hiding this comment

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

Review

Clean defer-until-idle mechanism that correctly solves the reported mid-stream notification interruption. The ref-based idle tracking avoids re-renders, and the render-body assignment + effect-based flush is correct React idiom. Test suite is thorough with 10 new cases covering deferral, flush, ordering, and cleanup.

Verdict

APPROVE — Correct, well-tested fix. Ship it.

@tanzhenxin tanzhenxin merged commit 662c2ab into QwenLM:main Apr 16, 2026
15 checks passed
@pic4xiu pic4xiu deleted the fix/defer-update-notification-until-idle branch April 16, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UX] Auto-update success notification inserted into conversation history during active session

2 participants