-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: thinking block designs should occupy lesser space #8685
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,17 +5,8 @@ import { ChatHistoryItem } from "core"; | |
| import { useEffect, useState } from "react"; | ||
| import styled from "styled-components"; | ||
|
|
||
| import { vscBackground } from "../.."; | ||
| import { AnimatedEllipsis } from "../../AnimatedEllipsis"; | ||
| import StyledMarkdownPreview from "../../StyledMarkdownPreview"; | ||
| import { ButtonContent, SpoilerButton } from "../../ui/SpoilerButton"; | ||
|
|
||
| const ThinkingTextContainer = styled.span` | ||
| display: inline-block; | ||
| min-width: fit-content; | ||
|
|
||
| padding-right: 1em; /* Reserve space for the ellipsis animation */ | ||
| `; | ||
|
|
||
| const MarkdownWrapper = styled.div` | ||
| & > div > *:first-child { | ||
|
|
@@ -39,7 +30,6 @@ function ThinkingBlockPeek({ | |
| index, | ||
| prevItem, | ||
| inProgress, | ||
| signature, | ||
| tokens, | ||
| }: ThinkingBlockPeekProps) { | ||
| const [open, setOpen] = useState(false); | ||
|
|
@@ -66,58 +56,49 @@ function ThinkingBlockPeek({ | |
|
|
||
| return duplicateRedactedThinkingBlock ? null : ( | ||
| <div className="thread-message"> | ||
| <div className="" style={{ backgroundColor: vscBackground }}> | ||
| <div | ||
| className="flex items-center justify-start pl-2 text-xs text-gray-300" | ||
| data-testid="thinking-block-peek" | ||
| > | ||
| <SpoilerButton onClick={() => setOpen(!open)}> | ||
| <ButtonContent> | ||
| {inProgress ? ( | ||
| <span> | ||
| {redactedThinking ? "Redacted Thinking" : "Thinking"} | ||
| <AnimatedEllipsis /> | ||
| </span> | ||
| ) : redactedThinking ? ( | ||
| "Redacted Thinking" | ||
| ) : ( | ||
| "Thought" + | ||
| (elapsedTime ? ` for ${elapsedTime}` : "") + | ||
| (tokens ? ` (${tokens} tokens)` : "") | ||
| )} | ||
| {open ? ( | ||
| <ChevronUpIcon className="h-3 w-3" /> | ||
| ) : ( | ||
| <ChevronDownIcon className="h-3 w-3" /> | ||
| )} | ||
| </ButtonContent> | ||
| </SpoilerButton> | ||
| <div className="mt-1 flex flex-col px-4"> | ||
| <div className="flex min-w-0 flex-row items-center justify-between gap-2"> | ||
| <div | ||
|
||
| className="text-description flex min-w-0 cursor-pointer flex-row items-center gap-1.5 text-xs transition-colors duration-200 ease-in-out hover:brightness-125" | ||
| data-testid="thinking-block-peek" | ||
| onClick={() => setOpen(!open)} | ||
| > | ||
| {inProgress ? ( | ||
| <span> | ||
| {redactedThinking ? "Redacted Thinking" : "Thinking"} | ||
| <AnimatedEllipsis /> | ||
| </span> | ||
| ) : redactedThinking ? ( | ||
| "Redacted Thinking" | ||
| ) : ( | ||
| "Thought" + | ||
| (elapsedTime ? ` for ${elapsedTime}` : "") + | ||
| (tokens ? ` (${tokens} tokens)` : "") | ||
| )} | ||
| {open ? ( | ||
| <ChevronUpIcon className="h-3 w-3" /> | ||
| ) : ( | ||
| <ChevronDownIcon className="h-3 w-3" /> | ||
| )} | ||
| </div> | ||
| </div> | ||
| <div | ||
| className={`ml-2 mt-2 overflow-y-auto transition-none duration-300 ease-in-out ${ | ||
| open ? "mb-2 mt-5 opacity-100" : "max-h-0 border-0 opacity-0" | ||
| className={`mt-2 overflow-y-auto transition-all duration-300 ease-in-out ${ | ||
| open ? "max-h-[50vh] opacity-100" : "max-h-0 opacity-0" | ||
| }`} | ||
| style={{ | ||
| borderLeft: | ||
| open && !redactedThinking | ||
| ? "2px solid var(--vscode-input-border, #606060)" | ||
| : "none", | ||
| }} | ||
| > | ||
| {redactedThinking ? ( | ||
| <div className="text-description-muted pl-4 text-xs"> | ||
| <div className="text-description pl-5 text-xs italic"> | ||
| Thinking content redacted due to safety reasons. | ||
| </div> | ||
| ) : ( | ||
| <> | ||
| <MarkdownWrapper className="-mt-1 px-0 pl-1"> | ||
| <StyledMarkdownPreview | ||
| isRenderingInStepContainer | ||
| source={content} | ||
| itemIndex={index} | ||
| /> | ||
| </MarkdownWrapper> | ||
| </> | ||
| <MarkdownWrapper> | ||
| <StyledMarkdownPreview | ||
| isRenderingInStepContainer | ||
| source={content} | ||
| itemIndex={index} | ||
| /> | ||
| </MarkdownWrapper> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Passing the current item as prevItem breaks ThinkingBlockPeek’s duplicate-suppression logic, so assistant steps will render an extra thinking block whenever a standalone thinking message precedes them.
Prompt for AI agents
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.
this will never be true because redactedThinking in props is always passed as undefined
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.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.