perf(cli): route wrapped and clients through cached folds - #101
Conversation
Walkthrough新增 AGENTS 聚合视图(AgentUsage 结构、ViewSet::AGENTS 位、AgentEntries 累加器)并接线进 AggregationEngine;wrapped 命令改为单次调用 load_aggregated_views_with_pricing 获取图与 agent 用量;clients 命令改为调用新增的 count_local_client_messages 入口获取计数,替代原双路径解析。 Changes核心聚合视图新增
Estimated code review effort: 3 (Moderate) | ~25 minutes wrapped 与 clients 命令数据路径统一
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant WrappedCmd as load_wrapped_data
participant Engine as AggregationEngine
participant Pricing as PricingService
participant Views as AggregatedViews
WrappedCmd->>Pricing: 初始化定价服务
WrappedCmd->>Engine: load_aggregated_views_with_pricing(ViewSet)
Engine->>Engine: 累积 graph / agent_entries
Engine-->>Views: 返回 graph, agent_usage
Views-->>WrappedCmd: aggregated.graph, aggregated.agent_usage
WrappedCmd->>WrappedCmd: build_top_agents(agent_usage)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
makoMakoGo
left a comment
There was a problem hiding this comment.
Review note: one inline comment left for the AGENTS message_count aggregation issue.
makoMakoGo
left a comment
There was a problem hiding this comment.
Re-reviewed latest head. The prior AGENTS message_count issue is fixed: the accumulator now uses msg.message_count.max(0), and the parity test covers a folded opencode message with message_count = 3.
I did not find new blockers in the updated diff. Overall design still matches #92: wrapped uses one cached aggregation fold for graph/time-metrics/agents, clients uses the lightweight count sink, opencode-only wrapped agent semantics are preserved, and TUI behavior remains untouched.
LGTM from code review. Note: the Native test workflow was still running when I reviewed; Core CI, Test & Coverage, and CodSpeed were green.
There was a problem hiding this comment.
3 issues found across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/tokscale-cli/src/commands/wrapped.rs (1)
358-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value将
"opencode"改为ClientId::OpenCode.as_str():这里和本文件其它客户端比较保持同一套常量来源,也能避免后续规范 ID 变更时出现字符串漂移。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-cli/src/commands/wrapped.rs` at line 358, Replace the hardcoded "opencode" string check in wrapped.rs with ClientId::OpenCode.as_str() so the client comparison uses the same identifier source as the rest of the file. Update the conditional around the agent.client comparison to reference ClientId::OpenCode directly, keeping client ID usage consistent and avoiding string drift if the canonical ID changes later.Source: Coding guidelines
crates/tokscale-core/src/aggregate/accumulators.rs (1)
14-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win已修复历史问题;建议核查与 TuiAcc 的重复归一化逻辑。
message_count累加已按msg.message_count.max(0)处理,解决了此前"固定 +1 导致折叠消息被低估"的问题,新增的agents_view_keeps_client_dimension测试也覆盖了message_count = 3场景,无需再关注。另外,
push()中按opencode/copilot/其它 client 分支选择不同的normalize_*_agent_name函数,这个分发逻辑与aggregate/tui.rs里TuiAcc(存在agent_map/agent_clients/agent_instances)很可能存在相似实现。建议核实是否可抽取共享辅助函数,避免两处归一化规则未来出现分叉。🔍 核查脚本
rg -n -B2 -A15 'normalize_opencode_agent_name|normalize_copilot_agent_name' crates/tokscale-core/src/aggregate/tui.rsAlso applies to: 318-369
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-core/src/aggregate/accumulators.rs` around lines 14 - 18, The normalization dispatch in push() is likely duplicating the same agent-name canonicalization logic that TuiAcc uses in aggregate/tui.rs. Review the existing normalize_opencode_agent_name, normalize_copilot_agent_name, and related client-specific branches, then factor the shared normalization behavior into a common helper or shared path used by both accumulators. Keep the client-based selection intact, but ensure both Accumulators rely on the same normalization rules to avoid future divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/tokscale-cli/src/commands/wrapped.rs`:
- Line 358: Replace the hardcoded "opencode" string check in wrapped.rs with
ClientId::OpenCode.as_str() so the client comparison uses the same identifier
source as the rest of the file. Update the conditional around the agent.client
comparison to reference ClientId::OpenCode directly, keeping client ID usage
consistent and avoiding string drift if the canonical ID changes later.
In `@crates/tokscale-core/src/aggregate/accumulators.rs`:
- Around line 14-18: The normalization dispatch in push() is likely duplicating
the same agent-name canonicalization logic that TuiAcc uses in aggregate/tui.rs.
Review the existing normalize_opencode_agent_name, normalize_copilot_agent_name,
and related client-specific branches, then factor the shared normalization
behavior into a common helper or shared path used by both accumulators. Keep the
client-based selection intact, but ensure both Accumulators rely on the same
normalization rules to avoid future divergence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c8ef6b3-74d6-4608-adfd-9be09341e699
📒 Files selected for processing (11)
crates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-cli/src/main.rscrates/tokscale-cli/src/tui/ui/agents.rscrates/tokscale-core/src/aggregate/accumulators.rscrates/tokscale-core/src/aggregate/config.rscrates/tokscale-core/src/aggregate/engine.rscrates/tokscale-core/src/aggregate/mod.rscrates/tokscale-core/src/aggregate/parity_tests.rscrates/tokscale-core/src/aggregate/views.rscrates/tokscale-core/src/clients.rscrates/tokscale-core/src/lib.rs
makoMakoGo
left a comment
There was a problem hiding this comment.
Final re-review on latest head. The follow-up commit addresses the valid bot findings cleanly:
ClientCountSinknow usesmessage.message_count.max(0)for headless Codex counts, matching the client totals basis.- A folded headless Codex count test covers
message_count = 4. - The AGENTS parity test no longer mutates
TZunnecessarily. - The wrapped opencode filter now uses
ClientId::OpenCode.as_str().
No new blockers found. LGTM from code review. Core CI, Test & Coverage, and CodSpeed are green; Native test was still running at the time of review.
Summary
Closes #92
Validation
Local memory check
Measured on WSL2 Ubuntu 24.04 with live local client data. Base is the previously installed local
tokscalebinary; PR istarget/release/tokscalefrom this branch. Each command had one warmup run, then 3 measured runs with/usr/bin/time -f "%M,%e"; RSS values are%M / 1024.tokscale --no-spinner clients --jsontokscale wrapped --year 2026 --agents --no-spinner --output ...tokscale wrapped --year 2026 --clients --no-spinner --output ...The wrapped commands exited successfully and produced non-empty PNG output.
clients --jsonkept the same JSON shape; exact live message counts can move during measurement while local Codex sessions are being written.Summary by cubic
Routes the
wrappedandclientsCLI paths through cached aggregation folds to avoid rescans and speed up results. Adds a cachedAGENTSview that preserves per-client agent usage and fixes message counting for accurate rankings. Closes #92.New Features
ViewSet::AGENTSandAgentUsageintokscale-corefor per-client agent metrics (tokens, cost, messages).opencode,copilot, others) and sorted for stable ranking.Bug Fixes
cc-mirror/*usage asclaudein client message counts.message_countto avoid undercounting.message_countfor headless Codex inclientsstats for accurate totals.Written for commit 7011c32. Summary will update on new commits.
Summary by CodeRabbit
新功能
Bug Fixes