mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(desktop): Branch button silently does nothing inside a branched chat tile (#71969)
* fix: Branch button is a dead no-op inside a branched chat tile session-tile.tsx wired onBranchInNewChat to () => undefined for tiled/branched sessions (nested branching isn't supported there), but the button in AssistantMessage's action bar rendered unconditionally regardless of whether a real handler was supplied. The button looked clickable but silently did nothing, with no visual feedback. - AssistantMessage now only renders the Branch button when onBranchInNewChat is actually provided, matching the existing pattern used for onDismissError/onRestoreToMessage. - session-tile.tsx no longer passes a no-op handler; the prop is simply omitted so the button doesn't render in tiles. - onBranchInNewChat is now optional on ChatViewProps, and the latestChatActions passthrough wrapper uses the existing latestOptional helper instead of an unconditional call. * test: assert Branch button visibility matches handler presence Adds coverage for the bug #2 fix: renders Thread with and without an onBranchInNewChat handler and asserts the Branch in new chat button is shown only when a real handler is supplied, hidden otherwise - covering both the normal open-chat case and the session-tile (branched chat) case that used to leave a dead, clickable button.
This commit is contained in:
parent
8eaaa5021c
commit
c92417e77f
5 changed files with 103 additions and 12 deletions
|
|
@ -71,7 +71,7 @@ interface ChatViewProps extends Omit<React.ComponentProps<'div'>, 'onSubmit'> {
|
|||
onCancel: () => Promise<void> | void
|
||||
onAddContextRef: (refText: string, label?: string, detail?: string) => void
|
||||
onAddUrl: (url: string) => void
|
||||
onBranchInNewChat: (messageId: string) => void
|
||||
onBranchInNewChat?: (messageId: string) => void
|
||||
maxVoiceRecordingSeconds?: number
|
||||
onAttachImageBlob: (blob: Blob) => Promise<boolean | void> | boolean | void
|
||||
onAttachDroppedItems: (candidates: DroppedFile[]) => Promise<boolean | void> | boolean | void
|
||||
|
|
|
|||
|
|
@ -170,7 +170,6 @@ function TileChat({
|
|||
onAddUrl={url => composer.addContextRefAttachment(`@url:${formatRefValue(url)}`, url)}
|
||||
onAttachDroppedItems={composer.attachDroppedItems}
|
||||
onAttachImageBlob={composer.attachImageBlob}
|
||||
onBranchInNewChat={() => undefined}
|
||||
onCancel={actions.cancelRun}
|
||||
onDeleteSelectedSession={() => undefined}
|
||||
onDismissError={actions.dismissError}
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ export function latestChatActions(actions: ChatActions): ChatActions {
|
|||
onAddUrl: (...args) => actions.onAddUrl(...args),
|
||||
onAttachDroppedItems: (...args) => actions.onAttachDroppedItems(...args),
|
||||
onAttachImageBlob: (...args) => actions.onAttachImageBlob(...args),
|
||||
onBranchInNewChat: (...args) => actions.onBranchInNewChat(...args),
|
||||
onBranchInNewChat: latestOptional(() => actions.onBranchInNewChat),
|
||||
onCancel: (...args) => actions.onCancel(...args),
|
||||
onDeleteSelectedSession: (...args) => actions.onDeleteSelectedSession(...args),
|
||||
onDismissError: latestOptional(() => actions.onDismissError),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,90 @@
|
|||
// Bug #2: the Branch-in-new-chat button used to render unconditionally even
|
||||
// when its handler was a no-op (session-tile.tsx passed `() => undefined`
|
||||
// for branched/tiled chats, where nested branching isn't supported). That
|
||||
// left a visibly clickable button that silently did nothing. The fix makes
|
||||
// AssistantMessage's action bar hide the button entirely when no handler is
|
||||
// supplied, matching how onDismissError/onRestoreToMessage already behave.
|
||||
import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react'
|
||||
import { cleanup, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { Thread } from '.'
|
||||
|
||||
const createdAt = new Date('2026-05-01T00:00:00.000Z')
|
||||
|
||||
class TestResizeObserver {
|
||||
observe() {}
|
||||
unobserve() {}
|
||||
disconnect() {}
|
||||
}
|
||||
vi.stubGlobal('ResizeObserver', TestResizeObserver)
|
||||
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) =>
|
||||
window.setTimeout(() => callback(performance.now()), 0)
|
||||
)
|
||||
vi.stubGlobal('cancelAnimationFrame', (id: number) => window.clearTimeout(id))
|
||||
vi.stubGlobal('CSS', { escape: (str: string) => str })
|
||||
Element.prototype.scrollTo = function scrollTo() {}
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
})
|
||||
|
||||
function userMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'user-1',
|
||||
role: 'user',
|
||||
content: [{ type: 'text', text: 'question one' }],
|
||||
attachments: [],
|
||||
createdAt,
|
||||
metadata: { custom: {} }
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function assistantMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-1',
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: 'done' }],
|
||||
status: { type: 'complete', reason: 'stop' },
|
||||
createdAt,
|
||||
metadata: {
|
||||
unstable_state: null,
|
||||
unstable_annotations: [],
|
||||
unstable_data: [],
|
||||
steps: [],
|
||||
custom: {}
|
||||
}
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function Harness({ onBranchInNewChat }: { onBranchInNewChat?: (messageId: string) => void }) {
|
||||
const runtime = useExternalStoreRuntime<ThreadMessage>({
|
||||
messages: [userMessage(), assistantMessage()],
|
||||
isRunning: false,
|
||||
onNew: async () => {}
|
||||
})
|
||||
return (
|
||||
<AssistantRuntimeProvider runtime={runtime}>
|
||||
<Thread onBranchInNewChat={onBranchInNewChat} />
|
||||
</AssistantRuntimeProvider>
|
||||
)
|
||||
}
|
||||
|
||||
describe('AssistantMessage branch button visibility (bug #2 fix)', () => {
|
||||
it('shows the Branch in new chat button when a handler is provided (open chat)', async () => {
|
||||
render(<Harness onBranchInNewChat={() => undefined} />)
|
||||
|
||||
expect(await screen.findByRole('button', { name: 'Branch in new chat' })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('hides the Branch in new chat button when no handler is provided (session-tile / branched chat)', async () => {
|
||||
render(<Harness />)
|
||||
|
||||
// Wait for the assistant message to actually mount before asserting
|
||||
// absence, so a missing button isn't just a false negative from an
|
||||
// unrendered message.
|
||||
await screen.findByText('done')
|
||||
|
||||
expect(screen.queryByRole('button', { name: 'Branch in new chat' })).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -151,15 +151,17 @@ const AssistantActionBar: FC<MessageActionProps> = ({ messageId, getMessageText,
|
|||
data-slot="aui_msg-actions"
|
||||
>
|
||||
<MessageAge />
|
||||
<TooltipIconButton
|
||||
onClick={() => {
|
||||
triggerHaptic('selection')
|
||||
onBranchInNewChat?.(messageId)
|
||||
}}
|
||||
tooltip={copy.branchNewChat}
|
||||
>
|
||||
<GitForkIcon className="size-3.5" />
|
||||
</TooltipIconButton>
|
||||
{onBranchInNewChat && (
|
||||
<TooltipIconButton
|
||||
onClick={() => {
|
||||
triggerHaptic('selection')
|
||||
onBranchInNewChat(messageId)
|
||||
}}
|
||||
tooltip={copy.branchNewChat}
|
||||
>
|
||||
<GitForkIcon className="size-3.5" />
|
||||
</TooltipIconButton>
|
||||
)}
|
||||
<CopyButton appearance="icon" buttonSize="icon" label={copy.copy} text={getMessageText} />
|
||||
<ReadAloudButton getText={getMessageText} messageId={messageId} />
|
||||
<ActionBarPrimitive.Reload asChild>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue