feat(chat): collapse turn activity#1727
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (27)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughThis PR implements automatic activity collapse for assistant message rendering: adds ChangesAutomatic Turn Activity Collapse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/renderer/src/components/message/messageActivityGroups.ts (1)
157-187: 💤 Low valueConsider i18n for duration unit strings.
The duration formatting hardcodes unit strings ("天", "小时", "分钟", "秒", "d", "h", "m", "s") rather than sourcing them from i18n files. While duration abbreviations are commonly hardcoded as standard symbols, the coding guideline states all user-facing strings should use vue-i18n keys. Consider extracting these to i18n files for full consistency, though this is lower priority since abbreviated units rarely vary within a locale.
As per coding guidelines: "All user-facing strings must use vue-i18n keys from
src/renderer/src/i18ninstead of hardcoded text".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/message/messageActivityGroups.ts` around lines 157 - 187, The formatActivityDuration function currently hardcodes unit strings for Chinese and non-Chinese locales; replace those literals with vue-i18n keys by using the i18n translation function (e.g. useI18n().t or a provided t) inside formatActivityDuration so each unit ("day", "hour", "minute", "second" and their short forms) is looked up from src/renderer/src/i18n; update the parts arrays in formatActivityDuration to call t('duration.day_short') / t('duration.day_cn') etc. (keep the existing logic for isChineseLocale and zero-second fallback) and ensure the translation keys are added to the i18n resource files.src/renderer/src/components/message/MessageBlockActivityGroup.vue (1)
21-37: 💤 Low valueConsider defensive validation for unrecognized block types.
The template silently skips blocks that don't match
reasoning_content,artifact-thinking(with content), ortool_call. While this may be intentional filtering, consider adding a development-mode warning if an unexpected block type is encountered to catch upstream data issues earlier.🛡️ Optional development guard
+import { isDevelopment } from '`@/lib/env`' + const buildActivityBlockKey = (block: DisplayAssistantMessageBlock, index: number): string => block.id ?? block.tool_call?.id ?? `${block.type}:${block.timestamp}:${index}` + +const isRecognizedBlockType = (block: DisplayAssistantMessageBlock): boolean => { + if (block.type === 'tool_call') return true + if ((block.type === 'reasoning_content' || block.type === 'artifact-thinking') && block.content) return true + if (isDevelopment) { + console.warn('[MessageBlockActivityGroup] Unrecognized or empty block:', block) + } + return false +}Then in template:
- <template v-for="(block, index) in blocks" :key="buildActivityBlockKey(block, index)"> + <template v-for="(block, index) in blocks" :key="buildActivityBlockKey(block, index)" v-if="isRecognizedBlockType(block)">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/message/MessageBlockActivityGroup.vue` around lines 21 - 37, Add a development-mode warning for unrecognized block types in the template rendering loop over blocks in MessageBlockActivityGroup.vue. After the existing v-if and v-else-if for known block types, add a v-else block that triggers a console.warn during development to log any unexpected block types. This will help catch upstream data issues early without affecting production behavior. Ensure this check references the iterated variable 'block.type' within the template's v-for scope.test/renderer/components/message/MessageItemAssistant.test.ts (1)
263-277: ⚡ Quick winConsider expanding test coverage for additional grouping scenarios.
The test correctly verifies the primary grouping behavior when both gating conditions are met. However, the test suite could benefit from covering additional edge cases:
status='sent'+isInGeneratingThread=true→ should NOT group (generating thread suppresses grouping)status='pending'+isInGeneratingThread=false→ should NOT group (pending status suppresses grouping)- Internal tool call filtering (tool with
name='update_plan'andextra.internalTool=trueshould be excluded from the group, per the context snippet)These additional test cases would provide stronger confidence that the grouping logic handles all combinations correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/renderer/components/message/MessageItemAssistant.test.ts` around lines 263 - 277, Add three unit tests for MessageItemAssistant to cover edge cases: (1) mount with message status 'sent' and isInGeneratingThread=true and assert no activity-group exists and individual blocks (MessageBlockThink/MessageBlockToolCall) are present; (2) mount with status 'pending' and isInGeneratingThread=false and assert no grouping and blocks are present; (3) mount with a tool call block whose tool has name 'update_plan' and extra.internalTool=true and assert that the internal tool call is excluded from the activity-group (i.e., group count reflects exclusion and the MessageBlockToolCall for that internal tool is not rendered in the group). Use the same helpers (createMessage, createThinkingBlock, createToolCallBlock) and component name MessageItemAssistant to create the test props and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/components/message/MessageBlockActivityGroup.vue`:
- Line 29: The forwarded `toggle-collapse` event is ambiguous; update
MessageBlockActivityGroup to include metadata so the parent can distinguish
source and state: change handleChildCollapseToggle to re-emit a payload like {
source: 'child-block', isCollapsed: boolean } (using the child's boolean), and
change the activity-group's own toggle emitter (the handler that flips
isExpanded) to emit { source: 'activity-group', isCollapsed: !isExpanded };
ensure MessageBlockThink events are treated as child-origin when re-emitting
from handleChildCollapseToggle and remove blind forwarding so every emitted
toggle-collapse includes the source and isCollapsed fields.
In `@src/renderer/src/i18n/da-DK/chat.json`:
- Around line 408-413: The strings in the activityCollapse section are currently
in English and must be translated into Danish. Replace the values for workedFor,
reasoningCount, toolCallCount, expandLabel, and collapseLabel with their
appropriate Danish translations, such as "Arbejdet i {duration}" for workedFor
and "Udvid {title}" for expandLabel, to ensure the section has correct Danish
text.
In `@src/renderer/src/i18n/de-DE/chat.json`:
- Around line 408-413: The activityCollapse translations are still in English;
update the values for the keys workedFor, reasoningCount, toolCallCount,
expandLabel, and collapseLabel in the activityCollapse object to proper German
strings while preserving placeholders (e.g., workedFor -> "Gearbeitet für
{duration}", reasoningCount -> "{count} Gedanke(n)", toolCallCount -> "{count}
Werkzeugaufruf(e)", expandLabel -> "Erweitern {title}", collapseLabel ->
"Einklappen {title}"); keep the exact placeholder names ({duration}, {count},
{title}) intact so existing interpolation in the code using
activityCollapse.workedFor, activityCollapse.reasoningCount,
activityCollapse.toolCallCount, activityCollapse.expandLabel, and
activityCollapse.collapseLabel continues to work.
In `@src/renderer/src/i18n/es-ES/chat.json`:
- Around line 408-413: The activityCollapse strings in the Spanish chat
translation file are still in English; update the entries in the
activityCollapse object to proper Spanish while preserving the existing
placeholders. Specifically, replace the text for workedFor, reasoningCount,
toolCallCount, expandLabel, and collapseLabel with Spanish equivalents, and keep
the same keys and interpolation tokens used by the i18n consumers.
In `@src/renderer/src/i18n/fa-IR/chat.json`:
- Around line 408-413: The `activityCollapse` section currently contains English
strings and requires proper Persian translations to support RTL text direction.
Replace the English text for the keys `workedFor`, `reasoningCount`,
`toolCallCount`, `expandLabel`, and `collapseLabel` with accurate Persian
translations, ensuring the placeholders `{duration}`, `{count}`, and `{title}`
remain intact for dynamic content substitution.
In `@src/renderer/src/i18n/fr-FR/chat.json`:
- Around line 408-413: The activityCollapse translations are still in English;
replace the English strings for the keys activityCollapse.workedFor,
activityCollapse.reasoningCount, activityCollapse.toolCallCount,
activityCollapse.expandLabel and activityCollapse.collapseLabel with proper
French equivalents (for example: "A travaillé pendant {duration}" for workedFor,
"{count} réflexion(s)" for reasoningCount, "{count} appel(s) d'outil)" for
toolCallCount, "Développer {title}" for expandLabel and "Réduire {title}" for
collapseLabel), ensuring placeholders {duration}, {count}, and {title} are
preserved exactly.
In `@src/renderer/src/i18n/he-IL/chat.json`:
- Around line 408-413: The activityCollapse strings in the he-IL chat.json
locale are still in English and need Hebrew translations. Update the entries for
workedFor, reasoningCount, toolCallCount, expandLabel, and collapseLabel in the
activityCollapse object to proper Hebrew, and ensure the translated text follows
RTL-friendly phrasing consistent with the rest of the he-IL locale file.
In `@src/renderer/src/i18n/ru-RU/chat.json`:
- Around line 408-414: The activityCollapse entries in the ru-RU locale still
contain English strings; update the "activityCollapse" object by replacing the
values for the keys workedFor, reasoningCount, toolCallCount, expandLabel, and
collapseLabel with proper Russian translations, preserving placeholders
({duration}, {count}, {title}) and implementing correct pluralization for
reasoningCount and toolCallCount (use Russian plural forms or the i18n plural
syntax your project uses) so UI shows fully translated labels.
In `@src/renderer/src/i18n/tr-TR/chat.json`:
- Around line 408-414: Replace the English strings under activityCollapse with
Turkish translations: set activityCollapse.workedFor to "{duration} boyunca
çalıştı", activityCollapse.reasoningCount to "{count} düşünce",
activityCollapse.toolCallCount to "{count} araç çağrısı",
activityCollapse.expandLabel to "{title} öğesini genişlet", and
activityCollapse.collapseLabel to "{title} öğesini daralt" so Turkish users see
localized labels.
In `@src/renderer/src/i18n/vi-VN/chat.json`:
- Around line 408-414: The activityCollapse entries are still in English; update
the Vietnamese translations for the keys inside the activityCollapse
object—specifically replace workedFor, reasoningCount, toolCallCount,
expandLabel, and collapseLabel with Vietnamese text while preserving the
{duration}, {count}, and {title} placeholders (suggestions: workedFor -> "Hoạt
động trong {duration}", reasoningCount -> "{count} suy nghĩ", toolCallCount ->
"{count} lần gọi công cụ", expandLabel -> "Mở rộng {title}", collapseLabel ->
"Thu gọn {title}").
---
Nitpick comments:
In `@src/renderer/src/components/message/messageActivityGroups.ts`:
- Around line 157-187: The formatActivityDuration function currently hardcodes
unit strings for Chinese and non-Chinese locales; replace those literals with
vue-i18n keys by using the i18n translation function (e.g. useI18n().t or a
provided t) inside formatActivityDuration so each unit ("day", "hour", "minute",
"second" and their short forms) is looked up from src/renderer/src/i18n; update
the parts arrays in formatActivityDuration to call t('duration.day_short') /
t('duration.day_cn') etc. (keep the existing logic for isChineseLocale and
zero-second fallback) and ensure the translation keys are added to the i18n
resource files.
In `@src/renderer/src/components/message/MessageBlockActivityGroup.vue`:
- Around line 21-37: Add a development-mode warning for unrecognized block types
in the template rendering loop over blocks in MessageBlockActivityGroup.vue.
After the existing v-if and v-else-if for known block types, add a v-else block
that triggers a console.warn during development to log any unexpected block
types. This will help catch upstream data issues early without affecting
production behavior. Ensure this check references the iterated variable
'block.type' within the template's v-for scope.
In `@test/renderer/components/message/MessageItemAssistant.test.ts`:
- Around line 263-277: Add three unit tests for MessageItemAssistant to cover
edge cases: (1) mount with message status 'sent' and isInGeneratingThread=true
and assert no activity-group exists and individual blocks
(MessageBlockThink/MessageBlockToolCall) are present; (2) mount with status
'pending' and isInGeneratingThread=false and assert no grouping and blocks are
present; (3) mount with a tool call block whose tool has name 'update_plan' and
extra.internalTool=true and assert that the internal tool call is excluded from
the activity-group (i.e., group count reflects exclusion and the
MessageBlockToolCall for that internal tool is not rendered in the group). Use
the same helpers (createMessage, createThinkingBlock, createToolCallBlock) and
component name MessageItemAssistant to create the test props and assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ee0248c-c972-4980-8661-430640eb3aec
📒 Files selected for processing (32)
docs/features/automatic-turn-activity-collapse/plan.mddocs/features/automatic-turn-activity-collapse/spec.mddocs/features/automatic-turn-activity-collapse/tasks.mdsrc/renderer/src/components/chat/messageListItems.tssrc/renderer/src/components/message/MessageBlockActivityGroup.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/messageActivityGroups.tssrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/pages/ChatPage.vuetest/renderer/components/MessageList.test.tstest/renderer/components/message/MessageBlockActivityGroup.test.tstest/renderer/components/message/MessageItemAssistant.test.tstest/renderer/components/message/messageActivityGroups.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
Summary by CodeRabbit
New Features
Documentation
Localization
Tests
Chores