fix(tui): order merged sources and balance model columns - #8
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a dynamic table layout system for the TUI and implements a token-based sorting mechanism for client contributions. The review feedback suggests enhancing the robustness of token calculations using saturating arithmetic, improving TUI visual stability by calculating column widths across the entire dataset rather than just visible rows, and addressing code duplication between the CLI and core crates. Additionally, it recommends using specialized crates for accurate character width calculation and simplifying the layout distribution logic.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@crates/tokscale-cli/src/tui/ui/models.rs`:
- Around line 65-67: The display_width() function uses chars().count() which
measures codepoints not terminal column width; update
crates/tokscale-cli/Cargo.toml to add the unicode-width crate and in
src/tui/ui/models.rs replace uses of chars().count() with
UnicodeWidthStr::width() (e.g., call s.width()) in display_width() and similarly
update truncate() to compute widths via UnicodeWidthStr::width(), then
clamp/convert the resulting usize to u16 (min(u16::MAX as usize)) to preserve
the previous bounds.
In `@crates/tokscale-core/src/lib.rs`:
- Line 1413: 在累加 token 时不要直接使用可能为负的 msg.tokens.total(),应与 TUI 保持一致先将负值截为 0
再累加;在对 totals.total_tokens 做更新的地方(当前使用 totals.total_tokens += msg.tokens.total()
的位置)改为使用非负值(例如对 msg.tokens.total() 应用 max(0) 或等效的零下限判断)再加到
totals.total_tokens,以确保 core 报表与 TUI 的 merged clients 排序一致。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c19f911f-9e6e-45e2-b223-9b2270c9e0fc
📒 Files selected for processing (3)
crates/tokscale-cli/src/tui/data/mod.rscrates/tokscale-cli/src/tui/ui/models.rscrates/tokscale-core/src/lib.rs
Summary
GroupBy::ModelandGroupBy::WorkspaceModelreports.Modelno longer consumes all extra space whileProvider/Sourceremain truncated.Sourcestops growing after it fits and leftover width is not forced into text columns.Closes #6.
Closes #7.
Verification
cargo test --package tokscale-core test_model_grouping_orders_merged_clients_by_total_tokenscargo test --package tokscale-cli test_aggregate_messages_orders_model_clients_by_total_tokenscargo test --package tokscale-cli tui::ui::models::testscargo check --package tokscale-clicargo build --release -p tokscale-cliLocal binary
Installed to
/home/travis/.local/bin/tokscale.Summary by CodeRabbit
版本更新说明