feat: Assistant marketplace/catalog screen - #434
Conversation
- Implemented the useAssistantCatalog hook to manage assistant catalog presets. - Added functionality to load presets, filter them, and create new assistants. - Created unit tests for the useAssistantCatalog hook to ensure correct behavior. - Mocked necessary dependencies for testing, including translations and assistant mutations.
Refactor the assistant catalog logic to support locale-aware sorting and remove redundant loading states. - Update `assistantCatalogService` to accept a locale for group sorting and fix a translation alias for 'Emotion'. - Simplify `useAssistantCatalog` by removing the unused `isLoading` state and ensuring presets sync correctly with language changes. - Update `AssistantCatalogScreen` to use the improved hook and add accessibility properties (roles and labels) to the UI components. - Update i18n strings to rename "Market" to "Library" for better consistency and add missing catalog labels. - Update unit tests to reflect changes in the `useAssistantCatalog` return type.
eeee0717
left a comment
There was a problem hiding this comment.
Review: 首轮综合审查(通用质量 / 错误处理 / 测试覆盖 / 类型设计 四个维度)
先说好的:service 层是真行为测试且覆盖近乎完整(normalize 防御分支、过滤、tab 构建、DTO 映射全有);两份 JSON 数据抽查干净(en/zh 各 780 条,id 唯一且两端完全对应,字段结构统一);i18n en/zh 键对称;路由接线正确;DTO mapper 设计是全 PR 最好的一块——unknown 标注切断 JSON 类型推断传播、读写模型分离、modelId 走 branded UniqueModelId 构造,与现有领域类型零重复。整体分层纪律高于仓库平均水准。
以下按优先级列出需要处理的问题。
必须处理(阻塞合并)
1. hook 的 addPreset 是死代码,真正的写路径反而零测试覆盖
AssistantCatalogScreen.tsx:41-60 的 handleConfirmAdd 没有用 useAssistantCatalog 暴露的 addPreset,而是自己 useAssistantMutations().createAssistant + 再调一遍 toCreateAssistantDtoFromCatalogPreset,把同一逻辑重新实现了一遍。后果:hook 测试里 mutation 成功/失败两条用例(useAssistantCatalog.test.tsx:85-105)测的是生产零调用的 API,而真正在跑的 toast/dialog 状态机(成功关 dialog、失败保留、finally 复位 isAdding)没有任何测试兜底。三个 review 维度独立撞到了同一个问题。
建议:Screen 改用 hook 的 addPreset(mapper 即可停止对 UI 层导出),再仿照 ChatInputSurface.test.tsx 的模式补 screen 层用例:确认后 DTO 正确传入、成功/失败 toast variant、失败时 dialog 不关闭、isAdding 期间按钮 disabled。
2. 添加失败全链路零日志
AssistantCatalogScreen.tsx:52-56 的 catch { 不接收 error,createMutation 无 onError,AssistantService.create 也不落日志——线上任何一次添加失败都拿不到诊断信息。项目有成熟惯例(loggerService.withContext,见 DbService.ts:16)。至少加一行 logger.error('Failed to add catalog preset', error as Error, { presetId: preset.id })。同理 normalizePresets(assistantCatalogService.ts:56-67)静默丢条目/静默清空时应 logger.warn 掉落数量——这份数据从桌面仓库同步,一次坏同步会让整个目录变成「暂无预设」而毫无线索。
3. defaultModel 映射是「重试永远失败」的陷阱
assistantCatalogService.ts:120-122 无条件 dto.modelId = createUniqueModelId(...),而 AssistantService.resolveCreateModelId(AssistantService.ts:354-357)对未注册模型直接抛 validation error;移动端用户几乎不可能恰好注册了桌面预设的模型,于是「添加失败,请重试」但重试永远失败,真实原因被 catch-all 吃掉。另外 createUniqueModelId 对非法 provider 会同步 throw(model.ts:78-95),mapper 不捕获。当前 780×2 条数据 0 条带 defaultModel 所以是 latent,但这段代码就是专为该字段写的,触发只差一次上游数据更新。建议:模型未注册/非法时降级为不带 modelId 创建并 log,而不是让整个添加失败。
4. 搜索无防抖,逐键对全量 prompt 做 toLowerCase
filterAssistantCatalogPresets(assistantCatalogService.ts:96-104)+ AssistantCatalogScreen.tsx:27-30:每敲一个字符就在 JS 线程同步 lowercase 780 条 prompt(平均 1797 字符、最长 54,599 字符、全量约 1.4MB),低端机输入会明显卡顿。建议:搜索加防抖,或预计算小写索引,或干脆不把 prompt 纳入搜索(name+description 已够,超长 prompt 命中反而让结果嘈杂)。
5. 两份 1.6MB JSON 顶层静态 import,未用语言也终身常驻 JS 堆
assistantCatalogService.ts:6-9。代码注释说「Dynamic require 不被 Metro 支持」——那指的是模板字符串动态路径;字面量路径的条件 require() 是支持的。改成在 loadAssistantCatalogPresets 内按语言 require('./data/agents-zh.json'),未选中语言的 module factory 不执行,堆占用减半(bundle 仍含双语,那是离线双语的固有代价)。
建议处理(可随本 PR 或后续跟进)
- 校验层过度承诺:
normalizePresets的类型谓词只验id/name却断言整个AssistantCatalogPreset,坏类型字段(如group: [123]、prompt: 42)会穿透到localeCompare/toLowerCase造成整屏渲染崩溃。全仓校验都用 zod,这里手写谓词是风格孤岛——建议换 zod schema 顺手做 id 去重断言(keyExtractor依赖 id 唯一,目前只是数据侧碰巧成立)。 '__all__'哨兵三处裸写(service:94、service:104、Screen:20):提取CATALOG_ALL_TAB_ID常量。- 「reloads presets when language changes」测试名不符实(
useAssistantCatalog.test.tsx:107-121):hook 用useState惰性初始化,已挂载实例语言切换不会重载,该测试靠重新 mount 通过。要么 hook 改useMemo(() => load(language), [language])让行为与测试名一致,要么改名并补文档性测试。enabled参数同理是 init-only 的语义欺骗,唯一调用方写死true,建议直接删。 - 死代码清理:
UseAssistantCatalogReturn零引用(要么给 hook 标注返回类型让Promise<{id}>收窄生效,要么删)、getPresetKey生产零调用、defaultModel.name/group连 mapper 都不读、i18ncommon.loading键全仓未引用。 - i18n 命名:屏幕标题在
library.assistant_catalog.*,其余键在assistant.catalog.*,建议收敛到后者;assistant.actions.catalog="Library" 与标题 "Assistant Library" 措辞也不统一。 - 数据里 15 条 emoji 带尾随空格(如
"🎯 "),mapper 的 trim 已吸收写侧,但展示侧preset.emoji || '🤖'(Screen:211)建议补.trim()。 - 失败 toast 与保持打开的 Dialog 的层叠关系建议真机验证一次(若 toast 层级低于 Dialog.Portal overlay,失败提示会被盖住)。
结论
REQUEST_CHANGES。功能方向和数据都没问题,架构分层也做得好;阻塞点集中在「声明与执行脱节」:写路径双通道且真实路径无测试、错误全链路无日志、defaultModel latent 陷阱、以及两个低成本的性能项。1-5 合计改动量不大(估计 <100 行),处理后即可合并。
…into feat/market
Summary
添加助手预设目录/市场页面
Changes
AssistantCatalogScreen)—— 按标签筛选的预设助手网格,支持搜索、预览和添加到助手列表的流程assistantCatalogService)—— 加载/解析/过滤捆绑的预设 JSON、标签页构建、DTO 映射useAssistantCatalog)—— 预设、标签页、筛选和创建 mutation 的状态管理/assistants/catalog)—— Expo Router 入口agents-en.json和agents-zh.json(从桌面版 cherry-studio 复制)library.assistant_catalog.*和assistant.actions.catalog键Notes
Preview