feat(tui): add monthly and weekly period tabs - #57
Conversation
Reviewer's GuideIntroduce Monthly and Weekly TUI period views derived from existing daily aggregates, add matching detail views and layout logic, and remove the Minutely tab along with its data aggregation, caching, settings and documentation, while documenting the architectural decision that coarser-than-daily period views must be built from daily data. Sequence diagram for opening monthly/weekly period detail viewsequenceDiagram
actor User
participant App
participant Data as tui_data
participant PeriodUi as period_rs
User->>App: handle_key_event(KeyCode::Enter)
App->>App: current_tab == Tab::Monthly | Tab::Weekly
App->>App: open_selected_period_detail(PeriodKind)
App->>App: get_sorted_periods(kind)
App->>Data: build_period_usage(&data.daily, kind)
Data-->>App: Vec<PeriodUsage>
App->>App: store PeriodDetailSelection
App->>App: enter_period_detail_sort_context()
App-->>User: status "Viewing period details..."
loop subsequent frames
App->>PeriodUi: render_monthly/render_weekly
PeriodUi->>App: is_period_detail_active()
PeriodUi->>App: get_sorted_period_detail_rows()
App-->>PeriodUi: Vec<PeriodDetailRow>
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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.
Hey - I've found 2 issues, and left some high level feedback:
- There are several call sites that recompute
build_period_usagedirectly (e.g.,update_data,get_current_list_len,period_detail_label,get_sorted_period_detail_rows,current_count_label); consider routing these throughget_sorted_periods(or a shared helper) so the aggregation and ordering logic live in one place and you avoid repeated recomputation per render.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are several call sites that recompute `build_period_usage` directly (e.g., `update_data`, `get_current_list_len`, `period_detail_label`, `get_sorted_period_detail_rows`, `current_count_label`); consider routing these through `get_sorted_periods` (or a shared helper) so the aggregation and ordering logic live in one place and you avoid repeated recomputation per render.
## Individual Comments
### Comment 1
<location path="crates/tokscale-cli/src/tui/app.rs" line_range="1795-1804" />
<code_context>
+ pub fn get_sorted_periods(&self, kind: PeriodKind) -> Vec<PeriodUsage> {
</code_context>
<issue_to_address>
**question (bug_risk):** Ascending cost/tokens sorts always prefer newer years; confirm this cross-year ordering is intentional.
In `get_sorted_periods`, the `Cost` and `Tokens` ascending branches still sort `section_year` in descending order:
```rust
(SortField::Cost, SortDirection::Ascending) => periods.sort_by(|a, b| {
b.section_year
.cmp(&a.section_year)
.then_with(|| a.cost.total_cmp(&b.cost))
.then_with(|| b.start_date.cmp(&a.start_date))
}),
```
So newer years always come first, and only within a year is the ordering truly ascending. If global ascending order is desired, you’d need to flip the year comparison for ascending sorts (or drop the year as a key). If the "newest-year-first" grouping is intentional, consider adding a short comment near this sort to document that behavior.
</issue_to_address>
### Comment 2
<location path="crates/tokscale-cli/src/tui/ui/period.rs" line_range="587-591" />
<code_context>
+ )
+ .height(1);
+
+ let detail_len = rows_data.len();
+ let start = scroll_offset.min(detail_len);
+ let end = (start + visible_height).min(detail_len);
+
+ if start >= detail_len {
+ return;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid early-return when scroll_offset exceeds current detail length to prevent a blank body.
In `render_detail`, the early `return` when `start >= detail_len` means a stale `scroll_offset` (e.g., after resize or data refresh) can render only the border/title with no rows or empty-state message. Instead of returning, either clamp `scroll_offset` here (e.g., derive `start` from `detail_len.saturating_sub(visible_height)` and recompute) or fall back to `start = 0` when `detail_len > 0` so some rows are always shown. The same applies to the `if start >= period_len { return; }` branch in `render_period`, which should use similar clamping to avoid blank views when indices drift.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request replaces the high-cardinality, per-message "Minutely" tab with new "Monthly" and "Weekly" period views in the TUI. Following the newly introduced ADR 0010, these coarser period views are derived efficiently on-demand from the already-aggregated daily buckets rather than being re-folded per-message in the main loop. This change allows for the removal of the minutely aggregation logic, its configuration settings, and associated caching mechanisms. A review comment identifies an issue in the Period Detail view where the model name could appear blank if display_name is empty, suggesting a fallback to the map's model key.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (1)
233-233:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win把
monthly示例改成“月/周期视图”描述。现在四个 README 里
tokscale monthly都被写成了“Daily view/每日视图”,会误导读者。请把这行注释统一改成和新月/周视图一致的文案,必要时再补一个weekly示例。🤖 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 `@README.md` at line 233, The comment for the tokscale monthly command is incorrectly labeled as "Daily view (shows daily breakdown)" when it should describe a monthly or periodic view instead. This misleading documentation appears across multiple README files in different languages. Update the comment at README.md line 233 to correctly describe monthly/periodic view behavior, then apply the same correction to the corresponding lines in README.zh-cn.md (line 228), README.ja.md (line 228), and README.ko.md (line 227). Optionally add a separate weekly example command if appropriate to provide complete documentation of the time period options.README.zh-cn.md (1)
552-570:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win把简中配置示例补上
scanner.extraScanPaths。英文版已经把这个配置项正式写进配置文档了,但简中 README 里还没出现,用户会找不到这项设置。请在 JSON 示例和表格里一起补上。
🤖 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 `@README.zh-cn.md` around lines 552 - 570, The Simplified Chinese README is missing the scanner.extraScanPaths configuration entry that already exists in the English version. Add scanner.extraScanPaths to both the JSON configuration example and the configuration table in README.zh-cn.md. Include it in the JSON example with an appropriate default value (empty array), and add a corresponding row to the configuration documentation table with the proper type (string[]), default value, and Chinese description matching the functionality described in the English version.README.ja.md (1)
248-270:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win把日文/韩文 README 同步到新的 8 视图 + 配置项。
这两份本地化文档还停留在旧的 6 视图文案,而且配置块里还缺
scanner.extraScanPaths和usageTabEnabled;读者在这些语言版本里看不到完整的新入口。
README.ja.md#L248-L270: 改成 8 视图说明,去掉旧的 6 视图表述。README.ja.md#L520-L538: 补回scanner.extraScanPaths和usageTabEnabled。README.ko.md#L248-L269: 同样更新。README.ko.md#L520-L537: 同样补回这两项。🤖 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 `@README.ja.md` around lines 248 - 270, Update the localized Japanese and Korean README files to reflect the expanded interactive TUI mode and new configuration options. At README.ja.md lines 248-270 (anchor site), update the "6つのビュー" section to describe 8 views instead of 6, and remove outdated view descriptions to match the current feature set. At README.ja.md lines 520-538 (sibling site), add the missing configuration items for scanner.extraScanPaths and usageTabEnabled to the settings documentation block. Apply the identical changes to README.ko.md: update lines 248-269 to change from 6 views to 8 views with accurate descriptions, and at lines 520-537, add the same two missing configuration items (scanner.extraScanPaths and usageTabEnabled) to maintain parity with the Japanese and English versions.
🤖 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/app.rs`:
- Around line 1798-1828: The sorting logic for Cost and Tokens is not performing
global sorting because it always compares section_year first before comparing
the actual sort field, causing items from later years to always rank before
items from earlier years regardless of their cost or token values. To fix this,
remove the section_year comparison from the beginning of all Cost and Tokens
sorting cases (both Ascending and Descending variants) in the match statement,
so that the primary sort is directly by the cost or tokens value with start_date
as the secondary criterion. The section_year grouping should only be applied
when sorting by the Date field. Additionally, add a test case that verifies
items are sorted correctly across different years when sorting by Cost or Tokens
to prevent regression.
In `@crates/tokscale-cli/src/tui/data/mod.rs`:
- Around line 953-955: The current logic in the active_days counting only
considers a day active if day.tokens.total() > 0, but this misses days that have
user activity (messages or conversation rounds) even when no tokens were
consumed. Modify the condition in the block where entry.active_days is being
incremented to check for actual activity such as message_count or interaction
rounds first, before checking tokens, so that days with conversations but zero
token consumption are still correctly counted as active days.
In `@crates/tokscale-cli/src/tui/ui/period.rs`:
- Around line 433-475: The function top_period_model incorrectly aggregates
models using only model_key as the BTreeMap entry key, which causes models with
the same name from different providers to be merged together, leading to
inflated token counts and incorrect provider/color information. Change the
aggregation key in the models BTreeMap from just model_key to a compound key
that includes the provider (such as a tuple of provider and model_key, or use
color_key as the aggregation key). Update the entry call to use this new
compound key while maintaining the original label, provider, and color_key
values in the inserted TopPeriodModel records.
---
Outside diff comments:
In `@README.ja.md`:
- Around line 248-270: Update the localized Japanese and Korean README files to
reflect the expanded interactive TUI mode and new configuration options. At
README.ja.md lines 248-270 (anchor site), update the "6つのビュー" section to
describe 8 views instead of 6, and remove outdated view descriptions to match
the current feature set. At README.ja.md lines 520-538 (sibling site), add the
missing configuration items for scanner.extraScanPaths and usageTabEnabled to
the settings documentation block. Apply the identical changes to README.ko.md:
update lines 248-269 to change from 6 views to 8 views with accurate
descriptions, and at lines 520-537, add the same two missing configuration items
(scanner.extraScanPaths and usageTabEnabled) to maintain parity with the
Japanese and English versions.
In `@README.md`:
- Line 233: The comment for the tokscale monthly command is incorrectly labeled
as "Daily view (shows daily breakdown)" when it should describe a monthly or
periodic view instead. This misleading documentation appears across multiple
README files in different languages. Update the comment at README.md line 233 to
correctly describe monthly/periodic view behavior, then apply the same
correction to the corresponding lines in README.zh-cn.md (line 228),
README.ja.md (line 228), and README.ko.md (line 227). Optionally add a separate
weekly example command if appropriate to provide complete documentation of the
time period options.
In `@README.zh-cn.md`:
- Around line 552-570: The Simplified Chinese README is missing the
scanner.extraScanPaths configuration entry that already exists in the English
version. Add scanner.extraScanPaths to both the JSON configuration example and
the configuration table in README.zh-cn.md. Include it in the JSON example with
an appropriate default value (empty array), and add a corresponding row to the
configuration documentation table with the proper type (string[]), default
value, and Chinese description matching the functionality described in the
English version.
🪄 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: 60f109ca-cdd8-46ee-9e4f-711d09037f69
📒 Files selected for processing (19)
README.ja.mdREADME.ko.mdREADME.mdREADME.zh-cn.mdcrates/tokscale-cli/src/main.rscrates/tokscale-cli/src/tui/app.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/data/mod.rscrates/tokscale-cli/src/tui/mod.rscrates/tokscale-cli/src/tui/settings.rscrates/tokscale-cli/src/tui/ui/footer.rscrates/tokscale-cli/src/tui/ui/header.rscrates/tokscale-cli/src/tui/ui/minutely.rscrates/tokscale-cli/src/tui/ui/mod.rscrates/tokscale-cli/src/tui/ui/period.rscrates/tokscale-cli/src/tui/ui/time_table.rsdocs/adr/0010-period-views-derive-from-daily.mddocs/plans/2026-06-12-memory-optimization.mddocs/plans/2026-06-13-architecture-track.md
💤 Files with no reviewable changes (3)
- crates/tokscale-cli/src/tui/ui/minutely.rs
- crates/tokscale-cli/src/tui/ui/time_table.rs
- crates/tokscale-cli/src/tui/settings.rs
What Changed
Why
Monthly and Weekly are coarser than daily, so they can be derived losslessly from daily usage instead of folding every message again. This keeps the per-message aggregation loop lean and avoids recreating the old Minutely gating/caching complexity.
Validation
rtk cargo fmt --checkrtk cargo test -p tokscale-clirtk rg -n "Minutely|minutely|minutelyTabEnabled|with_minutely_enabled|minute_bucket|MinutelyUsage|get_sorted_minutely|Tab::Minutely" .Notes
This PR targets
personal/local-clientsinmakoMakoGo/tokscale; opening againstmainwould include unrelated branch history.Summary by Sourcery
Add monthly and weekly period-based usage views to the TUI and remove the legacy minutely view and its per-message aggregation path, deriving coarse period data from existing daily aggregates instead.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
新功能
移除