Skip to content

relayburn-sdk: fix three small Rust correctness bugs#351

Merged
willwashburn merged 1 commit intomainfrom
rust-small-bugs
May 7, 2026
Merged

relayburn-sdk: fix three small Rust correctness bugs#351
willwashburn merged 1 commit intomainfrom
rust-small-bugs

Conversation

@willwashburn
Copy link
Copy Markdown
Member

Summary

Three small correctness fixes flagged in #337.

  1. Fix watch_loop.rs first-tick lag. The non-immediate branch was unconditionally consuming the first interval tick to "skip" an immediate run that branch never produced, so the first periodic tick fired at 2 * interval. Switch to interval_at(now + interval, period) so the first tick lands at ~interval in both modes. Adds a deterministic regression test using start_paused + tokio::time::advance.

  2. Rename now_isonow_lex_token. The function emits ts:NNN.NNN (a sortable lex token), not ISO-8601; the column doc was misleading. Rename + expand doc to call out the format contract; update the matching cutoff helper's pointer in burn state prune. Pure naming/doc fix; no behavior change. Actual ISO-8601 adoption tracked in Rust refactor: collapse 3 hand-rolled civil-date implementations onto the time crate #331.

  3. Rename save_cursor_changessave_cursors_if_changed + fix doc. The doc claimed per-key diffing; the impl is an equality check + full save. Rename and rewrite the doc to match. Per-key delta writes deferred to a separate perf PR.

Closes #337.

Test plan

  • cargo build --workspace --all-targets clean
  • cargo test --workspace passes (incl. new watch_loop deterministic-timing test)
  • cargo clippy --workspace --all-targets no new warnings

🤖 Generated with Claude Code

1. watch_loop non-immediate path fired its first periodic tick at
   2*interval instead of interval. The loop unconditionally consumed
   the first interval tick to "skip" an immediate run that the
   non-immediate branch never produced. Switch to interval_at(now +
   interval, period) so the first tick lands one interval out in both
   modes. Add a deterministic regression test using start_paused +
   tokio::time::advance.

2. now_iso emits ts:NNN.NNN (a sortable lex token), not ISO-8601.
   Rename to now_lex_token, expand the doc comment to call out the
   format contract, and update the matching cutoff helper's doc
   pointer in burn state's prune path. Actual ISO-8601 adoption is
   tracked in #331.

3. save_cursor_changes lied in its doc — it claimed per-key diffing
   but the impl is an equality check + full save. Rename to
   save_cursors_if_changed and rewrite the doc to match. Per-key
   delta writes deferred to a separate perf PR.

Closes #337.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c9829436-114e-4332-a101-f40863dea4c2

📥 Commits

Reviewing files that changed from the base of the PR and between e5e5b99 and 3b889a7.

📒 Files selected for processing (6)
  • crates/relayburn-cli/src/commands/state.rs
  • crates/relayburn-sdk/src/ingest.rs
  • crates/relayburn-sdk/src/ingest/cursors.rs
  • crates/relayburn-sdk/src/ingest/ingest.rs
  • crates/relayburn-sdk/src/ingest/watch_loop.rs
  • crates/relayburn-sdk/src/ledger/writer.rs

📝 Walkthrough

Walkthrough

This PR fixes three correctness bugs from issue #337. It standardizes ledger timestamps to a lexicographically sortable format, renames the cursor persistence function to match its actual semantics, and corrects watch loop periodic tick scheduling for non-immediate configurations.

Changes

Rust Correctness Fixes: Timestamp Format, Cursor Semantics, Watch Loop Timing

Layer / File(s) Summary
Ledger Timestamp Standardization
crates/relayburn-sdk/src/ledger/writer.rs, crates/relayburn-cli/src/commands/state.rs
now_iso() is replaced by now_lex_token() to reflect the actual ts:{secs:020}.{nanos:09} format. All call sites in append_stamp, append_content, and debug_now are updated. Documentation in the CLI state command is corrected to reference the lexicographic format.
Cursor Persistence Semantics
crates/relayburn-sdk/src/ingest/cursors.rs, crates/relayburn-sdk/src/ingest.rs, crates/relayburn-sdk/src/ingest/ingest.rs
save_cursor_changes() is renamed to save_cursors_if_changed() and documentation is corrected: the function skips writes only when cursors are equal, then persists the entire cursor map (not per-key diffs). Public re-exports and all call sites across five ingest functions (ingest_all, ingest_claude_projects, ingest_codex_sessions, ingest_opencode_sessions, ingest_claude_session) are updated.
Watch Loop First Tick Timing
crates/relayburn-sdk/src/ingest/watch_loop.rs
interval() with manual skip is replaced by interval_at(now + interval, interval) to ensure the first periodic tick for non-immediate configurations fires after one interval (not two). A regression test non_immediate_first_tick_lands_at_interval verifies correct timing.

Possibly related PRs

  • AgentWorkforce/burn#313: Updates crates/relayburn-cli/src/commands/state.rs for timestamp formatting and state CLI behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Three bugs scurry through the code,
Timestamps now in sortable mode,
Cursors saved with honest names,
Ticks that fire on rightful frames,
Hop hop, correctness is our shame!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: three small Rust correctness bugs related to watch_loop timing, timestamp naming, and cursor persistence function naming.
Description check ✅ Passed The description is well-detailed and directly related to the changeset. It explains all three fixes, references issue #337, provides test confirmation, and uses clear structured sections.
Linked Issues check ✅ Passed The PR fully addresses all three coding requirements from issue #337: fixes watch_loop first-tick lag using interval_at(), renames now_iso to now_lex_token with updated docs, and renames save_cursor_changes to save_cursors_if_changed with corrected documentation.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the three bugs identified in issue #337. The modifications to state.rs, ingest.rs, cursors.rs, ingest.rs, watch_loop.rs, and writer.rs are all necessary implementations of the fixes with no extraneous changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rust-small-bugs

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

@willwashburn willwashburn merged commit 2b56e6d into main May 7, 2026
8 checks passed
@willwashburn willwashburn deleted the rust-small-bugs branch May 7, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rust correctness: three small bugs (watch_loop first tick, now_iso label, cursor diff)

1 participant