From 4fb4d78989719153ce90176e39836e25747b9bb7 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 30 Jul 2026 02:04:40 -0500 Subject: [PATCH] fix(desktop): keep the browse scope through Tab, Backspace, and a mid-message pick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the commit engine, all from treating the scope as loose text: Tab-descend rebuilt the token as a bare `@apps/desktop/`, silently widening an explicit @folder: browse back to files and forcing the committed chip to re-guess its kind from a trailing slash. It now carries the scope down. Backspace only handled a path, so at `@folder:` it fell through to character deletion and nibbled back out through the directive syntax one key at a time. It now drops the scope as one unit, mirroring Tab's one-key descent. The rebuild fallback sliced tokenLength off the END of the draft, which assumed the trigger was the last thing in the editor — a pick made mid-message chopped the trailing prose off and stranded a partial `folder:` in front of the chip. rebuildAroundCaret splits around the caret instead, and is shared with the ascend path and the edit composer rather than hand-rolled three times. Also drop the chip's auto-inserted trailing space when the caret already has whitespace after it, so a mid-sentence pick doesn't leave a double space. --- .../composer/hooks/use-composer-trigger.ts | 123 ++++++++++++------ 1 file changed, 85 insertions(+), 38 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts index b178369c4c1d..e2839c62d1fc 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts @@ -12,15 +12,59 @@ import { slashCommandToken } from '../composer-utils' import { + appendComposerContents, + caretOffsetInEditor, composerPlainText, - placeCaretEnd, + placeCaretAtOffset, refChipElement, renderComposerContents, replaceBeforeCaret, + RICH_INPUT_SLOT, slashChipElement } from '../rich-editor' import { detectTrigger, textBeforeCaret, type TriggerState } from '../text-utils' +/** + * 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. + * + * The split is around the CARET, not the end of the draft. Slicing + * `length - tokenLength` off the end assumed the trigger token was the last + * thing in the editor: a completion picked mid-message chopped the trailing + * prose off and stranded a partial `folder:` in front of the chip, because the + * window it removed wasn't the token the user was typing. + */ +export function rebuildAroundCaret(editor: HTMLDivElement, tokenLength: number, insert: DocumentFragment | string) { + const current = composerPlainText(editor) + const caret = caretOffsetInEditor(editor) + const prefix = current.slice(0, Math.max(0, caret - tokenLength)) + const suffix = current.slice(caret) + + if (typeof insert === 'string') { + renderComposerContents(editor, `${prefix}${insert}${suffix}`) + placeCaretAtOffset(editor, prefix.length + insert.length) + + return + } + + // Measure before appending — moving a fragment empties it. Appending the + // element rather than re-serializing keeps mid-message slash pills alive: + // they have no text hydration, unlike `@` refs and the leading command. + const scratch = document.createElement('div') + + scratch.dataset.slot = RICH_INPUT_SLOT + scratch.append(insert.cloneNode(true)) + + const inserted = composerPlainText(scratch) + + renderComposerContents(editor, prefix) + editor.append(insert) + appendComposerContents(editor, suffix) + placeCaretAtOffset(editor, prefix.length + inserted.length) +} + interface CompletionSource { adapter: Unstable_TriggerAdapter | null loading: boolean @@ -238,16 +282,8 @@ export function useComposerTrigger({ // 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) - } + const rebuildAround = (insert: DocumentFragment | string) => + rebuildAroundCaret(editor, trigger.tokenLength, insert) // 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. @@ -256,7 +292,7 @@ export function useComposerTrigger({ if (runAction) { if (!replaceBeforeCaret(editor, trigger.tokenLength, document.createDocumentFragment())) { - rebuildWith(prefix => renderComposerContents(editor, prefix)) + rebuildAround('') } draftRef.current = composerPlainText(editor) @@ -290,12 +326,17 @@ export function useComposerTrigger({ if (descendInto) { const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/` + // Carry the browse scope down with the path. Dropping it turned an + // explicit `@folder:` browse into a bare `@apps/desktop/` token halfway + // through, so the next completion silently widened back to files and the + // committed chip had to re-guess the kind from a trailing slash. + const scope = trigger.scope ? `${trigger.scope}:` : '' const fragment = document.createDocumentFragment() - fragment.append(document.createTextNode(`@${path}`)) + fragment.append(document.createTextNode(`@${scope}${path}`)) if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) { - rebuildWith(prefix => renderComposerContents(editor, `${prefix}@${path}`)) + rebuildAround(`@${scope}${path}`) } return finish(true) @@ -325,59 +366,65 @@ export function useComposerTrigger({ ? refChipElement(directive[1], directive[2]) : null + // The trailing space is a convenience for "keep typing after the chip", so + // it's wrong when the caret already has whitespace in front of it — a pick + // made mid-sentence would leave a double space in the prose. + const followedBySpace = /^\s/.test(composerPlainText(editor).slice(caretOffsetInEditor(editor))) const fragment = document.createDocumentFragment() - chip ? fragment.append(chip, document.createTextNode(' ')) : fragment.append(document.createTextNode(text)) + chip + ? fragment.append(chip, ...(followedBySpace ? [] : [document.createTextNode(' ')])) + : fragment.append(document.createTextNode(followedBySpace ? text.trimEnd() : text)) 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}`) - } - }) + // The failed in-place attempt never consumed the fragment, so the chip + + // trailing space are re-inserted around the caret here. Moving the + // element (rather than re-serializing) keeps mid-message slash pills + // alive — they have no text hydration, unlike `@` refs and the leading + // command. + rebuildAround(chip ? fragment : text) } finish(keepTriggerOpen) } /** Backspace inside an `@` path drops the last segment (`a/b/` → `a/`) - * instead of one character. Descending is one Tab per level, so climbing - * back out should cost one key too rather than a held delete. Returns + * instead of one character, and once the path is empty it drops the browse + * scope (`@folder:` → `@`) rather than nibbling `:`, `r`, `e`, `d`… back + * through the directive syntax the user never typed. Descending is one Tab + * per level, so climbing back out costs one key per level too. Returns * false when the caret isn't in a path, so keydown falls through. */ const ascendTriggerPath = () => { const editor = editorRef.current - if (!editor || trigger?.kind !== '@' || !trigger.query.includes('/')) { + if (!editor || trigger?.kind !== '@') { + return false + } + + const scope = trigger.scope ? `${trigger.scope}:` : '' + + if (!trigger.value.includes('/') && !scope) { return false } // Trailing slash means we're listing a folder's children: drop that - // folder. Otherwise a partial segment is typed — drop just that. - const trimmed = trigger.query.replace(/\/$/, '') + // folder. Otherwise a partial segment is typed — drop just that. With the + // value already empty, the only thing left to drop is the scope itself. + const trimmed = trigger.value.replace(/\/$/, '') const parent = trimmed.slice(0, trimmed.lastIndexOf('/') + 1) + const next = trigger.value ? `${scope}${parent}` : '' recordUndoPoint?.() const fragment = document.createDocumentFragment() - fragment.append(document.createTextNode(`@${parent}`)) + fragment.append(document.createTextNode(`@${next}`)) // 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) + rebuildAroundCaret(editor, trigger.tokenLength, `@${next}`) } draftRef.current = composerPlainText(editor)