fix(research): collapse reduced MCTS to canonical baseline - #407
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe MCTS engine now treats ChangesMarket midpoint MCTS behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PredictionMctsRun
participant PredictionMctsEngine
participant FakeEvaluator
PredictionMctsRun->>PredictionMctsEngine: propose() with MarketMidpointOnly
PredictionMctsEngine->>PredictionMctsEngine: mark candidate node 0 pending
PredictionMctsEngine-->>PredictionMctsRun: return canonical candidate
PredictionMctsRun->>FakeEvaluator: observe candidate training
PredictionMctsRun->>PredictionMctsEngine: check expandability
PredictionMctsEngine-->>PredictionMctsRun: return false
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
0fab7a6 to
313a78f
Compare
e96da55 to
a482359
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e96da55d99
ℹ️ 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".
a482359 to
71bf456
Compare
71bf456 to
a444f10
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs (1)
858-910: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce the one-candidate limit inside
proposeand checkpoint validation.A midpoint-only advisor passes
blend_allowed; after observing the baseline, a direct secondpropose()bypasseshas_expandable_candidate()and expands that advisor. The resulting LLM child also passes checkpoint validation, so this profile is not actually limited to its canonical root candidate.
rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs#L858-L910: return a terminal error for everyMarketMidpointOnlycall after proposal0, and reject LLM advice for this profile.rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs#L586-L595: reject non-root/expanded MarketMidpointOnly checkpoint states.rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs#L1216-L1229: add a midpoint-only-advisor counterexample and assert a secondpropose()fails.As per PR objectives, this profile must evaluate exactly one canonical root candidate.
🤖 Prompt for 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. In `@rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs` around lines 858 - 910, Enforce the one-candidate MarketMidpointOnly contract: in propose, return a terminal error for every call after proposal 0 and reject LLM advice for this profile; in checkpoint validation, reject any non-root or expanded MarketMidpointOnly state. Update the midpoint-only advisor test to cover the counterexample and assert that a second propose() fails. Apply these changes at rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs:858-910, :586-595, and :1216-1229; the profile must evaluate exactly one canonical root candidate.
🤖 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.
Outside diff comments:
In `@rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs`:
- Around line 858-910: Enforce the one-candidate MarketMidpointOnly contract: in
propose, return a terminal error for every call after proposal 0 and reject LLM
advice for this profile; in checkpoint validation, reject any non-root or
expanded MarketMidpointOnly state. Update the midpoint-only advisor test to
cover the counterexample and assert that a second propose() fails. Apply these
changes at
rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rs:858-910,
:586-595, and :1216-1229; the profile must evaluate exactly one canonical root
candidate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c34c7f77-2664-48ce-accc-70659b46aa63
📒 Files selected for processing (4)
rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.jsonrust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.jsonrust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts.rsrust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts_run.rs
Direct engine proposals bypassed the caller expansion guard, and checkpoint validation admitted expanded reduced-profile state. Enforce the one-candidate/no-LLM contract at construction, proposal, and restore boundaries.
Closes part of #324.
Change contract
For the reduced-authority
MarketMidpointOnlyprofile, evaluate exactly one canonical root baseline candidate and expose no semantically equivalent scale-only or LLM expansion; preserve FullSurface behavior.A PRD is unnecessary because this is a small, already-specified correction inside the approved #324 shared-MCTS contract.
Out of scope
Authenticated evaluator metrics, typed receipts, experiment manifest,
hft-search-kernel, snapshot authority changes, CEX/alpha-harness, collector/runtime/deployment, and profitability claims.Dependency / merge order
Stacked on #405 (
codex/mission-v3-mcts-324-metrics). Merge order: #405 -> this PR -> authenticated evaluator layer.Focused validation
DeterministicMutation; new test requires the canonicalBaselinecandidate and no further expansion.cargo test --locked -p ploy-research prediction_mcts -- --nocapture(30 passed)cargo test --locked -p ploy-research(340 library + 4 binary passed)cargo clippy --locked -p ploy-research --features ml --all-targets --no-deps -- -D warningscargo fmt -p ploy-research -- --checksha256:fb89448ee15a3d62d43a4a260b1bb8b4794b6ff9ca7ad93421a8cecd290325e3Review size: 4 files, 70 insertions / 41 deletions; below the repository review limit.
Rollout / rollback impact
Research-only search semantics and policy identity. Rollback restores semantically redundant midpoint-weight candidates; immutable artifacts remain versioned and are not reinterpreted.
Summary by CodeRabbit
Bug Fixes
Configuration
Tests