fix(collector): accept closed same-hour reference fragments - #175
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReference-segment validation now accepts segments from the same or next consecutive UTC hour. Tests cover same-hour acceptance, larger-gap rejection, updated messaging, and aligned duplicate-trade timestamps. ChangesReference validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 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: 2e7eb4cc68
ℹ️ 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".
| if current < previous || current > previous + TimeDelta::hours(1) { | ||
| bail!("reference segments must be same or consecutive UTC hours"); |
There was a problem hiding this comment.
Reject backward same-hour fragments
When two fragments are in the same UTC hour, this condition accepts either order because current == previous regardless of their start_recorded_at/end_recorded_at values. A fragment recorded at 05:30 followed by one recorded at 05:10 therefore passes validation, after which combine_references concatenates them in that caller-supplied order and downstream selection retains the first metadata occurrence, making evidence provenance and output depend on an invalid ordering. Compare the authenticated timestamps for equal-hour fragments so backward inputs remain fail-closed.
Useful? React with 👍 / 👎.
| if current < previous || current > previous + TimeDelta::hours(1) { | ||
| bail!("reference segments must be same or consecutive UTC hours"); |
There was a problem hiding this comment.
Reject duplicate reference fragment identities
When an identical same-hour triplet is supplied more than once and that fragment contains no trade rows (for example, a metadata/settlement-only fragment alongside a separate trade fragment), this relaxed check accepts it and the later duplicate guard cannot detect it because it only tracks polymarket_trade record IDs. Validation then reports the same immutable source multiple times and concatenates its records multiple times, so a caller mistake changes authenticated provenance despite adding no new source data. Track and reject duplicate segment digests or file identities before combining the fragments.
Useful? React with 👍 / 👎.
Change contract
Accept an authenticated ordered set of closed Polymarket reference triplets when adjacent immutable fragments are from the same UTC hour or the immediately following hour; reject backward order and skipped hours.
Acceptance evidence
cargo test -p hft-collector --lib --locked— 182 passed, 2 ignored.cargo clippy -p hft-collector --lib --tests --locked -- -D warnings— passed.Out of scope
Raw collector discovery or deployment, OSS staging/copying, evidence publication, cohort/snapshot construction, evaluator/MCTS, and live execution.
Dependency / merge order
Implements #174 and unblocks #173. Merge before the #173 cloud artifact compilation is retried; no stacked base PR.
Rollout / rollback impact
The parser remains fail-closed for bound files, manifests, policy, duplicate trades, and completion evidence. Rollback reverts only the same-hour transition compatibility; immutable OSS artifacts are unchanged.
Summary by CodeRabbit