feat(research): expose authenticated partition view - #383
Conversation
📝 WalkthroughWalkthrough
ChangesAuthenticated partition identity
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant EventCohortPartition
participant authenticate_ready_event_cohort
participant AuthenticatedReadyEventCohort
participant AuthenticatedResearchSnapshot
EventCohortPartition->>authenticate_ready_event_cohort: partition identity
authenticate_ready_event_cohort->>AuthenticatedReadyEventCohort: AuthenticatedPartitionView
AuthenticatedReadyEventCohort->>AuthenticatedResearchSnapshot: partition_view
AuthenticatedResearchSnapshot-->>AuthenticatedResearchSnapshot: expose fresh or cached view
Possibly related issues
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.
🧹 Nitpick comments (1)
rust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rs (1)
3824-3829: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the real authentication path and all membership buckets.
The test manually seeds
partition_view, so it cannot catch regressions inAuthenticatedPartitionView::from_partitionorauthenticate_ready_event_cohort(Line 529). Empty crossing-excluded and held-out lists also leave those propagation paths untested. Add a focused test using a verified partition with distinct IDs in all three sets.As per coding guidelines, safety boundaries require targeted counterexample tests, not only workspace compilation.
Also applies to: 5000-5021
🤖 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/research_snapshot.rs` around lines 3824 - 3829, Update the test around AuthenticatedPartitionView to construct a verified partition and obtain the view through AuthenticatedPartitionView::from_partition and authenticate_ready_event_cohort, rather than manually seeding partition_view. Populate distinct market IDs in train_market_ids, crossing_excluded_market_ids, and held_out_market_ids, then assert all three buckets propagate correctly through the authenticated path.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.
Nitpick comments:
In `@rust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rs`:
- Around line 3824-3829: Update the test around AuthenticatedPartitionView to
construct a verified partition and obtain the view through
AuthenticatedPartitionView::from_partition and authenticate_ready_event_cohort,
rather than manually seeding partition_view. Populate distinct market IDs in
train_market_ids, crossing_excluded_market_ids, and held_out_market_ids, then
assert all three buckets propagate correctly through the authenticated path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c85476f-e835-4246-8dea-2e85e5656208
📒 Files selected for processing (2)
rust_hft/prediction-markets/crates/ploy-research/src/research_snapshot.rsrust_hft/prediction-markets/tasks/todo.md
Change contract
Expose the validated partition boundary plus train/crossing-excluded/held-out membership only through authenticated
ResearchSnapshotadmission and cache readback. The view is derived from the verified #322 partition; callers cannot provide or recompute it. Atomically synchronize the BTC/SOL mission policy digest becauseresearch_snapshot.rsis a policy identity input.Out of scope
MCTS/evaluator reward logic (#324), collector deployment or cutover (#326), runtime promotion, and result publication.
Dependency / merge order
Base:
5a4f1c5eadb39d472bea30eead236c985e5f85e0(main). Depends on merged #365 catalog partition and #373/#381 sealed snapshot admission. Merge this PR before rebuilding #324.Focused validation
cargo +1.91 test --locked -p ploy-research research_snapshot --lib(27 passed)cargo +1.91 test --locked -p ploy-research --lib(329 passed)cargo +1.91 test --locked -p ploy-research prediction_loop::tests::checked_in_btc_and_sol_templates_pin_current_brief_and_rust_policy --librustfmt +1.91 --edition 2021 --check crates/ploy-research/src/research_snapshot.rsgit diff --checkRollout / rollback impact
No production collector or execution change. Reverting this PR removes the read-only snapshot partition view and its synchronized template pin; authenticated snapshot admission remains fail-closed.