fix(binance): isolate replay-safe capture epochs - #124
Conversation
📝 WalkthroughWalkthroughThe archiver now tracks aggregate-trade observation in session-owned state. Segment rotation and closure use shared replay-readiness gating before seeding or writing checkpoints, and lifecycle tests cover the updated replay-safety behavior. ChangesReplay checkpoint gating
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant run_session
participant process_event
participant rotate_segment
participant replay_checkpoint_ready
participant write_checkpoints
run_session->>process_event: process aggregate-trade event
process_event->>process_event: record observed symbol
run_session->>rotate_segment: rotate with ProcessState
rotate_segment->>replay_checkpoint_ready: evaluate replay readiness
replay_checkpoint_ready-->>rotate_segment: readiness result
rotate_segment->>write_checkpoints: write checkpoints when ready
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd167ca2bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Change contract\nEach Binance websocket session is a fresh capture epoch, and only a safe scheduled rotation whose active books are all bridged and whose epoch archived a valid aggregate trade for every active symbol may seed replay-safe segment_open checkpoints at one pre-close rotation boundary.\n\nOut of scope\nBinance artifact verification and PR #120; cross-event aggregate-trade f/l continuity; downloader or OSS selection; research mapping; deployment, ECS, collector cutover, and live trading.\n\nDependency or merge order\nBased on main 44b554b after PR #119. Merge this producer contract before rebasing and finalizing PR #120.\n\nFocused validation\n- cargo test --locked -p hft-collector --features collector-binance --bin binance-lob-archiver: 36 passed\n- cargo test --locked -p hft-collector --features collector-binance: 159 library passed, 2 ignored; 36 archiver passed; 10 materializer passed; remaining bins passed\n- cargo clippy --locked -p hft-collector --all-targets --features collector-binance --no-deps -- -D warnings: passed\n- cargo fmt --package hft-collector -- --check: passed\n- git diff --check: passed\n- Red-to-green counterexamples cover per-symbol aggregate-trade coverage, unseeded rotation remaining unsafe after a later trade, and one pre-close timestamp shared by segment close/start/open.\n- Existing counterexamples cover snapshot-only epochs, trusted epochs without aggregate trades, reconnect-local state, and scheduled rotation seeding versus gap non-seeding.\n- Exact head 861335c: GitHub CI and CodeRabbit passed; independent Standards and Spec reviews found no issues; all review threads resolved.\n- Scope: one file, 403 insertions and 66 deletions, below the repository split threshold.\n\nRollout or rollback impact\nNo deployment or schema change. The producer changes only capture-epoch and segment-boundary replay safety. Reconnect, gap, exclusion, unseeded, and incomplete epochs remain fail-closed. Rollback is a revert of this PR; live trading remains disabled.