Fix buzz view/next rate display bugs (#257, #259, #260)#282
Merged
Conversation
The Beeminder API returns rates at full float precision (e.g. 0.21317778888888886), which `buzz view` dumped verbatim via %g. Add a formatRateValue helper that rounds to 4 decimal places, trims trailing zeros, and avoids scientific notation so large whole-number rates stay readable. Closes #260 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`buzz view`/`review` showed only the goal's end rate (the final road segment). Capture the API's `rcur` (current rate) and, when it differs from the end rate, display both — e.g. "0 hours / day (current), 0.21 (end)". Flat roads where the rates match still show a single rate. Closes #259 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`buzz next` surfaced the most urgent goal even when it was already overdue, printing "OVERDUE" instead of a countdown. Add filterOutOverdue and apply it (after the completed-goal filter) so `next` points at the soonest goal that still has time left. Closes #257 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🚀 Pre-release Build ReadyTest builds are ready! Install directly using the # Install the pre-release
bin install https://github.com/PinePeakDigital/buzz/releases/tag/pr-282-latest buzz-pr-282# Run the pre-release
buzz-pr-282# Uninstall the pre-release
bin remove buzz-pr-282Direct Download LinksOr download binaries directly from the pre-release page:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses three UX/logic bugs in buzz view / buzz next related to rate display precision, showing current vs end rate, and skipping already-overdue goals when selecting the “next” goal.
Changes:
- Add rate formatting to round to a readable precision and avoid scientific notation in
buzz view. - Display both current rate (
rcur) and end rate (rate) when they differ. - Filter out overdue goals in
buzz nextso it points at the soonest goal that still has time remaining.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| review.go | Adds formatRateValue and updates rate rendering to support readable rounding and current vs end rate display. |
| review_test.go | Adds tests for rate rounding and for current/end rate display behavior. |
| next.go | Applies an overdue-goal filter in buzz next after completed-goal filtering. |
| beeminder.go | Extends Goal with rcur and adds filterOutOverdue helper. |
| beeminder_test.go | Adds unit test coverage for filterOutOverdue. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds legacy ChangesOverdue filtering and rate display improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
next.go (1)
62-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a single
nowsnapshot for filtering and rendering.Line 65 filters with one
time.Now()call, but Line 76 formats the same goal with a later one. A goal that's about to expire can still slip through here and printOVERDUE, which is the bug this change is trying to eliminate.Proposed fix
func displayNextGoal() error { _, _, goals, err := loadConfigAndGoals() if err != nil { return err } + now := time.Now() // Skip goals that have already reached their end value — they have no // remaining work, so surfacing them as "next" would mislead the user into // acting on a completed goal. goals = filterOutEndValueReached(goals) // Skip overdue goals: "next" should point at the soonest goal that still // has time left, not one that's already past its deadline (which would // render as OVERDUE rather than a countdown). - goals = filterOutOverdue(goals, time.Now()) + goals = filterOutOverdue(goals, now) // If no goals, return error if len(goals) == 0 { return fmt.Errorf("no goals found") } // Get the first goal (most urgent) nextGoal := goals[0] // Format the output: "goalslug baremin timeframe" - timeframe := FormatGoalDueDate(nextGoal) + timeframe := FormatGoalDueDateAt(nextGoal, now)🤖 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 `@next.go` around lines 62 - 76, Take a single timestamp snapshot (e.g., now := time.Now()) and use it for both the overdue filtering and the output formatting so the same instant is used everywhere; replace the direct time.Now() call in filterOutOverdue(goals, time.Now()) with the snapshot and likewise ensure FormatGoalDueDate uses that same reference time (either by changing its signature to FormatGoalDueDate(goal, refTime) or adding a small helper like FormatGoalDueDateAt(goal, refTime)), then use that helper when formatting nextGoal so a goal cannot become OVERDUE between the filterOutOverdue and FormatGoalDueDate calls.
🤖 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 `@review.go`:
- Around line 335-344: The comparison uses raw floats (goal.Rcur vs goal.Rate)
which can differ but format to the same displayed value; change the branch to
compare the formatted/displayed rate strings instead (use the same formatting
helpers used for rendering, e.g., call formatRateValue or formatRate for
*goal.Rcur and *goal.Rate with goal.Runits/goal.Gunits and compare those
strings) and only render both "current" and "end" when the formatted strings
differ; update the logic around rateStr, formatRate, formatRateValue, goal.Rcur
and goal.Rate accordingly.
- Around line 250-253: The formatted rate can end up as "-0" when a small
negative value rounds to zero in formatRateValue; after computing rounded (using
rateDisplayDecimals), normalize negative zero to positive zero before calling
strconv.FormatFloat—e.g. detect if rounded equals 0 (or math.Signbit(rounded) &&
rounded == 0) and set rounded = 0.0 so the output never prints "-0".
---
Outside diff comments:
In `@next.go`:
- Around line 62-76: Take a single timestamp snapshot (e.g., now := time.Now())
and use it for both the overdue filtering and the output formatting so the same
instant is used everywhere; replace the direct time.Now() call in
filterOutOverdue(goals, time.Now()) with the snapshot and likewise ensure
FormatGoalDueDate uses that same reference time (either by changing its
signature to FormatGoalDueDate(goal, refTime) or adding a small helper like
FormatGoalDueDateAt(goal, refTime)), then use that helper when formatting
nextGoal so a goal cannot become OVERDUE between the filterOutOverdue and
FormatGoalDueDate calls.
🪄 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: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 252e114b-2792-433d-9bf1-5c3497ff3bcd
📒 Files selected for processing (5)
beeminder.gobeeminder_test.gonext.goreview.goreview_test.go
The live Beeminder goal endpoint returns the current rate as `currate`,
not `rcur` (the field name only `rcur`/`ravg` appeared in an older API
dump). As written, the current/end rate split would never trigger
because `Rcur` was always nil. Map `currate`, fall back to `rcur` via a
CurrentRate() helper for any payload that still uses the old name, and
verify against the live API:
Rate: 0.1623 hours / day (current), 0.1104 (end)
Follow-up to #259.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Compare formatted rate strings, not raw floats, when deciding the current/end split, so values that round equal don't render split - Normalize a small negative rate that rounds to zero to "0" (no "-0") - Use math.Pow10 for base-10 scaling in formatRateValue - Compare Unix timestamps directly in filterOutOverdue Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
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 (1)
next.go (1)
65-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse one reference timestamp for filtering and rendering.
Line 65 and Line 76 use different “now” values. A near-deadline goal can pass the overdue filter and still render as
OVERDUEimmediately after.Proposed fix
func displayNextGoal() error { _, _, goals, err := loadConfigAndGoals() if err != nil { return err } @@ - goals = filterOutOverdue(goals, time.Now()) + now := time.Now() + goals = filterOutOverdue(goals, now) @@ - timeframe := FormatGoalDueDate(nextGoal) + timeframe := FormatGoalDueDateAt(nextGoal, now)🤖 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 `@next.go` around lines 65 - 76, The code uses two different time.Now() calls (in filterOutOverdue(goals, time.Now()) and FormatGoalDueDate(nextGoal)) causing racey overdue detection; capture a single reference timestamp (e.g., now := time.Now()) and pass that same now into filterOutOverdue and into the rendering call (update FormatGoalDueDate to accept a time parameter or add a helper FormatGoalDueDateAt) so filterOutOverdue(goals, now) and FormatGoalDueDate(nextGoal, now) use the identical instant when selecting and formatting nextGoal.
🤖 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 `@next.go`:
- Around line 65-76: The code uses two different time.Now() calls (in
filterOutOverdue(goals, time.Now()) and FormatGoalDueDate(nextGoal)) causing
racey overdue detection; capture a single reference timestamp (e.g., now :=
time.Now()) and pass that same now into filterOutOverdue and into the rendering
call (update FormatGoalDueDate to accept a time parameter or add a helper
FormatGoalDueDateAt) so filterOutOverdue(goals, now) and
FormatGoalDueDate(nextGoal, now) use the identical instant when selecting and
formatting nextGoal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5959f698-eafd-40cb-ad05-a61f561febad
📒 Files selected for processing (5)
beeminder.gobeeminder_test.gonext.goreview.goreview_test.go
filterOutOverdue and the rendered countdown previously each called time.Now() independently, so a goal could pass the overdue filter and then render as OVERDUE moments later. Capture now once and thread it through both via the existing FormatGoalDueDateAt helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review-loop test-coverage findings (auto-applied, low-risk test-only): - formatRateValue's negative-zero normalization (-0 -> "0") was load-bearing but unexercised; add a -0.00001 case to TestFormatRate. - filterOutOverdue's strict "<" boundary (a goal due exactly at now is kept) was untested; add a due-now goal to TestFilterOutOverdue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Three small, independent fixes to
buzz view/buzz next, one commit each.#260 — round goal rate to a readable precision
buzz viewdumped the API's full-precision rate (0.21317778888888886 hours / day). A newformatRateValuehelper rounds to 4 decimals, trims trailing zeros, and avoids scientific notation so large whole-number rates (e.g.100000) stay readable.#259 — show current rate alongside end rate
The Rate line only showed the goal's end rate (the road's final segment). Now captures the API's
rcurand, when it differs from the end rate, shows both:Flat roads (current == end) still show a single rate, no redundant split.
#257 —
buzz nextskips overdue goalsbuzz nextwould surface an already-overdue goal and printOVERDUEinstead of a countdown. NewfilterOutOverduehelper (applied after the existing completed-goal filter) makesnextpoint at the soonest goal that still has time left.Tests
TestFormatRateTestReviewModelViewWithCurrentAndEndRate+TestReviewModelViewWithEqualCurrentAndEndRateTestFilterOutOverdueAll pass;
gofmt/go vetclean. Each commit builds independently. (The pre-existingTestParseDurationoverflow failure is unrelated — it's open bug #271.)Closes #257
Closes #259
Closes #260
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests