feat(alpha): add typed CEX baseline evidence contract - #662
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesCEX baseline evaluation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EvaluationPolicy
participant BaselineArtifact
participant FactorBank
participant BaselineGate
EvaluationPolicy->>BaselineArtifact: validate baseline policy and walk-forward bindings
BaselineArtifact->>BaselineGate: provide Ridge and CART evidence
FactorBank->>BaselineGate: provide factor-bank identity
BaselineGate->>EvaluationPolicy: return joint gate outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
This was referenced Aug 3, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
rust_hft/alpha-harness/domain/src/lib.rs (3)
2405-2411: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the expected factor ids once, and drop the redundant rebinding.
Line 2410 rebinds
expected_factor_idsto itself. Collect into a mutable binding instead.
CexBaselineGateV1::validate_binding(lines 2685-2691) derives the same list from the same source withBTreeSet, which also removes duplicates. This method usessort(), which keeps duplicates. Two derivations of one expected value can diverge. Extract a single helper and call it from both sites.♻️ Proposed shared helper
- let expected_factor_ids = factor_bank - .entries - .iter() - .map(|entry| entry.factor_id.clone()) - .collect::<Vec<_>>(); - let mut expected_factor_ids = expected_factor_ids; - expected_factor_ids.sort(); + let expected_factor_ids = expected_baseline_factor_ids(factor_bank);Add the helper next to
valid_baseline_artifact_id:fn expected_baseline_factor_ids(factor_bank: &CexFactorBankRevisionV2) -> Vec<String> { factor_bank .entries .iter() .map(|entry| entry.factor_id.clone()) .collect::<BTreeSet<_>>() .into_iter() .collect() }🤖 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/alpha-harness/domain/src/lib.rs` around lines 2405 - 2411, Extract a shared expected-factor-ID helper beside valid_baseline_artifact_id that collects factor IDs from CexFactorBankRevisionV2 entries into a BTreeSet and returns a deduplicated Vec<String>. Replace both derivations, including the block near CexBaselineGateV1::validate_binding, with calls to this helper, and make the local binding directly mutable where sorting remains necessary.
6072-6112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
#[rustfmt::skip], and assert the passing gate outcome.Two points:
- Lines 6091, 6092, 6093, 6107, and 6110 each hold one long statement.
#[rustfmt::skip]suppresses the formatting check instead of satisfying it. These lines encode the fold schedule and the model fixtures, which are the parts of this test a reviewer must read to confirm the contract. Extract named local helpers for the range tuples, the fold builder, and the two model values, then drop the attribute.- Line 6112 discards the gate with
let _gate. The passing shape(Some, Some, true, [])is a core outcome of this PR, and no assertion covers it. Assertpassed, the emptyfailure_codes, and the distinct artifact ids.💚 Proposed assertions for the passing gate
- let _gate = CexBaselineGateV1::new(&ridge, &cart).unwrap(); + let gate = CexBaselineGateV1::new(&ridge, &cart).unwrap(); + assert!(gate.passed); + assert!(gate.failure_codes.is_empty()); + assert_ne!(gate.ridge_artifact_id, gate.cart_artifact_id); + assert_eq!(gate.policy_hash, ridge.baseline_policy.content_hash().unwrap()); + gate.validate_binding(&bank, Some(&ridge), Some(&cart)).unwrap();Line 6093 re-implements the
artifact_partition_hashencoding by hand. See the comment on lines 2441-2474 for the shared-helper suggestion that removes the copy.🤖 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/alpha-harness/domain/src/lib.rs` around lines 6072 - 6112, Update baseline_artifacts_and_joint_gate_are_content_bound by removing #[rustfmt::skip], extracting named locals/helpers for the range tuples, fold builder, and Ridge/ShallowCart model fixtures so rustfmt can format the test. Reuse the existing shared artifact partition-hash helper instead of manually encoding the partition JSON. Replace the discarded _gate with assertions verifying passed is true, failure_codes is empty, and the Ridge and CART artifact IDs are distinct.
6114-6138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the negative cases that the PR objectives claim.
The objectives state that tampered dataset, partition, and artifact identities are rejected. This test covers partition drift at line 6120 and a non-finite Ridge coefficient at line 6116. Three claimed cases and several new validation branches have no coverage:
- a tampered
research_dataseton the artifact- a tampered
artifact_id- a CART deeper than
cart_max_depth, and aSplitwhosefeature_index >= factor_ids.len()- a drifted
CexBaselinePolicyV1field, whichvalidaterejects at lines 2110-2119- a
model_kindthat disagrees with the fold models, whichvalidaterejects at line 2367Each case is a few lines on top of the existing
ridgefixture.💚 Proposed added cases
let mut partition_drift = ridge; partition_drift.walk_forward_partition.content_sha256 = "0".repeat(64); assert!(partition_drift.validate().is_err()); + + let mut dataset_drift = partition_drift.clone(); + dataset_drift.walk_forward_partition = partition.clone(); + dataset_drift.research_dataset.content_sha256 = "1".repeat(64); + assert!(dataset_drift.validate().is_err()); + + let mut forged_id = dataset_drift.clone(); + forged_id.research_dataset = bank.research_dataset.clone(); + forged_id.artifact_id = format!("cex-baseline-artifact-{}", "2".repeat(64)); + assert!(forged_id.validate().is_err()); + + let mut wrong_kind = cart.clone(); + wrong_kind.model_kind = CexBaselineModelKindV1::Ridge; + wrong_kind.artifact_id = wrong_kind.expected_artifact_id().unwrap(); + assert!(wrong_kind.validate().is_err()); + + let mut unknown_feature = cart; + unknown_feature.folds[0].model = CexBaselineModelV1::ShallowCart { + root: CexBaselineCartNodeV1::Split { + feature_index: 99, + threshold: 0.0, + left: Box::new(CexBaselineCartNodeV1::Leaf { value: 0.1 }), + right: Box::new(CexBaselineCartNodeV1::Leaf { value: 0.2 }), + }, + }; + unknown_feature.artifact_id = unknown_feature.expected_artifact_id().unwrap(); + assert!(unknown_feature.validate().is_err());The proposed diff needs
ridge,partition, andcartto stay in scope, so clone instead of move at lines 6119 and 6110. Runcargo test -p alpha-domain --lockedfromrust_hft/after the change.As per coding guidelines: "Run the focused package tests with
cargo test -p alpha-domain --locked".🤖 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/alpha-harness/domain/src/lib.rs` around lines 6114 - 6138, Extend the validation test around the existing ridge, partition, and cart fixtures with negative cases for tampered artifact research_dataset, tampered artifact_id, over-depth CART nodes, out-of-range Split feature_index, drifted CexBaselinePolicyV1 fields, and mismatched model_kind versus fold models; assert each validation call fails. Clone ridge and partition instead of moving them so all cases remain in scope, keep cart available for its cases, and run cargo test -p alpha-domain --locked from rust_hft.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/alpha-harness/domain/src/lib.rs`:
- Around line 2593-2618: Update the EmptyFactorBank branch of validate_binding
to accept and use the mission artifact and policy, re-deriving the gate through
empty_factor_bank (or an equivalent constructor path) and comparing the result
with self, while retaining factor-bank revision validation. Update the
empty_factor_bank call sites and the tests around the existing lines 6131 and
6137 to pass the mission artifact and policy so mission_id and policy_hash are
independently verified.
- Around line 2502-2524: Update CexBaselineCartNodeV1::Leaf to carry a sample
count, then modify validate_cart_node to require that count to be at least
policy.cart_min_leaf while preserving the existing value, depth, feature-index,
and threshold checks. Ensure CexBaselineArtifactV1::validate passes the policy
through this validation so cart_min_leaf is enforced; do not leave the governed
parameter unread.
- Around line 2336-2375: Extend the per-fold validation in the baseline evidence
check to bind each fold to EvaluationWalkForwardV1: require purge_range and
embargo_range lengths to equal the configured purge_rows and embargo_rows, and
train_range length to meet initial_train_rows. Add a fold-order check requiring
each later validation window to start at or after the previous window’s end,
preventing duplicate or preceding validation windows. Preserve the existing
range, prediction, model, and fold-identity checks.
---
Nitpick comments:
In `@rust_hft/alpha-harness/domain/src/lib.rs`:
- Around line 2405-2411: Extract a shared expected-factor-ID helper beside
valid_baseline_artifact_id that collects factor IDs from CexFactorBankRevisionV2
entries into a BTreeSet and returns a deduplicated Vec<String>. Replace both
derivations, including the block near CexBaselineGateV1::validate_binding, with
calls to this helper, and make the local binding directly mutable where sorting
remains necessary.
- Around line 6072-6112: Update
baseline_artifacts_and_joint_gate_are_content_bound by removing
#[rustfmt::skip], extracting named locals/helpers for the range tuples, fold
builder, and Ridge/ShallowCart model fixtures so rustfmt can format the test.
Reuse the existing shared artifact partition-hash helper instead of manually
encoding the partition JSON. Replace the discarded _gate with assertions
verifying passed is true, failure_codes is empty, and the Ridge and CART
artifact IDs are distinct.
- Around line 6114-6138: Extend the validation test around the existing ridge,
partition, and cart fixtures with negative cases for tampered artifact
research_dataset, tampered artifact_id, over-depth CART nodes, out-of-range
Split feature_index, drifted CexBaselinePolicyV1 fields, and mismatched
model_kind versus fold models; assert each validation call fails. Clone ridge
and partition instead of moving them so all cases remain in scope, keep cart
available for its cases, and run cargo test -p alpha-domain --locked from
rust_hft.
🪄 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 Plus
Run ID: 1ba16c9a-dc99-4221-a8d1-4c4d42eb8faf
📒 Files selected for processing (1)
rust_hft/alpha-harness/domain/src/lib.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2803ec842
ℹ️ 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".
proerror77
force-pushed
the
codex/cex-baseline-domain-600
branch
from
August 3, 2026 21:37
c2803ec to
2595677
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change contract
Add the immutable typed policy, Ridge/CART artifact, fold/model, and joint sufficiency-gate contracts required by CEX baseline evaluation.
Issue relationship
Refs #600
Out of scope
Baseline fitting, Mission result persistence, MCTS admission, ACK/OSS execution, and Paper/Shadow/Live.
Dependencies and merge order
First PR in the #600 stack. Merge before the engine, producer, and MCTS admission PRs.
Focused validation
cargo test -p alpha-domain --locked --lib(50 passed)cargo clippy -p alpha-domain --locked --lib -- -D warningscargo fmt -p alpha-domain -- --checkRollout and rollback
Code-only domain contract. Roll back this commit before any dependent stack PR.
Scope exception
None.
Summary by CodeRabbit
New Features
Bug Fixes