perf(tui): cache overview summaries between data updates - #184
Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough概览数据现在由 Changes概览汇总与 TUI 渲染
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UsageData
participant App
participant OverviewSummary
participant OverviewSnapshot
UsageData->>App: 提供初始化或更新后的使用数据
App->>OverviewSummary: derive(data, main_session_count)
OverviewSummary-->>App: 返回汇总指标与 favorite
App->>OverviewSnapshot: overview_summary()
OverviewSnapshot-->>App: 渲染概览页面
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Derive snapshot aggregates when report projections are installed or replaced. Keep render ticks free of daily map rebuilds and per-character portrait allocations.
Derive one decimal-place cache rate in the Overview summary and use it for rendering, fun facts, and achievement thresholds.
Enforce a three-row portrait contract and compose full, compact, and minimal layouts without index-based slicing.
Derive cold loading and failure from installed-generation state, keep their diagnostics out of the footer, and remove the unused UsageData loading flag.
Use the shared token formatter for input-health and fun-fact counts instead of maintaining a duplicate implementation.
877eb86 to
0b1b9b1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/tokscale-cli/src/tui/ui/footer.rs (1)
222-241: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winOverview 页脚计数仍在每帧重新遍历 daily 数据，绕过了新增的
OverviewSummary缓存。
current_count_label针对Tab::Overview的分支在每次渲染时都会重新遍历app.data.daily → client_breakdown → models来统计 distinct 的 model/client 数量,而这与crates/tokscale-cli/src/tui/data/overview.rs中OverviewSummary::derive的遍历逻辑完全重复,后者已经在投影安装时算好了model_count/client_count并被缓存。本 PR 的目标正是"仅在安装的报告投影变化时重建汇总,terminal ticks/resize/主题切换都不应重新折叠 daily 数据",而页脚这里恰恰在每次 tick 都重新做了这个折叠,抵消了缓存收益。建议直接复用
app.overview_summary()已计算好的字段:♻️ 建议的修复
fn current_count_label(app: &App) -> String { match app.current_tab { Tab::Overview => { - let mut models = BTreeSet::new(); - let mut clients = BTreeSet::new(); - for day in &app.data.daily { - for (client, client_info) in &day.client_breakdown { - clients.insert(client.as_str()); - for model in client_info.models.values() { - models.insert(model.model_id.as_str()); - } - } - } + let summary = app.overview_summary(); format!( " ({} models · {} clients · {} days)", - models.len(), - clients.len(), + summary.model_count, + summary.client_count, app.data.daily.len() ) }🤖 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/tui/ui/footer.rs` around lines 222 - 241, 更新 current_count_label 的 Tab::Overview 分支，移除对 app.data.daily、client_breakdown 和 models 的重复遍历，改为复用 app.overview_summary() 中已缓存的 model_count 和 client_count 字段；保留现有 days 数量及标签格式。crates/tokscale-cli/src/tui/ui/overview_snapshot.rs (1)
173-215: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
None分支未对 portrait/slogan 做居中处理,与Some分支视觉不一致
Some分支所有内容都经过center_line处理,但None(无 favorite family,如零使用量)分支的portrait与slogan均未调用center_line,渲染时会左对齐而非居中,与portraits::lines的文档约定(调用方需自行对每行做居中)不符。🐛 建议修复
None => { - portrait = portraits::lines(app, OverviewFamily::Unknown); - slogan = Some(Line::from(Span::styled( - "no data yet", - Style::default().fg(app.theme.muted), - ))); + portrait = portraits::lines(app, OverviewFamily::Unknown) + .map(|line| center_line(line, width)); + slogan = Some(center_line( + Line::from(Span::styled( + "no data yet", + Style::default().fg(app.theme.muted), + )), + width, + )); }🤖 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/tui/ui/overview_snapshot.rs` around lines 173 - 215, Update the None branch of the favorite_family match to apply center_line to the Unknown portrait lines and the “no data yet” slogan, using the existing width value; keep the current styling and Some-branch behavior unchanged.
🤖 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.
Outside diff comments:
In `@crates/tokscale-cli/src/tui/ui/footer.rs`:
- Around line 222-241: 更新 current_count_label 的 Tab::Overview 分支，移除对
app.data.daily、client_breakdown 和 models 的重复遍历，改为复用 app.overview_summary() 中已缓存的
model_count 和 client_count 字段；保留现有 days 数量及标签格式。
In `@crates/tokscale-cli/src/tui/ui/overview_snapshot.rs`:
- Around line 173-215: Update the None branch of the favorite_family match to
apply center_line to the Unknown portrait lines and the “no data yet” slogan,
using the existing width value; keep the current styling and Some-branch
behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b141da3-dfce-49b2-8563-f806df1a63f9
📒 Files selected for processing (14)
crates/tokscale-cli/src/tui/app.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/data/mod.rscrates/tokscale-cli/src/tui/data/overview.rscrates/tokscale-cli/src/tui/ui/achievements.rscrates/tokscale-cli/src/tui/ui/footer.rscrates/tokscale-cli/src/tui/ui/loading.rscrates/tokscale-cli/src/tui/ui/mod.rscrates/tokscale-cli/src/tui/ui/overview_snapshot.rscrates/tokscale-cli/src/tui/ui/portraits.rscrates/tokscale-cli/src/tui/ui/spinner.rscrates/tokscale-core/src/aggregate/parity_tests.rscrates/tokscale-core/src/aggregate/tui.rscrates/tokscale-core/src/usage_views.rs
💤 Files with no reviewable changes (4)
- crates/tokscale-cli/src/tui/ui/spinner.rs
- crates/tokscale-core/src/usage_views.rs
- crates/tokscale-core/src/aggregate/parity_tests.rs
- crates/tokscale-core/src/aggregate/tui.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/tokscale-cli/src/tui/data/mod.rs
- crates/tokscale-cli/src/tui/app.rs
Exercise cache-read and cache-write token buckets through OverviewSummary::derive so the rendered cache rate cannot silently use the wrong aggregate.
1 participant
Summary
OverviewSummarywhenever the installed usage projection changesMotivation
The snapshot introduced in #183 rebuilt model, client, and family aggregates from every daily record on each render tick. It also split static portrait rows into per-character
Stringallocations. This follow-up moves data-dependent work to projection installation while leaving layout, theme mapping, and ticker animation in the renderer.Validation
cargo fmt --all -- --checkcargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo test --locked --workspace --all-featuresSummary by CodeRabbit