feat: replace static sleep with polling for datapoint verification after buzz add #188

Copilot · 2025-12-08T13:15:49Z

Polling Implementation - Final Improvements

Original prompt

We should switch from a static sleep to polling to ensure that the datapoint has been added before showing limsum after buzz add. Ideally we'd take an id of the added datapoint and then wait for a datapoint with that id to be in the recent data of the goal while polling.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

coderabbitai · 2025-12-08T13:15:55Z

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Co-authored-by: narthur <[email protected]>

github-actions · 2025-12-08T15:48:31Z

🚀 Pre-release Build Ready

Test builds are ready! Install directly using the bin command:

# Install the pre-release
bin install https://github.com/PinePeakDigital/buzz/releases/tag/pr-188-latest buzz-pr-188

# Run the pre-release
buzz-pr-188

# Uninstall the pre-release
bin remove buzz-pr-188

Direct Download Links

Or download binaries directly from the pre-release page:

💡 No GitHub login required for downloads!

🗑️ This pre-release will be automatically deleted when the PR is closed.

Copilot

Pull request overview

This PR replaces a fixed 2-second sleep with intelligent polling when verifying datapoint creation in the buzz add command. The change ensures the Beeminder server has processed the new datapoint before displaying the updated goal status, making the command both more reliable and typically faster.

Key changes:

Modified CreateDatapoint to return the created datapoint with its ID
Added WaitForDatapoint function that polls for the datapoint appearance (10s timeout, 500ms intervals)
Improved URL encoding using url.PathEscape and url.Values for safer API requests

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
beeminder.go	Added `WaitForDatapoint` polling function, modified `CreateDatapoint` to return created datapoint, improved URL escaping for goal slugs
main.go	Replaced fixed `limsumFetchDelay` with polling logic in `handleAddCommand`, improved error messaging for polling timeout
messages.go	Updated `submitDatapointCmd` to handle new `CreateDatapoint` return signature
beeminder_test.go	Added comprehensive tests for `WaitForDatapoint` (immediate success, timeout, error retry) and `CreateDatapoint` return value
handlers_test.go	Removed obsolete `TestLimsumFetchDelay` tests for the deleted static delay variable

Copilot · 2025-12-08T15:52:07Z

beeminder.go

+func WaitForDatapoint(config *Config, goalSlug, datapointID string, timeout, pollInterval time.Duration) (bool, error) {
+	deadline := time.Now().Add(timeout)
+
+	for time.Now().Before(deadline) {


The polling loop has an edge case: if the deadline is already passed when entering the loop, it will never execute. Consider checking at least once before checking the deadline, or documenting that zero or negative timeout values will result in no polling attempts.

Fixed in commit 680011f. Changed the loop structure to use an infinite for loop that always executes at least once, with timeout checking after each iteration. This ensures at least one polling attempt even with zero or negative timeout values.

Copilot · 2025-12-08T15:52:08Z

messages.go

+		_, err := CreateDatapoint(config, goalSlug, timestamp, value, comment)
 		return datapointSubmittedMsg{err: err}


The TUI's submitDatapointCmd discards the returned datapoint but doesn't implement polling like handleAddCommand does. This means the TUI may still show stale data after submitting a datapoint. Consider implementing similar polling in the TUI workflow to ensure data consistency, or document why it's not needed for the TUI use case.

Added clarifying comments in commit 680011f. The TUI doesn't need polling because it triggers a full goal refresh after datapoint submission (see datapointSubmittedMsg handling in main.go), which fetches updated data including the new datapoint and limsum. This approach is more appropriate for the TUI context.

Copilot · 2025-12-08T15:52:08Z

beeminder.go

+		if err != nil {
+			// Don't fail immediately on fetch errors, keep trying
+			time.Sleep(pollInterval)
+			continue
+		}


The error handling here continues polling on any fetch error, including permanent errors like authentication failures or non-existent goals. Consider distinguishing between retryable errors (like temporary network issues) and permanent errors (like 404 Not Found or 401 Unauthorized) to fail fast when appropriate.

Fixed in commit 680011f. Added isPermanentError() helper function that detects HTTP 401, 403, and 404 status codes in error messages. WaitForDatapoint now returns these errors immediately instead of continuing to poll, failing fast on authentication failures, permission issues, and non-existent goals.

Copilot · 2025-12-08T15:52:08Z

beeminder.go

+
+// WaitForDatapoint polls the goal's recent datapoints until the specified datapoint ID appears
+// or the timeout is reached. Returns true if the datapoint was found, false if timeout occurred.
+func WaitForDatapoint(config *Config, goalSlug, datapointID string, timeout, pollInterval time.Duration) (bool, error) {


The documentation comment should clarify that this function never returns an error (always returns nil for the error value). The current signature (bool, error) suggests errors are possible, but the implementation always returns nil for the error. Consider either:

Updating the doc comment to explicitly state "The error return value is always nil and reserved for future use", or

Simplifying the signature to return only bool if errors won't be needed

Fixed in commit 680011f. Updated documentation to clarify that the error return value is used to indicate permanent failures (e.g., authentication errors, non-existent goals) where continuing to poll would be futile. The function now returns errors for 401, 403, and 404 status codes.

narthur · 2025-12-08T17:21:11Z

@copilot apply changes based on the comments in this thread

…r timeout behavior Co-authored-by: narthur <[email protected]>

Initial plan

f15729f

Copilot AI assigned Copilot and narthur Dec 8, 2025

Copilot started work on behalf of narthur December 8, 2025 13:16 View session

Copilot AI and others added 3 commits December 8, 2025 13:23

feat: replace static sleep with polling for datapoint verification

3fd2412

Co-authored-by: narthur <[email protected]>

fix: add URL escaping for goalSlug and remove trailing whitespace

81aedb1

Co-authored-by: narthur <[email protected]>

fix: use url.Values for proper form data encoding

79c49ce

Co-authored-by: narthur <[email protected]>

Copilot AI changed the title ~~[WIP] Switch from static sleep to polling for datapoint check~~ feat: replace static sleep with polling for datapoint verification after buzz add Dec 8, 2025

Copilot finished work on behalf of narthur December 8, 2025 13:29

Copilot AI requested a review from narthur December 8, 2025 13:29

narthur marked this pull request as ready for review December 8, 2025 15:47

Copilot AI review requested due to automatic review settings December 8, 2025 15:47

Copilot started reviewing on behalf of narthur December 8, 2025 15:48 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Copilot started work on behalf of narthur December 8, 2025 17:21 View session

fix: improve WaitForDatapoint with permanent error handling and bette…

680011f

…r timeout behavior Co-authored-by: narthur <[email protected]>

Copilot finished work on behalf of narthur December 8, 2025 17:29

		_, err := CreateDatapoint(config, goalSlug, timestamp, value, comment)
		return datapointSubmittedMsg{err: err}

feat: replace static sleep with polling for datapoint verification after buzz add #188

Are you sure you want to change the base?

feat: replace static sleep with polling for datapoint verification after buzz add #188

Uh oh!

Conversation

Copilot AI commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Polling Implementation - Final Improvements

Uh oh!

coderabbitai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Other AI code review bot(s) detected

Uh oh!

github-actions bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Pre-release Build Ready

Direct Download Links

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

narthur commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Dec 8, 2025 •

edited

Loading

coderabbitai bot commented Dec 8, 2025 •

edited

Loading

github-actions bot commented Dec 8, 2025 •

edited

Loading