fix(chat): stabilize streaming interactions#1732
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughDocumentation and tests for several renderer regressions; switch Tailwind scan target to markstream-vue/dist/tailwind.js; Make reasoning headings inherit prose sizing; default smoothStreaming true; add ChatPage multi-frame session-restore bottom-settle and post-submit bottom forcing; add AgentRuntimePresenter user-pause for pending queues; bump two dependencies; set default PR base guidance to dev. ChangesRegression Fixes and Queue Pause Behavior
Repository Guidelines Update
Sequence DiagramsequenceDiagram
participant User
participant ChatPage
participant ScrollSettle
participant Queue as AgentRuntimePresenter
User->>ChatPage: submit message / switch session / stop
ChatPage->>ScrollSettle: schedule post-submit bottom scroll or session-restore settle
ScrollSettle->>ChatPage: read scrollHeight, set scrollTop to bottom
ScrollSettle->>ChatPage: observe ResizeObserver events and retry if height grows
User->>ChatPage: wheel/scroll/keydown intent
ChatPage->>ScrollSettle: cancel settle
User->>Queue: stop active queued turn (via cancelGeneration)
Queue->>Queue: mark session paused, block drain
User->>Queue: resumePendingQueue
Queue->>Queue: clear pause and resume drain
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/agentRuntimePresenter/index.ts (1)
621-631:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear the pause flag even when draining must wait.
Line 626 returns before the explicit resume action clears
userPausedPendingQueues. That means the UI resume path insrc/renderer/src/stores/ui/pendingInput.ts:114-124is ignored whenever the session is waiting for a follow-up answer, and once that follow-up is sent the later'completed'drain path is still blocked by the stale pause flag.Suggested fix
async resumePendingQueue(sessionId: string): Promise<void> { const state = await this.getSessionState(sessionId) if (!state) { throw new Error(`Session ${sessionId} not found`) } + this.userPausedPendingQueues.delete(sessionId) if (this.isAwaitingToolQuestionFollowUp(sessionId)) { return } - - this.userPausedPendingQueues.delete(sessionId) void this.drainPendingQueueIfPossible(sessionId, 'resume') }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/presenter/agentRuntimePresenter/index.ts` around lines 621 - 631, In resumePendingQueue, the code returns early when isAwaitingToolQuestionFollowUp(sessionId) is true but that prevents clearing the userPausedPendingQueues flag; move or add the deletion of the pause flag so userPausedPendingQueues.delete(sessionId) is executed regardless of the follow-up-waiting check (i.e., delete before or immediately after calling isAwaitingToolQuestionFollowUp), and keep the existing early return behavior so that drainPendingQueueIfPossible(sessionId, 'resume') is only invoked when appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/pages/ChatPage.vue`:
- Around line 451-517: The settleSessionRestoreScrollToBottom function only
cancels restore-settling on wheel/touchstart, allowing scrollbar drags and
keyboard scrolling to be overridden; update it to listen for additional
user-scroll intents (add 'scroll' on scrollContainer, pointer events like
'pointerdown'/'mousedown' to catch scrollbar drags, and keyboard events such as
'keydown' for PageUp/PageDown/Arrow/Home/End) and wire those to
cancelSessionRestoreScrollSettle via the existing
cancelSessionRestoreScrollIntentListeners mechanism; ensure you register these
same handlers on the same element(s) (scrollContainer.value) and remove them in
cancelSessionRestoreScrollIntentListeners so cancelSessionRestoreScrollSettle
still tears down RAF (sessionRestoreScrollFrame), ResizeObserver
(sessionRestoreResizeObserver) and timeout (sessionRestoreScrollTimer)
correctly.
In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts`:
- Line 3476: Replace the non-deterministic sleep calls "await new
Promise((resolve) => setTimeout(resolve, 10))" in the test file with
deterministic coordination: use Vitest's vi.waitFor to poll for the expected
condition (e.g., assert that the presenter state or mock call count has changed)
or refactor to await an event/promise returned by the code under test; update
both occurrences (the one at the shown line and the other at the noted nearby
line) to wait for a concrete observable condition instead of a fixed timeout so
the tests are reliable on slow CI/machines.
---
Outside diff comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Around line 621-631: In resumePendingQueue, the code returns early when
isAwaitingToolQuestionFollowUp(sessionId) is true but that prevents clearing the
userPausedPendingQueues flag; move or add the deletion of the pause flag so
userPausedPendingQueues.delete(sessionId) is executed regardless of the
follow-up-waiting check (i.e., delete before or immediately after calling
isAwaitingToolQuestionFollowUp), and keep the existing early return behavior so
that drainPendingQueueIfPossible(sessionId, 'resume') is only invoked when
appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86b56889-6647-4819-866c-53a20c117353
📒 Files selected for processing (19)
docs/issues/markdown-codeblock-session-scroll-regressions/plan.mddocs/issues/markdown-codeblock-session-scroll-regressions/spec.mddocs/issues/markdown-codeblock-session-scroll-regressions/tasks.mddocs/issues/reasoning-heading-font-size/plan.mddocs/issues/reasoning-heading-font-size/spec.mddocs/issues/reasoning-heading-font-size/tasks.mddocs/issues/stop-pauses-pending-queue/plan.mddocs/issues/stop-pauses-pending-queue/spec.mddocs/issues/stop-pauses-pending-queue/tasks.mdpackage.jsonsrc/main/presenter/agentRuntimePresenter/index.tssrc/renderer/src/assets/style.csssrc/renderer/src/components/markdown/MarkdownRenderer.vuesrc/renderer/src/components/think-content/ThinkContent.vuesrc/renderer/src/pages/ChatPage.vuetest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/renderer/assets/markstreamTailwindSource.test.tstest/renderer/components/ChatPage.test.tstest/renderer/components/think-content/ThinkContentStyle.test.ts
Collaborator
Author
|
Addressed the remaining review feedback in |
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.
Summary
UI Structure
BEFORE:
AFTER:
Tests
Summary by CodeRabbit
Bug Fixes
Improvements
Documentation