-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add collapsible reasoning blocks with auto-expand setting #7874
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.
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
Conversation
- Added autoExpandReasoningBlocks configuration setting to control default expansion state - Created CollapsibleReasoningBlock component that wraps ReasoningBlock with collapsible UI - Updated ChatRow to use CollapsibleReasoningBlock instead of ReasoningBlock - Added preview text display when reasoning block is collapsed - Integrated with ExtensionStateContext for settings management Fixes #7873
| ) : ( | ||
| <ChevronRight className="h-4 w-4 text-vscode-descriptionForeground" /> | ||
| )} | ||
| <span className="text-sm font-medium text-vscode-descriptionForeground">Reasoning</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header label "Reasoning" is hardcoded. Consider wrapping it in a translation function to support multiple languages.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed my own code and found 6 issues. Even I'm disappointed in myself.
| metadata, | ||
| }) => { | ||
| const extensionState = useContext(ExtensionStateContext) | ||
| const autoExpand = extensionState?.autoExpandReasoningBlocks ?? false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical issue: The new autoExpandReasoningBlocks setting isn't exposed in the Settings UI. Users have no way to toggle this feature through the interface. Could we add this to the AutoApproveSettings component or create a dedicated UI section for reasoning block preferences?
| metadata?: any | ||
| } | ||
|
|
||
| export const CollapsibleReasoningBlock: React.FC<CollapsibleReasoningBlockProps> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for this new component. Since this is a new UI component with state management and user interactions, shouldn't we add unit tests to ensure the collapsible behavior works correctly?
| ) : ( | ||
| <ChevronRight className="h-4 w-4 text-vscode-descriptionForeground" /> | ||
| )} | ||
| <span className="text-sm font-medium text-vscode-descriptionForeground">Reasoning</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Reasoning" label is hardcoded and not using i18n. For consistency with the rest of the UI, consider using:
| ts: number | ||
| isStreaming: boolean | ||
| isLast: boolean | ||
| metadata?: any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type safety concern: Using any type here. Could we define a proper type for metadata or remove it if it's not being used?
| return ( | ||
| <Collapsible open={isOpen} onOpenChange={setIsOpen}> | ||
| <div className="bg-vscode-editorWidget-background border border-vscode-editorWidget-border rounded-md overflow-hidden"> | ||
| <CollapsibleTrigger className="flex items-center justify-between w-full p-3 hover:bg-vscode-list-hoverBackground transition-colors"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessibility improvement: Consider adding aria-expanded attribute for better screen reader support:
| }, [autoExpand]) | ||
|
|
||
| // Extract first line or preview of the reasoning content | ||
| const getPreviewText = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance optimization: The getPreviewText() function is called on every render when collapsed. Consider memoizing this with useMemo since it only depends on content:
|
Not an issue |
Fixes #7873
Summary
This PR adds a configuration option to control the auto-expand/collapse behavior of the "thinking phase" output in the Roo Code extension.
Changes
autoExpandReasoningBlocksconfiguration setting to control default expansion stateCollapsibleReasoningBlockcomponent that wrapsReasoningBlockwith collapsible UIChatRowto useCollapsibleReasoningBlockinstead ofReasoningBlockExtensionStateContextfor settings managementFeatures
Testing
Screenshots
The reasoning block now has a collapsible UI with a preview when collapsed and full content when expanded.
Important
Adds collapsible reasoning blocks with an
autoExpandReasoningBlockssetting to control default expansion state in the chat interface.autoExpandReasoningBlockssetting inglobal-settings.tsto control reasoning block expansion.CollapsibleReasoningBlockinCollapsibleReasoningBlock.tsxto wrapReasoningBlockwith collapsible UI.ChatRow.tsxto useCollapsibleReasoningBlockfor reasoning messages.ExtensionStateContextfor settings management.autoExpandReasoningBlocksinwebviewMessageHandler.tsfor state updates.This description was created by
for 804532d. You can customize this summary. It will automatically update as commits are pushed.