feat(research): add durable prediction MCTS runner - #200
Conversation
|
Warning Review limit reached
Next review available in: 41 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 Run ID: 📒 Files selected for processing (6)
✨ 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: 413bf79ddf
ℹ️ 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".
| } else if state.advisor_call_consumed { | ||
| return Err( | ||
| "prediction MCTS advisor call was interrupted before durable response".to_string(), | ||
| ); |
There was a problem hiding this comment.
If the process exits after the durable advisor_call_consumed = true checkpoint but before state.advisor is saved, every subsequent resume enters this branch and returns an error forever. This bricks the durable run even though ordinary provider failures fall back to empty advice and the mission may still have unused LLM-call capacity; reconcile the abandoned reservation as a failure or retry it within the remaining budget.
Useful? React with 👍 / 👎.
| while state.training.len() < mission.search_budget.max_candidates { | ||
| if remaining_time(&state).is_zero() { |
There was a problem hiding this comment.
When baseline evaluation consumes the remaining wall-clock budget, the advisor call still runs before this sole deadline check; similarly, a training evaluation that returns after the deadline is observed and can be selected, and held-out evaluation is then started without another gate. Because the generic client/evaluator traits are not required to honor a zero timeout, this can spend calls and incorporate results outside the governed search window; check the deadline before and after every external stage.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
| let _lock = OutputLock::acquire(output_dir)?; | ||
| let state_path = output_dir.join("prediction-mcts-state.json"); |
There was a problem hiding this comment.
When prediction-mcts-state.json is absent but output_dir already contains stale artifacts or a baseline/training symlink, this binds a fresh run to that directory without validation. next_attempt_dir follows those preexisting child paths, so evaluator artifacts can be mixed with another run or written outside the configured root; reject nonempty unbound output and symlink components before creating state or attempts, as the existing run_or_resume path does.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
| let candidate = if let Some(pending) = state.pending.clone() { | ||
| pending | ||
| } else { | ||
| let candidate = engine.propose()?; |
There was a problem hiding this comment.
For a valid mission requesting 56–64 candidates, an unavailable or invalid advisor leaves only the fixed deterministic tree. With five weight components and depth three, that tree has at most 5 + 15 + 35 = 55 weight-unique non-baseline candidates because permutations collapse under seen_blends; this loop nevertheless calls engine.propose() until the requested count and returns an error instead of a normal partial BudgetExhausted result. Treat lack of another novel node as search-space exhaustion or constrain the accepted candidate budget to the available space.
Useful? React with 👍 / 👎.
|
|
||
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] | ||
| #[serde(tag = "kind", rename_all = "snake_case")] | ||
| #[serde(rename_all = "snake_case")] |
There was a problem hiding this comment.
Any PredictionMctsCheckpoint serialized by the previous revision encodes actions as internally tagged objects such as {"kind":"increase","component":"market_midpoint"}, while this annotation change expects externally tagged objects. Because CHECKPOINT_VERSION remains 1 and deserialization fails before checkpoint validation can run, existing persisted checkpoints can no longer resume; retain the old representation or add a backward-compatible deserializer and migration test.
AGENTS.md reference: AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
| Ok(evaluation) => evaluation, | ||
| Err(reason) => return pause(&mission, &state_path, &mut state, reason), | ||
| }; | ||
| engine.observe(&candidate.candidate_id, &evaluation)?; |
There was a problem hiding this comment.
On resume, candidate may come from the separately serialized state.pending; if its blend weights are altered while its candidate ID and recorded digest remain unchanged, the evaluator runs the altered blend and engine.observe() accepts the copied ID/digest as evidence for the original checkpoint node. The existing evaluate_and_observe() path prevents this by recomputing and comparing the candidate payload digest, so this runner must perform the same validation before invoking the evaluator to avoid crediting a reward to the wrong candidate.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
413bf79 to
2800d96
Compare
Closes #198
Change contract
Add the durable prediction adapter around the shared UCT kernel: bounded advisor input, deterministic checkpoint/resume, training-only observations, and post-selection held-out evaluation.
Out of scope
Official CLI switch, legacy rollback selector, evaluator wiring, collectors, snapshots, result publication, and execution.
Dependency / merge order
Stacked on #199; merge #199 first, then this PR, then #187.
Focused validation
cargo test -p ploy-research prediction_mcts --no-default-featurescargo test -p ploy-research checked_in_btc_and_sol_templates_pin_current_brief_and_rust_policy --no-default-featurescargo clippy -p ploy-research --no-default-features --no-deps -- -D warningscargo fmt --checkRollout / rollback impact
Library and tests only; no official entrypoint changes. Revert removes the dormant runner without touching legacy state.