Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ui/src/assets/iconfont.js

Large diffs are not rendered by default.

54 changes: 53 additions & 1 deletion ui/src/conversation-panel/components/message-footer/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,41 @@ function collectAnswerText(contents: ChatContent[]): string {
}
const answerText = computed(() => collectAnswerText(props.message.content))

// 重新生成沿用当前记录及原问题,由列表 Store 管理流式替换。
const canRegenerate = computed(() => Boolean(messageList && props.message.recordId && !messageList.streamLoading.value && !messageList.loading.value))
function regenerateAnswer() {
if (!canRegenerate.value) return
stopReading()
messageList?.regenerate(props.message)
}

// 浏览器朗读仅使用回答正文;切换回答或卸载时释放本组件发起的语音。
const speechSupported = typeof window !== 'undefined' && 'speechSynthesis' in window
const speaking = ref(false)
let utterance: SpeechSynthesisUtterance | undefined
function stopReading() {
if (utterance) {
utterance.onend = null
utterance.onerror = null
window.speechSynthesis.cancel()
utterance = undefined
}
speaking.value = false
}
function toggleReading() {
if (speaking.value) return stopReading()
if (!speechSupported || !answerText.value) return
window.speechSynthesis.cancel()
utterance = new SpeechSynthesisUtterance(answerText.value)
utterance.onend = utterance.onerror = () => {
speaking.value = false
utterance = undefined
}
speaking.value = true
window.speechSynthesis.speak(utterance)
}
onBeforeUnmount(stopReading)

