[shaohuzhang1]

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.

[shaohuzhang1]

Code Review

Potential Issues

1. No "user near bottom" detection — The docblock promises "only follow when the user is still near the bottom," but MutationObserver always calls follow.scrollBottom(). This will yank the scroll position even if the user scrolled up to read something. You likely need a check like if (isNearBottom(container)).

2. scrollBottom() on every mutation without throttling — Rapid DOM mutations (e.g., streaming text, character-by-character updates) trigger many synchronous layout reads/writes per frame, causing jank. Consider debouncing with requestAnimationFrame or checking inside the callback.

3. Feedback loop risk — If scrollBottom() itself mutates children or attributes that fall under the observer's scope (childList, subtree, characterData), it can re-trigger the observer and create an infinite loop. Ensure scrolling doesn't cause DOM changes, or disconnect/guard during the scroll call.

4. Missing attributes: true — Content expansion often occurs via CSS class/style changes (e.g., collapsed vs. expanded) without any child/character mutation. If your container grows this way, the observer won't fire. Consider adding attributes: true (but beware of noise).

5. No unmount cleanup for a mid-flight debounce — If you add debouncing, ensure pending timers/frames are cancelled in release(); otherwise callbacks can run after the element is detached.

6. scroll?.destroy() naming asymmetry — forceBottom/scrollBottom are imperative-scroll operations; destroy implies lifecycle. Verify Scroll.destroy() actually removes listeners and isn't just a no-op, since it's the only teardown available here.

7. flush: 'post' correctness — Good choice so the container exists in the DOM before measuring. However, immediate: true with flush: 'post' may still run before the browser has laid out (fonts, images). forceBottom() could compute an incorrect height on first mount; consider deferring initial scroll to the next requestAnimationFrame.

8. Return value absent — The composable returns nothing. Callers can't force a manual follow (e.g., "jump to latest" button). Consider exposing { scrollToBottom, release }.

9. Ref<HTMLElement | null | undefined> — Undefined and null are both handled identically; typically Ref<HTMLElement | null> suffices. Minor.

10. SSR safety — MutationObserver isn't guarded. If the module ever runs server-side, instantiation will throw. Guard with typeof MutationObserver !== 'undefined' or import from vue (some setups polyfill this; otherwise add a check).

Optimization Suggestions

import { 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 }
}

Summary

The 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.

[shaohuzhang1]

Here's my review of the diff:

Potential Issues

1. isExpanded no longer controlled by lifecycle — possible UX regression (unverifiable from this diff). Previously, content auto-collapsed when status transitioned from RUNNING to done. Now it defaults to false and never changes programmatically. If the MkCollapse is still bound to isExpanded, fine—but confirm nothing else relies on the old auto-collapse behavior (e.g., tests, snapshots, or parent expectations). Also note: the commit message references "与新建文件的差异区一致" and "流式输出时跟随到底部", but there's no visible logic change that ensures the follow only happens while RUNNING. If useBottomFollow auto-scrolls unconditionally, it could jump the viewport unexpectedly after completion.

2. ref given to v-model not shown in diff. With isExpanded = ref(false), make sure the <MkCollapse> binding (presumably v-model="isExpanded") remains intact throughout the template — it's not included in this diff hunk.

3. Auto-collapse removal loses user-state preservation semantics. Previously expanded content collapsed on completion; now it starts collapsed. Verify product intent — "思考中也只显示状态文案" suggests content is hidden during streaming even though new text is arriving. This is a deliberate change, but users who expand mid-stream rely on useBottomFollow to keep up; ensure it handles the case where the scroll container appears/disappears with isExpanded.

4. max-h-[400px] magic number duplicates layout knowledge. The comment says it matches the "新建文件的差异区" height. Consider extracting to a shared CSS variable/token so both places stay in sync.

5. useTemplateRef<HTMLElement>('reasoningScrollRef') is evaluated at setup time. That's fine if the composable watches the ref reactively (it should, per the Vue 3.5 pattern). Confirm useBottomFollow accepts a Readonly<ShallowRef> and reacts to the element being toggled via isExpanded — otherwise following won't engage after expansion.

