Skip to content

perf: improve tool calls in reasoning and multiple tool calls display#7742

Merged
Soulter merged 3 commits intomasterfrom
perf/tool-call
Apr 23, 2026
Merged

perf: improve tool calls in reasoning and multiple tool calls display#7742
Soulter merged 3 commits intomasterfrom
perf/tool-call

Conversation

@Soulter
Copy link
Copy Markdown
Member

@Soulter Soulter commented Apr 23, 2026

  • Updated LiveChatRoute and OpenApiRoute to replace manual message accumulation with BotMessageAccumulator.
  • Simplified message saving logic by using build_bot_history_content and collect_plain_text_from_message_parts.
  • Enhanced message processing to handle various message types (plain, image, record, file, video) more efficiently.
  • Improved reasoning handling by extracting thinking parts and displaying them correctly in the UI components.
  • Refactored message normalization and reasoning extraction logic in useMessages composable for better clarity and maintainability.
  • Updated ChatMessageList, MessageList, StandaloneChat, and ReasoningBlock components to accommodate new message structure and rendering logic.

Modifications / 改动点

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果


Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Unify bot message accumulation and reasoning handling across chat backends and UI to better support streaming reasoning content and structured tool call parts.

Bug Fixes:

  • Prevent duplicate handling of llm_result reasoning chains in the streaming pipeline and skip inappropriate forwarding of reasoning-only results in run_agent.

Enhancements:

  • Introduce a BotMessageAccumulator to consolidate streaming text, tools, and attachments into structured bot message parts for chat, live chat, and OpenAPI routes.
  • Normalize legacy reasoning fields into explicit 'think' message parts and derive plain text/reasoning text from parts when saving and rendering history.
  • Refine tool call emission in the tool loop agent runner to avoid duplicate or missing tool call result events and to stream reasoning chunks earlier and more consistently.
  • Centralize message part normalization and reasoning extraction utilities in useMessages and reuse them across ThreadPanel, MessageList, ChatMessageList, and StandaloneChat.
  • Enhance the ReasoningBlock component to render structured reasoning parts, including inline tool call details (e.g., Python/IPython), and improve preview behavior and transitions.

- Updated LiveChatRoute and OpenApiRoute to replace manual message accumulation with BotMessageAccumulator.
- Simplified message saving logic by using build_bot_history_content and collect_plain_text_from_message_parts.
- Enhanced message processing to handle various message types (plain, image, record, file, video) more efficiently.
- Improved reasoning handling by extracting thinking parts and displaying them correctly in the UI components.
- Refactored message normalization and reasoning extraction logic in useMessages composable for better clarity and maintainability.
- Updated ChatMessageList, MessageList, StandaloneChat, and ReasoningBlock components to accommodate new message structure and rendering logic.
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. area:webui The bug / feature is about webui(dashboard) of astrbot. labels Apr 23, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 3 issues, and left some high level feedback:

  • In BotMessageAccumulator.add_plain, the streaming flag no longer affects behavior (both branches just append to pending_text); consider either simplifying the method by removing the parameter or reintroducing distinct behavior so callers aren’t misled.
  • In ReasoningBlock.vue you reimplemented parseJsonSafe and tool-call normalization logic that also exists in useMessages; consider reusing the shared helpers to avoid duplicated logic that may drift over time (e.g., by importing parseJsonSafe and/or a common normalizeToolCall).
  • In tool_loop_agent_runner.step, llm_result with chain.type == 'reasoning' is now yielded once before the tools-call handling and again inside the skills_like fallback when no tool calls are returned, which can duplicate reasoning messages for that path; consider consolidating to a single emission.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `BotMessageAccumulator.add_plain`, the `streaming` flag no longer affects behavior (both branches just append to `pending_text`); consider either simplifying the method by removing the parameter or reintroducing distinct behavior so callers aren’t misled.
- In `ReasoningBlock.vue` you reimplemented `parseJsonSafe` and tool-call normalization logic that also exists in `useMessages`; consider reusing the shared helpers to avoid duplicated logic that may drift over time (e.g., by importing `parseJsonSafe` and/or a common `normalizeToolCall`).
- In `tool_loop_agent_runner.step`, `llm_result` with `chain.type == 'reasoning'` is now yielded once before the tools-call handling and again inside the `skills_like` fallback when no tool calls are returned, which can duplicate reasoning messages for that path; consider consolidating to a single emission.

## Individual Comments

