fix(research): pin static prediction policy graph - #76
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR replaces generated dependency fingerprints with checked-in v5 Linux dependency artifacts, validates their hashes and package composition during builds, updates runtime identity tests, and changes mission snapshot identifiers and related policy documentation. ChangesPrediction-policy identity
Sequence Diagram(s)sequenceDiagram
participant Cargo
participant build.rs
participant CheckedInGraph
participant RuntimeTests
participant MissionConfig
Cargo->>build.rs: execute build verification
build.rs->>CheckedInGraph: validate graph and canonical SHA-256
build.rs-->>RuntimeTests: expose verified dependency identity
RuntimeTests->>RuntimeTests: compare runtime identity and rejection cases
MissionConfig->>RuntimeTests: use updated policy snapshot identifier
🚥 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: 0c9bcb3f8c
ℹ️ 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".
| // The policy contract follows only target runtime dependencies. | ||
| // Build scripts and proc macros compile for the host and would | ||
| // otherwise make this Linux identity vary by developer machine. | ||
| "normal,no-proc-macro", |
There was a problem hiding this comment.
This filters all proc-macro edges out of the policy dependency fingerprint; Cargo 1.91.1 cargo tree --help documents no-proc-macro as an --edges filter, and several files that are explicitly part of the policy source set still rely on proc macros that generate target code (for example prediction_loop.rs:288 includes ploy-feed-loaders/src/database.rs, which uses #[derive(sqlx::FromRow)] at database.rs:227, and the evaluator uses #[tokio::main]). If a lockfile update changes one of those macro implementations or their parsing/codegen stack without changing the displayed normal packages, current_prediction_policy_snapshot_id() remains unchanged while the compiled evaluator/loader behavior can change, so stale missions can keep passing the content-addressed policy check.
Useful? React with 👍 / 👎.
0c9bcb3 to
c4ecbd6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/prediction-policy-dependencies.linux.sha256`:
- Line 1: Replace the all-zero value in
rust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256:1
with the actual Linux-generated sha256 digest, then recompute the derived search
policy snapshot ID and update search_policy_snapshot_id in
rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json:17
and
rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json:17
to match it.
🪄 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: 19139646-4ff1-41dd-8f5a-170ee1510b34
📒 Files selected for processing (7)
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/Cargo.tomlrust_hft/prediction-markets/crates/ploy-research/build.rsrust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256rust_hft/prediction-markets/crates/ploy-research/src/prediction_policy_identity.rsrust_hft/prediction-markets/tasks/todo.md
| @@ -0,0 +1 @@ | |||
| sha256:0000000000000000000000000000000000000000000000000000000000000000 | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Replace placeholder hash and update dependent mission snapshot IDs.
The file prediction-policy-dependencies.linux.sha256 currently contains an all-zeros placeholder hash. Because build.rs enforces an exact hash match on native Linux hosts (if native_linux_host() && canonical_hash != generated_hash { panic!(...) }), this placeholder will cause the build to panic and fail in Linux CI environments.
Additionally, the search_policy_snapshot_id in the mission templates was likely computed using this placeholder hash and will fail validation once the true dependency hash is applied.
rust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256#L1-L1: Replace the all-zeros placeholder with the actualsha256:digest generated on a Linux host.rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json#L17-L17: Updatesearch_policy_snapshot_idto match the new ID derived from the corrected dependency hash.rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json#L17-L17: Updatesearch_policy_snapshot_idto match the new ID derived from the corrected dependency hash.
📍 Affects 3 files
rust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256#L1-L1(this comment)rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json#L17-L17rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json#L17-L17
🤖 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/prediction-policy-dependencies.linux.sha256`
at line 1, Replace the all-zero value in
rust_hft/prediction-markets/crates/ploy-research/prediction-policy-dependencies.linux.sha256:1
with the actual Linux-generated sha256 digest, then recompute the derived search
policy snapshot ID and update search_policy_snapshot_id in
rust_hft/prediction-markets/config/research_missions/polymarket-btc-5m.example.json:17
and
rust_hft/prediction-markets/config/research_missions/polymarket-sol-5m.example.json:17
to match it.
c4ecbd6 to
5c6888e
Compare
97be133 to
d22c92a
Compare
d22c92a to
9108e89
Compare
Change contract
The research-only prediction policy identity consumes a reviewed, checked-in
x86_64-unknown-linux-gnudefault,dbdependency graph rather than recursively calling Cargo during every build. The v5 graph pins resolved package/source/checksum/feature evidence plus hashes forCargo.lockand its four path-dependency manifests. The build script fails closed if an input, graph hash, PostgreSQL profile, or forbidden authority/SQLite/host/proc-macro package drifts. BTC and SOL templates pin the resulting policy ID.Out of scope
Dependency / merge order
None. This is a standalone research CI fix against
main. After #76 merges, #75 must rebase or rerun CI against the updated base.Focused validation
cargo metadata --locked --no-deps --format-version 1cargo test --locked -p ploy-research runtime_identity_uses_a_canonical_checked_in_linux_graph_with_features --lib -- --test-threads=1cargo test --locked -p ploy-research checked_in_graph_validation_rejects_stale_and_forbidden_counterexamples --lib -- --test-threads=1cargo test --locked -p ploy-research checked_in_btc_and_sol_templates_pin_current_brief_and_rust_policy --lib -- --test-threads=1cargo fmt --all -- --checkandgit diff --check.d1e06…); the prior mismatch was recursive Cargo-tree build-context instability, not a macOS/Linux artifact. The checked-in reviewed graph hash issha256:3be7f78980c58e79d0ed94b61dd2ec17570fd60b73c661611fc296d10fcf539f. Final PR CI must pass both Linux build contexts.Rollout / rollback
Research identity only; no execution behavior changes. A dependency or manifest drift stops compilation until the reviewed graph/hash and mission pins are updated atomically. Rollback is one revert, with matching mission pins restored in that same revert.
Summary by CodeRabbit
Enhancements
Documentation