Skip to content

fix: Whether to display execution details depends on the configuration. #7820

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.

Sign up for GitHub

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

Merged
shaohuzhang1 merged 1 commit into from
Nov 10, 2026
Merged

fix: Whether to display execution details depends on the configuration. #7820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ui/src/conversation-panel/components/message-footer/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { datetimeFormat } from '@/utils/time'
import type { ChatMessage, ChatRecordMeta } from '../../core/types'
import { useExecutionDetailStore } from '../../right-sidebar/execution-detail/index'

import { MESSAGE_LIST_KEY } from '../message-list/index'
import { MESSAGE_LIST_KEY, useMessageListStore } from '../message-list/index'
import { useShareLinkStoreOptional } from '../message-list/share/index'
import { useShowShare } from '../../view/index'

Expand All @@ -24,14 +24,14 @@ onMounted(() => {
if (footerRef.value) footerObserver.observe(footerRef.value)
})
onBeforeUnmount(() => footerObserver?.disconnect())

// 智能体允许分享时显示分享入口；进入分享时预先勾选当前回答所属轮次。
const showShare = useShowShare()
const shareLink = useShareLinkStoreOptional()

// 回答生成中禁用分享。
const messageList = inject(MESSAGE_LIST_KEY, null)
const canShare = computed(() => Boolean(messageList?.messages.value.length && !messageList.streamLoading.value))

const show_exec=computed(()=>messageList?.appInfo.value?.show_exec)
const detail = useExecutionDetailStore()

const EMPTY: ChatRecordMeta = {
Expand Down Expand Up @@ -143,7 +143,7 @@ function openFeedback(status: '0' | '1') {
</el-button>

<!-- 执行详情:对话结束时已拉取并缓存,点击直接在右侧展示 -->
<el-button text @click="openExecutionDetail" class="-ml-1">
<el-button v-if="show_exec" text @click="openExecutionDetail" class="-ml-1">
<MkIcon name="icon_logs_outlined" />
<span>执行详情</span>
</el-button>

@shaohuzhang1 shaohuzhang1 Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here is a review of the code changes with identified issues and optimization suggestions:

1. Code Style & Formatting

  • Missing Spaces: const show_exec=computed(...) lacks spacing around the equals sign and arrow function. It should be const showExec = computed(() => ...).
  • Naming Convention: show_exec uses snake_case. Vue/JS conventions typically prefer camelCase for variables (showExec). Additionally, exec is an ambiguous abbreviation; executionDetailsVisible or canViewExecution would be clearer.
  • Import Usage: You imported useMessageListStore but do not seem to use it in the diff. If unused, remove it to prevent lint errors and confusion.

2. Logic & Safety Issues

  • Optional Chaining in Computed: 
    const show_exec = computed(() => messageList?.appInfo.value?.show_exec)
    While optional chaining prevents crashes, v-if="show_exec" will evaluate as false if messageList is null or undefined. This is safe, but ensure that the intent is to hide the button when data isn't ready, rather than hiding it due to a missing provider.
  • Reactivty Check: Ensure messageList?.appInfo is truly reactive. If appInfo is assigned once and doesn't change, accessing .value inside the computed property getter might cause unnecessary dependency tracking if not handled carefully. However, since messageList is injected, this is likely correct provided useMessageListStore returns reactive refs.

3. Optimization Suggestions

  • Hoisting Logic: The computed property show_exec is defined after canShare. Group related states together for better readability.
  • Template Clarity: In the template, v-if="show_exec" relies entirely on the state of messageList. If useShowShare() or other external factors affect visibility, consider combining them: 
    const showExecutionButton = computed(() => 
  Boolean(messageList?.appInfo?.value?.show_exec)
);
    (Note: Removed redundant .length check if show_exec is already a boolean flag).

Recommended Refactored Code Snippet

import { MESSAGE_LIST_KEY } from '../message-list/index'; // Removed unused useMessageListStore
// ... other imports

// 智能体允许分享时显示分享入口；进入分享时预先勾选当前回答所属轮次。
const showShare = useShowShare()
const shareLink = useShareLinkStoreOptional()

const messageList = inject(MESSAGE_LIST_KEY, null)
const detail = useExecutionDetailStore()

// 回答生成中禁用分享。
const canShare = computed(() => Boolean(messageList?.messages?.value?.length && !messageList?.streamLoading?.value))

// ✅ Fixed naming, spacing, and logic clarity
const showExec = computed(() => {
  // Optional chaining ensures safe access even if messageList or appInfo is null
  return Boolean(messageList?.appInfo?.value?.show_exec);
})

// ...

<el-button 
  v-if="showExec" 
  text 
  @click="openExecutionDetail" 
  class="-ml-1"
>
  <MkIcon name="icon_logs_outlined" />
  <span>执行详情</span>
</el-button>

Summary of Actions:

  1. Remove unused import useMessageListStore.
  2. Rename show_exec to showExec (camelCase).
  3. Fix Format: Add spaces around = and arrows.
  4. Keep v-if but ensure the variable name clearly indicates its boolean nature.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/conversation-panel/components/message-list/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const showUserAvatar = computed(() => appInfo.value?.show_user_avatar)
<!-- 助手消息结束后显示底部信息栏(知识来源/执行详情/tokens/耗时) -->
<MessageFooter
class="mt-3"
v-if="!isShared && msg.role === 'ASSISTANT' && (msg.write_ed || msg.record)"
v-if="(!isShared && msg.role === 'ASSISTANT' && (msg.write_ed || msg.record))&&!streamLoading"
:message="msg"
@resize="scrollRef?.followBottom()"
/>

@shaohuzhang1 shaohuzhang1 Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here is an analysis of the code snippet provided:

1. Irregularities & Issues

  • Formatting Spacing: There is a missing space between ) and ! in (msg.write_ed || msg.record))&&!streamLoading. While this is valid syntax, it violates standard formatting guidelines (e.g., Prettier, ESLint) and reduces readability.
    • Fix: Add a space: ...)) && !streamLoading.
  • Boolean Logic Order: In Vue templates, v-if conditions are evaluated every render cycle. The condition isShared && msg.role === 'ASSISTANT' ... involves multiple checks. If isShared is true, the rest of the expression short-circuits correctly. However, mixing template display logic (!isShared) with component state (!streamLoading) directly in the template can make the view harder to maintain.
  • Potential Race Condition/Stuttering: If streamLoading toggles frequently during streaming, this condition will cause the <MessageFooter> component to mount/unmount repeatedly. This leads to:
    • Unnecessary DOM re-renders.
    • Flickering if the footer has animations or transitions.
    • Performance overhead due to creating/destroying the component instance.

