Repository navigation
Cleanup when closing the chat while it is receiving a stream - #673
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (3 skipped: 3 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly describes the main change: cleanup when closing the chat during streaming. |
| Description check | |
The description directly explains the streaming scroll error, listener cleanup, logging changes, and reproduction steps. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (3 skipped: 3 unsupported.)
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 4004ff8c-430b-47e8-8185-4896e1efe2e3
📒 Files selected for processing (1)
src/components/ChattyLLM/ChattyLLMInputForm.vue
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
| }, | ||
|
|
||
| beforeUnmount() { | ||
| this.streamingMessage = null |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '430,525p' src/components/ChattyLLM/ChattyLLMInputForm.vue
sed -n '600,655p' src/components/ChattyLLM/ChattyLLMInputForm.vue
rg -n 'streamingMessage|scrollToBottomWhileStreaming|beforeUnmount|AbortController|abort\\(' src/components/ChattyLLM
sed -n '1,90p' src/components/ChattyLLM/ConversationBox.vueRepository: nextcloud/assistant
Length of output: 6988
🏁 Script executed:
set -e
printf '%s\n' '--- component outline ---'
ast-grep outline src/components/ChattyLLM/ChattyLLMInputForm.vue
printf '%s\n' '--- stream-related symbols ---'
rg -n -C 5 'streamingMessage|pollMessageGenerationCancel|fetch.*Stream|stream|onDownloadProgress|onmessage|EventSource|ReadableStream|axios' src/components/ChattyLLM/ChattyLLMInputForm.vue src/components/ChattyLLM
printf '%s\n' '--- cancellation definitions/usages ---'
rg -n -C 6 'pollMessageGenerationCancel|cancelTaskPositionPolling|pollMessageGenerationTimerId' .
printf '%s\n' '--- lifecycle and parent consumers ---'
rg -n -C 5 'ChattyLLMInputForm|beforeUnmount|unmount|active.*null|onSessionSelect' src/components/ChattyLLMRepository: nextcloud/assistant
Length of output: 41690
🏁 Script executed:
set -e
rg -n -C 8 'streamingMessage|pollMessageGenerationCancel|fetch.*Stream|stream|onDownloadProgress|onmessage|EventSource|ReadableStream|axios' src/components/ChattyLLM/ChattyLLMInputForm.vue src/components/ChattyLLM
rg -n -C 8 'pollMessageGenerationCancel|cancelTaskPositionPolling|pollMessageGenerationTimerId' srcRepository: nextcloud/assistant
Length of output: 42143
🏁 Script executed:
set -e
rg -n 'streamingMessage|pollMessageGenerationCancel|cancelTaskPositionPolling|pollMessageGenerationTimerId|stream|onDownloadProgress|onmessage|EventSource|ReadableStream|axios' src/components/ChattyLLM/ChattyLLMInputForm.vue
printf '%s\n' '--- component methods and lifecycle ---'
sed -n '1,180p' src/components/ChattyLLM/ChattyLLMInputForm.vue
sed -n '180,430p' src/components/ChattyLLM/ChattyLLMInputForm.vue
sed -n '430,700p' src/components/ChattyLLM/ChattyLLMInputForm.vue
printf '%s\n' '--- helper definitions ---'
rg -n -C 12 'pollMessageGenerationCancel|cancelTaskPositionPolling' srcRepository: nextcloud/assistant
Length of output: 42083
🌐 Web query:
github nextcloud assistant ChattyLLMInputForm.vue pollMessageGenerationCancel streamingMessage
💡 Result:
The closest match is **Nextcloud Assistant issue #398**, “Add Streaming Support for Chat Bot Responses.” It points to `src/components/ChattyLLM/ChattyLLMInputForm.vue` and describes the existing polling-based generation flow. The issue proposes streaming responses, but the search results don’t confirm symbols named `pollMessageGenerationCancel` or `streamingMessage` in that file. (GitHub issue 398 in nextcloud/assistant (link omitted to avoid creating a cross-reference))
Citations:
- 1: GitHub issue 398 in nextcloud/assistant (link omitted to avoid creating a cross-reference)
🏁 Script executed:
set -e
printf '%s\n' '--- generation poll and update path ---'
sed -n '1060,1195p' src/components/ChattyLLM/ChattyLLMInputForm.vue
printf '%s\n' '--- notification registration in ChattyLLMInputForm ---'
rg -n -C 20 'listenToTaskNotifications|listen\\(' src/components/ChattyLLM/ChattyLLMInputForm.vue
printf '%s\n' '--- streaming consumers and cleanup ---'
rg -n -C 12 'streaming|setTimeout|watch|beforeUnmount|onUnmounted' src/components/ChattyLLM/ChatMessage.vue src/components/ChattyLLM/ConversationBox.vueRepository: nextcloud/assistant
Length of output: 5172
🏁 Script executed:
set -e
sed -n '1060,1195p' src/components/ChattyLLM/ChattyLLMInputForm.vue
rg -n -C 20 'listenToTaskNotifications|listen\(' src/components/ChattyLLM/ChattyLLMInputForm.vue
rg -n -C 12 'streaming|setTimeout|watch|beforeUnmount|onUnmounted' src/components/ChattyLLM/ChatMessage.vue src/components/ChattyLLM/ConversationBox.vueRepository: nextcloud/assistant
Length of output: 25656
Invalidate task notifications when ChattyLLMInputForm unmounts.
When push notifications are enabled, listenToTaskNotifications registers a callback. beforeUnmount cancels polling but does not invalidate this callback. Because active still contains the session, a later notification can call updateStreamingMessage after streamingMessage was cleared. This recreates component state and schedules scrolling after the component has closed.
Suggested fix
messages: [], // null when failed to fetch
streamingMessage: null,
+ isUnmounted: false,
// only used while streaming to prevent auto scrolling after some user scrolling happened
userScrolled: false,
@@
beforeUnmount() {
+ this.isUnmounted = true
this.streamingMessage = null
this.pollMessageGenerationCancel?.()
@@
const pushChannel = 'taskprocessing:task_id_' + pushTaskId
const hasPush = listen(pushChannel, (type, body) => {
+ if (this.isUnmounted) {
+ return
+ }
console.debug('[assistant] received push notification', type, body)
…we don't try to scroll a dead UI element Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…nted Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
… is unmounted Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…y are ignored Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
e6b836f to
2d31916
Compare
To reproduce the scroll issue
Uncaught (in promise) TypeError: can't access property "scrollHeight", chatAreaElem is nullused to be spammed in the console each time a chunk is received via notify push🤖 AI (if applicable)