test(collector): add bounded one-shot tape seal benchmark - #778
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a deterministic synthetic 4 GiB market-tape benchmark, a manually dispatched workflow pinned to an exact ChangesMarket-tape benchmark
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant MainCheckout
participant RustBenchmark
participant StepSummary
GitHubActions->>MainCheckout: checkout refs/heads/main at exact SHA
GitHubActions->>RustBenchmark: compile and run one ignored benchmark
RustBenchmark->>GitHubActions: return status and benchmark evidence
GitHubActions->>StepSummary: publish filtered evidence
GitHubActions->>RustBenchmark: remove benchmark data
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.
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 @.github/scripts/test-market-tape-seal-benchmark.sh:
- Around line 46-49: Update .github/scripts/test-market-tape-seal-benchmark.sh
lines 46-49 to reject all credential configurations except the exact safe
persist-credentials: false setting, while preserving the existing checks. Update
.github/workflows/market-tape-seal-benchmark.yml lines 58-62 by adding
persist-credentials: false under the actions/checkout with block.
In `@rust_hft/tools/collector/src/polymarket_upload.rs`:
- Around line 3438-3442: Remove the MIN_FIXTURE_BYTES/MAX_FIXTURE_BYTES
assertion from the shared immutable-fixture test helper around
regular_identity(&fixture).unwrap(). Add the same synthetic size validation to
synthetic_immutable_fixture_reports_full_scan_and_seal_lookup_phases, while
keeping immutable_fixture_reports_full_scan_and_seal_lookup_phases limited to
verifying an immutable closed tape.
🪄 Autofix
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: fdd8c225-9b75-44b6-a604-f880d9d300a8
📒 Files selected for processing (4)
.github/scripts/test-market-tape-seal-benchmark.sh.github/workflows/market-tape-seal-benchmark.yml.github/workflows/ploy-ci.ymlrust_hft/tools/collector/src/polymarket_upload.rs
c061d65 to
4b0c974
Compare
b42e105 to
e21114d
Compare
Change contract
Add one GitHub-hosted, workflow-dispatch-only benchmark contract that deterministically generates a legal 4 GiB Polymarket market tape, runs the existing full-scan-versus-seal attribution/equivalence harness exactly once, and always removes its temporary data.
Issue relationship
Refs #716
Out of scope
Production, canary, Gate, Reference, ACK, ECS, OSS, ACR, credentials, image publication, uploader tuning, data-path semantics, and any automatic/scheduled benchmark execution.
Dependencies and merge order
Depends on merged PR #772 / main
1d94b43a42ca9abfa2783e84c416a4595952204d.Focused validation
Rollout and rollback
After exact-head CI/review and normal merge, manually dispatch once from the then-current exact main SHA. The workflow has read-only repository permission and no external connection. Rollback is deletion/revert of this manual-only workflow; it has no production effect.
Scope exception
None.
Summary by CodeRabbit
New Features
Bug Fixes