2. Optimization Suggestions

Option A: Extract to a Computed Property (Recommended)

Move the complex boolean logic into a computed property for better performance, testability, and readability.

// Script setup
const shouldShowFooter = computed(() => {
  if (isShared.value) return false;
  if (streamLoading.value) return false;
  if (msg.role !== 'ASSISTANT') return false; // Assuming 'msg' is available in scope
  
  return !!msg.write_ed || !!msg.record; // Ensure explicit boolean check if needed
});
<!-- Template -->
<MessageFooter
  v-if="shouldShowFooter"
  class="mt-3"
  :message="msg"
  @resize="scrollRef?.followBottom()"
/>

Option B: Use v-show Instead of v-if (If Footer is Heavy)

If <MessageFooter> is expensive to initialize and you only want to toggle visibility without unmounting, use v-show. However, note that v-show keeps the component mounted even when hidden.

<MessageFooter
  v-show="(!isShared && msg.role === 'ASSISTANT' && (msg.write_ed || msg.record)) && !streamLoading"
  ...
/>

Note: Only use v-show if the footer contains heavy initial computations or internal state that should persist. For simple UI elements, v-if is generally preferred.

3. Corrected Code Snippet (Quick Fix)

If you prefer to keep the inline logic, at least fix the formatting:

<MessageFooter
  class="mt-3"
  v-if="(!isShared && msg.role === 'ASSISTANT' && (msg.write_ed || msg.record)) && !streamLoading"
  :message="msg"
  @resize="scrollRef?.followBottom()"
/>

Summary

  • Critical Issue: None (code works functionally).
  • Best Practice Violation: Formatting ()&&! → ) && !).
  • Architectural Improvement: Extract boolean logic to a computed property to separate presentation logic from template markup and improve render performance stability.

Expand Down
Loading