mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): stop composer chips demoting to plaintext on every re-render
Two halves of one bug class: the composer treated 'rebuild the editor from serialized text' as routine, and serialized text couldn't express a slash pill. In-place commits: Chromium fragments text nodes around contenteditable=false chips, so the old commit path — whole token in ONE text node with the caret at its end — failed almost every keyboard pick and fell into the rebuild fallback. rangeBeforeCaret now walks the token backwards across sibling text nodes and refuses only at real boundaries (chip, <br>, block), making in-place replacement the default for picks, folder descends, Backspace path-ascends, and action items in both the main composer and the user-edit composer. Slash-pill hydration: renderComposerContents re-chipped @kind:value refs but never /command, so any surviving rebuild (draft restore, undo, the fallback above) demoted a committed command pill. A leading no-arg /command now hydrates back to its pill; arg-taking commands stay text since their tail may be uncommitted prose. Also: popover picks bank an undo point in the main composer, and Tab is swallowed while completions are in flight instead of moving focus out of the composer.
This commit is contained in:
parent
92856bc28a
commit
3ca4220fb3
4 changed files with 193 additions and 111 deletions
|
|
@ -16,6 +16,7 @@ import {
|
|||
placeCaretEnd,
|
||||
refChipElement,
|
||||
renderComposerContents,
|
||||
replaceBeforeCaret,
|
||||
slashChipElement
|
||||
} from '../rich-editor'
|
||||
import { detectTrigger, textBeforeCaret, type TriggerState } from '../text-utils'
|
||||
|
|
@ -29,6 +30,8 @@ interface UseComposerTriggerOptions {
|
|||
at: CompletionSource
|
||||
draftRef: MutableRefObject<string>
|
||||
editorRef: RefObject<HTMLDivElement | null>
|
||||
/** Bank the pre-commit state so a popover pick is a single undo step. */
|
||||
recordUndoPoint?: () => void
|
||||
requestMainFocus: () => void
|
||||
setComposerText: (text: string) => void
|
||||
slash: CompletionSource
|
||||
|
|
@ -47,6 +50,7 @@ export function useComposerTrigger({
|
|||
at,
|
||||
draftRef,
|
||||
editorRef,
|
||||
recordUndoPoint,
|
||||
requestMainFocus,
|
||||
setComposerText,
|
||||
slash
|
||||
|
|
@ -214,17 +218,31 @@ export function useComposerTrigger({
|
|||
return
|
||||
}
|
||||
|
||||
// Bank the pre-commit state first — every path below mutates the editor,
|
||||
// and a pick must be exactly one undo step.
|
||||
recordUndoPoint?.()
|
||||
|
||||
// Rebuild-from-text fallback for carets the range walk can't anchor (a
|
||||
// non-collapsed selection, a caret not preceded by contiguous text). It
|
||||
// re-renders the whole editor from serialized text, so it only runs when
|
||||
// the in-place path reports failure — never as the default.
|
||||
const rebuildWith = (render: (prefix: string) => void) => {
|
||||
const current = composerPlainText(editor)
|
||||
|
||||
render(current.slice(0, Math.max(0, current.length - trigger.tokenLength)))
|
||||
placeCaretEnd(editor)
|
||||
}
|
||||
|
||||
// Action items (e.g. "Browse all sessions…") run a side effect instead of
|
||||
// inserting a chip: strip the typed trigger token, then fire the action.
|
||||
const completionAction = (item.metadata as { action?: unknown } | undefined)?.action
|
||||
const runAction = typeof completionAction === 'string' ? COMPLETION_ACTIONS[completionAction] : undefined
|
||||
|
||||
if (runAction) {
|
||||
const current = composerPlainText(editor)
|
||||
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
|
||||
if (!replaceBeforeCaret(editor, trigger.tokenLength, document.createDocumentFragment())) {
|
||||
rebuildWith(prefix => renderComposerContents(editor, prefix))
|
||||
}
|
||||
|
||||
renderComposerContents(editor, prefix)
|
||||
placeCaretEnd(editor)
|
||||
draftRef.current = composerPlainText(editor)
|
||||
setComposerText(draftRef.current)
|
||||
closeTrigger()
|
||||
|
|
@ -247,19 +265,24 @@ export function useComposerTrigger({
|
|||
? String((item.metadata as { insertId?: unknown } | undefined)?.insertId ?? '')
|
||||
: ''
|
||||
|
||||
if (descendInto) {
|
||||
const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/`
|
||||
const current = composerPlainText(editor)
|
||||
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
|
||||
|
||||
renderComposerContents(editor, `${prefix}@${path}`)
|
||||
placeCaretEnd(editor)
|
||||
const finish = (keepOpen: boolean) => {
|
||||
draftRef.current = composerPlainText(editor)
|
||||
setComposerText(draftRef.current)
|
||||
requestMainFocus()
|
||||
window.setTimeout(refreshTrigger, 0)
|
||||
keepOpen ? window.setTimeout(refreshTrigger, 0) : closeTrigger()
|
||||
}
|
||||
|
||||
return
|
||||
if (descendInto) {
|
||||
const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/`
|
||||
const fragment = document.createDocumentFragment()
|
||||
|
||||
fragment.append(document.createTextNode(`@${path}`))
|
||||
|
||||
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
|
||||
rebuildWith(prefix => renderComposerContents(editor, `${prefix}@${path}`))
|
||||
}
|
||||
|
||||
return finish(true)
|
||||
}
|
||||
|
||||
// Picking a bare arg-taking command (e.g. `/personality`) shouldn't commit
|
||||
|
|
@ -280,67 +303,32 @@ export function useComposerTrigger({
|
|||
const slashKind = !expandsToArgs && trigger.kind === '/' ? slashChipKindForItem(item) : null
|
||||
const keepTriggerOpen = starter || (expandsToArgs && argumentMode !== 'text')
|
||||
|
||||
const finish = () => {
|
||||
draftRef.current = composerPlainText(editor)
|
||||
setComposerText(draftRef.current)
|
||||
requestMainFocus()
|
||||
keepTriggerOpen ? window.setTimeout(refreshTrigger, 0) : closeTrigger()
|
||||
}
|
||||
|
||||
const sel = window.getSelection()
|
||||
const range = sel?.rangeCount ? sel.getRangeAt(0) : null
|
||||
const node = range?.startContainer
|
||||
const offset = range?.startOffset ?? 0
|
||||
|
||||
if (!sel || !range || node?.nodeType !== Node.TEXT_NODE || offset < trigger.tokenLength) {
|
||||
const current = composerPlainText(editor)
|
||||
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
|
||||
|
||||
if (slashKind) {
|
||||
// Two-step arg picks (e.g. `/handoff` pill already inserted, now picking
|
||||
// the platform) land here because the caret sits past a contenteditable
|
||||
// chip. Rebuild the prefix and re-emit a single pill for the full command.
|
||||
renderComposerContents(editor, prefix)
|
||||
editor.append(slashChipElement(serialized, slashKind), document.createTextNode(' '))
|
||||
placeCaretEnd(editor)
|
||||
|
||||
return finish()
|
||||
}
|
||||
|
||||
renderComposerContents(editor, `${prefix}${text}`)
|
||||
placeCaretEnd(editor)
|
||||
|
||||
return finish()
|
||||
}
|
||||
|
||||
const replaceRange = document.createRange()
|
||||
replaceRange.setStart(node, offset - trigger.tokenLength)
|
||||
replaceRange.setEnd(node, offset)
|
||||
replaceRange.deleteContents()
|
||||
|
||||
const chip = slashKind
|
||||
? slashChipElement(serialized, slashKind)
|
||||
: directive
|
||||
? refChipElement(directive[1], directive[2])
|
||||
: null
|
||||
|
||||
if (chip) {
|
||||
const space = document.createTextNode(' ')
|
||||
const fragment = document.createDocumentFragment()
|
||||
fragment.append(chip, space)
|
||||
replaceRange.insertNode(fragment)
|
||||
const fragment = document.createDocumentFragment()
|
||||
|
||||
const caret = document.createRange()
|
||||
caret.setStart(space, 1)
|
||||
caret.collapse(true)
|
||||
sel.removeAllRanges()
|
||||
sel.addRange(caret)
|
||||
chip ? fragment.append(chip, document.createTextNode(' ')) : fragment.append(document.createTextNode(text))
|
||||
|
||||
return finish()
|
||||
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
|
||||
rebuildWith(prefix => {
|
||||
if (chip) {
|
||||
// The failed in-place attempt never consumed the fragment, so the
|
||||
// chip + trailing space land here instead. Appending the element
|
||||
// keeps mid-message slash pills alive — they have no text
|
||||
// hydration, unlike `@` refs and the leading command.
|
||||
renderComposerContents(editor, prefix)
|
||||
editor.append(fragment)
|
||||
} else {
|
||||
renderComposerContents(editor, `${prefix}${text}`)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
document.execCommand('insertText', false, text)
|
||||
finish()
|
||||
finish(keepTriggerOpen)
|
||||
}
|
||||
|
||||
/** Backspace inside an `@` path drops the last segment (`a/b/` → `a/`)
|
||||
|
|
@ -359,11 +347,23 @@ export function useComposerTrigger({
|
|||
const trimmed = trigger.query.replace(/\/$/, '')
|
||||
const parent = trimmed.slice(0, trimmed.lastIndexOf('/') + 1)
|
||||
|
||||
const current = composerPlainText(editor)
|
||||
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
|
||||
recordUndoPoint?.()
|
||||
|
||||
const fragment = document.createDocumentFragment()
|
||||
|
||||
fragment.append(document.createTextNode(`@${parent}`))
|
||||
|
||||
// In place first: the destructive re-render fallback rebuilds the editor
|
||||
// from text, which is exactly what used to demote a leading command pill
|
||||
// to plaintext on every Backspace inside a path.
|
||||
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
|
||||
const current = composerPlainText(editor)
|
||||
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
|
||||
|
||||
renderComposerContents(editor, `${prefix}@${parent}`)
|
||||
placeCaretEnd(editor)
|
||||
}
|
||||
|
||||
renderComposerContents(editor, `${prefix}@${parent}`)
|
||||
placeCaretEnd(editor)
|
||||
draftRef.current = composerPlainText(editor)
|
||||
setComposerText(draftRef.current)
|
||||
window.setTimeout(refreshTrigger, 0)
|
||||
|
|
|
|||
|
|
@ -346,7 +346,7 @@ export function ChatBar({
|
|||
triggerItems,
|
||||
triggerKeyConsumedRef,
|
||||
triggerLoading
|
||||
} = useComposerTrigger({ at, draftRef, editorRef, requestMainFocus, setComposerText, slash })
|
||||
} = useComposerTrigger({ at, draftRef, editorRef, recordUndoPoint, requestMainFocus, setComposerText, slash })
|
||||
|
||||
// Pull the live contentEditable text into draftRef + the AUI composer state
|
||||
// (which drives `hasComposerPayload` → the send button). Shared by the input
|
||||
|
|
@ -569,6 +569,17 @@ export function ChatBar({
|
|||
return
|
||||
}
|
||||
|
||||
// The popover is open but its items are still in flight (debounce + RPC).
|
||||
// Tab must not fall through to the browser — it would move focus out of
|
||||
// the composer mid-completion, which reads as the popover "eating" the
|
||||
// keypress. Swallow it; the refresh lands with the items.
|
||||
if (trigger && triggerLoading && triggerItems.length === 0 && event.key === 'Tab') {
|
||||
event.preventDefault()
|
||||
triggerKeyConsumedRef.current = true
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if (trigger && triggerItems.length > 0) {
|
||||
if (event.key === 'ArrowDown') {
|
||||
event.preventDefault()
|
||||
|
|
|
|||
|
|
@ -16,11 +16,22 @@ import {
|
|||
type SlashChipKind,
|
||||
slashIconElement
|
||||
} from '@/components/assistant-ui/directive-text'
|
||||
import {
|
||||
desktopSlashCommandArgumentMode,
|
||||
isDesktopSlashCommand,
|
||||
resolveDesktopCommand
|
||||
} from '@/lib/desktop-slash-commands'
|
||||
|
||||
export const RICH_INPUT_SLOT = 'composer-rich-input'
|
||||
|
||||
export const REF_RE = /@(file|folder|url|image|tool|line|terminal|session):(`[^`\n]+`|"[^"\n]+"|'[^'\n]+'|\S+)/g
|
||||
|
||||
/** A committed leading slash command: `/name` followed by whitespace. The
|
||||
* whitespace requirement is what separates a committed command (chips always
|
||||
* serialize with their auto-inserted trailing space) from one still being
|
||||
* typed, which must stay editable text. */
|
||||
const LEADING_SLASH_COMMAND_RE = /^\/[a-zA-Z][\w-]*(?=\s)/
|
||||
|
||||
const ESC: Record<string, string> = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }
|
||||
|
||||
export function escapeHtml(value: string) {
|
||||
|
|
@ -129,6 +140,24 @@ export function appendComposerContents(target: DocumentFragment | HTMLElement, t
|
|||
|
||||
export function renderComposerContents(target: HTMLElement, text: string) {
|
||||
target.replaceChildren()
|
||||
|
||||
// A leading `/command` hydrates back to its pill — parity with REF_RE for
|
||||
// `@` refs, so a full re-render from serialized text (draft restore, undo,
|
||||
// the trigger commit fallback) doesn't demote a committed command chip to
|
||||
// plain text. Only commands with NO argument stage qualify (skills, quick
|
||||
// commands, no-arg built-ins): their committed pill is exactly the bare
|
||||
// `/name`, so the boundary is unambiguous. Arg-taking commands (`/goal ship
|
||||
// it`, `/personality alice`) stay text — their tail may be prose that was
|
||||
// never committed. The trailing whitespace is load-bearing too: a committed
|
||||
// pill always serializes with its auto-inserted space, while a half-typed
|
||||
// `/wor` must stay editable text.
|
||||
const command = LEADING_SLASH_COMMAND_RE.exec(text)?.[0]
|
||||
|
||||
if (command && isDesktopSlashCommand(command) && desktopSlashCommandArgumentMode(command) === null) {
|
||||
target.append(slashChipElement(command, resolveDesktopCommand(command) ? 'command' : 'skill'))
|
||||
text = text.slice(command.length)
|
||||
}
|
||||
|
||||
appendComposerContents(target, text)
|
||||
}
|
||||
|
||||
|
|
@ -173,27 +202,84 @@ export function insertComposerContentsAtCaret(editor: HTMLElement, text: string)
|
|||
}
|
||||
}
|
||||
|
||||
/** Swap the `length` characters immediately before a collapsed caret for
|
||||
* `fragment`, leaving the caret after it. Returns whether it ran — a caret that
|
||||
* isn't inside a text node holding the whole token is left alone. */
|
||||
export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment: DocumentFragment) {
|
||||
/** Range covering exactly `length` serialized characters immediately before a
|
||||
* collapsed caret, spanning Chromium's split text nodes. Null when the caret
|
||||
* isn't a collapsed selection in `editor`, or when a chip/<br>/block boundary
|
||||
* interrupts before `length` characters are covered — a trigger token is
|
||||
* always contiguous text, so anything else means "don't touch the DOM here".
|
||||
*
|
||||
* This is what keeps chip insertion stable: Chromium fragments text nodes
|
||||
* around contenteditable=false chips on every edit, so any commit path that
|
||||
* demands the whole token inside ONE text node (the old check) degrades to a
|
||||
* full re-render as soon as a chip exists anywhere in the line. */
|
||||
export function rangeBeforeCaret(editor: HTMLElement, length: number): Range | null {
|
||||
const hit = composerSelectionRange(editor)
|
||||
|
||||
if (!hit?.range.collapsed) {
|
||||
return false
|
||||
if (!hit?.range.collapsed || length <= 0) {
|
||||
return null
|
||||
}
|
||||
|
||||
const { startContainer, startOffset } = hit.range
|
||||
let node: Node | null = hit.range.startContainer
|
||||
let offset = hit.range.startOffset
|
||||
|
||||
if (startContainer.nodeType !== Node.TEXT_NODE || startOffset < length) {
|
||||
return false
|
||||
// An element-positioned caret (common right after programmatic caret moves)
|
||||
// resolves to the end of the text node before it. A chip or <br> there means
|
||||
// no text token precedes the caret — bail rather than guess.
|
||||
if (node.nodeType !== Node.TEXT_NODE) {
|
||||
node = node.childNodes[offset - 1] ?? null
|
||||
|
||||
if (node?.nodeType !== Node.TEXT_NODE) {
|
||||
return null
|
||||
}
|
||||
|
||||
offset = (node.textContent || '').length
|
||||
}
|
||||
|
||||
let startNode = node as Text
|
||||
let startOffset = offset
|
||||
let remaining = length
|
||||
|
||||
while (remaining > 0) {
|
||||
if (startOffset >= remaining) {
|
||||
startOffset -= remaining
|
||||
remaining = 0
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
remaining -= startOffset
|
||||
|
||||
const prev: Node | null = startNode.previousSibling
|
||||
|
||||
if (prev?.nodeType !== Node.TEXT_NODE) {
|
||||
return null
|
||||
}
|
||||
|
||||
startNode = prev as Text
|
||||
startOffset = (prev.textContent || '').length
|
||||
}
|
||||
|
||||
const range = document.createRange()
|
||||
|
||||
range.setStart(startNode, startOffset)
|
||||
range.setEnd(hit.range.startContainer, hit.range.startOffset)
|
||||
|
||||
return range
|
||||
}
|
||||
|
||||
/** Swap the `length` characters immediately before a collapsed caret for
|
||||
* `fragment`, leaving the caret after it. Returns whether it ran. Spans split
|
||||
* text nodes (see rangeBeforeCaret) — a token typed around existing chips
|
||||
* still commits in place instead of falling back to a full re-render. */
|
||||
export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment: DocumentFragment) {
|
||||
const range = rangeBeforeCaret(editor, length)
|
||||
|
||||
if (!range) {
|
||||
return false
|
||||
}
|
||||
|
||||
const tail = fragment.lastChild
|
||||
|
||||
range.setStart(startContainer, startOffset - length)
|
||||
range.setEnd(startContainer, startOffset)
|
||||
range.deleteContents()
|
||||
range.insertNode(fragment)
|
||||
|
||||
|
|
@ -202,8 +288,11 @@ export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment
|
|||
}
|
||||
|
||||
range.collapse(true)
|
||||
hit.selection.removeAllRanges()
|
||||
hit.selection.addRange(range)
|
||||
|
||||
const selection = window.getSelection()
|
||||
|
||||
selection?.removeAllRanges()
|
||||
selection?.addRange(range)
|
||||
|
||||
return true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import {
|
|||
placeCaretEnd,
|
||||
refChipElement,
|
||||
renderComposerContents,
|
||||
replaceBeforeCaret,
|
||||
RICH_INPUT_SLOT
|
||||
} from '@/app/chat/composer/rich-editor'
|
||||
import { detectTrigger, textBeforeCaret, type TriggerState } from '@/app/chat/composer/text-utils'
|
||||
|
|
@ -321,41 +322,22 @@ export const UserEditComposer: FC<UserEditComposerProps> = ({ cwd, gateway, sess
|
|||
starter ? window.setTimeout(refreshTrigger, 0) : closeTrigger()
|
||||
}
|
||||
|
||||
const sel = window.getSelection()
|
||||
const range = sel?.rangeCount ? sel.getRangeAt(0) : null
|
||||
const node = range?.startContainer
|
||||
const offset = range?.startOffset ?? 0
|
||||
// In place first, spanning Chromium's split text nodes (see
|
||||
// rangeBeforeCaret). The re-render fallback only runs when the caret
|
||||
// genuinely can't anchor the token — it rebuilds from serialized text,
|
||||
// which re-chips `@` refs but resets the caret to the end.
|
||||
const fragment = document.createDocumentFragment()
|
||||
|
||||
if (!sel || !range || node?.nodeType !== Node.TEXT_NODE || offset < trigger.tokenLength) {
|
||||
directive
|
||||
? fragment.append(refChipElement(directive[1], directive[2]), document.createTextNode(' '))
|
||||
: fragment.append(document.createTextNode(text))
|
||||
|
||||
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
|
||||
const current = composerPlainText(editor)
|
||||
renderComposerContents(editor, `${current.slice(0, Math.max(0, current.length - trigger.tokenLength))}${text}`)
|
||||
placeCaretEnd(editor)
|
||||
|
||||
return finish()
|
||||
}
|
||||
|
||||
const replaceRange = document.createRange()
|
||||
replaceRange.setStart(node, offset - trigger.tokenLength)
|
||||
replaceRange.setEnd(node, offset)
|
||||
replaceRange.deleteContents()
|
||||
|
||||
if (directive) {
|
||||
const chip = refChipElement(directive[1], directive[2])
|
||||
const space = document.createTextNode(' ')
|
||||
const fragment = document.createDocumentFragment()
|
||||
fragment.append(chip, space)
|
||||
replaceRange.insertNode(fragment)
|
||||
|
||||
const caret = document.createRange()
|
||||
caret.setStart(space, 1)
|
||||
caret.collapse(true)
|
||||
sel.removeAllRanges()
|
||||
sel.addRange(caret)
|
||||
|
||||
return finish()
|
||||
}
|
||||
|
||||
document.execCommand('insertText', false, text)
|
||||
finish()
|
||||
},
|
||||
[aui, closeTrigger, recordUndoPoint, refreshTrigger, rememberInitialDraft, requestEditFocus, trigger]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue