feat(ploy-research): seal authenticated MCTS result evidence - #412
Conversation
📝 WalkthroughWalkthroughThe change binds authenticated selection evidence to durable lifecycle and content digests, adds validated content-addressed result receipts and three-receipt experiment manifests, persists receipts from authenticated trials, expands validation tests, and updates two mission snapshot identifiers. ChangesAuthenticated prediction artifacts
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ResearchTrial
participant SelectionEvidence
participant EvaluationArtifact
participant ResultReceipt
participant ArtifactStorage
ResearchTrial->>SelectionEvidence: obtain timestamped checkpoint and candidate digests
ResearchTrial->>EvaluationArtifact: validate held-out metrics
ResearchTrial->>ResultReceipt: construct authenticated receipt
ResultReceipt->>ArtifactStorage: write content-addressed JSON
ArtifactStorage-->>ResearchTrial: return receipt reference
ResearchTrial-->>ResearchTrial: return summary and receipt reference
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: 7dadf29f90
ℹ️ 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".
0d8cabe to
37896a9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts_authenticated.rs (2)
1897-2049: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider splitting this into per-behavior tests.
This single case now asserts metric rejection, task/metrics mismatch, path isolation, receipt cardinality, manifest readback, claim tampering, digest drift, lifecycle tampering, joint-state drift, and artifact tampering. A failure anywhere reports under a name that describes only the cardinality contract, which slows triage on a governance-critical evidence path. Separate
#[test]functions sharing the existingwrite_test_receipthelper would keep the setup cost low.🤖 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_authenticated.rs` around lines 1897 - 2049, Split experiment_manifest_requires_exactly_three_matching_typed_receipts into focused #[test] functions for metric rejection, task/metrics mismatch, path isolation, receipt cardinality, manifest readback, claim tampering, digest drift, lifecycle validation, joint-state drift, and artifact tampering. Reuse the existing setup helpers, especially write_test_receipt, and give each test a behavior-specific name while preserving the current assertions.
569-573: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared snapshot-hash validator instead of re-implementing the hex rule.
prediction_mission_v3.rsalready owns this rule (validate_snapshot_hash, called fromvalidate_prediction_mission_v3). Duplicating the 16-char lowercase-hex check here lets the two definitions drift, and it also folds a format error into the "claims are invalid" message.♻️ Suggested change
- if self.snapshot_hash.len() != 16 - || !self - .snapshot_hash - .bytes() - .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) - || self.unavailable_claims.profitability != RESEARCH_TRIAL_PROFITABILITY_CLAIM + crate::prediction_mission_v3::validate_snapshot_hash(&self.snapshot_hash)?; + if self.unavailable_claims.profitability != RESEARCH_TRIAL_PROFITABILITY_CLAIM || self.unavailable_claims.promotion != RESEARCH_TRIAL_PROMOTION_CLAIM {Requires
validate_snapshot_hashto be at leastpub(crate).#!/bin/bash rg -nP -C3 'fn validate_snapshot_hash' rust_hft/prediction-markets/crates/ploy-research/src🤖 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_authenticated.rs` around lines 569 - 573, Replace the inline snapshot_hash length and character checks in the affected validation logic with the shared validate_snapshot_hash function from prediction_mission_v3.rs, making that function at least pub(crate) so it is accessible. Preserve separate handling for format validation rather than folding snapshot-hash failures into the claims-invalid message.
🤖 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.
Nitpick comments:
In
`@rust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts_authenticated.rs`:
- Around line 1897-2049: Split
experiment_manifest_requires_exactly_three_matching_typed_receipts into focused
#[test] functions for metric rejection, task/metrics mismatch, path isolation,
receipt cardinality, manifest readback, claim tampering, digest drift, lifecycle
validation, joint-state drift, and artifact tampering. Reuse the existing setup
helpers, especially write_test_receipt, and give each test a behavior-specific
name while preserving the current assertions.
- Around line 569-573: Replace the inline snapshot_hash length and character
checks in the affected validation logic with the shared validate_snapshot_hash
function from prediction_mission_v3.rs, making that function at least pub(crate)
so it is accessible. Preserve separate handling for format validation rather
than folding snapshot-hash failures into the claims-invalid message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b996ea8a-dcfb-4fd5-86de-a0941147c6e8
📒 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_authenticated.rsrust_hft/prediction-markets/crates/ploy-research/src/prediction_mcts_run.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json
- rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json
37896a9 to
1ad3cb5
Compare
Change contract
After an admitted ResearchTrial completes post-selection held-out evaluation, content-address and durably checkpoint the evaluator artifact before sealing the result, then publish one task-isolated typed result receipt; accept a parent experiment manifest only when verified readback contains exactly one Settlement, one Up, and one Down receipt sharing the same admitted Mission, snapshot, partition, policy, immutable-image identity, and joint Up/Down state.
Closes #324.
Out of scope
#325 producer-to-receipt CI fixture, #233 ACK runtime trial and launch attestation, #234 immutable publication, collector deployment/#326, #205/#400/#401/#402, CEX, alpha-harness, hft-search-kernel, Paper/Shadow/Live, promotion, and profitability claims.
Dependencies and merge order
Depends on #408, merged at
74869ec32cc23e67b6c9bcd328bb36d67c88299d. This branch is rebased ontomain@eb095a786513facb1a59b1575a5111c94e36b4d1; merge this PR before starting #325.Focused validation
cargo test --locked -p ploy-research --quiet— 352 library tests plus 4 binary tests pass.cargo test --locked -p ploy-research --features ml --quiet— 376 library tests plus 4 binary tests pass.cargo clippy --locked -p ploy-research --all-targets --features ml --no-deps -- -D warnings— pass.rustfmt --edition 2021 --checkandgit diff --check eb095a78...1ad3cb5b— pass.sha256:e351eaab58e7db004d5e5defba4d249a8363183e32ef93a8269ef117f2686653; BTC/SOL templates match.414f0b6fa5ab15f5c22f90e13e1725fadbed3529f4ccbf23ca4f3379cd7cc3be./root/pr412_standardsand Spec reviewer/root/pr412_specreviewed exact rangeeb095a786513facb1a59b1575a5111c94e36b4d1...1ad3cb5b00ddcaa84b2a3838442b4c61b744fd29; final Spec result is CLEAN and Standards finding is resolved by this metadata update.Rollout and rollback
Research-only artifact/readback contract and checked-in policy fingerprint; no collector, runtime launcher, execution, or live-trading change. Roll back this PR atomically. Readers fail closed on incompatible run/checkpoint state, missing or mutated held-out artifacts, receipt/manifest schema, lifecycle evidence, or stale policy identity. Full repository revision, release-binary, and OCI launch attestation remain the separate #233 runtime contract, with immutable publication/readback in #234.
Scope exception
4 files / 1,228 additions / 38 deletions exceeds the 750-line reassessment threshold. This work was already split from typed evidence Layer 1 (#384) and the authenticated runner Layer 2 (#408). The remaining diff is one inseparable authenticated-result trust contract: the durable selection checkpoint must bind the content-addressed held-out artifact that the typed receipt rehashes, and the exact-three manifest must then re-read those receipts. Splitting any of those seams would temporarily permit mutable evaluator evidence or partial result publication. The named Standards and Spec reviewers above approved the exact final range. The unit rolls back safely only as one research-only artifact contract.