const openExecutionDetail = () => {
detail.showExecutionDetail(meta.value.executionDetails || [])
}
Expand Down Expand Up @@ -125,6 +160,24 @@ function openFeedback(status: '0' | '1') {
<MkIcon name="icon_copy_outlined" />
</el-button>
</MkTooltip>
<!-- 重新生成当前回答 -->
<span>
<MkTooltip content="重新生成">
<el-button text :disabled="!canRegenerate" @click="regenerateAnswer">
<MkIcon name="icon_refresh_outlined" />
</el-button>
</MkTooltip>
</span>

<!-- 朗读回答正文,再次点击停止 -->
<span>
<MkTooltip :content="speaking ? '停止朗读' : '朗读内容'">
<el-button text :disabled="!speechSupported || !answerText" @click="toggleReading">
<MkIcon :name="speaking ? 'icon_pause-round_outlined' : 'icon_speaker_outlined'" />
</el-button>
</MkTooltip>
</span>
<ButtonFeedback v-if="canFeedback" :current-status="currentStatus" :submitting="submitting" @select="openFeedback" />
<!-- 进入分享模式 -->
<span v-if="showShare">
<MkTooltip content="分享">
Expand All @@ -133,7 +186,6 @@ function openFeedback(status: '0' | '1') {
</el-button>
</MkTooltip>
</span>
<ButtonFeedback v-if="canFeedback" :current-status="currentStatus" :submitting="submitting" @select="openFeedback" />
<span v-if="meta.createTime" class="text-N600 ml-1">{{ datetimeFormat(meta.createTime) }}</span>
</div>
<el-collapse-transition>

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 provided code, highlighting potential issues, irregularities, and optimization suggestions.

1. Security & Privacy Concerns

  • XSS via Dynamic Icon Names: icon_pause-round_outlined and icon_speaker_outlined are hardcoded, which is safe. However, ensure that if dynamic names were ever used, they are sanitized. In this specific snippet, name bindings are static or derived from simple booleans (speaking ? ... : ...`), so XSS risk from icon names is low.
  • Content Injection in answerText: window.speechSynthesis.speak(utterance) uses answerText.value. If collectAnswerText returns raw HTML or unescaped content, it could lead to injection issues if the speech synthesis backend or underlying browser implementation mishandles the string. While SpeechSynthesisUtterance treats input as plain text, ensure collectAnswerText does not expose sensitive data unintentionally.

2. Performance & Reactivity Issues

  • Redundant computed Evaluation: answerText is a computed property. It will re-evaluate every time any reactive dependency changes. If props.message.content is a large object, collectAnswerText might be expensive. Consider memoizing or optimizing collectAnswerText if it performs complex parsing.
  • onBeforeUnmount Leak Risk: speechSupported is checked inside stopReading(), but onBeforeUnmount(stopReading) relies on window.speechSynthesis existing. If the component unmounts before window is properly defined (e.g., during initial SSR hydration or race conditions), stopReading will throw an error because window.speechSynthesis would be undefined.
    • Fix: Add a guard in stopReading:
      function stopReading() {
        if (!speechSupported) return; // Guard against SSR or unsupported env
        if (utterance) {
          // ... cancel logic
        }
        speaking.value = false;
      }

3. Bug Risks & Logic Flaws

  • Race Condition in toggleReading:

    utterance.onend = utterance.onerror = () => {
      speaking.value = false;
      utterance = undefined;
    };
    speaking.value = true;
    window.speechSynthesis.speak(utterance);

    If speechSynthesis.speak() triggers onend/onerror asynchronously very quickly (e.g., if the text is empty or error occurs immediately), speaking.value is set to false, but utterance is also set to undefined. However, if a user clicks "Stop" (toggleReading) while speaking is true, stopReading() cancels synthesis. The subsequent onend callback might fire after stopReading already cleared utterance and set speaking to false. This is generally handled by checking if (!speechSupported || !answerText.value) return at the start, but edge cases with rapid toggling may cause state desync.

    • Optimization: Check if speaking.value is already changing in the callback?
    • Better Pattern: Use a unique ID for each utterance to ignore stale callbacks.
  • regenerateAnswer State Conflict:
    You have two separate loading states: messageList.streamLoading.value and messageList.loading.value. Ensure these are mutually exclusive or that canRegenerate correctly accounts for all pending operations. If streamLoading is true, regenerate is disabled, which is good. But after regeneration starts, does loading become true? Ensure the UI reflects this transition smoothly.

  • Missing Error Handling for Synthesis:
    If speechSynthesis fails mid-speak (e.g., system error), onerror is called, but no user feedback (toast/error message) is provided. The UI just stops speaking silently. Consider adding a toast notification if speech fails unexpectedly.

4. Code Style & Best Practices

  • Vue Composition API Best Practices:
    • ✅ Good: Using computed for derived state.
    • ✅ Good: Cleanup via onBeforeUnmount.
    • ⚠️ Suggestion: Define utterance type more strictly. Currently let utterance: SpeechSynthesisUtterance | undefined. This is fine, but consider wrapping speech logic in a Composable (e.g., useSpeechSynthesis) for reusability across components.
  • Template Optimization:
    • You moved <ButtonFeedback> up in the template. Ensure the visual layout remains consistent. Moving elements in DOM without adjusting CSS flex/grid order might cause unexpected shifts.
    • ✅ Good: Conditional rendering with v-if for showShare and canFeedback avoids unnecessary DOM nodes.

5. Specific Suggestions for Improvement

A. Refactor Speech Logic into a Composable

Encapsulate speaking, utterance, stopReading, and toggleReading into a reusable hook to avoid cluttering the main component.

// composable.ts
export function useSpeech(contentRef: Ref<string>) {
  const supported = typeof window !== 'undefined' && 'speechSynthesis' in window;
  const speaking = ref(false);
  let currentUtterance: SpeechSynthesisUtterance | null = null;

  const stop = () => {
    if (supported && currentUtterance) {
      currentUtterance.onend = null;
      currentUtterance.onerror = null;
      window.speechSynthesis.cancel();
      currentUtterance = null;
    }
    speaking.value = false;
  };

  const toggle = () => {
    if (speaking.value) return stop();
    if (!supported || !contentRef.value) return;
    
    window.speechSynthesis.cancel(); // Clear queue
    const utt = new SpeechSynthesisUtterance(contentRef.value);
    utt.onend = () => { speaking.value = false; currentUtterance = null; };
    utt.onerror = () => { 
      speaking.value = false; 
      currentUtterance = null; 
      // Optionally handle error logging here
    };
    currentUtterance = utt;
    speaking.value = true;
    window.speechSynthesis.speak(utt);
  };

  onBeforeUnmount(stop);

  return { supported, speaking, toggle };
}

B. Improve canRegenerate Logic

Ensure messageList is defined before accessing its properties to avoid runtime errors if props.message is passed without context. Although guarded by &&, TypeScript might complain.

C. Accessibility (a11y)

  • Add aria-label to buttons if tooltips are not sufficient for screen readers.
  • Ensure aria-pressed="true" on the speak button when speaking is true.

Final Verdict

The code is structurally sound and follows Vue 3 Composition API patterns. The main improvements lie in extracting speech logic into a composable, adding guards for SSR/environment checks in cleanup, and providing better error feedback for TTS failures.

Expand Down
21 changes: 20 additions & 1 deletion ui/src/conversation-panel/components/message-list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ export function createMessageListStore(deps: MessageListDeps) {
}
const target = opts.newQuestion
? messages.value[messages.value.length - 1]
: messages.value.find((message) => message.id === `${opts.chatRecordId}_ASSISTANT`) || messages.value[messages.value.length - 1]
: messages.value.find(
(message) => (!!opts.chatRecordId && message.recordId === opts.chatRecordId) || message.id === `${opts.chatRecordId}_ASSISTANT`,
) || messages.value[messages.value.length - 1]
if (!target) return

const payload: Dict<unknown> = { message: opts.message, stream: true, re_chat: opts.reChat ?? false }
Expand All @@ -178,10 +180,17 @@ export function createMessageListStore(deps: MessageListDeps) {
if (opts.chatRecordId) payload.chat_record_id = opts.chatRecordId
if (opts.chunkId) payload.chunk_id = opts.chunkId

let replacementStarted = false
runStream(
() => chatMessage(cid, payload),
(chunk) => {
if (chunk?.chat_record_id && !target.recordId) target.recordId = chunk.chat_record_id
if (opts.reChat && !replacementStarted) {
target.content = []
target.record = undefined
target.write_ed = false
replacementStarted = true
}
appendChunk(target, chunk)
opts.onNext?.(chunk)
},
Expand All @@ -193,6 +202,15 @@ export function createMessageListStore(deps: MessageListDeps) {
)
}

// 原回答保留到重新生成收到首个分片,避免请求失败时丢失已展示内容。
const regenerate = (message: ChatMessage) => {
if (streamLoading.value || loading.value || !message.recordId) return
const index = messages.value.indexOf(message)
const question = messages.value[index - 1]
if (index < 1 || question?.role !== 'USER' || !question.content[0]) return
sendMessage({ message: question.content[0], newQuestion: false, reChat: true, chatRecordId: message.recordId })
}

// 优雅停止:仅通知后端取消,本地流继续读到 [DONE] 自行结束
const stop = () => {
const cid = getChatId()
Expand Down Expand Up @@ -245,6 +263,7 @@ export function createMessageListStore(deps: MessageListDeps) {
load,
loadMore,
sendMessage,
regenerate,
uploadFile,
onStreamProgress,
stop,

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.

The patch is syntactically valid, but there are several correctness/data-loss risks and some cleanup suggestions.

Main issues

  1. reChat can clear the wrong message

    const target = opts.newQuestion
      ? messages.value[messages.value.length - 1]
      : messages.value.find(...) || messages.value[messages.value.length - 1]

    For reChat, if the record cannot be found, this falls back to the last message. The first stream chunk then clears that message’s content. That is dangerous.

    • For opts.reChat, do not fall back to the last message; return if no matching target is found.
  2. Target lookup may match a non-assistant message

    (!!opts.chatRecordId && message.recordId === opts.chatRecordId) ||
    message.id === `${opts.chatRecordId}_ASSISTANT`

    The recordId branch does not verify message.role === 'ASSISTANT'. If any non-assistant message has the same recordId, you could clear the wrong message.

    • Add message.role === 'ASSISTANT'.
    • Also guard the legacy-ID branch when opts.chatRecordId is missing, to avoid matching "undefined_ASSISTANT".
  3. regenerate does not verify the target is an assistant message

    if (streamLoading.value || loading.value || !message.recordId) return
    const index = messages.value.indexOf(message)
    const question = messages.value[index - 1]
    if (index < 1 || question?.role !== 'USER' || !question.content[0]) return

    If regenerate is called on a USER message with a recordId, and the previous message is also USER, this can pass and send the wrong request.

    • Add if (message.role !== 'ASSISTANT') return.
    • Use question?.content?.[0] to avoid a runtime error if content is missing.
  4. recordId may not be updated on reChat

    if (chunk?.chat_record_id && !target.recordId) target.recordId = chunk.chat_record_id

    During a reChat, target.recordId is usually already set, so a new chat_record_id from the backend will be ignored. If reChat can return a new record ID, this leaves a stale ID.

    • Decide explicitly whether reChat should update it, e.g.:
      if (chunk?.chat_record_id && (!target.recordId || opts.reChat)) {
        target.recordId = chunk.chat_record_id
      }
      — but only if that matches the backend semantics.
  5. Old content is cleared on the first chunk, even if that chunk has no content

    if (opts.reChat && !replacementStarted) {
      target.content = []
      target.record = undefined
      target.write_ed = false
      replacementStarted = true
    }

    If the first chunk is only metadata, a ping, an error, or an empty delta, the old answer is cleared before any replacement content arrives.

    • Clear only when the first actual content-bearing chunk arrives.
    • Consider preserving the old content and restoring it if the stream fails before meaningful content is received.
  6. State reset is incomplete
    You reset content, record, and write_ed, but other streaming/error state may remain stale, e.g. error, status, finished, references, etc. Audit the ChatMessage shape and reset all fields that can affect rendering.

  7. regenerate ignores the result of sendMessage
    If sendMessage returns a Promise, callers of regenerate cannot await it or catch errors.

    • Return it:
      return sendMessage(...)
    • Or make regenerate async and await it.
  8. indexOf(message) relies on object identity
    If the caller ever passes a cloned or reconstructed message object, indexOf returns -1. If that is possible, use an ID-based lookup, e.g. findIndex(m => m.id === message.id).

  9. Potential retry/duplicate issue
    replacementStarted is local to one sendMessage call. If runStream internally retries after a partial response, the content will not be cleared again and chunks may be appended to a partial answer. Check whether runStream supports retries.

Minor suggestions

  • Simplify and guard the target lookup:
    const assistantId = opts.chatRecordId
      ? `${opts.chatRecordId}_ASSISTANT`
      : undefined
    
    const matched = opts.chatRecordId
      ? messages.value.find(
          (m) =>
            m.role === 'ASSISTANT' &&
            (m.recordId === opts.chatRecordId || m.id === assistantId),
        )
      : undefined
    
    const target = opts.newQuestion
      ? messages.value[messages.value.length - 1]
      : matched ?? (!opts.reChat ? messages.value[messages.value.length - 1] : undefined)
  • If recordId is not guaranteed unique, pass message.id or the index to sendMessage so the exact message can be targeted.
  • Consider naming it regenerateMessage for clarity if the store exports many actions.

Overall: the main fixes should be to prevent reChat from falling back to the last message, ensure the target is an assistant message, and clear old content only when actual replacement content arrives.

Expand Down
3 changes: 2 additions & 1 deletion ui/src/conversation-panel/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@

/* 对话正文:消息列表与底部输入区共用宽度和居中布局。 */
.mk-conversation-content {
box-sizing: border-box;
margin-inline: auto;
max-width: 800px;
max-width: calc(800px + var(--spacing) * 8);
width: 100%;
}

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.

The diff is syntactically valid CSS, but there are a few things to verify:

  1. --spacing must be defined and must resolve to a length

    • If --spacing is undefined, calc(800px + var(--spacing) * 8) becomes invalid at computed-value time.
    • In that case, max-width falls back to none, and because width: 100% is set, the element can expand to the full parent width. That is a likely layout regression.
    • Safer:
      max-width: calc(800px + (var(--spacing, 0px) * 8));
  2. box-sizing: border-box changes the width calculation

    • Before, max-width: 800px likely behaved as content-box unless a global reset already set border-box.
    • With border-box, padding and borders are included in the 800px. Increasing the max-width may be compensating for that, but only if the horizontal padding/border is exactly var(--spacing) * 8.
    • If the goal is to preserve an 800px content area, it is clearer to account for actual padding/border:
      max-width: calc(800px + var(--padding-inline) * 2);
      or keep a fixed max-width and let border-box include padding.
  3. Check whether --spacing is a length, not a unitless number

    • This is valid if --spacing is something like 4px or 0.25rem.
    • This is invalid if --spacing is 4 with no unit.
    • Also consider whether using a relative unit such as rem is intentional, since the max width will then change with font size.
  4. Browser support for calc() multiplication

    • Modern browsers support calc(var(--spacing) * 8).
    • If you need to support very old browsers, precompute the value or avoid multiplication in calc().
  5. Readability / line length

    • max-width controls line length. Increasing it from 800px to 800px + spacing * 8 may reduce readability if this element contains long-form conversation text.

Suggested safer version, assuming --spacing is a length:

.mk-conversation-content {
  box-sizing: border-box;
  margin-inline: auto;
  max-width: calc(800px + (var(--spacing, 0px) * 8));
  width: 100%;
}

If this width is reused elsewhere, consider storing it in a custom property:

.mk-conversation-content {
  --conversation-max-width: calc(800px + (var(--spacing, 0px) * 8));

  box-sizing: border-box;
  margin-inline: auto;
  max-width: var(--conversation-max-width);
  width: 100%;
}

Main action items: verify the --spacing token’s scope/type, confirm the border-box + increased max-width combination is intentional, and add a fallback to avoid an unintended full-width layout.

Expand Down
3 changes: 2 additions & 1 deletion ui/src/styles/STYLE_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ Element Plus 覆盖、主题变量和应用样式。Chat 入口还显式引入 E
也不额外设置 `min-height: 0`、`height: 100%` 或 `max-height: 100%`,消息列表负责内部滚动。

消息列表与底部输入区可共用 `mk-conversation-content`,在对话模块公共样式中统一设置
宽度 100%、最大宽度 800px 和水平居中。
`border-box`、宽度 100% 和水平居中。调用方配合 `p-4`,最大外宽为
`800px + var(--spacing) * 8`,使正文最大宽度保持 800px,窄容器下连同左右内边距一起收缩。

用户提问容器使用 `mk-conversation-question-content`,在对话模块公共样式中控制靠右、
随内容收缩及最大 85% 宽度。文字气泡使用 `mk-gray-card-lg`,换行规则在模板中组合工具类。
Expand Down
7 changes: 7 additions & 0 deletions ui/src/views/VIEW_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,13 @@ System 资源管理的修正内容与添加知识库选项仅包含当前智能
门户复用 Chat 分享 Store,抽屉点击分享后关闭并进入消息选择模式。
Chat/Portal 视图将左栏布局模式绑定到会话列表 Store;开启分享时,抽屉模式自动收起左栏。

### 对话消息操作

消息 Footer 在复制后提供重新生成和朗读内容入口。列表 Store 的 `regenerate(message)` 沿用
原问题与记录 ID,生成中及历史加载中禁用;收到首个分片后替换原回答,请求失败前保留原内容。
朗读使用浏览器语音合成,仅收集回答正文,不包含思考与工具参数;再次点击停止,卸载时清理语音。
不支持语音合成的浏览器禁用朗读按钮。

### 对话消息反馈

`conversation-panel/components/message-footer/feedback/` 内的 `ButtonFeedback.vue` 和 `FeedbackCard.vue`
Expand Down
Loading