feat: randomized jitter between place visits (Bundle E) - #37
Conversation
Closes #25. Adds ScraperConfig::place_panel_jitter (default 750ms): before each place visit the enrich loop sleeps place_panel_delay plus a random 0..=jitter, so navigations no longer happen at a fixed interval that bot-detection can flag. Jitter uses std-only entropy (sub-second wall-clock nanos) via a pure jitter_ms helper — no new dependency. Duration::ZERO disables it. Unit-tested for bounds and the zero case. https://claude.ai/code/session_01TPpTHPokxsZ3dQpRzg4NkD
|
Warning Review limit reached
More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughAdds ChangesPer-place jitter delay feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Address devil's-advocate / code review of Bundle E: - Seed quality (major): subsec_nanos correlated across the ~1.5s loop and was modulo-biased. time_seed now mixes full wall-clock nanos with a process-wide call counter through DefaultHasher (SipHash), so seeds are well-distributed and never repeat or correlate between successive place visits. - Guard the Duration::as_millis() u128 -> u64 cast with try_from (saturating). - Clarify docs: jitter is purely additive (place_panel_delay is the minimum), Duration::ZERO disables it, and place_panel_delay cross-references the jitter. https://claude.ai/code/session_01TPpTHPokxsZ3dQpRzg4NkD
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lib.rs`:
- Around line 375-378: The jitter calculation has two issues: first, the call to
jitter_ms at line 377 performs a lossy cast via as_millis() as u64 without
validating the result, and second, the jitter_ms function (around line 644)
performs max_ms + 1 which can overflow when max_ms equals u64::MAX, causing
panics in debug builds. Fix this by validating and clamping the milliseconds
value to a safe range before passing it to jitter_ms, and add explicit bounds
checking inside the jitter_ms function to safely handle the u64::MAX case
instead of blindly adding 1 to max_ms.
- Line 379: In the tokio::time::sleep call, replace the standard addition
operator between self.cfg.place_panel_delay and jitter with saturating_add to
prevent potential overflow panics. This defensive change ensures that when
extreme user-provided duration values are added together, the operation
gracefully clamps to the maximum Duration value instead of panicking, while
preserving the intended sleep behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b6b33e1-3492-4cd1-b8a9-f1eea5ab7870
📒 Files selected for processing (3)
CHANGELOG.mdREADME.mdsrc/lib.rs
| time_seed(), | ||
| self.cfg.place_panel_jitter.as_millis() as u64, | ||
| )); | ||
| tokio::time::sleep(self.cfg.place_panel_delay + jitter).await; |
There was a problem hiding this comment.
Use saturating duration addition for defensive runtime safety.
At Line 379, self.cfg.place_panel_delay + jitter can overflow and panic for extreme user-provided durations. saturating_add avoids a hard crash while preserving intent.
Proposed fix
- tokio::time::sleep(self.cfg.place_panel_delay + jitter).await;
+ tokio::time::sleep(self.cfg.place_panel_delay.saturating_add(jitter)).await;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tokio::time::sleep(self.cfg.place_panel_delay + jitter).await; | |
| tokio::time::sleep(self.cfg.place_panel_delay.saturating_add(jitter)).await; |
🤖 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 `@src/lib.rs` at line 379, In the tokio::time::sleep call, replace the standard
addition operator between self.cfg.place_panel_delay and jitter with
saturating_add to prevent potential overflow panics. This defensive change
ensures that when extreme user-provided duration values are added together, the
operation gracefully clamps to the maximum Duration value instead of panicking,
while preserving the intended sleep behavior.
2 participants
Overview
Bundle E — adds randomized jitter between place visits in the enrich path (#25).
Previously, with
enrich = true, the scraper navigated to every place URL at a fixedplace_panel_delayinterval (default 1.5 s) — a deterministic cadence that bot-detection can flag. This addsScraperConfig::place_panel_jitter(default 750 ms): before each place visit the loop sleepsplace_panel_delay + rand(0..=jitter).Details
jitter_ms(seed, max_ms)helper. The issue suggestedrand; a weak seed is sufficient here since this only de-regularises timing.Duration::ZEROdisables jitter (jitter_msreturns 0 whenmax_ms == 0).jitter_within_boundscovers the inclusive0..=maxrange and the zero case;config_defaultsasserts the new default.Test plan
cargo buildcargo test(9 passed — addsjitter_within_bounds)cargo fmt --all -- --checkcargo clippy --all-targets -- -D warningsCloses #25
https://claude.ai/code/session_01TPpTHPokxsZ3dQpRzg4NkD
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation