fix(polymarket): verify executable top against depth - #303
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)
📝 WalkthroughWalkthroughThe PR updates two Polymarket research mission snapshot identifiers and changes orderbook verification to compare top-of-book values with executable depth, including boundary-level handling, crossed-book rejection, and targeted tests. ChangesPolymarket orderbook verification
Research mission policy snapshots
Estimated code review effort: 3 (Moderate) | ~20 minutes 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: 3fd723c61d
ℹ️ 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".
| (None, Some(levels)) => levels | ||
| .iter() | ||
| .all(|level| !polymarket_tradeable_price(level.price)), |
There was a problem hiding this comment.
Filter non-executable levels before projecting depth
When a side contains a boundary level plus an executable top (for example, the new 0.02/0.03 ask fixture), this accepts the row but retains the 0.02 level. project_verified_polymarket_evidence copies every retained level into ResearchPmBookSnapshot (polymarket_evidence_projection.rs:155-179), while sweep_buy_to_stake_with_config and the sell sweep accept every price strictly between 0 and 1 (factors_v2.rs:8204-8207, 11126-11128). Consequently the executable-PnL evaluator consumes liquidity that this verifier explicitly classifies as non-executable, overstating fills and corrupting settlement/PnL labels whenever boundary depth is present. Apply the same strict predicate to projected/swept depth, rather than only when selecting the top.
Useful? React with 👍 / 👎.
Change contract
Verify each Polymarket top bid/ask against the unique best executable level in retained full depth using the collector's strict
0.02 < price < 0.98predicate; accept an absent top only when that side has no executable level.Closes #302.
Out of scope
Collector or producer behavior; evidence bytes, manifests, digests, or republishing; snapshot sampling; research projection; evaluator/MCTS; execution policy; Paper or Live.
Dependency / merge order
Independent focused verifier fix on current
main. Merge this PR, publish its exact-main research image, then rerun #232 against the unchanged evidence anchor. No stacked PR.Focused validation
cargo test -p ploy-market-data— 63 passedcargo test -p ploy-research checked_in_btc_and_sol_templates_pin_current_brief_and_rust_policy -- --nocapture— passedcargo fmt -p ploy-market-data -- --check— passedcargo clippy -p ploy-market-data --all-targets --no-deps -- -D warnings— passedgit diff --check— passed0.02/0.03with top0.03and complementary bid depth0.98/0.97with top0.97.Cloud reproduction / pending acceptance
snapshot-issue232-main-0724bfailed at the old absolute-depth predicate. Pinned read-only diagnosticsnapshot-issue232-book-tradeable-proof-0724bchecked all 532 book rows, retained 2,128 boundary levels, and found zero top mismatches under the collector predicate. Post-merge exact-main cloud acceptance remains pending by design.Rollout / rollback impact
One verifier predicate plus targeted tests and mechanically required governed mission pins. Rollout is the exact-main research image; rollback is a one-commit revert. Immutable evidence and successful artifacts are unchanged.
Workflow
Matt
diagnosing-bugsestablished the cloud counterexample; Matt TDD reproduced it before the surgical fix; Mattimplementand two-axiscode-reviewcompleted the focused issue. A PRD was unnecessary because this is one reproduced producer/verifier contract drift.Summary by CodeRabbit
Bug Fixes
Configuration