fix(usage): show session reset time and add configurable reset display#87
Open
jhjaggars wants to merge 1 commit intoHaleclipse:masterfrom
Open
fix(usage): show session reset time and add configurable reset display#87jhjaggars wants to merge 1 commit intoHaleclipse:masterfrom
jhjaggars wants to merge 1 commit intoHaleclipse:masterfrom
Conversation
Previously, the usage segment displayed the 5-hour (session) utilization percentage but paired it with the 7-day (weekly) reset time, causing a confusing mismatch. This also adds a duration display option. Changes: - Store both `five_hour_resets_at` and `seven_day_resets_at` in cache (replacing the single `resets_at` field which always used the weekly period) - Add `reset_period` option (default: "session") to control which reset time is shown; set to "weekly" to use the 7-day reset time instead - Add `reset_format` option (default: "time") to control display format: - "time": existing behavior, e.g. "2-22-13" - "duration": time remaining until reset, e.g. "4h 52m", "2d 3h", "45m" - Document all Usage segment options in README.md and README.zh.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideRefactors the usage segment to cache and select separate reset timestamps for the 5‑hour session and 7‑day weekly windows, adds configurable options to control which reset period is displayed and in what format (time vs duration), and documents the new options in both English and Chinese READMEs. Class diagram for updated usage segment cache and reset formattingclassDiagram
class UsageSegment {
+format_reset_time(reset_time_str)
+format_reset_duration(reset_time_str)
+get_cache_path()
+is_cache_valid(cache, cache_duration)
+fetch_api_usage(api_base_url, token, timeout)
+save_cache(cache)
+get_circle_icon(ratio)
}
class ApiUsageCache {
+five_hour_utilization: f64
+seven_day_utilization: f64
+five_hour_resets_at: Option~String~
+seven_day_resets_at: Option~String~
+cached_at: String
}
class UsageSegmentConfig {
+reset_period: String *implicit via options["reset_period"]*
+reset_format: String *implicit via options["reset_format"]*
}
UsageSegment --> ApiUsageCache : uses_for_caching
UsageSegment --> UsageSegmentConfig : reads_options_from
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider modeling
reset_periodandreset_formatas enums (withDeserializeandFromStr/TryFromimplementations) rather than raw strings so that invalid config values are caught explicitly instead of silently falling back to the default branch. - Since
ApiUsageCacherenamedresets_attofive_hour_resets_atand addedseven_day_resets_at, it may be safer to addserdeattributes (rename,default, etc.) to keep deserialization backward-compatible with existing cache files rather than relying on them to fail and be replaced.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider modeling `reset_period` and `reset_format` as enums (with `Deserialize` and `FromStr`/`TryFrom` implementations) rather than raw strings so that invalid config values are caught explicitly instead of silently falling back to the default branch.
- Since `ApiUsageCache` renamed `resets_at` to `five_hour_resets_at` and added `seven_day_resets_at`, it may be safer to add `serde` attributes (`rename`, `default`, etc.) to keep deserialization backward-compatible with existing cache files rather than relying on them to fail and be replaced.
## Individual Comments
### Comment 1
<location> `src/core/segments/usage.rs:76` </location>
<code_context>
+ return "now".to_string();
+ }
+
+ let total_minutes = remaining.num_minutes();
+ let days = total_minutes / (24 * 60);
+ let hours = (total_minutes % (24 * 60)) / 60;
</code_context>
<issue_to_address>
**suggestion:** Duration formatting can show `0m` when there is still almost a full minute remaining.
Because `remaining.num_minutes()` truncates fractional minutes, any remaining time under 60 seconds will display as `0m` rather than something like `"now"`. If you want very short durations to show `"now"`, consider a small threshold (e.g. treat durations < 60 seconds as `"now"`) or base this decision on `num_seconds()` instead of minutes.
```suggestion
if remaining.num_seconds() < 60 {
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| let reset_utc = dt.with_timezone(&Utc); | ||
| let remaining = reset_utc.signed_duration_since(now); | ||
|
|
||
| if remaining.num_seconds() <= 0 { |
There was a problem hiding this comment.
suggestion: Duration formatting can show 0m when there is still almost a full minute remaining.
Because remaining.num_minutes() truncates fractional minutes, any remaining time under 60 seconds will display as 0m rather than something like "now". If you want very short durations to show "now", consider a small threshold (e.g. treat durations < 60 seconds as "now") or base this decision on num_seconds() instead of minutes.
Suggested change
| if remaining.num_seconds() <= 0 { | |
| if remaining.num_seconds() < 60 { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reset_periodoption to explicitly control which reset time is shown ("session"or"weekly", default:"session").reset_formatoption to display the reset as a countdown duration instead of a clock time (e.g.4h 52minstead of2-22-13).New options
reset_period"session""session"(5-hour window) or"weekly"(7-day window)reset_format"time""time"(e.g.2-22-13) or"duration"(e.g.4h 52m)Example:
Test plan
reset_period = "weekly"shows the 7-day reset timereset_format = "duration"shows countdown (e.g.4h 52m)reset_format = "time"shows existing clock formatresets_atfield) trigger a fresh API fetch gracefully🤖 Generated with Claude Code
Summary by Sourcery
Align usage segment reset time with the displayed period and introduce configurable reset display options.
New Features:
Bug Fixes:
Enhancements:
Documentation: