fix(web): address skill row review feedback

This commit is contained in:
Yichen Jiang
2026-08-06 17:21:13 +08:00
parent 690f7ae035
commit a667d2cd64
39 changed files with 326 additions and 142 deletions

View File

@@ -2,5 +2,5 @@
# side as of the last confirmed-consistent state. Both languages carry equal authority;
# after editing either side, bring the other along and re-record with:
# pnpm run verify-translation-pairing --write packages/client/ui-skill/README.md
README.md: 2280c9302dbc46cff723752f88c47940f98417d5
README.zh.md: 0e9344ff63139f77461b02b48e18b0e94e54c223
README.md: ba9f1faae0f70a0f7bed4641e02703cc26bcb692
README.zh.md: f8210a885d201cbdc89d7a34704a819e80463d2c

View File

@@ -10,7 +10,7 @@ The `/client` export surface is the plugin body (`apply`/`inject`) only; the sou
## Skill tool row
The browser plugin also registers a keyed `skill` toolview in `conversation.chat.toolview`. A collapsed row renders the 16-pixel skill document-and-sparkle glyph, `Skill` title, separator, and requested skill name with the same neutral hierarchy as the Bash row; running calls carry the transcript shimmer, failures replace the name with the first error line, and interrupted calls use the warning state. A settled row expands as one whole-row disclosure into a bounded `Instructions` card containing the exact durable tool output, with the standard trajectory `Inspect` affordance when available. The row derives its name, lifecycle, and body only from the logged call/result slice, never from the current catalog, so cold replay remains stable even when installed skills or their descriptions change.
The browser plugin also registers a keyed `skill` toolview in `conversation.chat.toolview`. A collapsed row renders the 16-pixel skill document-and-sparkle glyph, `Skill` title, separator, and requested skill name with the same neutral hierarchy as the Bash row; running calls carry the transcript shimmer, failures replace the name with the first error line, and interrupted calls use the warning state. A settled row expands as one whole-row disclosure into a bounded `Instructions` card containing the exact durable tool output, with the standard trajectory `Inspect` affordance when available. The row derives its name, lifecycle, and body only from the logged call/result slice, using the history envelope's host-carried durable pair when pagination left the call event outside the window; it never reads the current catalog, so cold replay remains stable across page cuts and when installed skills or their descriptions change.
## Model Experience

View File

@@ -10,7 +10,7 @@ skill技能引用 source 的浏览器端:把 `/` 触发的 `skill` sourc
## skill 工具行
浏览器插件还会把一个 key 为 `skill` 的 toolview 注册进 `conversation.chat.toolview`。收起的行以与 Bash 行相同的中性色层级显示 16 像素的 skill 文档与闪光组合图标、`Skill` 标题、分隔符和请求加载的 skill 名称;运行中的调用带有 transcript文本记录的扫光效果失败时用错误首行替换名称中断调用则使用警告状态。已结算的行以整行作为展开入口展开后显示一个尺寸受限的 `Instructions` 卡片,其中原样呈现持久化的工具输出;可用时还会提供标准执行轨迹的 `Inspect` 入口。该行的名称、生命周期和正文只派生自已记录的调用/结果片段绝不读取当前 skill 目录,因此即使已安装的 skill 或其描述发生变化,冷回放仍保持稳定。
浏览器插件还会把一个 key 为 `skill` 的 toolview 注册进 `conversation.chat.toolview`。收起的行以与 Bash 行相同的中性色层级显示 16 像素的 skill 文档与闪光组合图标、`Skill` 标题、分隔符和请求加载的 skill 名称;运行中的调用带有 transcript文本记录的扫光效果失败时用错误首行替换名称中断调用则使用警告状态。已结算的行以整行作为展开入口展开后显示一个尺寸受限的 `Instructions` 卡片,其中原样呈现持久化的工具输出;可用时还会提供标准执行轨迹的 `Inspect` 入口。该行的名称、生命周期和正文只派生自已记录的调用/结果片段;分页将调用事件留在窗口外时,则使用 history envelope 中由 Host 携带的持久配对。该行绝不读取当前 skill 目录,因此冷回放在跨分页时,以及已安装的 skill 或其描述发生变化时均保持稳定。
## 模型体验

View File

@@ -4,7 +4,7 @@
import { useState, type KeyboardEvent, type ReactNode } from 'react'
import {
IconChevronDownOutline14, IconSkillOutline16, StateDot,
IconChevronDownOutline14, IconInspectOutline12, IconSkillOutline16, StateDot,
} from '@deepseek-ai/dsh-client-ui-primitives'
import type { ToolRowProps } from '@deepseek-ai/dsh-client-ui-conversation/client'
import type { PropsLocale } from '@deepseek-ai/dsh-client-ui-slots'
@@ -45,7 +45,8 @@ function skillName(argsRaw: string, callId: string): string {
return argsRaw === '' ? callId : firstLine(argsRaw)
}
/** Flatten the durable result exactly like the generic row's text fallback. */
/** Flatten durable result blocks under the generic tool-row text contract.
* Keep aligned with ui-conversation's contract/tool-call-model.ts `resultText`. */
function resultText(block: ToolRowProps['block']): string | null {
if (!('kind' in block)) return null
const parts: string[] = []
@@ -108,15 +109,6 @@ function stateStatus(state: SkillRowState, t: SkillRowProps['t']): string | null
}
}
/** Inspect affordance glyph shared with the transcript's other tool rows. */
function IconInspect() {
return (
<svg width="12" height="12" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg" aria-hidden>
<path d="M16 8L10.8571 12V10.552L14.1383 8L10.8571 5.448V4L16 8ZM5.14286 10.552L1.86171 8L5.14286 5.448V4L0 8L5.14286 12V10.552ZM9.02514 4L5.59657 12H6.84057L10.2691 4H9.02514Z" fill="currentColor" />
</svg>
)
}
/**
* Render one `skill` tool call as an accent summary and instructions disclosure.
* @param props - keyed toolview payload plus the skill locale seat.
@@ -129,7 +121,6 @@ export function SkillRow({ block, inspect, t }: SkillRowProps) {
const open = expanded && expandable
const status = stateStatus(model.state, t)
const summary = model.errorSummary ?? model.name
const ariaLabel = status === null ? `Skill ${summary}` : `${status} Skill ${summary}`
const toggleExpand = (): void => {
setExpanded(value => !value)
}
@@ -138,18 +129,20 @@ export function SkillRow({ block, inspect, t }: SkillRowProps) {
event.preventDefault()
toggleExpand()
}
const disclosureProps = expandable ? {
role: 'button' as const,
tabIndex: 0,
'aria-expanded': open,
onClick: toggleExpand,
onKeyDown: toggleFromKeyboard,
} : {}
const leading = disclosureLeading(model.state, open, expandable)
return (
<div className={css.card} data-tool="skill" data-state={model.state}>
<div
className={css.row}
data-expandable={expandable || undefined}
role={expandable ? 'button' : undefined}
tabIndex={expandable ? 0 : undefined}
aria-expanded={expandable ? open : undefined}
aria-label={expandable ? ariaLabel : undefined}
onClick={expandable ? toggleExpand : undefined}
onKeyDown={expandable ? toggleFromKeyboard : undefined}
{...disclosureProps}
>
<span className={css.leading}>{leading}</span>
{status !== null ? <span className={css.visuallyHidden}>{status}</span> : null}
@@ -167,7 +160,7 @@ export function SkillRow({ block, inspect, t }: SkillRowProps) {
</section>
{inspect !== undefined ? (
<button type="button" className={css.inspectButton} onClick={inspect}>
<IconInspect />
<IconInspectOutline12 />
Inspect
</button>
) : null}

View File

@@ -15,9 +15,10 @@ export const name = 'client-ui-skill-invariant'
export const inject = ['invariants']
/**
* No runtime invariant: a single slash-source registration whose disposal is
* proven by the HMR-safety spec — it emits no cordis events and owns no
* cross-plugin mutable state.
* No runtime invariant: the slash source, locale dictionaries, and keyed
* toolview are registry-owned registrations whose disposal is proven by the
* HMR-safety spec. They emit no cordis events and own no cross-plugin mutable
* state.
*/
const install: InvariantInstaller = () => {}

View File

@@ -14,6 +14,7 @@
import { Context } from 'cordis'
import { describe, expect, it, vi } from 'vitest'
import type { SessionId } from '@deepseek-ai/dsh-client-runtime/client'
import { SlotsService } from '@deepseek-ai/dsh-client-runtime/client'
import { SlashService } from '@deepseek-ai/dsh-client-ui-slash/client'
import type { ClientSessionContext, SlashSource } from '@deepseek-ai/dsh-client-ui-slash/client'
import { apply, inject } from '../src/client/index.ts'
@@ -25,33 +26,28 @@ type ListResult =
| { ok: false; error: { code: string; message: string; details: object } }
type ListFn = (payload: object, signal?: AbortSignal) => Promise<{ result: ListResult }>
interface PresentationRegistration {
name: string
key?: string
locale?: string
}
interface PresentationCapture {
registration?: PresentationRegistration
component?: unknown
slots: SlotsService
dictionaries: Array<{ namespace: string; dictionaries: unknown }>
localeDisposed: boolean
}
/** Provide the presentation registries and capture the plugin's registrations. */
function providePresentation(ctx: Context): PresentationCapture {
const capture: PresentationCapture = { dictionaries: [] }
const slots = new SlotsService(ctx)
slots.register({
name: 'root',
children: { 'conversation.chat.toolview': { kind: 'keyed', scope: 'session' } },
} as never, () => null)
const capture: PresentationCapture = {
slots,
dictionaries: [],
localeDisposed: false,
}
ctx.provide('locale', {
register(namespace: string, dictionaries: unknown) {
capture.dictionaries.push({ namespace, dictionaries })
return () => {}
},
})
ctx.provide('slots', {
inject(_name: string, factory: () => unknown) { factory() },
register(registration: PresentationRegistration, component: unknown) {
capture.registration = registration
capture.component = component
return () => {}
return () => { capture.localeDisposed = true }
},
})
return capture
@@ -110,10 +106,10 @@ describe('apply', () => {
ctx.provide('sessions', { subagentAddress: () => undefined })
const presentation = providePresentation(ctx)
await ctx.plugin({ inject: [...inject], apply }).await()
expect(presentation.registration).toEqual({
name: 'conversation.chat.toolview', key: 'skill', locale: 'skill',
})
expect(presentation.component).toBe(SkillToolRow)
const entry = presentation.slots.entries('conversation.chat.toolview')[0]
expect(entry?.options).toMatchObject({ key: 'skill' })
expect(entry?.locale).toBe('skill')
expect(entry?.component).toBe(SkillToolRow)
expect(presentation.dictionaries).toEqual([{
namespace: 'skill', dictionaries: {
zh: {
@@ -138,7 +134,7 @@ describe('apply', () => {
ctx.provide('sessions', {})
await ctx.plugin(SlashService).await()
ctx.provide('connection', { api: { skills: { list: listOk(CATALOG) } } })
providePresentation(ctx)
const presentation = providePresentation(ctx)
const fiber = ctx.plugin({ inject: [...inject], apply })
await fiber.await()
const slash = ctx.get('slash') as SlashService
@@ -153,6 +149,8 @@ describe('apply', () => {
// …and fiber teardown releases it.
await fiber.dispose()
expect(() => slash.registerSource(rival)).not.toThrow()
expect(presentation.slots.entries('conversation.chat.toolview')).toHaveLength(0)
expect(presentation.localeDisposed).toBe(true)
})
})

View File

@@ -53,7 +53,7 @@ describe('SkillRow', () => {
it('renders a compact Bash-shaped summary and discloses the exact instructions', () => {
const inspect = vi.fn()
const view = render(<SkillRow {...props(settled(), inspect)} />)
const row = screen.getByRole('button', { name: 'Skill dsh-manage-issues' })
const row = screen.getByRole('button', { name: 'Skilldsh-manage-issues' })
expect(row.getAttribute('aria-expanded')).toBe('false')
expect(view.container.querySelector('[data-tool="skill"]')?.getAttribute('data-state')).toBe('ok')
expect(view.container.querySelector('[data-tool="skill"] svg')?.getAttribute('width')).toBe('16')
@@ -97,7 +97,7 @@ describe('SkillRow', () => {
isError: true,
error: { name: 'SkillError', code: 'missing' },
}))} />)
const row = screen.getByRole('button', { name: 'skill 加载失败 Skill SkillError: missing resource' })
const row = screen.getByRole('button', { name: 'skill 加载失败SkillSkillError: missing resource' })
expect(view.container.querySelector('[data-tool="skill"]')?.getAttribute('data-state')).toBe('error')
expect(row.textContent).not.toContain('Check SKILL.md.')
fireEvent.click(row)
@@ -126,7 +126,7 @@ describe('SkillRow', () => {
isError: true,
error: { name: 'SkillError', code: 'missing' },
}))} />)
const errorRow = screen.getByRole('button', { name: 'skill 加载失败 Skill SkillError: missing' })
const errorRow = screen.getByRole('button', { name: 'skill 加载失败SkillSkillError: missing' })
fireEvent.click(errorRow)
expect(screen.getAllByText('SkillError: missing')).toHaveLength(2)
})