fix(alpha): gate live-capable formula missions before persistence - #67
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 (17)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughThe PR introduces shared live-formula capability validation, applies it to strategies, governed artifacts, missions, and evaluation, requires explicit live feature fields, adds OHLCV-derived research features, restricts durable execution, and records structured live-capability failures. ChangesLive Capability Enforcement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Mission
participant LiveValidator
participant AlphaStore
participant AutoResearchKernel
participant Evaluator
CLI->>Mission: provide engine and feature_fields
Mission->>LiveValidator: validate live mission and formula capabilities
LiveValidator-->>Mission: capability result
Mission->>AlphaStore: open state after successful preflight
AutoResearchKernel->>LiveValidator: validate candidate formula
LiveValidator-->>AutoResearchKernel: live or rejected
AutoResearchKernel->>Evaluator: evaluate live-compatible candidate
Possibly related PRs
🚥 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| fn temporary_db_path(name: &str) -> PathBuf { | ||
| std::env::temp_dir().join(format!( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1655-1663: Move artifact validation into the
evaluation_protocol_hash branch: apply artifact.validate_for_readback() only
when processing legacy bundles with an empty protocol hash, while canonical
bundles use the canonical validation path without permissive formula readback.
Preserve the existing hash validation and calculation behavior in both branches.
In `@rust_hft/alpha-harness/engine/src/lib.rs`:
- Around line 379-390: Update the live formula validation gate before evaluator
invocation to call the AST’s structural validate method first, then run
validate_live_formula only when that succeeds. Apply this to
CandidateArtifact::Formula in the proposal evaluation flow, preserving the
existing live_capability rejection and error mapping so malformed ASTs cannot
reach the evaluator.
🪄 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: fe63e78b-4f91-477a-9fef-dc151a3482cc
⛔ Files ignored due to path filters (1)
rust_hft/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
rust_hft/alpha-harness/app/Cargo.tomlrust_hft/alpha-harness/app/src/cli.rsrust_hft/alpha-harness/app/src/data_mission.rsrust_hft/alpha-harness/app/src/governance.rsrust_hft/alpha-harness/app/src/loop_control.rsrust_hft/alpha-harness/app/src/mission.rsrust_hft/alpha-harness/app/src/mission_runner.rsrust_hft/alpha-harness/domain/src/lib.rsrust_hft/alpha-harness/engine/Cargo.tomlrust_hft/alpha-harness/engine/src/formula_evaluator.rsrust_hft/alpha-harness/engine/src/lib.rsrust_hft/alpha-harness/engine/src/llm.rsrust_hft/alpha-harness/store/src/lib.rsrust_hft/research-core/factor-dsl/src/lib.rsrust_hft/strategy-framework/strategies/formula/src/lib.rs
| let live_capability = match &proposal.artifact { | ||
| CandidateArtifact::Formula(ast) => validate_live_formula(ast) | ||
| .map(|_| ()) | ||
| .map_err(|error| error.to_string()), | ||
| _ => Ok(()), | ||
| }; | ||
| match live_capability { | ||
| Err(error) => Err(("live_capability_reject", error)), | ||
| Ok(()) => self | ||
| .evaluator | ||
| .evaluate(&proposal, &evaluation_context) | ||
| .map_err(|error| ("evaluation_error", error)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the AST structure before invoking any evaluator.
This generic gate calls only validate_live_formula, while formula_evaluator.rs explicitly calls ast.validate() first. A malformed supported-node AST from deserialization/direct construction could reach a permissive evaluator and be persisted as Keep.
Proposed fix
CandidateArtifact::Formula(ast) => validate_live_formula(ast)
- .map(|_| ())
- .map_err(|error| error.to_string()),
+ .and_then(|_| ast.validate().map_err(Into::into))
+ .map_err(|error| error.to_string()),Prefer structural validation first if deterministic error precedence matters.
📝 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.
| let live_capability = match &proposal.artifact { | |
| CandidateArtifact::Formula(ast) => validate_live_formula(ast) | |
| .map(|_| ()) | |
| .map_err(|error| error.to_string()), | |
| _ => Ok(()), | |
| }; | |
| match live_capability { | |
| Err(error) => Err(("live_capability_reject", error)), | |
| Ok(()) => self | |
| .evaluator | |
| .evaluate(&proposal, &evaluation_context) | |
| .map_err(|error| ("evaluation_error", error)) | |
| let live_capability = match &proposal.artifact { | |
| CandidateArtifact::Formula(ast) => validate_live_formula(ast) | |
| .and_then(|_| ast.validate().map_err(Into::into)) | |
| .map_err(|error| error.to_string()), | |
| _ => Ok(()), | |
| }; | |
| match live_capability { | |
| Err(error) => Err(("live_capability_reject", error)), | |
| Ok(()) => self | |
| .evaluator | |
| .evaluate(&proposal, &evaluation_context) | |
| .map_err(|error| ("evaluation_error", error)) |
🤖 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/engine/src/lib.rs` around lines 379 - 390, Update the
live formula validation gate before evaluator invocation to call the AST’s
structural validate method first, then run validate_live_formula only when that
succeeds. Apply this to CandidateArtifact::Formula in the proposal evaluation
flow, preserving the existing live_capability rejection and error mapping so
malformed ASTs cannot reach the evaluator.
![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: 2daaf4dbdf
ℹ️ 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".
|
|
||
| fn validate_loop_args(args: &LoopRunArgs) -> anyhow::Result<()> { | ||
| mission::validate_live_mission_args(&args.mission)?; | ||
| if !matches!(args.mission.engine, EngineChoice::Mcts) { |
There was a problem hiding this comment.
When LoopRun accepts Mcts as the only live-capable durable engine here, the MCTS grammar still expands actions 0-2 to rank, delta, and mean in engine/src/engines/mcts.rs, all of which the new live gate rejects. With the CLI default seed 7, the first root expansion selects mean, so a one-candidate loop records only live_capability_reject and exhausts research without any evaluation. Please route loop MCTS through a live-only grammar or reject configurations that can propose research-only operators.
Useful? React with 👍 / 👎.
| pub(crate) fn validate_live_formula_engine(engine: EngineChoice) -> anyhow::Result<()> { | ||
| if matches!(engine, EngineChoice::Bayesian) { | ||
| bail!(BAYESIAN_WINDOW_SEARCH_LIVE_CAPABILITY_ERROR); | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
Only Bayesian is rejected in this live preflight, but OfflineRlEngine::ast_for_action can only emit rank, delta, mean, or zscore, and the new validate_live_formula path rejects all of those before evaluation. A standalone mission run --engine offline-rl with otherwise valid live feature fields therefore opens/updates the store and consumes candidates as live_capability_reject with no possible successful evaluation; either fail fast here or give OfflineRL a live-capable action set.
Useful? React with 👍 / 👎.
| match &proposal.artifact { | ||
| CandidateArtifact::Formula(ast) => { | ||
| ast.validate().map_err(|error| error.to_string())?; | ||
| validate_live_formula(ast).map_err(|error| error.to_string())?; |
There was a problem hiding this comment.
When this line starts rejecting formulas that purged-walk-forward-v3 used to evaluate before (for example signal, rank, or mean) while the evaluator version string stays at v3, existing persisted v3 walk-forward records are still accepted by governance::validated_walk_forward_evidence_in_lineage without revalidating the candidate. That can mark a research-only legacy candidate as WalkForwardKept even though a fresh evaluation now fails; please bump the formula evaluator version or filter stored candidates through the live contract before accepting v3 evidence.
Useful? React with 👍 / 👎.
| Ok(result) | ||
| }) | ||
| let live_capability = match &proposal.artifact { | ||
| CandidateArtifact::Formula(ast) => validate_live_formula(ast) |
There was a problem hiding this comment.
When a standalone mission run --engine llm is used, this new live-capability check rejects any formula the LLM returns with rank, delta, mean, or zscore. The LLM response schema in engine/src/llm.rs still explicitly allows those operators, and abandon does not add these crashes to prior outcomes, so such responses consume candidate/token budget and may be repeated instead of evaluated. Please narrow the LLM schema to live operators before enabling it under this gate.
Useful? React with 👍 / 👎.
2daaf4d to
eb91748
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb91748e69
ℹ️ 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".
| let persist_candidate = | ||
| is_novel && failure_class != "live_capability_reject"; |
There was a problem hiding this comment.
When standalone GP is run in small resume slices (for example --max-new-iterations 1) and its deterministic sequence hits a research-only operator such as delta or mean, this branch records only a crash iteration and no candidate/evaluation. The default history-replay checkpoint then has no observation to restore, so the next resume starts GP from the same seed and can regenerate the same rejected formula, spending candidate budget repeatedly on an already-rejected proposal; persist enough rejected-fingerprint state or make GP abandon/restore live rejects.
Useful? React with 👍 / 👎.
| "best_bid" | "best_ask" | "mid_price" | "spread" | "spread_bps" | "bid_size" | ||
| | "ask_size" | "book_imbalance" => LiveEventDomain::Snapshot, |
There was a problem hiding this comment.
The governed LOB materializer emits depth-qualified fields such as book_imbalance_top5, bid_depth_top5, and ofi_top5, but the live whitelist here only accepts the unqualified book_imbalance/size names. Any mission execute using the materializer's default imbalance or order-flow fields is now rejected by validate_live_feature_fields before state is created, leaving only mid_price/spread_bps from that artifact usable; either emit the whitelisted names or accept/runtime-map the materializer's live equivalents.
Useful? React with 👍 / 👎.
eb91748 to
9bfbea8
Compare
2 participants
Change contract
A Formula that cannot be constructed with live runtime semantics is rejected before evaluation and candidate persistence. Governance revalidates prior walk-forward evidence, direct mission and durable-loop entrypoints reject impossible engines before state access, and live MCTS emits only formulas accepted by the shared capability gate.
Scope and atomic exception
This PR is 18 files and +1261/-255 after rebase. The capability definition, kernel persistence rule, evaluator and governance checks, MCTS checkpoint contract, and CLI/loop preflight are one security boundary. Splitting them would leave a bypassable transition.
Explicit merge authority: proerror77 instructed this Codex task to complete the reviewed and green PR merges on 2026-07-17.
Included safeguards
Focused validation
cargo test -p alpha-engine -p alpha-domain -p alpha-store -p alpha-harness -p hft-factor-dsl -p hft-strategy-formula -- --test-threads=1(225 executed; one external LLM test ignored)cargo clippy -p alpha-harness -p alpha-engine -p alpha-domain -p alpha-store --all-targets -- -D warningscargo fmt --check -p alpha-harness -p alpha-storegit diff --checkDependency and merge order
Rebased on
mainafter #75 (0b8ce2b). No remaining PR dependency.Out of scope
trade_countin the raw OHLCV data contract. The current loader does not emit it, so a separate data-contract change must provide it before such a mission can pass dataset registration.Rollback
Revert this PR to restore the prior gate behavior. It introduces no live activation, order path, or deployment mutation.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation