Conversation
5291e71 to
2fab15f
Compare
2fab15f to
a64147c
Compare
op7418
left a comment
There was a problem hiding this comment.
PR #169 Review: Context Ring 实时与压缩状态支持
总体评价
非常实用的功能,在输入框区域展示上下文 token 使用情况的环形进度条,并支持压缩状态显示。数据流设计合理,从 SDK 事件到 SSE 到前端状态的整条链路都考虑到了。
需要改进的地方
1. MessageInput 中内联了大量渲染逻辑
Context usage ring 的渲染使用了一个 IIFE (() => { ... })() 包裹了约 60 行代码直接内联在 JSX 中。建议提取为独立的 ContextUsageRing 组件,接收 contextTokens、maxContext、isCompacting、contextStale 作为 props。这样也更便于单独测试和复用。
2. 颜色阈值硬编码
ratio > 0.7 ? '#ef4444' : ratio > 0.5 ? '#eab308' : '#22c55e'建议使用 Tailwind CSS 变量或至少定义为常量,方便主题适配。直接用十六进制颜色会在暗色主题下可能有对比度问题。
3. contextTokens 的 useMemo 遍历所有消息
const contextTokens = useMemo(() => {
for (let i = messages.length - 1; i >= 0; i--) { ... }
}, [messages]);这个 memo 在每次 messages 数组引用变化时都会重新执行,但实际上只关心最后一条 assistant 消息的 token_usage。如果消息列表很长,反向遍历虽然效率尚可,但 messages 数组引用频繁变化(每次追加流式文本)会导致频繁重计算。建议考虑将 context token 信息直接通过 snapshot 传递,而非从消息列表反推。
4. sanitizeEnvValue 的 eslint-disable 注释被错误删除
- // eslint-disable-next-line no-control-regex
+ 删除了 ESLint disable 注释但保留了空行,后面的正则表达式 /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g 可能会触发 ESLint 的 no-control-regex 规则。如果项目已在 ESLint 配置中全局禁用了该规则则无影响,否则需要保留注释。
5. extractTokenUsage 中的 modelUsage 类型断言链较复杂
const modelUsage = (msg as Record<string, unknown>).modelUsage as ...多层 as 断言降低了类型安全性。建议定义一个明确的接口来描述 SDK result message 的扩展字段,或者使用运行时类型检查。
6. spin 动画未定义
压缩状态使用了 animation: 'spin 1.5s linear infinite',但没有看到对应的 CSS @keyframes spin 定义。Tailwind 内置了 animate-spin,但直接在 inline style 中使用 spin 关键帧需要确保全局 CSS 中有定义。
7. 缺少单元测试
Context Ring 的状态计算逻辑(ratio 计算、token 格式化、颜色选择)可以用纯函数提取并编写单元测试。
优点
- SSE 事件类型设计清晰(
compact_boundary、compactingstatus、context_tokens) contextTokensPendingRefresh状态处理了压缩后等待刷新的中间态- i18n 支持完整(en.ts 和 zh.ts 都已更新)
streamingContextTokens和contextWindow的分层设计合理
概述
测试