feat(research): add prediction MCTS training evidence - #199
Conversation
📝 WalkthroughWalkthroughAdds training-only settlement probability reporting, exposes the new report API, and adds an optional evaluator mode that validates a prediction-MCTS candidate and emits content-addressed training evidence. Two mission templates receive updated policy snapshot identifiers. ChangesPrediction MCTS training evidence
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CandidateJson
participant MondayPredictionEvaluator
participant TrainingReportBuilder
participant ReportOutputDirectory
MondayPredictionEvaluator->>CandidateJson: Load PredictionMctsCandidate
MondayPredictionEvaluator->>MondayPredictionEvaluator: Validate candidate and governed prior
MondayPredictionEvaluator->>TrainingReportBuilder: Build training-only probability report
TrainingReportBuilder-->>MondayPredictionEvaluator: Return training baselines
MondayPredictionEvaluator->>ReportOutputDirectory: Write content-addressed evidence artifact
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 71f2c3111e
ℹ️ 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".
| if prior.mission_id.as_deref() != Some(candidate.identity.mission_id.as_str()) | ||
| || prior.data_snapshot_id.as_deref() | ||
| != Some(candidate.identity.data_snapshot_id.as_str()) | ||
| || prior.symbols.as_slice() != [candidate.identity.symbol.as_str()] | ||
| || prior.horizon.as_deref() != Some(candidate.identity.horizon.as_str()) | ||
| || prior.probability_blends[0] != candidate.probability_blend |
There was a problem hiding this comment.
When the externally supplied candidate JSON has a forged or stale candidate_id or probability_blend_sha256, this check still accepts it because it compares only the identity and blend payload; the unverified values are then copied into a content-addressed artifact and the evaluator reports success. Recompute and verify the blend digest and candidate identity before scoring so this advertised fail-closed mode cannot emit evidence bound to a different candidate.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
| let event_count = training | ||
| .iter() | ||
| .map(|row| row.event_id.as_str()) | ||
| .collect::<HashSet<_>>() | ||
| .len(); |
There was a problem hiding this comment.
When the training slice contains events without a settlement label, a fillable full-depth entry, or a usable candidate probability, this counts those events even though build_settlement_probability_report_with_surface subsequently excludes their rows. The emitted event_count can therefore materially overstate the number of settlement events behind the reported Brier score and log loss; derive it from the eligible samples for the selected candidate model instead.
Useful? React with 👍 / 👎.
| min_bucket_observations: options.review.min_observations.max(20), | ||
| ..Default::default() | ||
| }, | ||
| time_cohort: settlement_time_cohort, |
There was a problem hiding this comment.
When this mode is invoked with a --time-cohort-boundary-ms different from the mission boundary that produced the candidate, no validation detects it because neither PredictionMctsIdentity nor the governed prior carries that boundary, and this value is passed directly into training. Moving the boundary later admits intended held-out events into the reward cohort, while PredictionMctsEngine::observe accepts the resulting arbitrary nonempty cohort ID, so a misconfigured runner can leak holdout outcomes into MCTS search; bind and validate the mission boundary before evaluation.
AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rust_hft/prediction-markets/crates/ploy-research/src/bin/monday-prediction-evaluator.rs (2)
1636-1706: 🧹 Nitpick | 🔵 TrivialRun sidecar-focused checks given the new evidence contract is consumed by
ploy-agent-sidecar.
PredictionMctsEvaluation/SettlementTrainingEvidencepopulated here are validated and consumed byobserve()in the sidecar crate (per provided context). Beyond the evaluator-binary tests already listed for this PR, consider also runningcargo test -p ploy-agent-sidecarto confirm the identity/blend-digest checks inobserve()still accept evidence shaped this way.As per path instructions: "Run sidecar checks with
cargo test -p ploy-agent-sidecar."🤖 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/bin/monday-prediction-evaluator.rs` around lines 1636 - 1706, Run the required sidecar-focused validation with cargo test -p ploy-agent-sidecar, confirming observe() accepts the PredictionMctsEvaluation and SettlementTrainingEvidence identity and blend-digest fields populated in this training path.Source: Path instructions
1636-1636: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
--prediction-mcts-training-candidate-jsonsilently takes precedence over--reprice-pilot-10sif both are passed.The training-candidate branch returns early (Line 1705) with no check that
reprice_pilot_10s(or other mutually exclusive modes) wasn't also requested. If an orchestration script mistakenly sets both flags, the reprice pilot silently never runs instead of failing loudly.validate_reprice_pilot_configalready validates other flag interactions for the reprice path — consider adding an equivalent guard for this new dormant mode.🛡️ Proposed guard
if let Some(candidate_path) = prediction_mcts_training_candidate_json.as_deref() { + if reprice_pilot_10s { + panic!("--prediction-mcts-training-candidate-json cannot be combined with --reprice-pilot-10s"); + } let candidate: PredictionMctsCandidate = std::fs::read(candidate_path)As per coding guidelines: "Never simplify away validation, security, data-loss prevention, or other trust-boundary checks."
Also applies to: 1707-1747
🤖 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/bin/monday-prediction-evaluator.rs` at line 1636, Guard the prediction_mcts_training_candidate_json branch before its early return so it rejects reprice_pilot_10s and any other mutually exclusive modes instead of silently taking precedence. Reuse or extend validate_reprice_pilot_config where appropriate, and ensure invalid flag combinations fail loudly before entering the training-candidate flow.Source: Coding guidelines
🤖 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 `@rust_hft/prediction-markets/crates/ploy-research/src/factors_v2.rs`:
- Around line 12658-12679: Add an explicit non-empty assertion for
report.baselines in settlement_training_report_never_scores_the_held_out_event
before the existing all-baselines n == 1 check, ensuring the test cannot pass
vacuously while preserving its current assertions.
---
Nitpick comments:
In
`@rust_hft/prediction-markets/crates/ploy-research/src/bin/monday-prediction-evaluator.rs`:
- Around line 1636-1706: Run the required sidecar-focused validation with cargo
test -p ploy-agent-sidecar, confirming observe() accepts the
PredictionMctsEvaluation and SettlementTrainingEvidence identity and
blend-digest fields populated in this training path.
- Line 1636: Guard the prediction_mcts_training_candidate_json branch before its
early return so it rejects reprice_pilot_10s and any other mutually exclusive
modes instead of silently taking precedence. Reuse or extend
validate_reprice_pilot_config where appropriate, and ensure invalid flag
combinations fail loudly before entering the training-candidate flow.
🪄 Autofix (Beta)
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
Run ID: 212445e5-7fcf-4217-9ff5-a19b895ede86
📒 Files selected for processing (5)
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/bin/monday-prediction-evaluator.rsrust_hft/prediction-markets/crates/ploy-research/src/factors_v2.rsrust_hft/prediction-markets/crates/ploy-research/src/lib.rs
| #[test] | ||
| fn settlement_training_report_never_scores_the_held_out_event() { | ||
| let (start, boundary, end, rows, options) = short_settlement_time_cohort_case(); | ||
| let options = SettlementProbabilityWalkForwardOptions { | ||
| time_cohort: Some(SettlementProbabilityTimeCohort::new(boundary, 300).unwrap()), | ||
| ..options | ||
| }; | ||
| let report = build_settlement_training_probability_report_with_prior( | ||
| &rows, start, end, None, options, | ||
| ); | ||
|
|
||
| assert_eq!(report.event_count, 1); | ||
| assert!(report.baselines.iter().all(|baseline| baseline.n == 1)); | ||
| assert_eq!( | ||
| report.training_cohort_id, | ||
| format!( | ||
| "settlement-training-before-{}-300s", | ||
| boundary.timestamp_millis() | ||
| ) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test doesn't assert baselines is non-empty, so it can pass vacuously.
assert!(report.baselines.iter().all(|baseline| baseline.n == 1)) (Line 12670) is true for an empty Vec too. If a future regression made build_settlement_training_probability_report_with_prior always return empty baselines (e.g., threshold/model-selection bug), this test would still pass. Compare with the sibling test settlement_probability_short_time_cohort_emits_one_held_out_window, which explicitly asserts !report.windows.is_empty() before checking window contents.
🧪 Proposed fix
assert_eq!(report.event_count, 1);
+ assert!(!report.baselines.is_empty());
assert!(report.baselines.iter().all(|baseline| baseline.n == 1));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn settlement_training_report_never_scores_the_held_out_event() { | |
| let (start, boundary, end, rows, options) = short_settlement_time_cohort_case(); | |
| let options = SettlementProbabilityWalkForwardOptions { | |
| time_cohort: Some(SettlementProbabilityTimeCohort::new(boundary, 300).unwrap()), | |
| ..options | |
| }; | |
| let report = build_settlement_training_probability_report_with_prior( | |
| &rows, start, end, None, options, | |
| ); | |
| assert_eq!(report.event_count, 1); | |
| assert!(report.baselines.iter().all(|baseline| baseline.n == 1)); | |
| assert_eq!( | |
| report.training_cohort_id, | |
| format!( | |
| "settlement-training-before-{}-300s", | |
| boundary.timestamp_millis() | |
| ) | |
| ); | |
| } | |
| #[test] | |
| fn settlement_training_report_never_scores_the_held_out_event() { | |
| let (start, boundary, end, rows, options) = short_settlement_time_cohort_case(); | |
| let options = SettlementProbabilityWalkForwardOptions { | |
| time_cohort: Some(SettlementProbabilityTimeCohort::new(boundary, 300).unwrap()), | |
| ..options | |
| }; | |
| let report = build_settlement_training_probability_report_with_prior( | |
| &rows, start, end, None, options, | |
| ); | |
| assert_eq!(report.event_count, 1); | |
| assert!(!report.baselines.is_empty()); | |
| assert!(report.baselines.iter().all(|baseline| baseline.n == 1)); | |
| assert_eq!( | |
| report.training_cohort_id, | |
| format!( | |
| "settlement-training-before-{}-300s", | |
| boundary.timestamp_millis() | |
| ) | |
| ); | |
| } |
🤖 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/factors_v2.rs` around
lines 12658 - 12679, Add an explicit non-empty assertion for report.baselines in
settlement_training_report_never_scores_the_held_out_event before the existing
all-baselines n == 1 check, ensuring the test cannot pass vacuously while
preserving its current assertions.
Closes #197
Change contract
Add a fail-closed evaluator mode that scores one typed MCTS probability candidate on training settlement events only.
Out of scope
Shared runner, official entrypoint switch, LLM selection, checkpointing, hold-out scoring, token execution, collectors, snapshots, publication, and live execution.
Dependency / merge order
Base layer. Merge before #198, then #187.
Focused validation
cargo test -p ploy-research settlement_training_report_never_scores_the_held_out_event --no-default-featurescargo test -p ploy-research --bin monday-prediction-evaluator --features db --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 --features db --no-default-features --no-deps -- -D warningscargo fmt --checkRollout / rollback impact
Dormant evaluator flag only; no production entrypoint changes. Revert this PR to remove the capability.
Summary by CodeRabbit
New Features
Configuration