Skip to content

fix: reasoning style #7843

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
wangdan-fit2cloud merged 1 commit into from
Nov 11, 2026
Merged

fix: reasoning style #7843

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
2 changes: 1 addition & 1 deletion ui/src/conversation-panel/components/content/items/group/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const currentActivity = computed(() => {
<template #label>
<span v-if="content.status === 'RUNNING'" class="mk-conversation-loading" />
<span class="min-w-0 max-w-[300px] truncate" :title="content.name">{{ content.name }}</span>
<span v-if="!isExpanded && currentActivity" class="min-w-0 truncate" :title="currentActivity">{{ currentActivity }}</span>
<!-- <span v-if="!isExpanded && currentActivity" class="min-w-0 truncate" :title="currentActivity">{{ currentActivity }}</span> -->
<el-tag v-if="isFailed" size="small" :type="statusType">{{ statusText }}</el-tag>
</template>

@shaohuzhang1 shaohuzhang1 Nov 11, 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.

Observations & Suggestions:

  1. Commented-out code should be removed, not left in. Dead code (the commented <span>) clutters the file and will drift out of sync. If it's needed later, rely on Git history. Delete it.

  2. Potential unused variable/refs after removal. With that <span> gone, currentActivity (computed at line ~78) may no longer be referenced anywhere. If so:

    • Remove the currentActivity computed property to avoid dead code and unnecessary re-computation.
    • Also check whether imports/helpers feeding currentActivity are now unused (linters like ESLint no-unused-vars should flag these).

  3. Behavioral change — intentional? This removes the inline "current activity" text from collapsed tabs. If this is a temporary debug toggle, use a feature flag / v-if="false" with a TODO comment rather than raw HTML comments, or better, gate via a config constant.

  4. Duplicate :title concern (if restored). Both the name span and activity span set :title. When overlayed/clipped they can conflict for tooltip UX — fine as-is but worth noting.

  5. Accessibility: The loading indicator <span class="mk-conversation-loading" /> is decorative; consider aria-hidden="true" plus an aria-label or visually-hidden text ("Running") for screen readers.

  6. Whitespace/formatting: No other irregularities in the diff itself; indentation matches surrounding code.

Recommended action: Delete the commented span entirely; if currentActivity becomes orphaned, remove it and its dependencies; confirm the activity-text removal is intended product behavior.

Expand Down
7 changes: 3 additions & 4 deletions ui/src/workflow-canvas/ai-generate/panel/activity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ export function createActivity() {
}
last.text += event.delta
emit(scopeKey, { type, id: last.id, content: event.delta, ...(type === 'REASONING' ? { status: 'RUNNING' } : {}) })
lastActivity.value = tail(last.text)
// lastActivity.value = tail(last.text)
lastActivity.value = event.type === 'reasoning' ? tail(last.text) : ''
} else if (event.type === 'tool_start') {
settle(scopeKey)
const delegate = DELEGATE_TOOLS.has(event.name)
Expand All @@ -206,9 +207,7 @@ export function createActivity() {
const name = toolTitle(toolCall)
emit(
toolCall.scopeKey,
toolCall.delegate
? { type: 'GROUP', id: toolCall.contentId, name }
: { type: 'TOOL', id: toolCall.contentId, name, arguments: event.delta },
toolCall.delegate ? { type: 'GROUP', id: toolCall.contentId, name } : { type: 'TOOL', id: toolCall.contentId, name, arguments: event.delta },
)
} else if (event.type === 'tool_end') {
const toolCall = toolCallById.get(event.id)

@shaohuzhang1 shaohuzhang1 Nov 11, 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.

Irregularities / Issues

  1. Potential bug — inconsistent event.type casing. The field is written as event.type === 'reasoning' (lowercase) here, but in the surrounding code the emit uses type === 'REASONING' (uppercase). If event.type is actually the uppercase 'REASONING', this condition will never match, so lastActivity.value will always be ''. Verify the canonical values and make casing consistent.

  2. Reasoning output suppressed for non-reasoning events. Clearing lastActivity.value to '' on any non-reasoning delta discards prior tail state. Confirm this is intentional (e.g., resetting the "current activity" preview). If you only want to skip updating for non-reasoning events, use an early return instead of assigning ''.

  3. Behavioral regression risk. Previously lastActivity.value = tail(last.text) tracked all text; now it only tracks reasoning. Any consumer relying on lastActivity for assistant message streaming will break.

  4. Commented-out code left in place. The // lastActivity.value = tail(last.text) line should be removed rather than commented, to avoid dead-code confusion.

Minor / Style

  1. Ternary duplication. event.type === 'reasoning' ? tail(last.text) : '' — if the guard above is already if (/* reasoning */), this is redundant. Prefer restructuring:

    lastActivity.value = type === 'REASONING' ? tail(last.text) : ''

    using the already-computed type.

  2. Formatting change in the emit(...) call is purely cosmetic (collapsed ternary onto one line). No functional issue, but very long lines may violate lint/prettier config — run the formatter.

  3. Parameter name shadowing. In the emit object, arguments: event.delta — arguments is a reserved-ish identifier in classic functions; fine as an object key, but worth confirming the downstream schema expects that exact name.

Optimization suggestions

  • Avoid recomputing tail(last.text) multiple times; compute once if reused.
  • Consider deriving isReasoning from the normalized type variable (already available) to prevent divergence between type and event.type checks.
  • Drop the stale commented line and rely on version control history.

Action items

  • Confirm whether event.type is 'reasoning' or 'REASONING'.
  • Decide intended semantics of clearing vs. skipping update.
  • Remove commented code; re-run formatter/linter.

Expand Down
Loading