mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): one label per reference, on every surface
Picking a folder showed three different names for it: the popover row said
`desktop/`, the editor mid-browse said `apps/desktop/`, and the committed chip
said `desktop`. Each surface derived its own label from the value.
Upstream keeps ONE label on the directive node and hands it to every consumer
verbatim (`DirectiveNode.__label = item.label`, rendered by `decorate()` and
carried through `:type[label]{name=id}`). Our wire format is `@kind:value`,
which can't carry a label, so the same invariant is held by deriving both ends
from refChipLabel: the popover row now shows exactly what the chip will show,
and the commit path passes the picked row's label into the chip rather than
letting it re-derive one.
refChipLabel keeps the directory for the reason it already keeps a URL's path —
a bare basename can't tell two references apart, and `src`, `index.ts`, and
`main.tsx` repeat all over a repo. Browsing into apps/desktop/ only to be
handed a chip reading `desktop` throws away the context you navigated for.
This commit is contained in:
parent
1f72da4f88
commit
47180dec83
4 changed files with 155 additions and 9 deletions
131
apps/desktop/src/app/chat/composer/directive-label.test.ts
Normal file
131
apps/desktop/src/app/chat/composer/directive-label.test.ts
Normal file
|
|
@ -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<typeof s, { kind: 'mention' }> => 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)
|
||||
})
|
||||
})
|
||||
|
|
@ -363,7 +363,12 @@ 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
|
||||
|
|
|
|||
|
|
@ -31,8 +31,10 @@ describe('hermesDirectiveFormatter.parse', () => {
|
|||
it('still parses unquoted paths', () => {
|
||||
const segments = hermesDirectiveFormatter.parse('@file:src/main.tsx the entry point')
|
||||
|
||||
// The label keeps its directory: it's the same string the `@` popover row
|
||||
// showed, and a bare `main.tsx` can't tell two files apart.
|
||||
expect(segments).toEqual([
|
||||
{ kind: 'mention', type: 'file', label: 'main.tsx', id: 'src/main.tsx' },
|
||||
{ kind: 'mention', type: 'file', label: 'src/main.tsx', id: 'src/main.tsx' },
|
||||
{ kind: 'text', text: ' the entry point' }
|
||||
])
|
||||
})
|
||||
|
|
|
|||
|
|
@ -332,10 +332,18 @@ function parseDirectiveText(text: string): Unstable_DirectiveSegment[] {
|
|||
return segments
|
||||
}
|
||||
|
||||
/** Display text for a `@kind:value` chip. Shared with the composer's
|
||||
* contenteditable chips so a link reads the same before and after send: the
|
||||
* host leads (scheme and `www.` are noise) and the path rides along for the
|
||||
* chip's `truncate` to cut — a bare hostname can't tell two links apart. */
|
||||
/** The single display label for a `@kind:value` reference — used by the `@`
|
||||
* popover row, the composer chip, and the sent-message chip alike, so a
|
||||
* reference reads the same everywhere. Upstream keeps one label on the
|
||||
* directive node and hands it to every consumer verbatim; our wire format
|
||||
* (`@kind:value`) can't carry a label, so this is the shared deriver that
|
||||
* holds the same invariant.
|
||||
*
|
||||
* Paths keep their directory for the reason links keep theirs: a bare
|
||||
* basename can't tell two references apart (`src`, `index.ts`, `main.tsx`
|
||||
* repeat all over a repo), and browsing into `apps/desktop/` only to be
|
||||
* handed a chip reading `desktop` throws away the context you navigated for.
|
||||
* The chip's `truncate` cuts the overflow. */
|
||||
export function refChipLabel(type: string, id: string): string {
|
||||
if (type === 'terminal') {
|
||||
return id || 'terminal'
|
||||
|
|
@ -356,9 +364,9 @@ export function refChipLabel(type: string, id: string): string {
|
|||
}
|
||||
}
|
||||
|
||||
const tail = id.split(/[\\/]/).filter(Boolean).pop()
|
||||
|
||||
return tail || id
|
||||
// `./` is noise the completer emits, not part of the reference. A trailing
|
||||
// slash is kept — it's what distinguishes a folder from a file.
|
||||
return id.replace(/^\.\//, '') || id
|
||||
}
|
||||
|
||||
function safeEmbeddedImages(text: string) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue