diff --git a/apps/desktop/src/app/chat/composer/directive-label.test.ts b/apps/desktop/src/app/chat/composer/directive-label.test.ts new file mode 100644 index 00000000000..f6efe18b42f --- /dev/null +++ b/apps/desktop/src/app/chat/composer/directive-label.test.ts @@ -0,0 +1,131 @@ +import type { Unstable_TriggerItem } from '@assistant-ui/core' +import { act, renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { hermesDirectiveFormatter } from '@/components/assistant-ui/directive-text' + +import { classify } from './hooks/use-at-completions' +import { useComposerTrigger } from './hooks/use-composer-trigger' +import { composerPlainText, RICH_INPUT_SLOT } from './rich-editor' + +/** A row exactly as tui_gateway's complete.path emits it, run through the + * real classify() the popover uses. */ +function backendRow(text: string, display: string, meta: string): Unstable_TriggerItem { + const c = classify({ text, display, meta }) + + return { + id: `${text}|0`, + type: c.type, + label: c.display, + metadata: { icon: c.type, display: c.display, meta: c.meta, rawText: text, insertId: c.insertId } + } +} + +function typed(text: string) { + const editor = document.createElement('div') + + editor.contentEditable = 'true' + editor.dataset.slot = RICH_INPUT_SLOT + document.body.append(editor) + editor.append(document.createTextNode(text)) + + const range = document.createRange() + + range.selectNodeContents(editor) + range.collapse(false) + + const sel = window.getSelection() + + sel?.removeAllRanges() + sel?.addRange(range) + + const editorRef = { current: editor as HTMLDivElement | null } + + const { result } = renderHook(() => + useComposerTrigger({ + at: { adapter: null, loading: false }, + draftRef: { current: text }, + editorRef, + requestMainFocus: vi.fn(), + setComposerText: vi.fn(), + slash: { adapter: null, loading: false } + }) + ) + + act(() => result.current.refreshTrigger()) + + return { editor, result } +} + +/** The label the sent message renders for a committed draft. */ +function sentLabel(draft: string) { + return hermesDirectiveFormatter + .parse(draft) + .filter((s): s is Extract => s.kind === 'mention') + .map(s => s.label) + .join(',') +} + +describe('one label per reference, on every surface', () => { + it('the popover row, the committed chip, and the sent chip all read the same', () => { + const cases = [ + { text: '@folder:apps/desktop/', display: 'desktop/', meta: 'dir' }, + { text: '@file:apps/desktop/src/main.tsx', display: 'main.tsx', meta: 'apps/desktop/src' }, + { text: '@folder:apps/desktop/src/', display: 'src/', meta: 'dir' } + ] + + for (const entry of cases) { + const item = backendRow(entry.text, entry.display, entry.meta) + const { editor, result } = typed('@desk') + + act(() => result.current.replaceTriggerWithChip(item)) + + const row = String((item.metadata as { display: string }).display) + const chip = editor.querySelector('[data-ref-text]')?.textContent ?? '' + + expect(chip).toBe(row) + expect(sentLabel(composerPlainText(editor))).toBe(row) + } + }) + + it('a folder pick reads as its path, not a bare basename', () => { + // `src` and `desktop` repeat all over a repo — the row you picked said + // where it was, and the chip has to keep saying it. + const item = backendRow('@folder:apps/desktop/', 'desktop/', 'dir') + + expect(item.label).toBe('apps/desktop/') + + const { editor, result } = typed('@desk') + + act(() => result.current.replaceTriggerWithChip(item)) + + expect(editor.querySelector('[data-ref-text]')?.textContent).toBe('apps/desktop/') + }) + + it('Tab-descend leaves the live query, and the scope when there is one', () => { + const { editor, result } = typed('@folder:desk') + + act(() => + result.current.replaceTriggerWithChip(backendRow('@folder:apps/desktop/', 'desktop/', 'dir'), { + descend: true + }) + ) + + // Mid-browse the editor holds the live query, scope included — that's the + // path being typed, not a label, and it's what the next completion reads. + expect(composerPlainText(editor)).toBe('@folder:apps/desktop/') + }) + + it('a url still reads host + path on every surface', () => { + const item = backendRow('@url:https://github.com/NousResearch/hermes-agent/pull/74533', '', '') + const { editor, result } = typed('@gith') + + act(() => result.current.replaceTriggerWithChip(item)) + + const expected = 'github.com/NousResearch/hermes-agent/pull/74533' + + expect(item.label).toBe(expected) + expect(editor.querySelector('[data-ref-text]')?.textContent).toBe(expected) + expect(sentLabel(composerPlainText(editor))).toBe(expected) + }) +}) diff --git a/apps/desktop/src/app/chat/composer/directive-scope.test.ts b/apps/desktop/src/app/chat/composer/directive-scope.test.ts new file mode 100644 index 00000000000..08d1804dfba --- /dev/null +++ b/apps/desktop/src/app/chat/composer/directive-scope.test.ts @@ -0,0 +1,164 @@ +import type { Unstable_TriggerItem } from '@assistant-ui/core' +import { act, renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { useComposerTrigger } from './hooks/use-composer-trigger' +import { pathifyRefs } from './path-refs' +import { composerPlainText, insertComposerContentsAtCaret, RICH_INPUT_SLOT } from './rich-editor' +import { detectTrigger, openDirectiveScope, textBeforeCaret } from './text-utils' +import { linkifyUrls } from './url-refs' + +function folderItem(rel: string): Unstable_TriggerItem { + const rawText = `@folder:${rel}/` + + return { + id: `${rawText}|0`, + type: 'folder', + label: rel.split('/').filter(Boolean).pop() ?? rel, + metadata: { icon: 'folder', display: `${rel}/`, meta: 'dir', rawText, insertId: `${rel}/` } + } +} + +/** Literally-typed text, caret `fromEnd` characters before the end. */ +function typed(text: string, fromEnd = 0) { + const editor = document.createElement('div') + + editor.contentEditable = 'true' + editor.dataset.slot = RICH_INPUT_SLOT + document.body.append(editor) + + const node = document.createTextNode(text) + + editor.append(node) + + const range = document.createRange() + + range.setStart(node, text.length - fromEnd) + range.collapse(true) + + const sel = window.getSelection() + + sel?.removeAllRanges() + sel?.addRange(range) + + return editor +} + +function withTrigger(editor: HTMLDivElement, draft: string) { + const editorRef = { current: editor as HTMLDivElement | null } + + const { result } = renderHook(() => + useComposerTrigger({ + at: { adapter: null, loading: false }, + draftRef: { current: draft }, + editorRef, + requestMainFocus: vi.fn(), + setComposerText: vi.fn(), + slash: { adapter: null, loading: false } + }) + ) + + act(() => result.current.refreshTrigger()) + + return result +} + +/** The composer's paste handler, minus the clipboard plumbing. */ +function paste(editor: HTMLDivElement, text: string) { + insertComposerContentsAtCaret(editor, pathifyRefs(linkifyUrls(text)), openDirectiveScope(editor)) +} + +describe('directive scope is a browse mode, not text to maintain', () => { + it('Tab-descend carries the scope down instead of dropping to a bare path', () => { + const editor = typed('@folder:apps/deskt') + const result = withTrigger(editor, '@folder:apps/deskt') + + expect(result.current.trigger).toMatchObject({ kind: '@', scope: 'folder', value: 'apps/deskt' }) + + act(() => result.current.replaceTriggerWithChip(folderItem('apps/desktop'), { descend: true })) + + expect(composerPlainText(editor)).toBe('@folder:apps/desktop/') + }) + + it('Backspace climbs the path, then drops the whole scope', () => { + const editor = typed('@folder:apps/desktop/') + const result = withTrigger(editor, '@folder:apps/desktop/') + + act(() => result.current.ascendTriggerPath()) + expect(composerPlainText(editor)).toBe('@folder:apps/') + + act(() => result.current.refreshTrigger()) + act(() => result.current.ascendTriggerPath()) + expect(composerPlainText(editor)).toBe('@folder:') + + // The scope is one unit: Backspace drops it whole rather than nibbling + // back through `:`, `r`, `e`, `d`, `l`, `o`, `f`. + act(() => result.current.refreshTrigger()) + act(() => result.current.ascendTriggerPath()) + expect(composerPlainText(editor)).toBe('@') + }) + + it('leaves Backspace alone when there is no scope and no path', () => { + const editor = typed('@apps') + const result = withTrigger(editor, '@apps') + + let handled = true + + act(() => { + handled = result.current.ascendTriggerPath() + }) + + expect(handled).toBe(false) + }) + + it('a pick mid-message keeps the trailing prose and consumes the whole token', () => { + const editor = typed('@folder:apps/deskt and some trailing words', 24) + const result = withTrigger(editor, '@folder:apps/deskt and some trailing words') + + act(() => result.current.replaceTriggerWithChip(folderItem('apps/desktop'))) + + expect(composerPlainText(editor)).toBe('@folder:`apps/desktop/` and some trailing words') + expect(editor.querySelector('[data-ref-kind="folder"]')).not.toBeNull() + }) + + it('pasting into an open @url: scope consumes it instead of stacking', () => { + const editor = typed('refer to @url:') + + paste(editor, 'https://github.com/NousResearch/hermes-agent/pull/74533') + + expect(composerPlainText(editor)).toBe('refer to @url:`https://github.com/NousResearch/hermes-agent/pull/74533`') + expect(editor.textContent).not.toContain('@url:@url:') + }) + + it('a normal paste with no open scope is untouched', () => { + const editor = typed('look at ') + + paste(editor, 'https://example.com/x') + + expect(composerPlainText(editor)).toBe('look at @url:`https://example.com/x`') + }) + + it('scope parsing leaves an unscoped @ query alone', () => { + expect(detectTrigger('@apps/desk')).toMatchObject({ kind: '@', value: 'apps/desk' }) + expect(detectTrigger('@apps/desk')?.scope).toBeUndefined() + }) + + it('openDirectiveScope only fires on an EMPTY scope', () => { + // The count is what a paste consumes: `@url:` is 5 characters of syntax + // the user never typed and shouldn't be left holding. + expect(openDirectiveScope(typed('@url:'))).toBe(5) + expect(openDirectiveScope(typed('@url:https://x.com'))).toBe(0) + expect(openDirectiveScope(typed('plain text'))).toBe(0) + }) + + it('chips stay atomic to scope detection', () => { + const editor = typed('@folder:apps/desktop/') + const result = withTrigger(editor, '@folder:apps/desktop/') + + act(() => result.current.replaceTriggerWithChip(folderItem('apps/desktop'))) + + // A committed chip is one object-replacement char, so a fresh `@` typed + // after it opens an unscoped browse rather than inheriting the old scope. + expect(detectTrigger(`${textBeforeCaret(editor)}@`)?.scope).toBeUndefined() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-at-completions.test.ts b/apps/desktop/src/app/chat/composer/hooks/use-at-completions.test.ts new file mode 100644 index 00000000000..b133a7060ff --- /dev/null +++ b/apps/desktop/src/app/chat/composer/hooks/use-at-completions.test.ts @@ -0,0 +1,105 @@ +import { act, renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { queryClient } from '@/lib/query-client' + +import { useAtCompletions } from './use-at-completions' + +function gatewayStub(latencyMs = 40) { + const calls: string[] = [] + + const gateway = { + request: vi.fn(async (_method: string, params: { word: string }) => { + calls.push(params.word) + await new Promise(r => setTimeout(r, latencyMs)) + + return { items: [{ text: `@folder:${params.word.slice(1)}x/`, display: 'x/', meta: 'dir' }] } + }) + } + + return { calls, gateway } +} + +function setup(latencyMs = 40) { + const { calls, gateway } = gatewayStub(latencyMs) + + const { result } = renderHook(() => + useAtCompletions({ gateway: gateway as never, sessionId: 's1', cwd: '/repo' }) + ) + + return { calls, result } +} + +/** Type a burst of keystrokes `gapMs` apart, like a person. */ +async function type(result: { current: { adapter: { search?: (q: string) => unknown } } }, queries: string[], gapMs: number) { + for (const q of queries) { + act(() => { + result.current.adapter.search?.(q) + }) + await act(async () => { + await vi.advanceTimersByTimeAsync(gapMs) + }) + } +} + +describe('PERF: @ path completions are cached and skip the debounce', () => { + it('serves a repeated query with no round trip and no spinner', async () => { + vi.useFakeTimers() + queryClient.clear() + + const { calls, result } = setup() + + // First visit to `apps/` pays the round trip. + await type(result, ['apps/'], 0) + await act(async () => { + await vi.advanceTimersByTimeAsync(200) + }) + + const afterFirst = calls.length + + expect(afterFirst).toBe(1) + + // Walk away and come back — Tab in, Backspace out, retype. Every one of + // these used to be a fresh git ls-files + rank on the backend. + await type(result, ['apps/desktop/', 'apps/', 'apps/desktop/', 'apps/'], 0) + await act(async () => { + await vi.advanceTimersByTimeAsync(200) + }) + + // Two distinct paths, so exactly two round trips total — the repeats are free. + expect(calls.length).toBe(2) + expect(result.current.loading).toBe(false) + + vi.useRealTimers() + }) + + it('a cached query paints without waiting out the debounce', async () => { + vi.useFakeTimers() + queryClient.clear() + + const { calls, result } = setup() + + await type(result, ['apps/'], 0) + await act(async () => { + await vi.advanceTimersByTimeAsync(200) + }) + + expect(calls.length).toBe(1) + + // Re-ask for the cached query and advance by far less than the 60ms + // debounce. A cached answer resolves in a microtask, so it must paint + // without the timer and without ever flipping the spinner on. + act(() => { + result.current.adapter.search?.('apps/') + }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(1) + }) + + expect(result.current.loading).toBe(false) + expect(calls.length).toBe(1) + + vi.useRealTimers() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-at-completions.ts b/apps/desktop/src/app/chat/composer/hooks/use-at-completions.ts index d56a6d57e71..5a53f46e9ea 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-at-completions.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-at-completions.ts @@ -1,7 +1,9 @@ import type { Unstable_TriggerAdapter, Unstable_TriggerItem } from '@assistant-ui/core' import { useCallback } from 'react' +import { refChipLabel } from '@/components/assistant-ui/directive-text' import type { HermesGateway } from '@/hermes' +import { cachedPathCompletion, hasCachedPathCompletion } from '@/lib/slash-completion-cache' import { normalize } from '@/lib/text' import type { CompletionEntry, CompletionPayload } from './use-live-completion-adapter' @@ -60,7 +62,14 @@ function classify(entry: CompletionEntry): { return { type: kind, insertId: rest, - display: textValue(entry.display, rest || `@${kind}:`), + // The row shows exactly what picking it produces. Upstream keeps one + // label per item and hands it to the chip verbatim (DirectiveNode's + // `__label = item.label`); our wire format is `@kind:value`, which can't + // carry a label the way their `:type[label]{name=id}` does, so the same + // invariant is held by deriving both ends from refChipLabel. Without + // this the list said `desktop/`, the editor said `apps/desktop/`, and + // the chip said `desktop` — three names for one folder. + display: rest ? refChipLabel(kind, rest) : textValue(entry.display, `@${kind}:`), meta: textValue(entry.meta) } } @@ -82,6 +91,11 @@ export function useAtCompletions(options: { const { gateway, sessionId, cwd } = options const enabled = Boolean(gateway) + // Cache key: the completion depends on the query AND the directory it's + // resolved against, so a cwd or session change can't serve another tree's + // listing. + const cacheKey = useCallback((query: string) => `${cwd ?? ''}|${sessionId ?? ''}|${query}`, [cwd, sessionId]) + const fetcher = useCallback( async (query: string): Promise => { const starters = starterEntries(query) @@ -102,7 +116,14 @@ export function useAtCompletions(options: { } try { - const result = await gateway.request<{ items?: CompletionEntry[] }>('complete.path', params) + // De-duplicated the same way `/` completions are. Walking a path is + // inherently repetitive — Tab into a folder, Backspace out, retype a + // segment — and every one of those steps used to be a fresh + // `git ls-files` + rank on the backend (~40ms of the ~50ms round trip + // measured on this repo's 8k files). + const result = await cachedPathCompletion(cacheKey(query), () => + gateway.request<{ items?: CompletionEntry[] }>('complete.path', params) + ) const items = result.items ?? [] return { items: items.length > 0 ? items : starters, query } @@ -110,7 +131,7 @@ export function useAtCompletions(options: { return { items: starters, query } } }, - [gateway, sessionId, cwd] + [cacheKey, gateway, sessionId, cwd] ) const toItem = useCallback((entry: CompletionEntry, index: number): Unstable_TriggerItem => { @@ -135,7 +156,13 @@ export function useAtCompletions(options: { } }, []) - return useLiveCompletionAdapter({ enabled, fetcher, toItem }) + // A query already in cache skips both the debounce and the loading state. + // This is what makes walking a tree feel instant rather than merely fast: + // the 60ms debounce exists to avoid a request per keystroke, and it buys + // nothing when the answer is already in hand. + const isCached = useCallback((query: string) => hasCachedPathCompletion(cacheKey(query)), [cacheKey]) + + return useLiveCompletionAdapter({ enabled, fetcher, isCached, toItem }) } /** Re-export `classify` for use by the formatter (insertion side). */ 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 b178369c4c1..51c57a7679e 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) @@ -322,62 +363,73 @@ export function useComposerTrigger({ const chip = slashKind ? slashChipElement(serialized, slashKind) : directive - ? refChipElement(directive[1], directive[2]) + ? // Carry the picked row's own label into the chip rather than letting + // it re-derive one from the value. Upstream's DirectiveNode does the + // same (`__label = item.label`), and it's what makes the list and the + // chip agree: you get the string you just read, not a second guess at + // it. Falls back to the shared deriver for callers with no label. + refChipElement(directive[1], directive[2], (item.metadata as { display?: string })?.display || item.label) : 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) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 19b71ed6543..2a957564066 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -67,7 +67,7 @@ import { import { useComposerScope } from './scope' import { ComposerStatusStack } from './status-stack' import { CodingStatusRow } from './status-stack/coding-row' -import { extractClipboardImageBlobs } from './text-utils' +import { extractClipboardImageBlobs, openDirectiveScope } from './text-utils' import { ComposerTriggerPopover } from './trigger-popover' import type { ChatBarProps } from './types' import { isRedoShortcut, isUndoShortcut } from './undo-history' @@ -491,8 +491,13 @@ export function ChatBar({ // Links in the paste land as `@url:` chips rather than a wall of URL text — // the same reference the "Add URL" dialog inserts, parsed in place so a link // mid-sentence keeps its position. Bare `@path` tokens promote the same way. + // A paste into an open `@url:`/`@file:` scope CONSUMES that scope instead of + // stacking on it — the scope is the browse mode the user is pasting into, + // not text they typed and want to keep (`@url:@url:\`https://…\``). + const scope = openDirectiveScope(event.currentTarget) + recordUndoPoint() - insertComposerContentsAtCaret(event.currentTarget, pathifyRefs(linkifyUrls(pastedText))) + insertComposerContentsAtCaret(event.currentTarget, pathifyRefs(linkifyUrls(pastedText)), scope) scheduleFlushEditorToDraft(event.currentTarget) } @@ -1138,6 +1143,7 @@ export function ChatBar({ loading={triggerLoading} onHover={setTriggerActive} onPick={replaceTriggerWithChip} + scope={trigger.scope} /> )} {!poppedOut && ( diff --git a/apps/desktop/src/app/chat/composer/inline-references.test.ts b/apps/desktop/src/app/chat/composer/inline-references.test.ts new file mode 100644 index 00000000000..952f6bd85ad --- /dev/null +++ b/apps/desktop/src/app/chat/composer/inline-references.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' + +import { refAttrs, refAttrsHtml } from '@/components/assistant-ui/directive-text' +import { REFERENCE_STYLES, referenceKind, referenceStyle } from '@/components/assistant-ui/reference-kinds' + +/** + * There is ONE inline-reference system: `class="ref"` + `data-ref=""`. + * A pasted link, an `@file:` chip, a `/skill`, a `@session:` the agent wrote — + * all the same markup, styled by the `.ref` rules in styles.css. + */ +describe('the inline reference contract', () => { + it('marks any element as a reference of a given kind', () => { + expect(refAttrs('file')).toEqual({ className: 'ref', 'data-ref': 'file' }) + expect(refAttrsHtml('skill')).toBe('class="ref" data-ref="skill"') + }) + + it('an unkinded reference is a plain link, not a broken one', () => { + // A bare external link has no kind — it keeps the default link colour + // rather than being tagged with a wrong one. + expect(refAttrs()).toEqual({ className: 'ref' }) + expect(refAttrsHtml()).toBe('class="ref"') + }) + + it('normalises an unknown kind instead of emitting it raw', () => { + // A kind CSS has no rule for would silently render unstyled; coercing to + // `other` keeps it inside the system. + expect(refAttrs('wat')['data-ref']).toBe('other') + expect(referenceKind('wat')).toBe('other') + }) + + it('ships no colour from TypeScript — the theme owns every accent', () => { + // The whole point of keying on `data-ref`: a skin restyles all references + // at once, and no hex or color-mix() is hardcoded in a component. + for (const [kind, style] of Object.entries(REFERENCE_STYLES)) { + expect(style, `${kind} must not carry a colour`).not.toHaveProperty('color') + } + + expect(JSON.stringify(refAttrs('url'))).not.toMatch(/color|#[0-9a-f]{3}/i) + }) + + it('gives every kind a glyph and a label', () => { + for (const [kind, style] of Object.entries(REFERENCE_STYLES)) { + expect(style.codicon, `${kind} codicon`).toBeTruthy() + expect(style.label, `${kind} label`).toBeTruthy() + + // Emoji rows render the emoji itself instead of a glyph. + if (kind !== 'emoji') { + expect(style.paths.length, `${kind} paths`).toBeGreaterThan(0) + } + } + }) + + it('keeps commands and skills visually distinct', () => { + // Different data-ref values, so the stylesheet can accent them apart. + expect(refAttrs('skill')['data-ref']).not.toBe(refAttrs('command')['data-ref']) + expect(referenceStyle('skill').codicon).not.toBe(referenceStyle('command').codicon) + }) +}) + +describe('references are text, not badges', () => { + it('carries no layout, padding, or background of its own', () => { + // Everything visual lives in the stylesheet. If a component starts adding + // its own chrome here, that's the drift this system exists to prevent. + const { className } = refAttrs('file') + + expect(className).toBe('ref') + + for (const chrome of ['bg-', 'rounded', 'px-', 'py-', 'border', 'inline-flex', 'text-[']) { + expect(className).not.toContain(chrome) + } + }) +}) diff --git a/apps/desktop/src/app/chat/composer/rich-editor.ts b/apps/desktop/src/app/chat/composer/rich-editor.ts index 9958c3a4b09..6285a4dcfd0 100644 --- a/apps/desktop/src/app/chat/composer/rich-editor.ts +++ b/apps/desktop/src/app/chat/composer/rich-editor.ts @@ -7,15 +7,15 @@ * plain-text round-trip. */ import { - DIRECTIVE_CHIP_CLASS, directiveIconElement, directiveIconSvg, formatRefValue, + refAttrsHtml, refChipLabel, - slashChipClass, type SlashChipKind, slashIconElement } from '@/components/assistant-ui/directive-text' +import { referenceKind } from '@/components/assistant-ui/reference-kinds' import { slashCommandMatches, type SlashCommandScanOptions } from './slash-refs' @@ -60,42 +60,38 @@ export function refChipHtml(kind: string, rawValue: string, displayLabel?: strin const label = displayLabel || refChipLabel(kind, id) - return `${directiveIconSvg(kind)}${escapeHtml(label)}` + return `${directiveIconSvg(kind)}${escapeHtml(label)}` } export function refChipElement(kind: string, rawValue: string, displayLabel?: string) { const id = unquoteRef(rawValue) const text = `@${kind}:${quoteRefValue(id)}` const chip = document.createElement('span') - const label = document.createElement('span') chip.contentEditable = 'false' chip.title = id chip.dataset.refText = text chip.dataset.refId = id chip.dataset.refKind = kind - chip.className = DIRECTIVE_CHIP_CLASS - label.className = 'truncate' - label.textContent = displayLabel || refChipLabel(kind, id) - chip.append(directiveIconElement(kind), label) + chip.className = 'ref' + chip.dataset.ref = referenceKind(kind) + chip.append(directiveIconElement(kind), document.createTextNode(displayLabel || refChipLabel(kind, id))) return chip } -/** A non-editable pill for a picked slash command (`/skin nous`, `/tropes`). +/** A non-editable reference for a picked slash command (`/skin nous`, `/tropes`). * `data-ref-text` carries the literal command so `composerPlainText` round-trips * it back to the exact text that gets submitted. */ export function slashChipElement(command: string, kind: SlashChipKind, label?: string) { const chip = document.createElement('span') - const text = document.createElement('span') chip.contentEditable = 'false' chip.dataset.refText = command chip.dataset.slashKind = kind - chip.className = slashChipClass(kind) - text.className = 'truncate' - text.textContent = label || command - chip.append(slashIconElement(kind), text) + chip.className = 'ref' + chip.dataset.ref = kind + chip.append(slashIconElement(kind), document.createTextNode(label || command)) return chip } @@ -228,8 +224,24 @@ function atTokenBoundary(editor: HTMLElement, range: Range | null): boolean { * — Chromium's editing pipeline is ~O(n²) on large multiline blobs. * * The text arrives whole rather than typed, so a `/command` ending it is - * complete rather than half-written and chips like the rest. */ -export function insertComposerContentsAtCaret(editor: HTMLElement, text: string) { + * complete rather than half-written and chips like the rest. + * + * `consumeBefore` characters immediately before the caret are swallowed by the + * insert. That's how a paste into an open `@url:` scope replaces the scope + * instead of stacking on it (`@url:@url:\`https://…\``). */ +export function insertComposerContentsAtCaret(editor: HTMLElement, text: string, consumeBefore = 0) { + const scoped = consumeBefore > 0 ? rangeBeforeCaret(editor, consumeBefore) : null + + if (scoped) { + scoped.deleteContents() + scoped.collapse(true) + + const selection = window.getSelection() + + selection?.removeAllRanges() + selection?.addRange(scoped) + } + const hit = composerSelectionRange(editor) const fragment = document.createDocumentFragment() diff --git a/apps/desktop/src/app/chat/composer/text-utils.test.ts b/apps/desktop/src/app/chat/composer/text-utils.test.ts index ddb15ec677d..a2e54b2c07e 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.test.ts @@ -4,19 +4,19 @@ import { blobDedupeKey, detectTrigger, extractClipboardImageBlobs } from './text describe('detectTrigger', () => { it('detects a bare slash trigger with an empty query', () => { - expect(detectTrigger('/')).toEqual({ kind: '/', query: '', tokenLength: 1 }) + expect(detectTrigger('/')).toEqual({ kind: '/', query: '', tokenLength: 1, value: '' }) }) it('detects a slash command query', () => { - expect(detectTrigger('/skill')).toEqual({ kind: '/', query: 'skill', tokenLength: 6 }) + expect(detectTrigger('/skill')).toEqual({ kind: '/', query: 'skill', tokenLength: 6, value: 'skill' }) }) it('detects a bare at-mention trigger with an empty query', () => { - expect(detectTrigger('@')).toEqual({ kind: '@', query: '', tokenLength: 1 }) + expect(detectTrigger('@')).toEqual({ kind: '@', query: '', tokenLength: 1, value: '' }) }) it('detects an at-mention query', () => { - expect(detectTrigger('@file')).toEqual({ kind: '@', query: 'file', tokenLength: 5 }) + expect(detectTrigger('@file')).toEqual({ kind: '@', query: 'file', tokenLength: 5, value: 'file' }) }) it('returns null for plain text', () => { @@ -27,17 +27,20 @@ describe('detectTrigger', () => { expect(detectTrigger('/personality ')).toEqual({ kind: '/', query: 'personality ', - tokenLength: 13 + tokenLength: 13, + value: 'personality ' }) expect(detectTrigger('/personality alic')).toEqual({ kind: '/', query: 'personality alic', - tokenLength: 17 + tokenLength: 17, + value: 'personality alic' }) expect(detectTrigger('/tools enable foo')).toEqual({ kind: '/', query: 'tools enable foo', - tokenLength: 17 + tokenLength: 17, + value: 'tools enable foo' }) }) @@ -54,14 +57,25 @@ describe('detectTrigger', () => { it('keeps the at-mention live while walking into subfolders', () => { // A `/` inside the query is path navigation, not the end of the token — // the popover has to stay open so the next directory level can load. - expect(detectTrigger('@./')).toEqual({ kind: '@', query: './', tokenLength: 3 }) - expect(detectTrigger('@./src')).toEqual({ kind: '@', query: './src', tokenLength: 6 }) - expect(detectTrigger('@~/Desktop/')).toEqual({ kind: '@', query: '~/Desktop/', tokenLength: 11 }) - expect(detectTrigger('@/usr/local')).toEqual({ kind: '@', query: '/usr/local', tokenLength: 11 }) + expect(detectTrigger('@./')).toEqual({ kind: '@', query: './', tokenLength: 3, value: './' }) + expect(detectTrigger('@./src')).toEqual({ kind: '@', query: './src', tokenLength: 6, value: './src' }) + expect(detectTrigger('@~/Desktop/')).toEqual({ + kind: '@', + query: '~/Desktop/', + tokenLength: 11, + value: '~/Desktop/' + }) + expect(detectTrigger('@/usr/local')).toEqual({ + kind: '@', + query: '/usr/local', + tokenLength: 11, + value: '/usr/local' + }) expect(detectTrigger('@apps/desktop/src')).toEqual({ kind: '@', query: 'apps/desktop/src', - tokenLength: 17 + tokenLength: 17, + value: 'apps/desktop/src' }) }) @@ -70,20 +84,48 @@ describe('detectTrigger', () => { // assistant-ui's Lexical DirectivePlugin gets the same semantics from node // boundaries: typing a trigger right after a chip (no space) still opens // the popover, and a chip inside a token ends it. - expect(detectTrigger('\uFFFC@Desk')).toEqual({ kind: '@', query: 'Desk', tokenLength: 5 }) + expect(detectTrigger('\uFFFC@Desk')).toEqual({ kind: '@', query: 'Desk', tokenLength: 5, value: 'Desk' }) // Not position 0, so it's an inline reference — not a command invocation. - expect(detectTrigger('\uFFFC/cle')).toEqual({ inline: true, kind: '/', query: 'cle', tokenLength: 4 }) + expect(detectTrigger('\uFFFC/cle')).toEqual({ + inline: true, + kind: '/', + query: 'cle', + tokenLength: 4, + value: 'cle' + }) // The placeholder itself never leaks into a query. expect(detectTrigger('@a\uFFFCb')).toBeNull() }) - it('keeps the at-mention live for a typed ref kind with a path', () => { + it('splits a typed ref kind off as the browse scope', () => { + // `@folder:apps/` is ONE token with TWO parts. The kind is the mode the + // user is browsing in, so it's held as `scope` rather than left in `value` + // for every consumer to re-parse (or, worse, to preserve by hand). expect(detectTrigger('@file:src/main.tsx')).toEqual({ kind: '@', query: 'file:src/main.tsx', - tokenLength: 18 + scope: 'file', + tokenLength: 18, + value: 'src/main.tsx' }) - expect(detectTrigger('@folder:apps/')).toEqual({ kind: '@', query: 'folder:apps/', tokenLength: 13 }) + expect(detectTrigger('@folder:apps/')).toEqual({ + kind: '@', + query: 'folder:apps/', + scope: 'folder', + tokenLength: 13, + value: 'apps/' + }) + // A scope with nothing typed after it is the empty-browse state the + // popover renders a header for. + expect(detectTrigger('@url:')).toEqual({ kind: '@', query: 'url:', scope: 'url', tokenLength: 5, value: '' }) + }) + + it('only treats a KNOWN kind as a scope', () => { + // `@teknium1:` is a handle with a colon, not a directive — inventing a + // scope for it would make Backspace eat the whole word. + expect(detectTrigger('@teknium1:')?.scope).toBeUndefined() + expect(detectTrigger('@teknium1:')?.value).toBe('teknium1:') + expect(detectTrigger('@localhost:8080')?.scope).toBeUndefined() }) it('still ends the at-mention token at whitespace', () => { @@ -92,15 +134,28 @@ describe('detectTrigger', () => { expect(detectTrigger('look at @apps/desktop')).toEqual({ kind: '@', query: 'apps/desktop', - tokenLength: 13 + tokenLength: 13, + value: 'apps/desktop' }) }) it('treats a mid-message slash as an inline reference', () => { // Skills have to be reachable anywhere in a prompt, not just at position 0. - expect(detectTrigger('hello /')).toEqual({ kind: '/', inline: true, query: '', tokenLength: 1 }) - expect(detectTrigger('hello /clean')).toEqual({ kind: '/', inline: true, query: 'clean', tokenLength: 6 }) - expect(detectTrigger('text\n/skill')).toEqual({ kind: '/', inline: true, query: 'skill', tokenLength: 6 }) + expect(detectTrigger('hello /')).toEqual({ kind: '/', inline: true, query: '', tokenLength: 1, value: '' }) + expect(detectTrigger('hello /clean')).toEqual({ + kind: '/', + inline: true, + query: 'clean', + tokenLength: 6, + value: 'clean' + }) + expect(detectTrigger('text\n/skill')).toEqual({ + kind: '/', + inline: true, + query: 'skill', + tokenLength: 6, + value: 'skill' + }) }) it('does not carry arg completion into an inline slash reference', () => { diff --git a/apps/desktop/src/app/chat/composer/text-utils.ts b/apps/desktop/src/app/chat/composer/text-utils.ts index 3224716b27c..e8828fe890b 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.ts @@ -9,9 +9,27 @@ export interface TriggerState { inline?: boolean kind: '@' | '/' | ':' query: string + /** The `@kind:` prefix the user scoped the browse to, when there is one. */ + scope?: DirectiveScope tokenLength: number + /** `query` minus the `scope:` prefix — the value actually being typed. */ + value: string } +/** Directive kinds the `@` popover can scope a browse to. Mirrors the starter + * rows in use-at-completions and the gateway's `complete.path` prefixes. */ +export const DIRECTIVE_SCOPES = ['file', 'folder', 'url', 'image', 'tool', 'git'] as const + +export type DirectiveScope = (typeof DIRECTIVE_SCOPES)[number] + +// Picking "attach a folder" types `@folder:` into the editor, and everything +// after it is the value being browsed. Parsing that prefix off the query is +// what lets the rest of the composer treat it as the BROWSE MODE it is rather +// than characters the user has to maintain by hand — Tab-descending has to +// carry it down, Backspace has to drop it whole, and a chip landing on it has +// to consume it. +const AT_SCOPE_RE = new RegExp(`^(${DIRECTIVE_SCOPES.join('|')}):(.*)$`) + // `@` triggers stop at the first whitespace — `@file:path` and `@diff` are // single tokens, and a path is part of that token: `@./src/`, `@~/Desktop/`, // and `@file:src/foo` all have to keep the popover live while the user walks @@ -146,6 +164,15 @@ export function textBeforeCaret(editor: HTMLDivElement): string | null { return serializeTextBefore(editor, range.startContainer, range.startOffset) } +/** How many characters of directive scope the caret is sitting inside (`@url:` + * with nothing typed after it), or 0. A paste lands INTO that scope: the scope + * text is consumed rather than left in front of the chip as leftover syntax. */ +export function openDirectiveScope(editor: HTMLDivElement): number { + const trigger = detectTrigger(textBeforeCaret(editor) ?? '') + + return trigger?.kind === '@' && trigger.scope && !trigger.value ? trigger.tokenLength : 0 +} + export function detectTrigger(textBefore: string): TriggerState | null { // An inline `/skill` is a reference dropped into prose, so it carries no args // and the whole match is the token the chip replaces. Checked before the @@ -156,19 +183,28 @@ export function detectTrigger(textBefore: string): TriggerState | null { if (inline) { const query = inline[2] ?? '' - return { inline: true, kind: '/', query, tokenLength: 1 + query.length } + return { inline: true, kind: '/', query, tokenLength: 1 + query.length, value: query } } const command = SLASH_COMMAND_TRIGGER_RE.exec(textBefore) if (command) { - return { kind: '/', query: command[2], tokenLength: 1 + command[2].length } + return { kind: '/', query: command[2], tokenLength: 1 + command[2].length, value: command[2] } } const at = AT_TRIGGER_RE.exec(textBefore) if (at) { - return { kind: '@', query: at[2], tokenLength: 1 + at[2].length } + const query = at[2] + const scoped = AT_SCOPE_RE.exec(query) + + return { + kind: '@', + query, + ...(scoped ? { scope: scoped[1] as DirectiveScope } : {}), + tokenLength: 1 + query.length, + value: scoped ? (scoped[2] ?? '') : query + } } // After `@` so a directive starter's colon (`@file:`) stays an `@` query. @@ -177,7 +213,7 @@ export function detectTrigger(textBefore: string): TriggerState | null { const emoji = $reactionsEnabled.get() ? EMOJI_TRIGGER_RE.exec(textBefore) : null if (emoji) { - return { kind: ':', query: emoji[2], tokenLength: 1 + emoji[2].length } + return { kind: ':', query: emoji[2], tokenLength: 1 + emoji[2].length, value: emoji[2] } } return null diff --git a/apps/desktop/src/app/chat/composer/trigger-popover-parity.test.tsx b/apps/desktop/src/app/chat/composer/trigger-popover-parity.test.tsx new file mode 100644 index 00000000000..e3f2ab08eba --- /dev/null +++ b/apps/desktop/src/app/chat/composer/trigger-popover-parity.test.tsx @@ -0,0 +1,151 @@ +import { render, screen } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { ComposerTriggerPopover } from './trigger-popover' + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + composer: { + lookupLoading: 'Loading…', + lookupNoMatches: 'No matches', + lookupTry: 'Try', + lookupOr: 'or' + } + } + }) +})) + +function atItem(type: string, display: string, rawText: string, meta = '') { + return { + id: `${rawText}|0`, + type, + label: display, + metadata: { icon: type, display, meta, rawText, insertId: display } + } +} + +function slashItem(command: string, group: string, meta = '') { + return { + id: `${command}|0`, + type: 'slash', + label: command.slice(1), + metadata: { command, display: command, meta, group, action: '', rawText: command } + } +} + +const noop = () => {} + +/** The rendered shape of one row: does it have an icon, and how is it laid out? + * Icons are codicons — an ``, not an SVG. */ +function rowShape(root: HTMLElement) { + const row = root.querySelector('button') as HTMLElement + const icon = row.querySelector('i.codicon') + + return { + hasIcon: Boolean(icon), + iconName: icon?.className.match(/codicon-([\w-]+)/)?.[1], + classes: row.className + } +} + +describe('@ and / are one menu', () => { + it('a slash row has an icon, just like an @ row', () => { + const at = render( + + ) + + const atShape = rowShape(at.container) + at.unmount() + + const slash = render( + + ) + + const slashShape = rowShape(slash.container) + + // The whole point: `/` used to render a stacked, icon-less row. + expect(slashShape.hasIcon).toBe(true) + expect(atShape.hasIcon).toBe(true) + expect(slashShape.classes).toBe(atShape.classes) + + // And the glyph reflects the kind, not one generic bullet. + expect(slashShape.iconName).toBe('zap') + expect(atShape.iconName).toBe('folder') + }) + + it('renders the name and description for both kinds', () => { + const { rerender } = render( + + ) + + expect(screen.getByText('/work')).toBeTruthy() + expect(screen.getByText('Start in a worktree')).toBeTruthy() + + rerender( + + ) + + expect(screen.getByText('src/main.tsx')).toBeTruthy() + expect(screen.getByText('src')).toBeTruthy() + }) + + it('an emoji row stays icon-less — the emoji IS the icon', () => { + const { container } = render( + + ) + + expect(rowShape(container).hasIcon).toBe(false) + }) + + it('labels the active browse scope from the shared vocabulary', () => { + render( + + ) + + expect(screen.getByText('Folders')).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/trigger-popover.tsx b/apps/desktop/src/app/chat/composer/trigger-popover.tsx index 0cf57700c14..32b40323034 100644 --- a/apps/desktop/src/app/chat/composer/trigger-popover.tsx +++ b/apps/desktop/src/app/chat/composer/trigger-popover.tsx @@ -1,39 +1,14 @@ import type { Unstable_TriggerItem } from '@assistant-ui/core' import { Fragment } from 'react' +import { referenceKind, referenceStyle } from '@/components/assistant-ui/reference-kinds' import { Codicon } from '@/components/ui/codicon' import { GlyphSpinner } from '@/components/ui/glyph-spinner' import { useI18n } from '@/i18n' import { cn } from '@/lib/utils' import { COMPLETION_DRAWER_BELOW_CLASS, COMPLETION_DRAWER_CLASS, CompletionDrawerEmpty } from './completion-drawer' - -const AT_ICON_BY_TYPE: Record = { - diff: 'diff', - file: 'book', - folder: 'folder', - git: 'git-branch', - image: 'file-media', - simple: 'symbol-misc', - staged: 'diff-added', - tool: 'tools', - url: 'globe' -} - -function atIcon(item: Unstable_TriggerItem) { - const meta = item.metadata as { rawText?: string } | undefined - const raw = meta?.rawText || item.label - - if (raw.startsWith('@diff')) { - return AT_ICON_BY_TYPE.diff - } - - if (raw.startsWith('@staged')) { - return AT_ICON_BY_TYPE.staged - } - - return AT_ICON_BY_TYPE[item.type] || AT_ICON_BY_TYPE.simple -} +import type { DirectiveScope } from './text-utils' interface RowMeta { display?: string @@ -41,12 +16,41 @@ interface RowMeta { meta?: string } -const ROW_BASE_CLASS = [ - 'relative flex w-full cursor-default select-none rounded-md px-2 py-1 text-left', +/** The kind a row represents, for its icon. `@` rows carry it as the item type; + * `/` rows carry it as the completion group (Skills / Themes / Commands). */ +function rowKind(item: Unstable_TriggerItem, isSlash: boolean): string { + const meta = item.metadata as (RowMeta & { rawText?: string }) | undefined + + if (isSlash) { + const group = meta?.group?.trim() + + return group === 'Skills' ? 'skill' : group === 'Themes' ? 'theme' : 'command' + } + + // The gateway's simple refs (`@diff`, `@staged`) share one item type, so the + // glyph comes from the directive itself. + const raw = meta?.rawText || item.label + + if (raw.startsWith('@diff')) { + return 'diff' + } + + if (raw.startsWith('@staged')) { + return 'staged' + } + + return item.type +} + +const ROW_CLASS = [ + 'relative flex w-full cursor-default select-none items-center gap-2 rounded-md px-2 py-1 text-left', 'outline-hidden transition-colors hover:bg-(--ui-bg-tertiary)', 'data-[highlighted]:bg-(--ui-bg-tertiary) data-[highlighted]:text-foreground' ].join(' ') +const GROUP_HEADER_CLASS = + 'select-none px-2 pb-0.5 text-[0.625rem] font-semibold uppercase tracking-wider text-(--ui-text-tertiary)' + interface ComposerTriggerPopoverProps { activeIndex: number items: readonly Unstable_TriggerItem[] @@ -55,8 +59,24 @@ interface ComposerTriggerPopoverProps { onHover: (index: number) => void onPick: (item: Unstable_TriggerItem) => void placement?: 'bottom' | 'top' + /** The `@kind:` browse the list is filtered to, when there is one. Rendered + * as a header so the scope reads as the mode it is — the raw `@folder:` in + * the editor otherwise looks like syntax the user has to finish by hand. */ + scope?: DirectiveScope } +/** + * The composer's completion list, for every trigger. + * + * `@` and `/` render through the SAME row: icon, name, description. They used + * to be two layouts in one file — `@` horizontal with an icon, `/` stacked with + * none — which is why picking a file and picking a skill felt like features + * from different apps. Icons and accents come from the shared reference + * vocabulary, so a row looks like the chip it will become. + * + * `:` emoji is the one exception: the emoji IS the icon, so it renders as a + * single display string (Slack's exact shape). + */ export function ComposerTriggerPopover({ activeIndex, items, @@ -64,11 +84,13 @@ export function ComposerTriggerPopover({ loading, onHover, onPick, - placement = 'top' + placement = 'top', + scope }: ComposerTriggerPopoverProps) { const { t } = useI18n() const copy = t.composer const isSlash = kind === '/' + const isEmoji = kind === ':' let lastGroup: string | undefined @@ -80,6 +102,7 @@ export function ComposerTriggerPopover({ onMouseDown={event => event.preventDefault()} role="listbox" > + {scope &&
{referenceStyle(scope).label}
} {items.length === 0 ? ( loading ? (
@@ -93,7 +116,7 @@ export function ComposerTriggerPopover({ {copy.lookupTry} @file: {copy.lookupOr}{' '} @folder:. - ) : kind === ':' ? ( + ) : isEmoji ? ( <> {copy.lookupTry} :joy:. @@ -114,61 +137,28 @@ export function ComposerTriggerPopover({ const isFirstHeader = lastGroup === undefined lastGroup = group || lastGroup const active = index === activeIndex + const refKind = referenceKind(rowKind(item, isSlash)) return ( - {showHeader && ( -
- {group} -
- )} + {showHeader &&
{group}
}