feat(sight): add optimization tips and savings breakdown to token savings page#1007
Conversation
|
linyizhou seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
ebc78c7 to
3bb756a
Compare
jfeng18
left a comment
There was a problem hiding this comment.
感谢 @husterL9!后端算好展示串 + severity 查找表 + ?? [] 防御,方向很对 👍 复核后:
建议处理:
- 评级死区:
<5%警告、≥15%良好、≥30%优秀,但 [5%,15%) 区间没有任何评级(handlers.rs:431/468/474),确认是否有意?若非,建议把警告区间扩到<15%。 - 阈值缺判别测试:测试用 3/25/50,边界 5/15/30 都没覆盖——把
>=改成>测试也不会失败,建议补边界用例。 - 前端 banner 未覆盖:详情面板里
compression_ratio.toFixed()在测试中从未展开渲染,API mock 若缺该字段会 crash;建议加一个"展开详情"的测试。
小点: compression_ratio 未 clamp,外部 stats.db 若 after>before 会显示负压缩率。
3bb756a to
ff2ae1e
Compare
|
感谢 review! 已全部处理:
后端测试 17 passed / 0 failed。关于与 #985/#990 的 rebase,合入时我会注意顺序处理冲突。 |
4fc4c67 to
676cb5b
Compare
jfeng18
left a comment
There was a problem hiding this comment.
Thanks for the work on optimization tips — the generate_optimization_tips logic and test coverage look well thought out.
One issue: the backend additions (get_token_savings, compute_compression_ratio, build_explanation, generate_optimization_tips etc.) are in handlers.rs, but mod.rs routes /api/token-savings to token_savings.rs. The new endpoint in handlers.rs is never registered, so none of this backend code will be reached at runtime.
The frontend references item.compression_ratio.toFixed(1) and item.explanation, but the actually-served DTO from token_savings.rs does not include these fields — this will cause a runtime error.
Could you take a look?
676cb5b to
8f76176
Compare
|
感谢指出!已修复路由 vs 实际端点不一致的问题: 根因: 修复方案:
验证结果: |
jfeng18
left a comment
There was a problem hiding this comment.
LGTM overall — optimization tips and compression breakdown are useful additions.
One readability issue:
build_explanation uses Unicode escapes for Chinese text (e.g. \u{5de5}\u{5177} instead of "工具"). This makes the format strings nearly impossible to read or maintain. Please replace with Chinese string literals directly.
Also: this PR has merge conflicts with main (likely from #1099 baseline_tokens). Please rebase onto latest main before merge.
98e89c3 to
f3d3283
Compare
9c6e791 to
bf536a7
Compare
jfeng18
left a comment
There was a problem hiding this comment.
TokenSavingsPage.tsx:178 — 重构 DiffView 时旧条件渲染 {item.optimization_reason && (...)} 的开头被移除,但闭合 )} 残留。当前文件 <div 开标签 16 个、</div> 闭标签 15 个,第 157 行的 overflow 容器未正确关闭,TSX 无法编译。
另外第 157-177 行新增的两列对比与第 199-213 行原有 fallback 分支内容近乎相同,确认下是否需要去重。
其余 LGTM,后端逻辑已正确落入 token_savings.rs,阈值和边界测试覆盖完整。
bf536a7 to
efd12dd
Compare
efd12dd to
e11b587
Compare
Description
为 Token Savings 页面增加优化建议面板和节省排行面板,提升可操作性和可解释性。
后端 (handlers.rs)
OptimizationItemDto新增compression_ratio(压缩率)和explanation(中文说明)OptimizationTip结构体(level/title/description)get_token_savings根据节省率、组件启用状态生成多级优化建议前端 (TokenSavingsPage.tsx + apiClient.ts)
OptimizationTipsPanel:按 success/info/warning 分级展示优化建议SavingsBreakdownPanel:跨会话 Top 5 节省排行,进度条可视化DiffView增强:explanation banner + before/after token 数量标注测试 (TokenSavingsPage.test.tsx)
Related Issue
closes #1006
Type of Change
Scope
sight(agentsight)Checklist
sight:cargo testpass (578 passed, 0 failed)Cargo.lock)Testing
cargo test: 578 passed, 0 failed, 28 ignoredAdditional Notes
前端以 embed 模式编译进二进制,已验证本地部署正常。