Skip to content

fix: merge assistant activity groups#1739

Merged
zerob13 merged 1 commit into
devfrom
codex/fix-merged-activity-groups
Jun 5, 2026
Merged

fix: merge assistant activity groups#1739
zerob13 merged 1 commit into
devfrom
codex/fix-merged-activity-groups

Conversation

@zerob13
Copy link
Copy Markdown
Collaborator

@zerob13 zerob13 commented Jun 5, 2026

Summary

  • Ignore empty reasoning metadata blocks while building assistant activity groups, so MiniMax-style signed empty reasoning chunks no longer split one work span.
  • Keep activity details mounted and animate height/opacity on expand/collapse to reduce layout jitter.
  • Tighten the summary chevron/title spacing.

UI

BEFORE
[>] Already worked for 9s · 5 thoughts · 4 tool calls
(details hard-toggle with display none)

AFTER
[>] Already worked for 9s · 5 thoughts · 4 tool calls
(details animate open/closed; collapsed details are inert)

Tests

  • pnpm exec vitest run test/renderer/components/message/messageActivityGroups.test.ts test/renderer/components/message/MessageBlockActivityGroup.test.ts test/renderer/components/message/MessageItemAssistant.test.ts
  • pnpm run format
  • pnpm run i18n
  • pnpm run lint
  • commit hook: pnpm run typecheck

Summary by CodeRabbit

  • New Features

    • Improved activity grouping to properly merge assistant work separated by metadata blocks.
    • Enhanced expand/collapse animation with smoother transitions and no layout shifts.
    • Replaced icon swapping with chevron rotation for clearer expand/collapse indication.
  • Bug Fixes

    • Fixed collapsed activity group details to prevent interactive element exposure.
  • Accessibility

    • Improved keyboard and screen reader handling for collapsed activity groups.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR implements "Merged Activity Groups" by filtering empty reasoning metadata blocks from activity grouping so they do not split assistant work spans, and refactors the expand/collapse UI from v-show to an always-mounted grid-based transition that animates height and opacity while preventing interaction on hidden details via aria-hidden and inert.

Changes

Merged Activity Groups Feature

Layer / File(s) Summary
Specification and Planning Documentation
docs/issues/merged-activity-groups/plan.md, docs/issues/merged-activity-groups/spec.md, docs/issues/merged-activity-groups/tasks.md
Three new docs define the feature: spec outlines expected behavior for empty reasoning blocks and UI constraints; plan details implementation of grouping logic and transition-based collapse; tasks list completed work items.
Empty Reasoning Block Detection and Grouping Integration
src/renderer/src/components/message/messageActivityGroups.ts
New isEmptyReasoningBlock helper detects reasoning blocks with non-string or whitespace-only content; buildAssistantRenderItems skips such blocks during iteration, preventing them from being buffered into activity groups or rendered as standalone items.
Activity Group Collapse UI - Always-Mounted Transitions
src/renderer/src/components/message/MessageBlockActivityGroup.vue
Component replaces v-show with always-rendered expandable shell using dynamic grid-template-rows and opacity classes; toggle icon becomes single rotating chevron-right; collapsed state marked with aria-hidden and inert to prevent focus and pointer interaction on mounted hidden controls.
Component Tests - Collapse DOM and Accessibility Assertions
test/renderer/components/message/MessageBlockActivityGroup.test.ts
Tests updated to assert aria-hidden, inert, and grid row classes on the always-mounted shell container; verifies collapsed state disables interaction and expanded state re-enables it.
Integration Test - Empty Reasoning Block Merging
test/renderer/components/message/messageActivityGroups.test.ts
New test verifies that a sequence of empty signed reasoning blocks interleaved with tool calls and visible content collapses into a single activity-group; validates merged group contains correct block/tool-call content and reports accurate reasoning/tool-call counts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ThinkInAIXYZ/deepchat#1727: Both PRs directly modify activity grouping and collapse behavior in messageActivityGroups.ts and MessageBlockActivityGroup.vue—this PR's empty-reasoning filtering and always-mounted collapse UI build on collapse-turn activity work.

Poem

🐰 Hops with glee through grouped refrains,
Empty thoughts that leave no stains—
Grid rows glide with silent grace,
Collapsed shells find their hiding place,
Merged activities skip the empty beat,
Making rabbit-coded logic sweet!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: fixing activity group merging to ignore empty reasoning blocks, which is the core feature implemented across the codebase changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-merged-activity-groups

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
docs/issues/merged-activity-groups/plan.md (1)

25-25: 💤 Low value

Consider hyphenating compound adjective for consistency.

The phrase "provider signed" could be hyphenated to "provider-signed" when used as a compound adjective modifying "empty reasoning blocks" for improved technical writing consistency.

📝 Optional style improvement
-- Add renderer regression coverage around provider signed empty reasoning blocks.
+- Add renderer regression coverage around provider-signed empty reasoning blocks.
🤖 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 `@docs/issues/merged-activity-groups/plan.md` at line 25, Change the compound
adjective "provider signed" to a hyphenated form "provider-signed" in the
sentence that reads "Add renderer regression coverage around provider signed
empty reasoning blocks" so it consistently uses "provider-signed" when modifying
"empty reasoning blocks"; update any identical occurrences in the same document
to match this hyphenation.
🤖 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.

Nitpick comments:
In `@docs/issues/merged-activity-groups/plan.md`:
- Line 25: Change the compound adjective "provider signed" to a hyphenated form
"provider-signed" in the sentence that reads "Add renderer regression coverage
around provider signed empty reasoning blocks" so it consistently uses
"provider-signed" when modifying "empty reasoning blocks"; update any identical
occurrences in the same document to match this hyphenation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fe865eac-295c-402a-bc0f-abd5de89e44d

📥 Commits

Reviewing files that changed from the base of the PR and between f1e6a9e and eb03eb3.

📒 Files selected for processing (7)
  • docs/issues/merged-activity-groups/plan.md
  • docs/issues/merged-activity-groups/spec.md
  • docs/issues/merged-activity-groups/tasks.md
  • src/renderer/src/components/message/MessageBlockActivityGroup.vue
  • src/renderer/src/components/message/messageActivityGroups.ts
  • test/renderer/components/message/MessageBlockActivityGroup.test.ts
  • test/renderer/components/message/messageActivityGroups.test.ts

@zerob13 zerob13 merged commit 6ed3b14 into dev Jun 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant