-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: integrate tool call icons with status indicators and daisy chaining #4279
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
Changes from all commits
38ba815
0b47c91
928705f
f78e139
50391b8
07b6024
f64f008
99390ea
1e52f7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,12 @@ import { extractImagePaths, removeImagePathsFromText } from '../utils/imageUtils | |
| import { formatMessageTimestamp } from '../utils/timeUtils'; | ||
| import MarkdownContent from './MarkdownContent'; | ||
| import ToolCallWithResponse from './ToolCallWithResponse'; | ||
| import ToolCallChain from './ToolCallChain'; | ||
| import { | ||
| identifyConsecutiveToolCalls, | ||
| shouldHideMessage, | ||
| getChainForMessage, | ||
| } from '../utils/toolCallChaining'; | ||
| import { | ||
| Message, | ||
| getTextContent, | ||
|
|
@@ -18,6 +24,7 @@ import { | |
| import ToolCallConfirmation from './ToolCallConfirmation'; | ||
| import MessageCopyLink from './MessageCopyLink'; | ||
| import { NotificationEvent } from '../hooks/useMessageStream'; | ||
| import { cn } from '../utils'; | ||
|
|
||
| interface GooseMessageProps { | ||
| // messages up to this index are presumed to be "history" from a resumed session, this is used to track older tool confirmation requests | ||
|
|
@@ -60,30 +67,77 @@ export default function GooseMessage({ | |
| } | ||
|
|
||
| const cotRaw = match[1].trim(); | ||
| const visible = text.replace(match[0], '').trim(); | ||
| return { visibleText: visible, cotText: cotRaw.length > 0 ? cotRaw : null }; | ||
| const visibleText = text.replace(regex, '').trim(); | ||
|
|
||
| return { | ||
| visibleText, | ||
| cotText: cotRaw || null, | ||
| }; | ||
| }; | ||
|
|
||
| const { visibleText: textWithoutCot, cotText } = splitChainOfThought(textContent); | ||
| // Split out Chain-of-Thought | ||
| const { visibleText, cotText } = splitChainOfThought(textContent); | ||
|
|
||
| // Extract image paths from the visible part of the message (exclude CoT) | ||
| const imagePaths = extractImagePaths(textWithoutCot); | ||
| // Extract image paths from the message content | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this doing same as before but unrelated/stylistic changes?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess needed as below uses visibleText as what was textWithoutCot?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bit of a simplified variable as opposed to continuing the train of textWithoutX textWithoutY rabbit hole. |
||
| const imagePaths = extractImagePaths(visibleText); | ||
|
|
||
| // Remove image paths from text for display | ||
| const displayText = | ||
| imagePaths.length > 0 ? removeImagePathsFromText(textWithoutCot, imagePaths) : textWithoutCot; | ||
| imagePaths.length > 0 ? removeImagePathsFromText(visibleText, imagePaths) : visibleText; | ||
|
|
||
| // Memoize the timestamp | ||
| const timestamp = useMemo(() => formatMessageTimestamp(message.created), [message.created]); | ||
|
|
||
| // Get tool requests from the message | ||
| const toolRequests = getToolRequests(message); | ||
|
|
||
| // Get current message index | ||
| const messageIndex = messages.findIndex((msg) => msg.id === message.id); | ||
|
|
||
| // Enhanced chain detection that works during streaming | ||
| const toolCallChains = useMemo(() => { | ||
| // Always run chain detection, but handle streaming messages specially | ||
| const chains = identifyConsecutiveToolCalls(messages); | ||
|
|
||
| // If this message is streaming and has tool calls but no text, | ||
| // check if it should extend an existing chain | ||
| if (isStreaming && toolRequests.length > 0 && !displayText.trim()) { | ||
| // Look for an existing chain that this message could extend | ||
| const previousMessage = messageIndex > 0 ? messages[messageIndex - 1] : null; | ||
| if (previousMessage) { | ||
| const prevToolRequests = getToolRequests(previousMessage); | ||
|
|
||
| // If previous message has tool calls (with or without text), extend its chain | ||
| if (prevToolRequests.length > 0) { | ||
| // Find if previous message is part of a chain | ||
| const prevChain = chains.find((chain) => chain.includes(messageIndex - 1)); | ||
| if (prevChain) { | ||
| // Extend the existing chain to include this streaming message | ||
| const extendedChains = chains.map((chain) => | ||
| chain === prevChain ? [...chain, messageIndex] : chain | ||
| ); | ||
| return extendedChains; | ||
| } else { | ||
| // Create a new chain with previous and current message | ||
| return [...chains, [messageIndex - 1, messageIndex]]; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return chains; | ||
| }, [messages, isStreaming, messageIndex, toolRequests, displayText]); | ||
|
|
||
| // Check if this message should be hidden (part of chain but not first) | ||
| const shouldHide = shouldHideMessage(messageIndex, toolCallChains); | ||
|
|
||
| // Get the chain this message belongs to | ||
| const messageChain = getChainForMessage(messageIndex, toolCallChains); | ||
|
|
||
| // Extract URLs under a few conditions | ||
| // 1. The message is purely text | ||
| // 2. The link wasn't also present in the previous message | ||
| // 3. The message contains the explicit http:// or https:// protocol at the beginning | ||
| const messageIndex = messages?.findIndex((msg) => msg.id === message.id); | ||
| const previousMessage = messageIndex > 0 ? messages[messageIndex - 1] : null; | ||
| const previousUrls = previousMessage ? extractUrls(getTextContent(previousMessage)) : []; | ||
| const urls = toolRequests.length === 0 ? extractUrls(displayText, previousUrls) : []; | ||
|
|
@@ -145,10 +199,17 @@ export default function GooseMessage({ | |
| appendMessage, | ||
| ]); | ||
|
|
||
| // If this message should be hidden (part of chain but not first), don't render it | ||
| if (shouldHide) { | ||
| return null; | ||
| } | ||
|
|
||
| // Determine rendering logic based on chain membership and content | ||
| const isFirstInChain = messageChain && messageChain[0] === messageIndex; | ||
|
|
||
| return ( | ||
| <div className="goose-message flex w-[90%] justify-start min-w-0"> | ||
| <div className="flex flex-col w-full min-w-0"> | ||
| {/* Chain-of-Thought (hidden by default) */} | ||
| {cotText && ( | ||
| <details className="bg-bgSubtle border border-borderSubtle rounded p-2 mb-2"> | ||
| <summary className="cursor-pointer text-sm text-textSubtle select-none"> | ||
|
|
@@ -160,61 +221,73 @@ export default function GooseMessage({ | |
| </details> | ||
| )} | ||
|
|
||
| {/* Visible assistant response */} | ||
| {displayText && ( | ||
| <div className="flex flex-col group"> | ||
| <div className={`goose-message-content py-2`}> | ||
| <div ref={contentRef}>{<MarkdownContent content={displayText} />}</div> | ||
| <div ref={contentRef} className="w-full"> | ||
| <MarkdownContent content={displayText} /> | ||
| </div> | ||
|
|
||
| {/* Render images if any */} | ||
| {/* Image previews */} | ||
| {imagePaths.length > 0 && ( | ||
| <div className="flex flex-wrap gap-2 mt-2 mb-2"> | ||
| <div className="mt-4"> | ||
| {imagePaths.map((imagePath, index) => ( | ||
| <ImagePreview key={index} src={imagePath} alt={`Image ${index + 1}`} /> | ||
| <ImagePreview key={index} src={imagePath} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Only show timestamp and copy link when not streaming */} | ||
| <div className="relative flex justify-start"> | ||
| {toolRequests.length === 0 && !isStreaming && ( | ||
| <div className="text-xs font-mono text-text-muted pt-1 transition-all duration-200 group-hover:-translate-y-4 group-hover:opacity-0"> | ||
| {timestamp} | ||
| </div> | ||
| )} | ||
| {displayText && | ||
| message.content.every((content) => content.type === 'text') && | ||
| !isStreaming && ( | ||
| {toolRequests.length === 0 && ( | ||
| <div className="relative flex justify-start"> | ||
| {!isStreaming && ( | ||
| <div className="text-xs font-mono text-text-muted pt-1 transition-all duration-200 group-hover:-translate-y-4 group-hover:opacity-0"> | ||
| {timestamp} | ||
| </div> | ||
| )} | ||
| {message.content.every((content) => content.type === 'text') && !isStreaming && ( | ||
| <div className="absolute left-0 pt-1"> | ||
| <MessageCopyLink text={displayText} contentRef={contentRef} /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
| {toolRequests.length > 0 && ( | ||
| <div className="relative flex flex-col w-full"> | ||
| {toolRequests.map((toolRequest) => ( | ||
| <div className={`goose-message-tool pb-2`} key={toolRequest.id}> | ||
| <ToolCallWithResponse | ||
| // If the message is resumed and not matched tool response, it means the tool is broken or cancelled. | ||
| isCancelledMessage={ | ||
| messageIndex < messageHistoryIndex && | ||
| toolResponsesMap.get(toolRequest.id) == undefined | ||
| } | ||
| toolRequest={toolRequest} | ||
| toolResponse={toolResponsesMap.get(toolRequest.id)} | ||
| notifications={toolCallNotifications.get(toolRequest.id)} | ||
| isStreamingMessage={isStreaming} | ||
| append={append} | ||
| /> | ||
| <div className={cn(displayText && 'mt-2')}> | ||
| {isFirstInChain ? ( | ||
| <ToolCallChain | ||
| messages={messages} | ||
| chainIndices={messageChain} | ||
| toolCallNotifications={toolCallNotifications} | ||
| toolResponsesMap={toolResponsesMap} | ||
| messageHistoryIndex={messageHistoryIndex} | ||
| isStreaming={isStreaming} | ||
| /> | ||
| ) : !messageChain ? ( | ||
| <div className="relative flex flex-col w-full"> | ||
| <div className="flex flex-col gap-3"> | ||
| {toolRequests.map((toolRequest) => ( | ||
| <div className="goose-message-tool" key={toolRequest.id}> | ||
| <ToolCallWithResponse | ||
| isCancelledMessage={ | ||
| messageIndex < messageHistoryIndex && | ||
| toolResponsesMap.get(toolRequest.id) == undefined | ||
| } | ||
| toolRequest={toolRequest} | ||
| toolResponse={toolResponsesMap.get(toolRequest.id)} | ||
| notifications={toolCallNotifications.get(toolRequest.id)} | ||
| isStreamingMessage={isStreaming} | ||
| append={append} | ||
| /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <div className="text-xs text-text-muted pt-1 transition-all duration-200 group-hover:-translate-y-4 group-hover:opacity-0"> | ||
| {!isStreaming && timestamp} | ||
| </div> | ||
| </div> | ||
| ))} | ||
| <div className="text-xs text-text-muted pt-1 transition-all duration-200 group-hover:-translate-y-4 group-hover:opacity-0"> | ||
| {!isStreaming && timestamp} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| )} | ||
|
|
||
|
|
@@ -229,9 +302,8 @@ export default function GooseMessage({ | |
| </div> | ||
|
|
||
| {/* TODO(alexhancock): Re-enable link previews once styled well again */} | ||
| {/* eslint-disable-next-line no-constant-binary-expression */} | ||
| {false && urls.length > 0 && ( | ||
| <div className="flex flex-wrap mt-[16px]"> | ||
| {urls.length > 0 && ( | ||
| <div className="mt-4"> | ||
| {urls.map((url, index) => ( | ||
| <LinkPreview key={index} url={url} /> | ||
| ))} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { formatMessageTimestamp } from '../utils/timeUtils'; | ||
| import { Message, getToolRequests } from '../types/message'; | ||
| import { NotificationEvent } from '../hooks/useMessageStream'; | ||
| import ToolCallWithResponse from './ToolCallWithResponse'; | ||
|
|
||
| interface ToolCallChainProps { | ||
| messages: Message[]; | ||
| chainIndices: number[]; | ||
| toolCallNotifications: Map<string, NotificationEvent[]>; | ||
| toolResponsesMap: Map<string, import('../types/message').ToolResponseMessageContent>; | ||
| messageHistoryIndex: number; | ||
| isStreaming?: boolean; | ||
| } | ||
|
|
||
| export default function ToolCallChain({ | ||
| messages, | ||
| chainIndices, | ||
| toolCallNotifications, | ||
| toolResponsesMap, | ||
| messageHistoryIndex, | ||
| isStreaming = false, | ||
| }: ToolCallChainProps) { | ||
| const lastMessageIndex = chainIndices[chainIndices.length - 1]; | ||
| const lastMessage = messages[lastMessageIndex]; | ||
| const timestamp = lastMessage ? formatMessageTimestamp(lastMessage.created) : ''; | ||
|
|
||
| return ( | ||
| <div className="relative flex flex-col w-full"> | ||
| <div className="flex flex-col gap-3"> | ||
| {chainIndices.map((messageIndex) => { | ||
| const message = messages[messageIndex]; | ||
| const toolRequests = getToolRequests(message); | ||
|
|
||
| return toolRequests.map((toolRequest) => ( | ||
| <div key={toolRequest.id} className="goose-message-tool"> | ||
| <ToolCallWithResponse | ||
| isCancelledMessage={ | ||
| messageIndex < messageHistoryIndex && | ||
| toolResponsesMap.get(toolRequest.id) == undefined | ||
| } | ||
| toolRequest={toolRequest} | ||
| toolResponse={toolResponsesMap.get(toolRequest.id)} | ||
| notifications={toolCallNotifications.get(toolRequest.id)} | ||
| isStreamingMessage={isStreaming} | ||
| /> | ||
| </div> | ||
| )); | ||
| })} | ||
| </div> | ||
|
|
||
| <div className="text-xs text-text-muted pt-1 transition-all duration-200 group-hover:-translate-y-4 group-hover:opacity-0"> | ||
| {!isStreaming && timestamp} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import React from 'react'; | ||
| import { cn } from '../utils'; | ||
|
|
||
| export type ToolCallStatus = 'pending' | 'loading' | 'success' | 'error'; | ||
|
|
||
| interface ToolCallStatusIndicatorProps { | ||
| status: ToolCallStatus; | ||
| className?: string; | ||
| } | ||
|
|
||
| export const ToolCallStatusIndicator: React.FC<ToolCallStatusIndicatorProps> = ({ | ||
| status, | ||
| className, | ||
| }) => { | ||
| const getStatusStyles = () => { | ||
| switch (status) { | ||
| case 'success': | ||
| return 'bg-green-500'; | ||
| case 'error': | ||
| return 'bg-red-500'; | ||
| case 'loading': | ||
| return 'bg-yellow-500 animate-pulse'; | ||
| case 'pending': | ||
| default: | ||
| return 'bg-gray-400'; | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div | ||
| className={cn( | ||
| 'absolute -top-0.5 -right-0.5 w-2 h-2 rounded-full border border-background-default', | ||
| getStatusStyles(), | ||
| className | ||
| )} | ||
| aria-label={`Tool status: ${status}`} | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Wrapper component that adds a status indicator to a tool icon | ||
| */ | ||
| interface ToolIconWithStatusProps { | ||
| ToolIcon: React.ComponentType<{ className?: string }>; | ||
| status: ToolCallStatus; | ||
| className?: string; | ||
| } | ||
|
|
||
| export const ToolIconWithStatus: React.FC<ToolIconWithStatusProps> = ({ | ||
| ToolIcon, | ||
| status, | ||
| className, | ||
| }) => { | ||
| return ( | ||
| <div className={cn('relative inline-block', className)}> | ||
| <ToolIcon className="w-3 h-3 flex-shrink-0" /> | ||
| <ToolCallStatusIndicator status={status} /> | ||
| </div> | ||
| ); | ||
| }; |
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.
is this change part of this, a little lost as to what the functional change is
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.
No function change
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.
Why is it needed though?
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.
Vibes - will revert
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.
Ahhh I see regex was used within the chain logic and kept consistent @zanesq