feat(sight): add strategy breakdown to token savings page#985
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. |
f5230c1 to
acff014
Compare
jfeng18
left a comment
There was a problem hiding this comment.
感谢 @husterL9 这个策略拆分!特别赞这次补齐了后端 Rust 测试 👍 复核后几个建议:
建议处理:
- 节省率口径不一致:
get_session_savings用compounded/(actual+compounded)(handlers.rs:872),而列表页get_token_savings用saved/actual(:1182)——同一 session 在 AtifViewer 卡片与列表页会显示不同的百分比,建议统一口径。 - 饼图重复切片:
strategy_breakdown按原始operation聚合(:703)、用 label 展示(:724),多个未知 operation 会生成多个都叫"其他优化"的切片;建议按 strategy label/id 聚合。 - 全表扫描:
get_session_savings用list_sessions_for_savings(0, i64::MAX)取全部 session 再.find单条,每次打开 AtifViewer 都会全表聚合;建议加按 session_id 的单条查询。
小点: compress-toon/compress-schema 归入 tool/mcp 桶的依据建议加注释说明。
另:本 PR 与 #990/#1007 改了同一批文件(handlers.rs / TokenSavingsPage.tsx),合并需注意顺序与 rebase;三者共往 handlers.rs 加了约千行,建议考虑抽出 server/token_savings.rs 收口。
- Add per-strategy pie chart (compress-response, compress-schema, rewrite-command, compress-toon) to savings summary card - Add strategy column with colored badges and hover tooltips to optimization items table - Extend /api/token-savings response with strategy_breakdown array - Add session-scoped token savings endpoint - Enhance ATIF viewer page with improvements - Update related tests
21ab2b4 to
275bd1d
Compare
|
感谢 review,已全部修复并 force push:
PTAL 🙏 |
jfeng18
left a comment
There was a problem hiding this comment.
Thanks for the solid work on strategy breakdown! Two items to address:
P1 — Test env pollution
token_savings.rs L668/702/740/773: set_var("HOME", ...) is never restored after each test. Since ENV_MUTEX only guards within this module, other tests in the same binary (e.g. genai::default_path(), tokenless::default_stats_path()) may observe the stale value under parallel execution, causing intermittent CI failures.
P2 — Pie chart colors always fall back to grey
StrategyBreakdown.strategy is populated with the Chinese label (e.g. "Schema 压缩") via grand_strategy_map key (L377), but the frontend STRATEGY_CONFIG is keyed by operation name (compress-schema). The lookup STRATEGY_CONFIG[b.strategy] never matches, so all pie slices render in the default #9ca3af grey.
275bd1d to
93baa36
Compare
- Fix savings rate formula inconsistency: unify get_session_savings to use compounded/total_tokens (same as get_token_savings list page) - Fix duplicate pie chart slices: aggregate strategy_breakdown by label instead of raw operation, so unknown ops merge into one slice - Fix full table scan: add get_session_for_savings() method that queries by session_id directly instead of list_all + .find - Add classification comments explaining compress-toon/compress-schema bucket assignment rationale - Extract token_savings.rs module from handlers.rs (~800 lines) to reduce file size and improve maintainability
93baa36 to
0bf87e5
Compare
jfeng18
left a comment
There was a problem hiding this comment.
Both issues addressed. HOME env var is now properly saved/restored in all 4 test functions, and StrategyBreakdown.strategy correctly uses the operation key for known ops. LGTM.
Description
Add per-strategy visualization and breakdown to the token savings dashboard, enabling users to see which optimization strategies (response compression, schema compression, command rewriting, TOON encoding) contribute most to their token savings.
Changes:
/api/token-savingsresponse withstrategy_breakdownarray; add session-scoped/api/token-savings/session/{id}endpointRelated Issue
closes #984
Type of Change
Scope
sight(agentsight)Checklist
sight:cargo testpassCargo.lock)Testing
cargo testpasses (2 passed, 28 ignored, 0 failed)