Repository navigation
fix: [Q&A Page] Thought processes, tool calls, and sub-agent executions are collapsed by default during streaming output; users can expand them as needed. #7925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,15 @@ | ||
| <script setup lang="ts"> | ||
| import { ref, watch } from 'vue' | ||
| import { ref, useTemplateRef } from 'vue' | ||
| import { useBottomFollow } from '@/conversation-panel/core/bottom-follow' | ||
|
|
||
| defineOptions({ name: 'ConversationReasoningContent' }) | ||
|
|
||
| const props = defineProps<{ content: { content?: string | null; status?: string } }>() | ||
| const isExpanded = ref(true) | ||
| let prevStatus = props.content.status | ||
| defineProps<{ content: { content?: string | null; status?: string } }>() | ||
| // 默认收起,思考中也只显示状态文案,手动展开后查看内容 | ||
| const isExpanded = ref(false) | ||
|
|
||
| watch( | ||
| () => props.content.status, | ||
| (status) => { | ||
| if (prevStatus === 'RUNNING' && status !== 'RUNNING') { | ||
| isExpanded.value = false | ||
| } | ||
| prevStatus = status | ||
| }, | ||
| ) | ||
| // 思考内容限高滚动,高度与新建文件的差异区一致;流式输出时跟随到底部 | ||
| useBottomFollow(useTemplateRef<HTMLElement>('reasoningScrollRef')) | ||
| </script> | ||
|
|
||
| <template> | ||
|
|
@@ -35,7 +29,7 @@ watch( | |
| {{ content.status === 'RUNNING' ? '思考中' : '思考完成' }} | ||
| </span> | ||
| </template> | ||
| <div class="text-N500! whitespace-pre-wrap break-words"> | ||
| <div ref="reasoningScrollRef" class="max-h-[400px] overflow-y-auto text-N500! whitespace-pre-wrap break-words"> | ||
| {{ content.content }} | ||
| </div> | ||
| </MkCollapse> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's my review of the diff: Potential Issues
Minor / Style
Optimization Suggestions
Summary: No outright bugs in the shown hunks, but verify (a) the |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { onBeforeUnmount, watch, type Ref } from 'vue' | ||
| import { Scroll } from './scroll' | ||
|
|
||
| /** | ||
| * 限高滚动容器的底部跟随:容器挂载后定位到底部,内容变化时仅在用户仍位于底部附近才跟随。 | ||
| * 容器随展开 / 收起挂载和卸载,跟随实例与内容监听随之创建和释放。 | ||
| */ | ||
| export function useBottomFollow(containerRef: Ref<HTMLElement | null | undefined>) { | ||
| let scroll: Scroll | undefined | ||
| let observer: MutationObserver | undefined | ||
|
|
||
| function release() { | ||
| observer?.disconnect() | ||
| observer = undefined | ||
| scroll?.destroy() | ||
| scroll = undefined | ||
| } | ||
|
|
||
| watch( | ||
| containerRef, | ||
| (container) => { | ||
| release() | ||
| if (!container) return | ||
| const follow = new Scroll(container) | ||
| scroll = follow | ||
| follow.forceBottom() | ||
| observer = new MutationObserver(() => follow.scrollBottom()) | ||
| observer.observe(container, { childList: true, subtree: true, characterData: true }) | ||
| }, | ||
| { flush: 'post', immediate: true }, | ||
| ) | ||
|
|
||
| onBeforeUnmount(release) | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code ReviewPotential Issues
Optimization Suggestionsimport { onBeforeUnmount, watch, type Ref } from 'vue'
import { Scroll } from './scroll'
export function useBottomFollow(containerRef: Ref<HTMLElement | null>) {
let scroll: Scroll | undefined
let observer: MutationObserver | undefined
let rafId = 0
const isNearBottom = (el: HTMLElement) =>
el.scrollHeight - el.scrollTop - el.clientHeight < 32
function release() {
cancelAnimationFrame(rafId)
rafId = 0
observer?.disconnect()
observer = undefined
scroll?.destroy()
scroll = undefined
}
watch(
containerRef,
(container) => {
release()
if (!container || typeof MutationObserver === 'undefined') return
const follow = new Scroll(container)
scroll = follow
requestAnimationFrame(() => follow.forceBottom())
observer = new MutationObserver(() => {
if (!isNearBottom(container)) return
cancelAnimationFrame(rafId)
rafId = requestAnimationFrame(() => follow.scrollBottom())
})
observer.observe(container, { childList: true, subtree: true, characterData: true })
},
{ flush: 'post', immediate: true },
)
onBeforeUnmount(release)
return { release }
}SummaryThe main functional gap is the missing "near bottom" guard — the current implementation defeats its own stated behavior. After that, throttling via rAF and guarding against observer feedback loops are the highest-value fixes. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are my observations on this diff:
Potential Issues
Behavioral regression (intentional but worth validating):
useBottomFollowis called unconditionally with a template ref:useTemplateRef<HTMLElement>('groupScrollRef')resolves only after mount, and only when the element exists. Since the target div is insidev-if="isExpanded && childContents.length", the ref will benullwhenever the group is collapsed or has no children.useBottomFollowhandles anull/initially-undefined ref gracefully (attaching watchers lazily, re-resolving when the ref later becomes non-null after expansion). If it only grabsref.valueonce on setup oronMounted, following won't work after the first expand/collapse cycle.v-iftoggle.currentActivity/ collapse header now shows even when collapsed:currentActivitycomputed is actually rendered in the header (this diff doesn't show the header, so verify elsewhere).Removed dynamic getter means
content.statuschanges no longer affect expansion:RUNNING → DONEwould auto-collapse. Now it stays as user left it. That's presumably desired ("manual wins"), just note it.Optimization / Cleanliness
max-h-[400px]hardcoded: Consider extracting to a shared constant / CSS variable alongside the "new file diff region" height mentioned in the comment, so the two stay in sync.overflow-y-auto+ border styling: Withborder-l-2 pl-3, the scrollbar will sit at the far right of the div including padding. Minor, but if scrollbar aesthetics matter, consider inner wrapper.Accessibility: A scrollable container with fixed max-height should ideally have
tabindex="0"(and possiblyrole/aria-label) so keyboard users can scroll it. Same could apply to the follow-scroll region — decide whether full a11y is required.Ref naming:
groupScrollRefused via string key inuseTemplateRef— fine in Vue 3.5+. Ensure the project's Vue version supportsuseTemplateRef(it was introduced in 3.5). If the codebase targets <3.5, this import will fail.Test coverage: Add/adjust tests verifying:
status.Import ordering:
useBottomFollowis imported fromcore, placed after the component import — acceptable, but keep the project's lint grouping consistent.Summary
The main risk is
useBottomFollowreceiving a ref that is frequentlynulldue to thev-if. Please confirm its implementation handles lazy/conditional elements, and confirm the Vue version supportsuseTemplateRef. Otherwise the diff is straightforward and the behavior change looks intentional.