Skip to content

fix: saturate timer tick payload#68

Merged
geoffjay merged 1 commit into
geoffjay:mainfrom
nvphungdev:fix/timer-tick-overflow
May 17, 2026
Merged

fix: saturate timer tick payload#68
geoffjay merged 1 commit into
geoffjay:mainfrom
nvphungdev:fix/timer-tick-overflow

Conversation

@nvphungdev

Copy link
Copy Markdown
Contributor

Summary

  • Saturate timer tick values at i64::MAX before storing them in Value::Integer.
  • Reuse the same tick payload builder for immediate, refresh, and interval emissions.
  • Add a regression test for u64::MAX tick values.

Fixes #39

Validation

  • cargo fmt --manifest-path crates/nemo-data/Cargo.toml -- --check
  • git diff --check

Blocked:

  • cargo test -p nemo-data timer stalled while updating the workspace GPUI git dependency (https://github.com/zed-industries/zed); stopped after retrying to avoid leaving a hanging cargo process.

@geoffjay geoffjay added the review-agent Dispatches the review agent to review a pull request label May 17, 2026

@geoffjay geoffjay left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Summary

Clean fix for timer tick overflow (#39). Saturates u64 tick values at i64::MAX instead of the silent wraparound from as i64, and deduplicates payload-building by extracting create_tick_payload_for.

Issues (blocking)

None.

Suggestions (non-blocking)

None — this is well-scoped and correct.

Approved

LGTM. The saturation behavior is correct, the deduplication eliminates a real maintenance hazard, and the test covers the overflow case.

@geoffjay geoffjay added merge-queue Approved by reviewer, queued for merge by conductor and removed review-agent Dispatches the review agent to review a pull request labels May 17, 2026
@codecov

codecov Bot commented May 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.45%. Comparing base (46d8ab7) to head (01e0870).

Files with missing lines Patch % Lines
crates/nemo-data/src/sources/timer.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   52.35%   52.45%   +0.09%     
==========================================
  Files          71       71              
  Lines        6101     6097       -4     
==========================================
+ Hits         3194     3198       +4     
+ Misses       2907     2899       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@geoffjay geoffjay merged commit b24dc29 into geoffjay:main May 17, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Approved by reviewer, queued for merge by conductor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix potential integer overflow in timer tick count

2 participants