Hedge advisor: match basket overrides on (code, currency) only#95
Merged
Conversation
The override matcher was keyed on (contract_code, currency, delivery_months) with delivery_months hardcoded to 3. This only matched when settlement fell at the start of a quarter; for any mid-quarter settle the actual front-month is 1 or 2 months out, so well-formed overrides silently fell through to the synthetic at-current-coupon CTD. Match on (contract_code, currency) only. delivery_months becomes a tiebreaker among multiple matches: prefer DEFAULT_DELIVERY_MONTHS, else smallest. Single-match cases — including back-month-only and mid-quarter front-months — now adopt the override. Test back_month_only_override_falls_back_to_synthetic flips and is renamed back_month_only_override_is_used.
Both helpers were pub-exported through hedging::, risk::, and the convex facade, but no caller outside convex-analytics imports them — they're only used by make_default_future and the unit tests. Drop the cross-crate exports and tighten the function visibility to pub(crate).
📝 WalkthroughWalkthroughThis PR bumps the version to 0.13.0, restricts two internal helper functions to crate-private visibility, removes their public re-exports across module boundaries, and refactors basket override matching to accept back-month-only overrides by matching on ChangesAPI Visibility and Hedging Logic
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The auto-tenor heuristic in pick_swap_tenor bucketed modified duration into a fixed liquid set with hard thresholds. Two failure modes: 1. MD is the wrong signal at the book level when KRD has structure. A book with weighted-average MD 7.5y but KRD concentrated at 10Y was hedged with a 5Y swap that did nothing for the actual exposure. 2. Float boundary at d < 7.5 mapped near-7.5 MDs to the lower bucket nondeterministically depending on the seventh decimal place. Replace with a single-leg KRD projection: for each candidate liquid tenor, build a unit par swap, compute its risk on the position's KRD ladder, scale to neutralize parallel DV01, and measure residual L2 norm. Pick the tenor that minimizes residual; tie-break to longer. Fall back to MD-based snap (with longer-on-tie) only when the position has no usable KRD ladder. Both interest_rate_swap and cash_bond_pair share the picker. Existing strategy tests that assert tenor=10Y (long_10y_corporate fixture) keep passing because projection picks the same answer when KRD already points there. Three new tests: - pick_swap_tenor_uses_krd_projection_not_md_bucket: book MD 7.5, KRD heavy at 10Y -> 10Y (was 5Y under MD-bucketing). - pick_swap_tenor_falls_back_to_md_snap_when_ladder_empty: empty ladder, MD 7.5 -> 10Y (round-up tie at midpoint). - snap_to_liquid_swap_tenor_round_up_at_midpoints: 3.5/7.5/15/25 all round to the longer tenor. NaN-MD test signature updated for the new function shape.
- cargo fmt --all (re-flowed two long re-export lists) - ctd module doc referenced approximate_cme_cf which is now pub(crate); rephrased as crate-internal context to drop the broken intra-doc link
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/convex-analytics/src/risk/hedging/strategies.rs (1)
733-738: ⚡ Quick winAdd regression coverage for the remaining tiebreak branches.
The new selector still has two untested branches here: preferring
DEFAULT_DELIVERY_MONTHSover a smaller month like1, and picking the smallest month when no default3exists (for example6vs9). One focused unit test for each would pin down the new matching contract.🤖 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 `@crates/convex-analytics/src/risk/hedging/strategies.rs` around lines 733 - 738, Add two unit tests that exercise the untested branches around the `chosen` selection: (1) a test where the input `matches` contains an entry with delivery_months == DEFAULT_DELIVERY_MONTHS and another with a smaller month (e.g. 1) and assert the selector returns the DEFAULT_DELIVERY_MONTHS entry; (2) a test where no entry has delivery_months == DEFAULT_DELIVERY_MONTHS (e.g. entries 6 and 9) and assert the selector returns the entry with the smallest delivery_months (6). Locate the selection logic that assigns to the `chosen` variable and invoke that function/path with appropriately constructed match structs (setting their `delivery_months` field) to verify each branch.
🤖 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 `@crates/convex-analytics/src/risk/hedging/strategies.rs`:
- Around line 835-907: The code currently treats a ladder with any
NaN/non-finite bucket as usable, which later yields NaN l2 and can silently pick
a bad tenor; update the KRD usability check so usable_krd requires all
position.key_rate_buckets have finite partial_dv01 (and tenor_years if relevant)
before proceeding, and if any bucket value is non-finite return the
modified-duration fallback via
snap_to_liquid_swap_tenor(position.modified_duration_years); adjust the check
that builds usable_krd (and/or short-circuit before the candidate loop) rather
than trying to sanitize values inside the l2 computation (references:
position.key_rate_buckets, usable_krd, partial_dv01, unit_risk, l2, and
snap_to_liquid_swap_tenor).
---
Nitpick comments:
In `@crates/convex-analytics/src/risk/hedging/strategies.rs`:
- Around line 733-738: Add two unit tests that exercise the untested branches
around the `chosen` selection: (1) a test where the input `matches` contains an
entry with delivery_months == DEFAULT_DELIVERY_MONTHS and another with a smaller
month (e.g. 1) and assert the selector returns the DEFAULT_DELIVERY_MONTHS
entry; (2) a test where no entry has delivery_months == DEFAULT_DELIVERY_MONTHS
(e.g. entries 6 and 9) and assert the selector returns the entry with the
smallest delivery_months (6). Locate the selection logic that assigns to the
`chosen` variable and invoke that function/path with appropriately constructed
match structs (setting their `delivery_months` field) to verify each branch.
🪄 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: 05c7e5b6-3dba-4d05-9054-427854aa5cd8
📒 Files selected for processing (3)
crates/convex-analytics/src/risk/hedging/ctd.rscrates/convex-analytics/src/risk/hedging/strategies.rscrates/convex/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/convex/src/lib.rs
- crates/convex-analytics/src/risk/hedging/ctd.rs
Comment on lines
+835
to
+907
| let krd_threshold = position.dv01.abs() * 1e-9; | ||
| let usable_krd = !position.key_rate_buckets.is_empty() | ||
| && position | ||
| .key_rate_buckets | ||
| .iter() | ||
| .any(|b| b.partial_dv01.abs() > krd_threshold); | ||
| if !usable_krd || !position.dv01.is_finite() || position.dv01 == 0.0 { | ||
| return Ok(snap_to_liquid_swap_tenor(position.modified_duration_years)); | ||
| } | ||
|
|
||
| let (frequency, day_count, idx) = swap_conventions_for(position.currency)?; | ||
| let tenor_grid: Vec<f64> = position | ||
| .key_rate_buckets | ||
| .iter() | ||
| .map(|b| b.tenor_years) | ||
| .collect(); | ||
| let curve_t = curve_tenor_to(discount_curve, settlement); | ||
|
|
||
| let mut best: Option<(f64, f64)> = None; | ||
| for &candidate in LIQUID_SWAP_TENORS.iter() { | ||
| let par = RateCurveDyn::par_swap_rate( | ||
| discount_curve, | ||
| curve_t + candidate, | ||
| frequency.periods_per_year(), | ||
| ) | ||
| .map_err(|e| AnalyticsError::CurveError(e.to_string()))?; | ||
| let unit = InterestRateSwap { | ||
| tenor_years: candidate, | ||
| fixed_rate_decimal: par, | ||
| fixed_frequency: frequency, | ||
| fixed_day_count: day_count, | ||
| floating_index: idx.into(), | ||
| // Direction is irrelevant; sign cancels in DV01-neutral sizing. | ||
| side: SwapSide::PayFixed, | ||
| notional: Decimal::ONE, | ||
| currency: position.currency, | ||
| }; | ||
| let unit_risk = interest_rate_swap_risk( | ||
| &unit, | ||
| discount_curve, | ||
| discount_curve_id, | ||
| settlement, | ||
| Some(&tenor_grid), | ||
| )?; | ||
| if unit_risk.dv01.abs() < 1e-12 { | ||
| continue; | ||
| } | ||
| let scale = -position.dv01 / unit_risk.dv01; | ||
| let l2_sq: f64 = position | ||
| .key_rate_buckets | ||
| .iter() | ||
| .map(|pb| { | ||
| let unit_at = unit_risk | ||
| .buckets | ||
| .iter() | ||
| .find(|ub| (ub.tenor_years - pb.tenor_years).abs() < 1e-9) | ||
| .map(|ub| ub.partial_dv01) | ||
| .unwrap_or(0.0); | ||
| let residual = pb.partial_dv01 + scale * unit_at; | ||
| residual * residual | ||
| }) | ||
| .sum(); | ||
| let l2 = l2_sq.sqrt(); | ||
| // `<=` with ascending iteration preserves "ties → longer" without | ||
| // a separate equality branch; on a real curve ties don't occur. | ||
| if best.map_or(true, |(_, cur)| l2 <= cur) { | ||
| best = Some((candidate, l2)); | ||
| } | ||
| } | ||
|
|
||
| Ok(best | ||
| .map(|(t, _)| t) | ||
| .unwrap_or_else(|| snap_to_liquid_swap_tenor(position.modified_duration_years))) |
There was a problem hiding this comment.
Treat non-finite KRD ladders as unusable before scoring candidates.
Right now a ladder with one finite bucket and one NaN bucket skips the MD fallback, then computes l2 = NaN. Because best starts empty, the first candidate is kept and later <= comparisons against NaN never replace it, so this can silently pick an arbitrary tenor instead of falling back.
Proposed fix
fn pick_swap_tenor(
position: &RiskProfile,
discount_curve: &RateCurve<DiscreteCurve>,
discount_curve_id: &str,
settlement: Date,
) -> AnalyticsResult<f64> {
let krd_threshold = position.dv01.abs() * 1e-9;
let usable_krd = !position.key_rate_buckets.is_empty()
+ && position
+ .key_rate_buckets
+ .iter()
+ .all(|b| b.tenor_years.is_finite() && b.partial_dv01.is_finite())
&& position
.key_rate_buckets
.iter()
.any(|b| b.partial_dv01.abs() > krd_threshold);
if !usable_krd || !position.dv01.is_finite() || position.dv01 == 0.0 {
return Ok(snap_to_liquid_swap_tenor(position.modified_duration_years));
@@
let l2 = l2_sq.sqrt();
+ if !l2.is_finite() {
+ continue;
+ }
// `<=` with ascending iteration preserves "ties → longer" without
// a separate equality branch; on a real curve ties don't occur.
if best.map_or(true, |(_, cur)| l2 <= cur) {
best = Some((candidate, l2));
}🤖 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 `@crates/convex-analytics/src/risk/hedging/strategies.rs` around lines 835 -
907, The code currently treats a ladder with any NaN/non-finite bucket as
usable, which later yields NaN l2 and can silently pick a bad tenor; update the
KRD usability check so usable_krd requires all position.key_rate_buckets have
finite partial_dv01 (and tenor_years if relevant) before proceeding, and if any
bucket value is non-finite return the modified-duration fallback via
snap_to_liquid_swap_tenor(position.modified_duration_years); adjust the check
that builds usable_krd (and/or short-circuit before the candidate loop) rather
than trying to sanitize values inside the l2 computation (references:
position.key_rate_buckets, usable_krd, partial_dv01, unit_risk, l2, and
snap_to_liquid_swap_tenor).
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.
Summary
Two correctness fixes plus one cleanup in the hedge advisor's strategy layer.
1. Match basket overrides on
(code, currency)onlyOverride matcher in
lookup_basket_overridekeyed on(contract_code, currency, delivery_months)withdelivery_monthshardcoded to 3. Only matched when settlement fell at the start of a quarter; for any mid-quarter settle the actual front-month is 1 or 2 months out, so well-formed basket overrides silently fell through to the synthetic at-current-coupon CTD.Match now keys on
(contract_code, currency).delivery_monthsis a tiebreaker among multiple matches (preferDEFAULT_DELIVERY_MONTHS, else smallest). Single-match cases — back-month-only, and mid-quarter front-months — now adopt the override.back_month_only_override_falls_back_to_synthetictest flipped and renamedback_month_only_override_is_used.2. Pick swap tenor by KRD projection, not MD bucket
pick_swap_tenorbucketed modified duration into a fixed liquid set with hard<thresholds. Two failure modes: (a) MD is the wrong signal at the book level when KRD has structure — a book with MD 7.5y but KRD concentrated at 10Y was hedged with a 5Y swap that did nothing for the actual exposure; (b) the< 7.5boundary mapped near-7.5 MDs to 5Y nondeterministically depending on the seventh decimal place of float math.Replaced with a single-leg KRD projection: for each candidate liquid tenor, build a unit par swap, compute its risk on the position's KRD ladder, scale to neutralize parallel DV01, measure residual L2 norm; pick the tenor that minimizes residual, tie-break to longer. MD-based snap is the fallback only when the position has no usable KRD ladder.
Shared by
interest_rate_swapandcash_bond_pair. Existing strategy tests that assertedtenor = 10Ykeep passing because thelong_10y_corporatefixture's KRD already points to 10Y.3. Drive-by: demote two crate-internal helpers
approximate_cme_cfanddeliverable_to_bondwerepub-exported through three layers but had no consumer outsideconvex-analytics— only used bymake_default_futureand unit tests. Demoted topub(crate)and dropped from re-exports inhedging/mod.rs,risk/mod.rs, and theconvexfacade.4. Workspace version bump
0.12.1→0.13.0.Test plan
cargo test --package convex-analytics --lib risk::hedging— 106/106 pass (was 103; +3 new picker tests)cargo build --workspace --release— clean, no warningsdelivery_months: 1and confirm the supplied basket flows through tobond_future_riskrather than the synthetic Aug-2036 CTDpropose_hedgesIRS leg picks 10YSummary by CodeRabbit
Chores
Refactor
Tests