feat(research): add prediction MCTS adapter - #195
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89496e2eb7
ℹ️ 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".
| pub mission_id: String, | ||
| pub data_snapshot_id: String, | ||
| pub symbol: String, | ||
| pub horizon: String, |
There was a problem hiding this comment.
When a valid mission is recreated with the same mission ID, snapshot, symbol, and horizon but a different time_cohort_boundary_ms, this identity—and therefore PredictionMctsConfig—remains equal, so restore_checkpoint accepts reward statistics learned under the old train/validation split. Because docs/architecture/PREDICTION_MARKETS.md defines that boundary as part of immutable mission identity, this can silently let one cohort steer another experiment; include the boundary or a digest of the full governed mission in the checkpoint and evidence identity.
AGENTS.md reference: rust_hft/prediction-markets/AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| || self.config.exploration < 0.0 | ||
| || self.config.max_depth == 0 | ||
| || self.config.max_candidates == 0 | ||
| || self.proposed > self.config.max_candidates |
There was a problem hiding this comment.
When a persisted checkpoint is corrupted or tampered with after its pending candidates have been observed, it can retain the existing nodes and seen_blends but reset proposed to zero; this validation only rejects counts above the maximum, and the empty pending map provides no conflicting ordinal. restore_checkpoint consequently accepts it and propose can grant up to another full candidate budget, violating the governed run limit, so validate the count against the immutable seen/history state or authenticate the checkpoint.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
| package:hft-instrument@0.1.0|source=path|checksum=none|features=default | ||
| package:hft-integration@0.1.0|source=path|checksum=none|features=default | ||
| package:hft-ports@0.1.0|source=path|checksum=none|features=default | ||
| package:hft-search-kernel@0.1.0|source=path|checksum=none|features= |
There was a problem hiding this comment.
Because the newly added path dependency is recorded with checksum=none, while neither POLICY_INPUTS nor prediction_policy_sources() includes research-core/search-kernel/Cargo.toml or src/lib.rs, a later change to UCT selection or backpropagation changes prediction behavior without changing current_prediction_policy_snapshot_id(). Missions and checked-in templates can therefore continue claiming the old content-addressed policy while running different search mechanics; hash the kernel manifest and source, or a source-artifact digest, into the policy identity.
AGENTS.md reference: rust_hft/prediction-markets/AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| WeightComponent::MarketMidpoint => proposed.market_midpoint_weight += MUTATION_STEP, | ||
| WeightComponent::ChainlinkDigital => proposed.chainlink_digital_weight += MUTATION_STEP, | ||
| WeightComponent::DistanceLobVol => proposed.distance_lob_vol_weight += MUTATION_STEP, | ||
| WeightComponent::EventSurface => proposed.event_surface_weight += MUTATION_STEP, | ||
| WeightComponent::ExistingModel => proposed.existing_model_weight += MUTATION_STEP, |
There was a problem hiding this comment.
For a valid large-magnitude blend—for example, five weights of 1e100—IEEE-754 rounding makes every weight + 0.25 equal to the original weight. Each expansion is consequently treated as already seen; with max_depth == 1, the first propose call consumes all root actions and returns an error despite a nonzero candidate budget. Since proposal validation permits these finite weights, normalize or canonicalize the blend before mutation, or use a scale-relative step.
Useful? React with 👍 / 👎.
| { | ||
| return Err("training evidence identity does not match the candidate".to_string()); | ||
| } | ||
| let reward = evaluation.training_settlement.reward()?; |
There was a problem hiding this comment.
When evaluation evidence is replayed from persistence or supplied by another evaluator implementation, a caller can construct SettlementTrainingEvidence with matching candidate, digest, and identity fields but any nonempty training_cohort_id; reward() validates only the aggregate ranges. observe then backpropagates that score without binding an event set or content-addressed cohort artifact to the mission boundary, so held-out or cross-cohort scores placed in training_settlement can steer the tree. Require verified cohort/evaluation provenance before accepting the reward.
AGENTS.md reference: rust_hft/prediction-markets/AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
Change contract
Add the second real adapter to the shared UCT seam: typed Polymarket probability-blend candidates expand deterministically under MCTS, while only event-disjoint training-cohort settlement evidence can update rewards.
Out of scope
Switching the official prediction runner, deleting legacy prediction/Alpha Search state, changing collectors or snapshots, result publication, cloud deployment, and live execution.
Dependency or merge order
Parent PRD: #184. Implements #186. Stacked on #190 / #185 and must merge after #190. Later order remains #187 -> #188 -> #189.
Focused validation
cargo test --locked --manifest-path prediction-markets/Cargo.toml -p ploy-research prediction_mcts::tests --lib(7 passed)cargo test --locked --manifest-path prediction-markets/Cargo.toml -p ploy-research prediction_policy_identity_excludes_runtime_and_oms_authority --lib(1 passed)cargo test --locked --manifest-path prediction-markets/Cargo.toml -p ploy-research checked_in_btc_and_sol_templates_pin_current_brief_and_rust_policy --lib(1 passed)git diff --checkScope guardrail exception
This focused adapter is 9 files and +903/-6, above the 750-line assessment point. Named standards reviewer
standards_review_182approved the atomic exception: typed candidate identity, training-only reward, checkpoint/resume, payload/provenance binding, and their counterexample tests form one fail-closed contract. Splitting them would create an independently mergeable adapter with an attribution or checkpoint-authority gap. Cargo.lock, Linux policy graph, policy source binding, and pinned mission templates are direct generated/trust-boundary consequences.Rollout / rollback impact
The adapter is not selected by the official runner in this PR. Revert removes the inactive adapter and restores the prior policy snapshot.
Closes #186