### Comment 1
<location path="astrbot/dashboard/routes/chat.py" line_range="147-155" />
<code_context>
+            self._append_think_part(result_text)
+            return
+
+        if streaming:
+            self.pending_text += result_text
+        else:
+            self.pending_text += result_text
</code_context>
<issue_to_address>
**suggestion:** The `streaming` flag in `add_plain` is currently redundant and could be simplified or used meaningfully.

In `BotMessageAccumulator.add_plain`, the `streaming` conditional is redundant since both branches do `self.pending_text += result_text`. Either make the non-streaming path behave differently (e.g., overwrite instead of append, as before) or remove the conditional. If non-streaming can receive multiple `plain` chunks, clearly define whether they should be concatenated or replaced.

```suggestion
        if chain_type == "reasoning":
            self._flush_pending_text()
            self._append_think_part(result_text)
            return

        # For streaming, accumulate partial chunks; for non-streaming, treat the latest
        # plain text as the authoritative value and overwrite any previous pending text.
        if streaming:
            self.pending_text += result_text
        else:
            self.pending_text = result_text
```
</issue_to_address>

### Comment 2
<location path="dashboard/src/components/chat/message_list_comps/ReasoningBlock.vue" line_range="208-213" />
<code_context>
-  return String(value);
-}
-
-function parseJsonSafe(value: unknown) {
-  if (typeof value !== "string") return value;
-  try {
-    return JSON.parse(value);
-  } catch {
-    return value;
-  }
-}
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating `parseJsonSafe` logic that already exists in `useMessages`.

This component reimplements `parseJsonSafe`, but the same helper is already exported from `@/composables/useMessages`. Please import and reuse that shared utility to keep parsing behavior consistent across components and avoid future divergence when handling tool call payloads.

Suggested implementation:

```

```

To fully implement the suggestion, you should also:

1. Update the imports at the top of `ReasoningBlock.vue` to use the shared helper from `@/composables/useMessages`. For example, if `useMessages` is already imported:
   - Change something like:
   ```ts
   import { useMessages } from "@/composables/useMessages";
   ```
   to:
   ```ts
   import { useMessages, parseJsonSafe } from "@/composables/useMessages";
   ```
   Or, if `parseJsonSafe` is a named export in a different way, adjust accordingly.

2. Ensure all existing usages of `parseJsonSafe` in this component remain unchanged (they will now reference the imported function instead of the removed local implementation).

3. If `useMessages` is not yet imported in this file, add a new import line:
   ```ts
   import { parseJsonSafe } from "@/composables/useMessages";
   ```
</issue_to_address>

### Comment 3
<location path="dashboard/src/composables/useMessages.ts" line_range="767" />
<code_context>
   };
 }

