Skip to content

fix(desktop): Branch button silently does nothing inside a branched chat tile - #71969

Merged
austinpickett merged 2 commits into
NousResearch:mainfrom
alelpoan:fix/branch-button-dead-in-tile
Jul 27, 2026
Merged

fix(desktop): Branch button silently does nothing inside a branched chat tile#71969
austinpickett merged 2 commits into
NousResearch:mainfrom
alelpoan:fix/branch-button-dead-in-tile

Conversation

@alelpoan

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a dead-button UX bug: inside a session opened as a tile (i.e. a chat that was itself created via Branch in new chat), the Branch button on assistant replies is fully visible and looks clickable — but clicking it does absolutely nothing, with no error, no toast, no feedback of any kind.

Nested branching from a tile isn't supported by design (session-tile.tsx intentionally wires onBranchInNewChat to a no-op), but the button itself rendered unconditionally regardless of whether a real handler existed, so the "unsupported here" state was invisible to the user. This is the same class of problem as an earlier fixed bug where a dead Refresh button gave no feedback either.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx: the Branch button in AssistantActionBar now only renders when onBranchInNewChat is actually provided ({onBranchInNewChat && (...)}), matching the existing pattern already used for onDismissError and onRestoreToMessage — both of which are conditionally rendered for the same reason (per the comment in latest-actions.ts: presence, not just callability, is what these children gate on).
  • apps/desktop/src/app/chat/session-tile.tsx: no longer passes a no-op onBranchInNewChat={() => undefined} for tiled/branched chats; the prop is simply omitted, so the button now doesn't render there at all instead of rendering dead.
  • apps/desktop/src/app/chat/index.tsx: onBranchInNewChat on ChatViewProps is now optional (?), since omitting it is now a legitimate, intentional state rather than something every caller had to fake with a no-op.
  • apps/desktop/src/app/contrib/latest-actions.ts: the latestChatActions passthrough wrapper now uses the existing latestOptional helper for onBranchInNewChat instead of an unconditional call, consistent with how onDismissError/onRestoreToMessage are already wrapped.
  • apps/desktop/src/components/assistant-ui/thread/assistant-message.test.tsx (new): covers both states — button visible when a handler is passed, button absent when it isn't.

How to Test

  1. Open a normal chat, send a message, hover an assistant reply — the Branch icon is visible and clickable, as before (no regression).
  2. Use Branch in new chat to open a branched tab/tile.
  3. Hover an assistant reply inside that branched tile — before this fix, the Branch icon was visible and clicking it did nothing; after the fix, the icon isn't shown at all for messages in a tile, so there's nothing misleading to click.
  4. Run the new test file directly: npx vitest run src/components/assistant-ui/thread/assistant-message.test.tsx

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

image

alelpoan added 2 commits July 26, 2026 16:01
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.
Adds coverage for the bug NousResearch#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.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 26, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — approved

Verified the fix end to end. It's a clean, minimal fix that extends the codebase's existing conventions rather than inventing anything.

What it does right

  • AssistantActionBar now gates the Branch button on {onBranchInNewChat && (...)}, matching the exact pattern already used for onDismissError (assistant-message.tsx:114). Presence-gating is the established contract for these action-bar children.
  • latest-actions.ts switches onBranchInNewChat to the existing latestOptional() helper, consistent with every other optional handler (onDismissError, onRestoreToMessage, onTranscribeAudio, ...).
  • session-tile.tsx drops the () => undefined no-op instead of faking a handler, and ChatViewProps.onBranchInNewChat becomes optional to make omission a legitimate state. The chain is internally consistent.
  • Tooltip copy.branchNewChat resolves to 'Branch in new chat', matching the test's getByRole name.

Verification performed

  • mergeable: MERGEABLE / CLEAN — no conflicts.
  • The 4 changed source files typecheck with zero errors.
  • New test passes (2/2). Confirmed non-vacuous: reverting the conditional render makes the "button hidden when no handler" case fail as expected, then passes again once restored.

No blocking issues. LGTM.


Reviewed by Hermes Agent

@austinpickett
austinpickett merged commit c92417e into NousResearch:main Jul 27, 2026
37 checks passed
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…hat tile (NousResearch#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.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…hat tile (NousResearch#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 NousResearch#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants