feat(feedback): add the Web surface for message feedback

Consume the durable message-feedback sidecar from #2217 in the browser:
per-message Like/Dislike with an optional note, contributed through a
declared assistant-actions slot.

- carry MessageId on finalized AssistantMessageNode so a target is nameable
- declare conversation.chat.assistant-actions and render it in the
  IconActions row between copy and branch
- hold one FeedbackController per Session with per-item ifVersion CAS,
  reconciling a version-conflict from the reply's authoritative item
- mount messageFeedbackRemote alongside goalsRemote
This commit is contained in:
Chinesezjc
2026-08-11 16:58:40 +08:00
parent 5d591e55c1
commit 526febc44d
72 changed files with 2095 additions and 22 deletions

View File

@@ -0,0 +1,197 @@
// @vitest-environment jsdom
/**
* ui-feedback browser half on a real cordis Context with fake slots/remote
* faces: the plugin registers the feedback entry at
* conversation.chat.assistant-actions, one controller per Session backs every
* message in that Session, a reconnect refreshes only Sessions that were
* already read, and registration plus controller disposal ride the plugin
* fiber (HMR safety). The node half and the invariant companion are exercised
* over the same Context.
*/
import { Context, Service } from '@deepseek-ai/cordis'
import { afterEach, describe, expect, it } from 'vitest'
import { cleanup } from '@testing-library/react'
import { SlotsService, type SessionId } from '@deepseek-ai/dsh-client-runtime/client'
import { LocaleService } from '@deepseek-ai/dsh-client-locale/client'
import type { MessageId } from '@deepseek-ai/dsh-client-connection/client'
import type { MessageFeedbackItem, MessageFeedbackVersion } from '@deepseek-ai/dsh-message-feedback/types'
import type { FeedbackInjected } from '../src/client/slots.ts'
import { apply, inject } from '../src/client/index.ts'
import { apply as nodeApply } from '../src/index.ts'
afterEach(cleanup)
const sid = (k: string): SessionId => k as SessionId
const MSG = 'm-1' as MessageId
const seeded: MessageFeedbackItem = {
messageId: MSG,
rating: 'positive',
version: 'v1' as MessageFeedbackVersion,
createdAt: 1,
updatedAt: 1,
}
/** Boot the plugin over fake faces; the Remote namespace records every call. */
async function bench() {
const ctx = new Context()
const calls: { method: string; request: unknown }[] = []
const messageFeedback = {
list: (request: unknown) => {
calls.push({ method: 'list', request })
return Promise.resolve({ ok: true as const, value: { items: [seeded] } })
},
put: (request: unknown) => {
calls.push({ method: 'put', request })
return Promise.resolve({ ok: true as const, value: seeded })
},
delete: (request: unknown) => {
calls.push({ method: 'delete', request })
return Promise.resolve({ ok: true as const, value: { absent: true as const } })
},
}
class RemoteService extends Service {
constructor(serviceCtx: Context) {
super(serviceCtx, 'remote')
}
}
new RemoteService(ctx)
ctx.provide('remote.messageFeedback', messageFeedback)
await ctx.plugin(SlotsService).await()
ctx.slots.register({
name: 'root',
children: { 'conversation.chat.assistant-actions': { kind: 'list', scope: 'session' } },
} as never, (() => null) as never)
ctx.provide('locale', new LocaleService(ctx))
const fiber = ctx.plugin({ inject: [...inject], apply })
return {
ctx,
fiber,
calls,
entry: () => {
const entry = ctx.slots.entries('conversation.chat.assistant-actions')[0]
if (entry === undefined) return undefined
return {
...entry.options,
locale: entry.locale,
inject: entry.inject as unknown as ((sessionId: SessionId) => FeedbackInjected) | undefined,
}
},
}
}
describe('ui-feedback browser plugin', () => {
it('registers the feedback entry with the documented id, order, and locale', async () => {
const b = await bench()
await b.fiber.await()
expect(b.entry()).toMatchObject({ id: 'feedback', order: 10, locale: 'feedback' })
expect(b.entry()?.inject).toBeTypeOf('function')
})
it('exposes the feedback hook plus the ensure/rate/clear verbs', async () => {
const b = await bench()
await b.fiber.await()
const face = b.entry()!.inject!(sid('s1'))
expect(face.hooks.feedback.getSnapshot()).toMatchObject({ status: 'cold' })
expect(face.ensure).toBeTypeOf('function')
expect(face.rate).toBeTypeOf('function')
expect(face.clear).toBeTypeOf('function')
})
it('shares one controller across every message in the same Session', async () => {
const b = await bench()
await b.fiber.await()
const first = b.entry()!.inject!(sid('s1'))
const second = b.entry()!.inject!(sid('s1'))
expect(first.hooks.feedback).toBe(second.hooks.feedback)
await first.ensure()
await second.ensure()
expect(b.calls.filter(call => call.method === 'list')).toHaveLength(1)
})
it('keeps separate Sessions on separate controllers', async () => {
const b = await bench()
await b.fiber.await()
const one = b.entry()!.inject!(sid('s1'))
const two = b.entry()!.inject!(sid('s2'))
expect(one.hooks.feedback).not.toBe(two.hooks.feedback)
await one.ensure()
await two.ensure()
expect(b.calls.filter(call => call.method === 'list').map(call => call.request)).toEqual([
{ sessionId: 's1' },
{ sessionId: 's2' },
])
})
it('routes rate and clear to the Remote with the addressed message', async () => {
const b = await bench()
await b.fiber.await()
const face = b.entry()!.inject!(sid('s1'))
expect(await face.rate(MSG, 'negative', 'wrong answer')).toEqual({ ok: true })
expect(await face.clear(MSG)).toEqual({ ok: true })
expect(b.calls.filter(call => call.method === 'put')[0]?.request).toMatchObject({
sessionId: 's1', messageId: MSG, rating: 'negative', note: 'wrong answer',
})
expect(b.calls.filter(call => call.method === 'delete')[0]?.request).toMatchObject({
sessionId: 's1', messageId: MSG,
})
})
it('refreshes only Sessions already read when the connection resets', async () => {
const b = await bench()
await b.fiber.await()
const warm = b.entry()!.inject!(sid('warm'))
await warm.ensure()
b.entry()!.inject!(sid('cold'))
const before = b.calls.filter(call => call.method === 'list').length
b.ctx.emit('connection/reset')
await Promise.resolve()
const reads = b.calls.filter(call => call.method === 'list')
expect(reads).toHaveLength(before + 1)
expect(reads.at(-1)?.request).toEqual({ sessionId: 'warm' })
})
it('withdraws the registration and disposes controllers with the plugin fiber', async () => {
const b = await bench()
await b.fiber.await()
const face = b.entry()!.inject!(sid('s1'))
await face.ensure()
await b.fiber.dispose()
expect(b.ctx.slots.entries('conversation.chat.assistant-actions')).toHaveLength(0)
// A disposed controller refuses further mutations, so no request outlives the fiber.
const before = b.calls.length
expect(await face.rate(MSG, 'positive')).toMatchObject({ ok: false, error: { code: 'disposed' } })
expect(b.calls).toHaveLength(before)
})
it('re-registers cleanly when the plugin is reloaded', async () => {
const b = await bench()
await b.fiber.await()
await b.fiber.dispose()
const reloaded = b.ctx.plugin({ inject: [...inject], apply })
await reloaded.await()
expect(b.ctx.slots.entries('conversation.chat.assistant-actions')).toHaveLength(1)
expect(b.entry()).toMatchObject({ id: 'feedback' })
})
it('the node half applies without host-side behavior', () => {
// The invariant companion is mounted by the vitest-wide invariant host on
// every Context this suite creates; its registration is covered there.
expect(() => { nodeApply() }).not.toThrow()
})
})

View File

@@ -0,0 +1,273 @@
/**
* FeedbackController: the browser-local object layer over one Session's
* message-feedback sidecar. These specs pin the per-item compare-and-set
* contract — every mutation sends the version last observed, a conflict
* reconciles from the authoritative item carried by the reply, mutations
* serialize per Session, and a disposed controller stops publishing.
*/
import { describe, expect, it, vi } from 'vitest'
import type { MessageId, SessionId } from '@deepseek-ai/dsh-client-connection/client'
import type {
MessageFeedbackItem, MessageFeedbackVersion,
} from '@deepseek-ai/dsh-message-feedback/types'
import { FeedbackController, type MessageFeedbackRemote } from '../src/client/controller.ts'
const SESSION = 's-1' as SessionId
const MSG = 'm-1' as MessageId
const OTHER = 'm-2' as MessageId
const version = (v: string): MessageFeedbackVersion => v as MessageFeedbackVersion
function item(overrides: Partial<MessageFeedbackItem> = {}): MessageFeedbackItem {
return {
messageId: MSG,
rating: 'positive',
version: version('v1'),
createdAt: 1,
updatedAt: 1,
...overrides,
}
}
/** A recording fake Remote whose per-method answers are scripted per call. */
function fakeRemote(script: Partial<MessageFeedbackRemote> = {}) {
const calls: { method: string; request: unknown }[] = []
const record = <K extends keyof MessageFeedbackRemote>(
method: K,
real: MessageFeedbackRemote[K] | undefined,
fallback: Awaited<ReturnType<MessageFeedbackRemote[K]>>,
): MessageFeedbackRemote[K] =>
((request: Parameters<MessageFeedbackRemote[K]>[0]) => {
calls.push({ method, request })
return real === undefined
? Promise.resolve(fallback)
: (real as (input: typeof request) => ReturnType<MessageFeedbackRemote[K]>)(request)
}) as MessageFeedbackRemote[K]
const remote: MessageFeedbackRemote = {
list: record('list', script.list, { ok: true, value: { items: [] } }),
put: record('put', script.put, { ok: true, value: item() }),
delete: record('delete', script.delete, { ok: true, value: { absent: true } }),
}
return { remote, calls }
}
describe('FeedbackController', () => {
it('seeds the view from one list read and keys items by message id', async () => {
const seeded = item({ note: 'good' })
const { remote, calls } = fakeRemote({
list: () => Promise.resolve({ ok: true, value: { items: [seeded] } }),
})
const controller = new FeedbackController(remote, SESSION)
expect(controller.getSnapshot().status).toBe('cold')
expect(await controller.ensure()).toEqual({ ok: true })
const view = controller.getSnapshot()
expect(view.status).toBe('ready')
expect(view.items.get(MSG)).toEqual(seeded)
expect(calls).toEqual([{ method: 'list', request: { sessionId: SESSION } }])
})
it('collapses concurrent loads onto one in-flight read', async () => {
const { remote, calls } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
await Promise.all([controller.ensure(), controller.ensure(), controller.refresh()])
expect(calls.filter(call => call.method === 'list')).toHaveLength(1)
})
it('sends ifVersion null for a first rating and the observed version afterwards', async () => {
const first = item({ version: version('v1') })
const second = item({ version: version('v2'), rating: 'negative' })
const { remote, calls } = fakeRemote({
put: request => Promise.resolve({
ok: true,
value: (request as { rating: string }).rating === 'positive' ? first : second,
}),
})
const controller = new FeedbackController(remote, SESSION)
expect(await controller.rate(MSG, 'positive')).toEqual({ ok: true })
expect(await controller.rate(MSG, 'negative')).toEqual({ ok: true })
const puts = calls.filter(call => call.method === 'put').map(call => call.request)
expect(puts[0]).toMatchObject({ messageId: MSG, rating: 'positive', ifVersion: null })
expect(puts[1]).toMatchObject({ messageId: MSG, rating: 'negative', ifVersion: version('v1') })
expect(controller.getSnapshot().items.get(MSG)).toEqual(second)
})
it('forwards an optional note and omits the field when absent', async () => {
const { remote, calls } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
await controller.rate(MSG, 'positive', 'helpful')
await controller.rate(OTHER, 'negative')
const puts = calls.filter(call => call.method === 'put').map(call => call.request as Record<string, unknown>)
expect(puts[0]?.note).toBe('helpful')
expect(puts[1]).not.toHaveProperty('note')
})
it('reconciles a version conflict from the authoritative item without refetching', async () => {
const authoritative = item({ version: version('v9'), rating: 'negative', note: 'changed elsewhere' })
const { remote, calls } = fakeRemote({
put: () => Promise.resolve({
ok: false,
error: { code: 'version-conflict', current: authoritative },
}),
})
const controller = new FeedbackController(remote, SESSION)
expect(await controller.rate(MSG, 'positive')).toEqual({
ok: false,
error: { code: 'version-conflict', message: 'feedback changed elsewhere' },
})
expect(controller.getSnapshot().items.get(MSG)).toEqual(authoritative)
expect(calls.filter(call => call.method === 'list')).toHaveLength(1)
})
it('drops the local item when a conflict reports the feedback is gone', async () => {
const { remote } = fakeRemote({
list: () => Promise.resolve({ ok: true, value: { items: [item()] } }),
delete: () => Promise.resolve({
ok: false,
error: { code: 'version-conflict', current: null },
}),
})
const controller = new FeedbackController(remote, SESSION)
await controller.ensure()
expect(await controller.clear(MSG)).toMatchObject({ ok: false, error: { code: 'version-conflict' } })
expect(controller.getSnapshot().items.has(MSG)).toBe(false)
})
it('deletes with the observed version and removes the item on success', async () => {
const { remote, calls } = fakeRemote({
list: () => Promise.resolve({ ok: true, value: { items: [item({ version: version('v7') })] } }),
})
const controller = new FeedbackController(remote, SESSION)
await controller.ensure()
expect(await controller.clear(MSG)).toEqual({ ok: true })
expect(calls.filter(call => call.method === 'delete')[0]?.request)
.toEqual({ sessionId: SESSION, messageId: MSG, ifVersion: version('v7') })
expect(controller.getSnapshot().items.has(MSG)).toBe(false)
})
it('treats clearing an unrated message as already satisfied without a call', async () => {
const { remote, calls } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
expect(await controller.clear(MSG)).toEqual({ ok: true })
expect(calls.filter(call => call.method === 'delete')).toHaveLength(0)
})
it('serializes mutations so each one compares against the committed version', async () => {
let inFlight = 0
let overlapped = false
const versions = [version('v1'), version('v2')]
let index = 0
const { remote, calls } = fakeRemote({
put: async () => {
inFlight += 1
if (inFlight > 1) overlapped = true
await Promise.resolve()
inFlight -= 1
const next = versions[index] ?? version('vN')
index += 1
return { ok: true, value: item({ version: next }) }
},
})
const controller = new FeedbackController(remote, SESSION)
await Promise.all([controller.rate(MSG, 'positive'), controller.rate(MSG, 'negative')])
expect(overlapped).toBe(false)
const puts = calls.filter(call => call.method === 'put').map(call => call.request as Record<string, unknown>)
expect(puts[0]?.ifVersion).toBeNull()
expect(puts[1]?.ifVersion).toBe(version('v1'))
})
it('publishes an error status when the list read is rejected by the Host', async () => {
const { remote } = fakeRemote({
list: () => Promise.resolve({ ok: false, error: { code: 'session-not-found', sessionId: SESSION } }),
})
const controller = new FeedbackController(remote, SESSION)
expect(await controller.ensure()).toMatchObject({ ok: false, error: { code: 'session-not-found' } })
expect(controller.getSnapshot()).toMatchObject({
status: 'error',
error: 'this session is no longer persisted',
})
})
it('settles a transport throw as a result instead of rejecting', async () => {
const { remote } = fakeRemote({ list: () => Promise.reject(new Error('socket closed')) })
const controller = new FeedbackController(remote, SESSION)
expect(await controller.ensure()).toEqual({
ok: false,
error: { code: 'transport', message: 'socket closed' },
})
expect(controller.getSnapshot().status).toBe('error')
})
it('settles a mutation transport throw without corrupting the view', async () => {
const { remote } = fakeRemote({ put: () => Promise.reject(new Error('socket closed')) })
const controller = new FeedbackController(remote, SESSION)
expect(await controller.rate(MSG, 'positive')).toEqual({
ok: false,
error: { code: 'transport', message: 'socket closed' },
})
expect(controller.getSnapshot().items.has(MSG)).toBe(false)
})
it('notifies subscribers on publication and stops after unsubscribe', async () => {
const { remote } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
const listener = vi.fn()
const unsubscribe = controller.subscribe(listener)
await controller.ensure()
const seen = listener.mock.calls.length
expect(seen).toBeGreaterThan(0)
unsubscribe()
await controller.rate(MSG, 'positive')
expect(listener).toHaveBeenCalledTimes(seen)
})
it('contains a throwing subscriber at the observable boundary', async () => {
const { remote } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
controller.subscribe(() => { throw new Error('subscriber exploded') })
const healthy = vi.fn()
controller.subscribe(healthy)
await controller.ensure()
expect(healthy).toHaveBeenCalled()
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})
it('refuses mutations and stops publishing once disposed', async () => {
const { remote, calls } = fakeRemote()
const controller = new FeedbackController(remote, SESSION)
await controller.ensure()
const listener = vi.fn()
controller.subscribe(listener)
controller.dispose()
const before = calls.length
expect(await controller.rate(MSG, 'positive')).toMatchObject({ ok: false, error: { code: 'disposed' } })
expect(calls).toHaveLength(before)
expect(listener).not.toHaveBeenCalled()
})
})

View File

@@ -0,0 +1,172 @@
// @vitest-environment jsdom
/**
* FeedbackActions rendering and gestures: the rating buttons reflect the
* shared view, re-clicking the active rating retracts it, the note editor
* saves through the same rate verb, the Session's feedback is read on first
* interaction rather than on mount, and a rejected mutation surfaces inline
* without losing the authoritative state.
*/
import { useSyncExternalStore } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { cleanup, fireEvent, render, waitFor } from '@testing-library/react'
import { makeTranslate } from '@deepseek-ai/dsh-client-test-runtime'
import { zh as commonZh } from '@deepseek-ai/dsh-client-locale/src/locales/zh.ts'
import type { MessageId } from '@deepseek-ai/dsh-client-connection/client'
import type { MessageFeedbackItem, MessageFeedbackVersion } from '@deepseek-ai/dsh-message-feedback/types'
import { FeedbackActions } from '../src/client/FeedbackActions.tsx'
import type { FeedbackActionResult, FeedbackView } from '../src/client/controller.ts'
import { zh } from '../src/client/locales.ts'
afterEach(cleanup)
const MSG = 'm-1' as MessageId
const t = makeTranslate(zh, commonZh)
function item(overrides: Partial<MessageFeedbackItem> = {}): MessageFeedbackItem {
return {
messageId: MSG,
rating: 'positive',
version: 'v1' as MessageFeedbackVersion,
createdAt: 1,
updatedAt: 1,
...overrides,
}
}
/** Render the controls over a fixed view and recording verbs. */
function mount(options: {
current?: MessageFeedbackItem | undefined
rateResult?: FeedbackActionResult
clearResult?: FeedbackActionResult
} = {}) {
const view: FeedbackView = {
status: 'ready',
items: new Map(options.current === undefined ? [] : [[MSG, options.current]]),
error: null,
}
const ensure = vi.fn(() => Promise.resolve<FeedbackActionResult>({ ok: true }))
const rate = vi.fn(() => Promise.resolve(options.rateResult ?? { ok: true as const }))
const clear = vi.fn(() => Promise.resolve(options.clearResult ?? { ok: true as const }))
const useFeedback = (<T,>(select: (v: FeedbackView) => T): T =>
useSyncExternalStore(() => () => {}, () => select(view))) as never
const props = { messageId: MSG, ensure, rate, clear, useFeedback, t } as unknown as
Parameters<typeof FeedbackActions>[0]
return { ...render(<FeedbackActions {...props} />), ensure, rate, clear }
}
describe('FeedbackActions', () => {
it('renders both rating buttons unpressed with no recorded feedback', () => {
const ui = mount()
expect(ui.getByLabelText(zh['action.like']).getAttribute('aria-pressed')).toBe('false')
expect(ui.getByLabelText(zh['action.dislike']).getAttribute('aria-pressed')).toBe('false')
})
it('marks the recorded rating pressed and offers to retract it', () => {
const ui = mount({ current: item({ rating: 'negative' }) })
expect(ui.getByLabelText(zh['action.dislikeActive']).getAttribute('aria-pressed')).toBe('true')
expect(ui.getByLabelText(zh['action.like']).getAttribute('aria-pressed')).toBe('false')
})
it('reads the Session feedback on first interaction, once', () => {
const ui = mount()
const like = ui.getByLabelText(zh['action.like'])
fireEvent.pointerEnter(like)
fireEvent.pointerEnter(like)
fireEvent.focus(ui.getByLabelText(zh['action.dislike']))
expect(ui.ensure).toHaveBeenCalledTimes(1)
})
it('does not read the Session feedback on mount', () => {
const ui = mount()
expect(ui.ensure).not.toHaveBeenCalled()
})
it('rates a message that has no feedback yet', async () => {
const ui = mount()
fireEvent.click(ui.getByLabelText(zh['action.like']))
await waitFor(() => { expect(ui.rate).toHaveBeenCalledWith(MSG, 'positive', undefined) })
expect(ui.clear).not.toHaveBeenCalled()
})
it('replaces the opposite rating and carries the existing note forward', async () => {
const ui = mount({ current: item({ rating: 'positive', note: 'keep me' }) })
fireEvent.click(ui.getByLabelText(zh['action.dislike']))
await waitFor(() => { expect(ui.rate).toHaveBeenCalledWith(MSG, 'negative', 'keep me') })
})
it('retracts the feedback when the active rating is clicked again', async () => {
const ui = mount({ current: item({ rating: 'positive' }) })
fireEvent.click(ui.getByLabelText(zh['action.likeActive']))
await waitFor(() => { expect(ui.clear).toHaveBeenCalledWith(MSG) })
expect(ui.rate).not.toHaveBeenCalled()
})
it('saves a typed note through the rate verb and closes the editor', async () => {
const ui = mount({ current: item({ rating: 'positive' }) })
fireEvent.click(ui.getByText(zh['note.open']))
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: ' precise and short ' } })
fireEvent.click(ui.getByText(zh['note.save']))
await waitFor(() => { expect(ui.rate).toHaveBeenCalledWith(MSG, 'positive', 'precise and short') })
await waitFor(() => { expect(ui.queryByLabelText(zh['note.aria'])).toBeNull() })
})
it('clears the note when the editor is emptied', async () => {
const ui = mount({ current: item({ rating: 'positive', note: 'old note' }) })
fireEvent.click(ui.getByText('old note'))
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: ' ' } })
fireEvent.click(ui.getByText(zh['note.save']))
await waitFor(() => { expect(ui.rate).toHaveBeenCalledWith(MSG, 'positive', undefined) })
})
it('seeds the editor with the recorded note and abandons it on cancel', () => {
const ui = mount({ current: item({ rating: 'positive', note: 'old note' }) })
fireEvent.click(ui.getByText('old note'))
expect((ui.getByLabelText(zh['note.aria']) as HTMLTextAreaElement).value).toBe('old note')
fireEvent.click(ui.getByText(zh['note.cancel']))
expect(ui.queryByLabelText(zh['note.aria'])).toBeNull()
expect(ui.rate).not.toHaveBeenCalled()
})
it('offers no note editor before a rating is recorded', () => {
const ui = mount()
expect(ui.queryByText(zh['note.open'])).toBeNull()
})
it('reports a lost race with the conflict copy', async () => {
const ui = mount({
rateResult: { ok: false, error: { code: 'version-conflict', message: 'feedback changed elsewhere' } },
})
fireEvent.click(ui.getByLabelText(zh['action.like']))
await waitFor(() => { expect(ui.getByText(zh['error.conflict'])).toBeTruthy() })
})
it('reports any other failure with the generic copy', async () => {
const ui = mount({
rateResult: { ok: false, error: { code: 'target-not-found', message: 'no such message' } },
})
fireEvent.click(ui.getByLabelText(zh['action.like']))
await waitFor(() => { expect(ui.getByText(zh['error.generic'])).toBeTruthy() })
})
})