6. defineProps result unused (no props const). Fine under vue/consistent-define-props-style rules, but double-check other tooling doesn't require a named variable. Otherwise lint-clean.

7. whitespace-pre-wrap + overflow-y-auto: long single tokens can still overflow horizontally unless break-words (overflow-wrap: break-word) applies; it's present, good. But add overflow-x-hidden or pr-* padding to avoid a scrollbar overlaying content vertically.

8. Accessibility: scroll container has no focusability/tabindex. Keyboard users can't scroll the reasoning content without focusing an ancestor. Consider tabindex="0" or a role="region" with a label.

Minor / Style

• Typo in naming: text-N500! looks like a custom utility; ensure ! suffix convention matches the project (Tailwind v4?).
• Composable name useBottomFollow(...) immediately invoked — fine, but consider calling it with the return value assigned if you later need pause/resume on user scroll.
• Removing the watch eliminates reliance on prevStatus, which was fragile (didn't handle initial RUNNING → undefined etc.). Good simplification overall.

Optimization Suggestions

• Bind height via a shared token: class="max-h-[var(--reasoning-max-height,400px)]".
• Add shrink-0/min-h-0 as appropriate inside the collapse body so flex layouts don't distort the scroll area.
• Ensure useBottomFollow respects user scrolling (only auto-follow when near bottom), otherwise streaming will fight manual scroll-up.
• Keep a test/snapshot covering the "expand during RUNNING" path to lock in the new intended behavior.

Summary: No outright bugs in the shown hunks, but verify (a) the MkCollapse binding still wires isExpanded, (b) useBottomFollow gates following to streaming/near-bottom state, and (c) the behavioral change (start collapsed, no auto-collapse) is intentional and documented/tested.

[shaohuzhang1]

Here are my observations on this diff:

Potential Issues

1. Behavioral regression (intentional but worth validating):

   • The default state changed from "expanded when RUNNING or no status" to always collapsed.
   • If any UX flow, test, or screenshot relies on the old auto-expand behavior for running groups, this will break. Confirm product agrees.

2. useBottomFollow is called unconditionally with a template ref:

   • useTemplateRef<HTMLElement>('groupScrollRef') resolves only after mount, and only when the element exists. Since the target div is inside v-if="isExpanded && childContents.length", the ref will be null whenever the group is collapsed or has no children.
   • Verify that useBottomFollow handles a null/initially-undefined ref gracefully (attaching watchers lazily, re-resolving when the ref later becomes non-null after expansion). If it only grabs ref.value once on setup or onMounted, following won't work after the first expand/collapse cycle.
   • Also confirm it re-hooks when the element is unmounted/remounted by the v-if toggle.

3. currentActivity / collapse header now shows even when collapsed:

   • The comment previously said "collapsed shows latest child activity." Now that default is collapsed, ensure the currentActivity computed is actually rendered in the header (this diff doesn't show the header, so verify elsewhere).

4. Removed dynamic getter means content.status changes no longer affect expansion:

   • Previously a transition from RUNNING → DONE would auto-collapse. Now it stays as user left it. That's presumably desired ("manual wins"), just note it.

Optimization / Cleanliness

5. 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.

6. overflow-y-auto + border styling: With border-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.

7. Accessibility: A scrollable container with fixed max-height should ideally have tabindex="0" (and possibly role/aria-label) so keyboard users can scroll it. Same could apply to the follow-scroll region — decide whether full a11y is required.

8. Ref naming: groupScrollRef used via string key in useTemplateRef — fine in Vue 3.5+. Ensure the project's Vue version supports useTemplateRef (it was introduced in 3.5). If the codebase targets <3.5, this import will fail.

9. Test coverage: Add/adjust tests verifying:

   • Default collapsed regardless of status.
   • Manual toggle persists and is unaffected by status change.
   • Bottom-follow triggers on child content updates while expanded.

10. Import ordering: useBottomFollow is imported from core, placed after the component import — acceptable, but keep the project's lint grouping consistent.

Summary

The main risk is useBottomFollow receiving a ref that is frequently null due to the v-if. Please confirm its implementation handles lazy/conditional elements, and confirm the Vue version supports useTemplateRef. Otherwise the diff is straightforward and the behavior change looks intentional.