Expand AI group actions by default and add toggle switch#98
Expand AI group actions by default and add toggle switch#98mbergo wants to merge 4 commits intomatt1398:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a more intuitive and efficient way to manage the expansion and collapse of AI groups within the UI. By shifting from individual group expansion tracking to a default state with manual overrides, it enables a new 'expand/collapse all' feature, enhancing user control and streamlining the chat history interaction. The changes improve both the underlying state management and the user interface for a smoother experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
mbergo
changed the title
coderabbitai
bot
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to control the default expansion state of AI groups in the chat history. The core change involves refactoring the tabUISlice state management to replace expandedAIGroupIds with aiGroupsExpandedByDefault (a boolean) and toggledAIGroupIds (a set for individual overrides). A new 'Expand/Collapse All' button, using new lucide-react icons, has been added to the ChatHistory component's sticky header, enabling users to toggle the global default expansion state for AI groups. Toggling this global setting also clears any previously set individual group overrides. The STICKY_BUTTON_OFFSET in ChatHistory.tsx was adjusted to accommodate the always-present sticky header. Additionally, the package.json file was updated to include tar.gz as a new build target, and corresponding unit tests were updated to reflect the new expansion logic.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a Linux Changes
Possibly related PRs
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/renderer/components/chat/ChatHistory.tsx (1)
233-237: Keep the sticky offset and reclaimed spacing in one place.Line 236 hard-codes
44, while Line 813 separately hard-codes-2remto reclaim the same header space. The next button-size tweak can make navigation targets hide under the sticky bar again. Please derive both from one source of truth or measure the sticky container height.Also applies to: 813-813
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/ChatHistory.tsx` around lines 233 - 237, The hard-coded sticky header size is duplicated (STICKY_BUTTON_OFFSET = 44 and a separate "-2rem" reclamation) — consolidate into a single source of truth by deriving the reclaimed spacing from STICKY_BUTTON_OFFSET (or by measuring the sticky container height at runtime) and use that value wherever you currently use 44 or "-2rem"; update the ChatHistory component to export/use STICKY_BUTTON_OFFSET for both the scroll offset and the reclaimed spacing (or compute stickyHeight via a ref to the sticky container and set both the scroll offset and the negative-margin/reclaim style from that measured value) so future button-size changes only require one update.test/renderer/store/tabUISlice.test.ts (1)
392-449: Add a regression for the auto-expand interaction.These cases cover the slice transitions in isolation, but they never exercise the
sessionDetailSliceloop that auto-expands every AI group. A test that collapses all, runs that path, and asserts the groups stay collapsed would lock down the bug above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/store/tabUISlice.test.ts` around lines 392 - 449, Add a regression test that collapses all AI groups for a tab (use initTabUIState + toggleAIGroupsExpandedByDefaultForTab to set default collapsed), then invoke the sessionDetailSlice auto-expand loop (call the function or dispatch the action in sessionDetailSlice that performs the automatic expansion of AI groups) and assert that isAIGroupExpandedForTab remains false for each group; this ensures the sessionDetailSlice auto-expand path respects the collapsed-by-default state and clears/does not reapply overrides.src/renderer/hooks/useTabUI.ts (1)
111-117: Avoid duplicating the expansion formula in the hook.Line 114-116 re-implements the same XOR rule that
src/renderer/store/slices/tabUISlice.tsalready owns at Line 177-182. The slice tests will not catch a future divergence here, so this should live in one shared helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useTabUI.ts` around lines 111 - 117, The hook useTabUI.ts currently re-implements the XOR expansion rule in its isAIGroupExpanded callback; instead export the single-source helper from the tabUISlice (where the logic currently lives) and call that helper from useTabUI's isAIGroupExpanded. Concretely: remove the inline XOR logic in isAIGroupExpanded, add or use an exported function from src/renderer/store/slices/tabUISlice.ts (e.g., an exported helper that computes expanded state from aiGroupsExpandedByDefault and toggledAIGroupIds), import that helper into useTabUI.ts, and delegate the computation to it so the rule is maintained in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/store/CLAUDE.md`:
- Around line 36-39: The documentation for tabUISlice omits the
aiGroupsExpandedByDefault flag, making toggledAIGroupIds ambiguous; update the
CLAUDE.md model to document aiGroupsExpandedByDefault alongside
toggledAIGroupIds and explain how the two interact (i.e.,
aiGroupsExpandedByDefault provides the base expansion state per tab and
toggledAIGroupIds contains exceptions relative to that default), and reference
the tabUISlice slice name and both symbols (aiGroupsExpandedByDefault,
toggledAIGroupIds) so readers understand how expansion state is computed.
In `@src/renderer/store/slices/tabUISlice.ts`:
- Around line 185-225: The auto-expand path is repopulating toggledAIGroupIds
even when a tab's aiGroupsExpandedByDefault is false; update the refresh/load
logic in sessionDetailSlice to respect the per-tab default by checking
getAIGroupsExpandedByDefaultForTab(tabId) before calling expandAIGroupForTab
(skip calling expandAIGroupForTab when the default is false), or alternatively
modify expandAIGroupForTab to no-op if getAIGroupsExpandedByDefaultForTab(tabId)
is false; reference expandAIGroupForTab, getAIGroupsExpandedByDefaultForTab, and
toggleAIGroupsExpandedByDefaultForTab when making the change so the collapse-all
state survives refresh.
---
Nitpick comments:
In `@src/renderer/components/chat/ChatHistory.tsx`:
- Around line 233-237: The hard-coded sticky header size is duplicated
(STICKY_BUTTON_OFFSET = 44 and a separate "-2rem" reclamation) — consolidate
into a single source of truth by deriving the reclaimed spacing from
STICKY_BUTTON_OFFSET (or by measuring the sticky container height at runtime)
and use that value wherever you currently use 44 or "-2rem"; update the
ChatHistory component to export/use STICKY_BUTTON_OFFSET for both the scroll
offset and the reclaimed spacing (or compute stickyHeight via a ref to the
sticky container and set both the scroll offset and the negative-margin/reclaim
style from that measured value) so future button-size changes only require one
update.
In `@src/renderer/hooks/useTabUI.ts`:
- Around line 111-117: The hook useTabUI.ts currently re-implements the XOR
expansion rule in its isAIGroupExpanded callback; instead export the
single-source helper from the tabUISlice (where the logic currently lives) and
call that helper from useTabUI's isAIGroupExpanded. Concretely: remove the
inline XOR logic in isAIGroupExpanded, add or use an exported function from
src/renderer/store/slices/tabUISlice.ts (e.g., an exported helper that computes
expanded state from aiGroupsExpandedByDefault and toggledAIGroupIds), import
that helper into useTabUI.ts, and delegate the computation to it so the rule is
maintained in one place.
In `@test/renderer/store/tabUISlice.test.ts`:
- Around line 392-449: Add a regression test that collapses all AI groups for a
tab (use initTabUIState + toggleAIGroupsExpandedByDefaultForTab to set default
collapsed), then invoke the sessionDetailSlice auto-expand loop (call the
function or dispatch the action in sessionDetailSlice that performs the
automatic expansion of AI groups) and assert that isAIGroupExpandedForTab
remains false for each group; this ensures the sessionDetailSlice auto-expand
path respects the collapsed-by-default state and clears/does not reapply
overrides.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7074204d-d633-4a20-8d49-526eade38a89
📒 Files selected for processing (6)
package.jsonsrc/renderer/components/chat/ChatHistory.tsxsrc/renderer/hooks/useTabUI.tssrc/renderer/store/CLAUDE.mdsrc/renderer/store/slices/tabUISlice.tstest/renderer/store/tabUISlice.test.ts
| ## Key Pattern: Per-Tab UI Isolation | ||
| `tabUISlice` maintains independent UI state per tab using tabId: | ||
| - `expandedAIGroupIds`, `expandedDisplayItemIds`, `expandedSubagentTraceIds` | ||
| - `toggledAIGroupIds`, `expandedDisplayItemIds`, `expandedSubagentTraceIds` | ||
| - Ensures expanding a group in tab A doesn't affect tab B |
There was a problem hiding this comment.
Document aiGroupsExpandedByDefault in this model too.
Line 38 now lists only toggledAIGroupIds, but that set is only interpretable relative to aiGroupsExpandedByDefault. Without the default flag here, the store docs no longer describe how AI-group expansion is actually represented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/store/CLAUDE.md` around lines 36 - 39, The documentation for
tabUISlice omits the aiGroupsExpandedByDefault flag, making toggledAIGroupIds
ambiguous; update the CLAUDE.md model to document aiGroupsExpandedByDefault
alongside toggledAIGroupIds and explain how the two interact (i.e.,
aiGroupsExpandedByDefault provides the base expansion state per tab and
toggledAIGroupIds contains exceptions relative to that default), and reference
the tabUISlice slice name and both symbols (aiGroupsExpandedByDefault,
toggledAIGroupIds) so readers understand how expansion state is computed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
67-88: Duplicate CI work on push events.The
quality-gatejob runspnpm check(which includes typecheck, lint, test, and build) on every push. However, thevalidatejob already runs typecheck, lint, and build, and thetestjob already runs tests—both on all events including push. This causes typecheck, lint, and build to run twice on pushes, wasting CI minutes.Consider one of these approaches:
- Remove
quality-gatesincevalidate+testalready cover all checks- Make
quality-gatea summary gate that depends on the other jobs without re-running checks:quality-gate: if: github.event_name == 'push' needs: [validate, test] runs-on: ubuntu-latest steps: - run: echo "All checks passed"- Split by event type: Run
validate/testonly on PRs andquality-gateonly on push🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 67 - 88, The quality-gate job is duplicating work by re-running pnpm check on push; modify the quality-gate job (named quality-gate) so it no longer re-runs checks but instead depends on the existing validate and test jobs: remove the pnpm check step and add a needs: [validate, test] entry (keep runs-on and a simple step such as echo "All checks passed" or similar) so validate handles typecheck/lint/build and test handles tests without double execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 67-88: The quality-gate job is duplicating work by re-running pnpm
check on push; modify the quality-gate job (named quality-gate) so it no longer
re-runs checks but instead depends on the existing validate and test jobs:
remove the pnpm check step and add a needs: [validate, test] entry (keep runs-on
and a simple step such as echo "All checks passed" or similar) so validate
handles typecheck/lint/build and test handles tests without double execution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8fc4b44a-c07b-455e-9ad8-18f4780ce728
📒 Files selected for processing (1)
.github/workflows/ci.yml
* Initial plan * feat: add tar.gz to Linux release build targets ---------
- Add push trigger for main branch (in addition to version tags) - Gate mac and Windows release jobs to tag pushes only (require signing secrets) - Linux job runs on both main branch pushes and version tags
- Add aiGroupsExpandedByDefault flag to TabUIState (default: true) - Replace expandedAIGroupIds with toggledAIGroupIds for XOR override logic - Add toggleAIGroupsExpandedByDefaultForTab action to flip default and clear overrides - Expose aiGroupsExpandedByDefault and toggleAIGroupsExpandedByDefault in useTabUI hook - Add Collapse All / Expand All button in ChatHistory sticky header (always visible) - Fix sticky header offset to constant 44px (header always rendered now) - Update tests to reflect new expand-by-default behavior
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/components/chat/ChatHistory.tsx (1)
763-772: Add accessibility attributes to the Expand/Collapse All button.The button lacks
aria-labelandtitleattributes, which impacts accessibility and discoverability. The Context button nearby implicitly conveys meaning through its text, but a tooltip would improve UX for the toggle button.♿ Proposed accessibility improvement
<button onClick={toggleAIGroupsExpandedByDefault} className="pointer-events-auto flex items-center gap-1 rounded-md px-2.5 py-1.5 text-xs shadow-lg backdrop-blur-md transition-colors hover:opacity-80" style={{ backgroundColor: 'var(--context-btn-bg)', color: 'var(--color-text-secondary)', }} + title={aiGroupsExpandedByDefault ? 'Collapse all AI groups' : 'Expand all AI groups'} + aria-pressed={aiGroupsExpandedByDefault} > {aiGroupsExpandedByDefault ? 'Collapse All' : 'Expand All'} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/ChatHistory.tsx` around lines 763 - 772, The Expand/Collapse All button in ChatHistory.tsx (the element using toggleAIGroupsExpandedByDefault and reading aiGroupsExpandedByDefault) needs accessibility attributes added: add a dynamic aria-label and a title that reflect the current state (e.g., "Collapse all AI groups" when aiGroupsExpandedByDefault is true and "Expand all AI groups" when false) so screen readers and tooltips convey the action; ensure the attributes are set on the same <button> element that already calls toggleAIGroupsExpandedByDefault.test/renderer/store/tabUISlice.test.ts (1)
85-132: Consider adding direct test coverage fortoggleAIGroupsExpandedByDefaultForTab.The test suite thoroughly covers
toggleAIGroupExpansionForTab(per-group toggles), but there's no direct test for the newtoggleAIGroupsExpandedByDefaultForTabaction that flips the default and clears overrides. This is a key behavior change where clicking "Collapse All" should:
- Set
aiGroupsExpandedByDefaulttofalse- Clear
toggledAIGroupIds💡 Suggested test case
it('should toggle aiGroupsExpandedByDefault and clear overrides', () => { store.getState().initTabUIState('tab-1'); // Create some overrides first store.getState().toggleAIGroupExpansionForTab('tab-1', 'group-1'); expect(store.getState().isAIGroupExpandedForTab('tab-1', 'group-1')).toBe(false); // Toggle the default (Collapse All) store.getState().toggleAIGroupsExpandedByDefaultForTab('tab-1'); // Default should now be false, overrides cleared const tabState = store.getState().tabUIStates.get('tab-1'); expect(tabState?.aiGroupsExpandedByDefault).toBe(false); expect(tabState?.toggledAIGroupIds.size).toBe(0); // All groups should now be collapsed by default expect(store.getState().isAIGroupExpandedForTab('tab-1', 'group-1')).toBe(false); expect(store.getState().isAIGroupExpandedForTab('tab-1', 'group-2')).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/store/tabUISlice.test.ts` around lines 85 - 132, Add a unit test that directly covers toggleAIGroupsExpandedByDefaultForTab: init a tab via initTabUIState('tab-1'), create an override using toggleAIGroupExpansionForTab('tab-1','group-1') and assert isAIGroupExpandedForTab returns false, then call toggleAIGroupsExpandedByDefaultForTab('tab-1') and assert the tab entry in tabUIStates has aiGroupsExpandedByDefault === false and toggledAIGroupIds.size === 0, and finally assert isAIGroupExpandedForTab('tab-1','group-1') and isAIGroupExpandedForTab('tab-1','group-2') reflect the new default (both collapsed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/chat/ChatHistory.tsx`:
- Around line 763-772: The Expand/Collapse All button in ChatHistory.tsx (the
element using toggleAIGroupsExpandedByDefault and reading
aiGroupsExpandedByDefault) needs accessibility attributes added: add a dynamic
aria-label and a title that reflect the current state (e.g., "Collapse all AI
groups" when aiGroupsExpandedByDefault is true and "Expand all AI groups" when
false) so screen readers and tooltips convey the action; ensure the attributes
are set on the same <button> element that already calls
toggleAIGroupsExpandedByDefault.
In `@test/renderer/store/tabUISlice.test.ts`:
- Around line 85-132: Add a unit test that directly covers
toggleAIGroupsExpandedByDefaultForTab: init a tab via initTabUIState('tab-1'),
create an override using toggleAIGroupExpansionForTab('tab-1','group-1') and
assert isAIGroupExpandedForTab returns false, then call
toggleAIGroupsExpandedByDefaultForTab('tab-1') and assert the tab entry in
tabUIStates has aiGroupsExpandedByDefault === false and toggledAIGroupIds.size
=== 0, and finally assert isAIGroupExpandedForTab('tab-1','group-1') and
isAIGroupExpandedForTab('tab-1','group-2') reflect the new default (both
collapsed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d68fee4d-b41c-4293-9fcf-833407ab7220
📒 Files selected for processing (4)
src/renderer/components/chat/ChatHistory.tsxsrc/renderer/hooks/useTabUI.tssrc/renderer/store/slices/tabUISlice.tstest/renderer/store/tabUISlice.test.ts
Use electron-builder with --publish never and upload artifacts via gh CLI using GITHUB_TOKEN scoped to GITHUB_REPOSITORY. This avoids electron-builder inferring the upstream repo from the git remote and trying to publish there instead.
|
Thanks for the PR! Expand/collapse all is a useful feature. A few things before we can merge:
|
2 participants
commit 2ac8362
This pull request implements a significant refactor of the per-tab AI group expansion logic in the UI state, introducing a new "expand/collapse all" feature and simplifying how group expansion is managed. The changes replace the old per-group expansion set with a default-expanded flag and a set of manual overrides, making the behavior more intuitive, especially when toggling all groups at once. Additionally, the UI is updated to include a sticky "Expand/Collapse All" button at the top of the chat history panel.
Key changes include:
AI Group Expansion Logic Refactor
expandedAIGroupIdsset inTabUIStatewith a newaiGroupsExpandedByDefaultboolean and atoggledAIGroupIdsset, which tracks manual overrides from the default state. This allows for efficient "expand/collapse all" operations and simplifies per-group expansion logic. [1] [2]isAIGroupExpandedForTab,toggleAIGroupExpansionForTab, andexpandAIGroupForTab. [1] [2]getAIGroupsExpandedByDefaultForTabandtoggleAIGroupsExpandedByDefaultForTab, enabling toggling the default expansion state and clearing manual overrides when toggling all. [1] [2]UI Updates for Expand/Collapse All
ChatHistory, allowing users to expand or collapse all AI groups in the current tab with one click. The button updates its icon and tooltip based on the current default state. [1] [2]Hook and Type Updates
useTabUIhook to expose the newaiGroupsExpandedByDefaultstate andtoggleAIGroupsExpandedByDefaultmethod, and to use the new expansion logic. [1] [2] [3] [4] [5]Testing and Documentation
tabUISlice.test.tsto cover the new default/override logic, ensuring correct per-tab isolation and toggle behavior. [1] [2] [3]CLAUDE.md) to describe the new state model and clarify the role oftoggledAIGroupIds.Miscellaneous
tar.gzas a build target inpackage.json.These changes improve the usability and maintainability of the per-tab AI group expansion feature, and lay the groundwork for further UI enhancements.
Summary by CodeRabbit
New Features
Chores