+export function normalizeMessageParts(
+  parts: unknown,
+  legacyReasoning = "",
</code_context>
<issue_to_address>
**issue (complexity):** Consider tightening the normalization boundary so all message processing uses a single canonical shape and shared helpers for reasoning vs display slices.

You can reduce the new complexity around normalization and thinking/display partitioning without changing behavior by tightening the “canonical shape” boundary and reusing it consistently.

### 1. Collapse normalization into a single canonical pathway

Right now you have:

- `normalizePartsInternal(parts)`
- `normalizeMessageParts(parts, legacyReasoning)`
- `extractReasoningText(parts | unknown, legacyReasoning)`

`extractReasoningText` can be simplified if everything it touches is already canonical. You can make `normalizeMessageParts` your one public normalizer, and keep `normalizePartsInternal` private:

```ts
// keep this file-local (non-exported)
function normalizePartsInternal(parts: unknown): MessagePart[] {
  if (typeof parts === "string") {
    return parts ? [{ type: "plain", text: parts }] : [];
  }
  if (!Array.isArray(parts)) return [];
  return parts.map((part: any) => {
    if (!part || typeof part !== "object") {
      return { type: "plain", text: String(part ?? "") };
    }
    if (part.type === "reasoning") {
      return {
        ...part,
        type: "think",
        think: String(part.think ?? part.text ?? ""),
      };
    }
    return { ...part };
  });
}

export function normalizeMessageParts(
  parts: unknown,
  legacyReasoning = "",
): MessagePart[] {
  const normalizedParts = normalizePartsInternal(parts);
  if (legacyReasoning && !normalizedParts.some((p) => p.type === "think")) {
    normalizedParts.unshift({ type: "think", think: legacyReasoning });
  }
  return normalizedParts;
}

// assumes canonical parts – no `unknown` / no legacy reasoning here
export function extractReasoningText(parts: MessagePart[]): string {
  const text = parts
    .filter((part) => part.type === "think")
    .map((part) => String(part.think || ""))
    .join("");
  return text;
}
```

Callers that currently pass `unknown` to `extractReasoningText` can normalize once upfront:

```ts
const normalizedMessage = normalizeMessageParts(
  content.message || [],
  content.reasoning || "",
);
const normalizedContent: ChatContent = {
  type: content.type || (record.sender_id === "bot" ? "bot" : "user"),
  message: normalizedMessage,
  reasoning: extractReasoningText(normalizedMessage) || (content.reasoning || ""),
  // ...
};
```

This keeps all behavior but makes the call sites clearly responsible for the one-time normalization.

### 2. Centralize thinking vs display slicing

`thinkingParts` and `displayParts` both:

- Normalize `content.message`/`content.reasoning`.
- Find the first non-empty part.
- Then diverge on how to slice.

You can factor the shared traversal into a helper and use it in both:

```ts
function ensureCanonicalParts(content: ChatContent): MessagePart[] {
  return Array.isArray(content.message)
    ? content.message
    : normalizeMessageParts(content.message, content.reasoning || "");
}

function splitThinkingAndDisplay(parts: MessagePart[]): {
  thinking: MessagePart[];
  display: MessagePart[];
} {
  const start = firstNonEmptyPartIndex(parts);
  if (start < 0) return { thinking: [], display: [] };

  let index = start;
  while (index < parts.length && isThinkingPart(parts[index])) {
    index += 1;
  }

  const thinking = parts.slice(start, index);
  const display = parts.slice(index).filter((part) => !isEmptyPlainPart(part));
  return { thinking, display };
}

export function thinkingParts(content: ChatContent): MessagePart[] {
  const parts = ensureCanonicalParts(content);
  const { thinking } = splitThinkingAndDisplay(parts);

  if (thinking.length > 0) return thinking;
  const fallbackReasoning = String(content.reasoning || "");
  return fallbackReasoning ? [{ type: "think", think: fallbackReasoning }] : [];
}

export function displayParts(content: ChatContent): MessagePart[] {
  const parts = ensureCanonicalParts(content);
  return splitThinkingAndDisplay(parts).display;
}
```

This keeps your current behavior (including the fallback to `content.reasoning`) but removes duplicated index/slice logic and makes the “thinking vs display” rules inspectable in one place.

### 3. Treat `content.reasoning` as derived inside mutations

`appendReasoningPart` updates both `message` and `reasoning`, and calls `extractReasoningText` which currently re-normalizes. Once `extractReasoningText` assumes canonical parts, you can avoid re-normalization and clarify that `reasoning` is always derived from `message`:

```ts
export function appendReasoningPart(record: ChatRecord, text: string) {
  markMessageStarted(record);
  if (!text) return;

  const content = record.content;
  const last = content.message[content.message.length - 1];

  if (last?.type === "think") {
    last.think = `${String(last.think || "")}${text}`;
  } else {
    content.message.push({ type: "think", think: text });
  }

  // message is already canonical here
  content.reasoning = extractReasoningText(content.message);
}
```

If you later centralize `reasoning` computation (e.g. just before persistence/UI), this function can simply stop writing `content.reasoning` altogether, but for now this change alone removes one normalization hop.

These tweaks keep all features and behaviours intact while tightening the normalization boundary and reducing branching/duplication.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/dashboard/routes/chat.py Outdated
Comment thread dashboard/src/components/chat/message_list_comps/ReasoningBlock.vue Outdated
Comment thread dashboard/src/composables/useMessages.ts
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the message handling logic to support a structured 'message parts' format, specifically improving the management and rendering of reasoning (thinking) content and tool calls. It introduces the BotMessageAccumulator class to centralize message assembly on the backend and updates several frontend components to handle the new data structure. A high-severity issue was identified in the BotMessageAccumulator.add_plain method, where non-streaming text is incorrectly appended rather than replaced, which could lead to duplicated content in non-streaming responses.

Comment thread astrbot/dashboard/routes/chat.py Outdated
Soulter and others added 2 commits April 23, 2026 17:41
- Introduced a new ReasoningSidebar component for displaying reasoning details.
- Refactored MessageList and StandaloneChat components to utilize renderBlocks for improved message part handling.
- Added ReasoningTimeline component to visualize reasoning steps.
- Updated message handling logic to differentiate between thinking and content blocks.
- Enhanced localization for reasoning-related terms in English, Russian, and Chinese.
- Improved styling for various components to ensure consistency and readability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@Soulter Soulter merged commit bb6619f into master Apr 23, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. area:webui The bug / feature is about webui(dashboard) of astrbot. size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant