Harden research and live trading risk loop - #53
proerror77 wants to merge 7 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 52 files, which is 2 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (52)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
Agent Teams read-only review found merge blockers; keeping this PR draft and not merging:
Branch is also behind current main and has a |
# Conflicts: # .gitignore
| .duration_since(std::time::UNIX_EPOCH) | ||
| .unwrap() | ||
| .as_nanos(); | ||
| let directory = std::env::temp_dir().join(format!( |
| .unwrap() | ||
| .as_nanos(); | ||
| let directory = | ||
| std::env::temp_dir().join(format!("monday-backtest-chain-{}-{id}", std::process::id())); |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 256b9b4f6f
ℹ️ 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 old_quantity = self.keyed_quantity.get(&key).copied().unwrap_or_default(); | ||
| let next_quantity = old_quantity + incoming; | ||
| let old_notional = self.keyed_notional.get(&key).copied().unwrap_or_default(); | ||
| let next_notional = next_quantity.abs() * price; |
There was a problem hiding this comment.
When multiple same-symbol intents for the same venue/product are reviewed in one batch at different limit prices, this replaces the prior keyed notional with next_quantity * current_price. For example, a 10-lot buy at 100 followed by a 10-lot buy at 1 changes the projected gross notional from 1000 to 20, so ProjectedExposureRiskManager, portfolio budgets, and tokenized-security caps can approve a batch that actually commits far more notional than the configured limit.
Useful? React with 👍 / 👎.
| .evidence_source | ||
| .as_deref() | ||
| .is_some_and(|source| !source.trim().is_empty()) | ||
| && context.evidence_venue == intent.target_venue |
There was a problem hiding this comment.
For tokenized-security intents where both intent.target_venue and compliance_context.evidence_venue are unset, this equality treats missing venue evidence as valid. In the review() path, or in review_with_venue_specs() when the default venue fallback is used, such an intent can pass the securities-token filter with no venue-scoped eligibility/depth evidence, even though the policy is intended to require authoritative evidence for the execution venue.
Useful? React with 👍 / 👎.
| base_risk_manager, | ||
| system_risk_config.strategy_overrides.clone(), |
There was a problem hiding this comment.
When any strategy_overrides entry exists, this wraps the projected exposure manager inside StrategyAwareRiskManager. The engine calls review_with_venue_specs, but StrategyAwareRiskManager does not override that method, so the trait default calls review() separately per intent; the projected exposure layer only sees one order at a time, allowing cross-venue batches such as buy 60 on Binance and sell 60 on Bitget to bypass a 100-unit gross cap whenever an override is configured.
Useful? React with 👍 / 👎.
| && context.evidence_observed_at.is_some_and(|observed_at| { | ||
| hft_core::now_micros().saturating_sub(observed_at) <= self.evidence_max_age_us |
There was a problem hiding this comment.
When evidence_observed_at is greater than the local clock, saturating_sub returns 0, so future-dated tokenized-security evidence is treated as fresh. With a clock-skewed or malformed evidence producer, stale market-quality/corporate-action evidence can remain valid until wall clock catches up; require observed_at <= now before applying the max-age check.
Useful? React with 👍 / 👎.
| let reconciliation_ready = match truth_age_secs { | ||
| None => false, |
There was a problem hiding this comment.
The production Kubernetes baseline starts from quotes_only: true and probes /readiness, but quotes-only runtimes never publish RuntimeTruthStatus because no live balance reconciliation runs. This new None => false path makes /readiness return 503 forever even after the market-data runtime is healthy, so the baseline pod never becomes Ready.
Useful? React with 👍 / 👎.
| && (!runtime_truth.reconciliation_complete | ||
| || !runtime_truth.reconciliation_healthy | ||
| || runtime_truth.observed_at_us == 0 | ||
| || reconciliation_age_us > MAX_RECONCILIATION_AGE_US) |
There was a problem hiding this comment.
For Paper and Shadow deployments, the activation adapter sets the venue to execution_mode = Paper, while SystemRuntime only publishes reconciliation truth for live-mode balance reconciliation. With this new condition, every otherwise healthy PortfolioSnapshot in Paper/Shadow has observed_at_us == 0 and is rewritten to Decayed, so runtime_stage_is_healthy can never accept Paper/Shadow evidence for promotion.
Useful? React with 👍 / 👎.
|
Final Agent Teams review found remaining merge blockers, so this PR is returned to Draft. Blockers: post-submit duplicate OrderId can create an untracked cross-client order; Paper/Shadow promotion truth and quotes-only readiness are unreachable in the real startup path; projected exposure lacks pending-order reservations and can reject reduce-only exits; source-segment validation still has an in-place inode mutation gap. Local integration fixes and tests are preserved, but will not be pushed as merge-ready until these contracts are closed and independently re-reviewed. |
|
Superseded by focused draft PRs: #59 (Prediction L2 market data), #60 (authoritative account reconciliation), #61 (batch exposure and tokenized policy), #62 (runtime health/readiness), #64 (LiveSmall promotion gate), and #65 (evidence-bound executable L2 replay). The prior Paper/Shadow |
What changed
Why
The previous pipeline could accept self-attested backtest provenance, under-model executable liquidity, net exposure across unrelated venues, and treat incomplete reconciliation as healthy runtime evidence.
Impact
Research results now fail closed on unverified data. Live intake and Agentic promotion remain blocked whenever reconciliation, risk, or data integrity is incomplete. LiveSmall remains disabled.
Validation
cargo check --workspace --all-targets