address review: dedup strategy trace! P&L and clarify Overflow doc

Two small correctness / docs fixes surfaced in the review on #371.

- `strategies::short_strangle::calculate_profit_at` and
  `strategies::short_straddle::calculate_profit_at` were calling
  `pnl_at_expiration` three times per leg: twice inside the `trace!`
  argument list and once inside `d_sum`. Besides the wasted work
  (`pnl_at_expiration` builds several `Position` clones and re-runs
  premium and fee arithmetic), it meant the `trace!` snapshot and the
  returned total could differ if any leg had intermittent errors or
  side-effecting dependencies. Both implementations now compute
  `call_pnl` / `put_pnl` once, feed them into `d_sum` to get `total`,
  and use the three cached locals in the `trace!` line and the `Ok`
  return.

- `DecimalError::Overflow` docs were reworded to match the actual
  behaviour: the operands are part of the `#[error]` display
  template and therefore visible in any formatted error string that
  reaches downstream modules via the `#[from]` cascade. The previous
  note claimed the operands were "never re-emitted through the
  public API surface", which is incorrect. The doc now explicitly
  points callers that need to redact operand values towards a
  custom `Display` / match-and-reformat path.

No functional API change. `cargo test --lib` passes (3751 / 5
ignored / 0 regressions). `cargo clippy --lib --all-features`
reports zero warnings. `make pre-push` is clean.

Refs #335, PR #371

Author: joaquinbejar

src/error/decimal.rs

@@ -139,10 +139,14 @@ pub enum DecimalError {
     /// `"pricing::black_scholes::discount_strike"`) so failures can be
     /// traced back to their kernel without walking the call stack.
     ///
-    /// The operands are captured verbatim for post-mortem debugging; they
-    /// are never re-emitted through the public API surface because
-    /// `DecimalError` already reaches every domain module through the
-    /// existing `#[from]` cascade.
+    /// The operands are captured verbatim for post-mortem debugging and
+    /// are included in the formatted error output (see the `#[error]`
+    /// template below). This `DecimalError` variant is propagated across
+    /// domain modules through the existing `#[from]` cascade, so the
+    /// operand values will appear in any log or wrapper-error string
+    /// produced downstream. Callers that need to redact them for
+    /// production logging should match on the `Overflow` variant
+    /// explicitly and reformat rather than rely on the default `Display`.
     #[error("Decimal {operation} overflow: {lhs} op {rhs}")]
     Overflow {
         /// Short static tag identifying the call-site.

src/greeks/numerical.rs

@@ -82,11 +82,7 @@ pub fn numerical_gamma(option: &Options) -> Result<Decimal, GreeksError> {
     //   p_plus - 2*p + p_minus.
     // Build `2*p` via `d_mul` so an overflow on the doubled price does
     // not silently saturate before the checked `d_sub` / `d_add`.
-    let two_p = d_mul(
-        dec!(2.0),
-        p.to_dec(),
-        "greeks::numerical::gamma::two_p",
-    )?;
+    let two_p = d_mul(dec!(2.0), p.to_dec(), "greeks::numerical::gamma::two_p")?;
     let step = d_sub(p_plus.to_dec(), two_p, "greeks::numerical::gamma::step")?;
     let numer = d_add(step, p_minus.to_dec(), "greeks::numerical::gamma::numer")?;
     let h_squared = d_mul(H, H, "greeks::numerical::gamma::h_squared")?;

src/pricing/chooser.rs

@@ -163,21 +163,13 @@ fn simple_chooser_price(option: &Options, choice_date_days: f64) -> Result<Decim
         dividend_discount_t,
         "pricing::chooser::price::leg_s_t_discounted",
     )?;
-    let leg_s_t = d_mul(
-        leg_s_t_discounted,
-        n_d1,
-        "pricing::chooser::price::leg_s_t",
-    )?;
+    let leg_s_t = d_mul(leg_s_t_discounted, n_d1, "pricing::chooser::price::leg_s_t")?;
     let leg_k_t_discounted = d_mul(
         k.to_dec(),
         discount_t,
         "pricing::chooser::price::leg_k_t_discounted",
     )?;
-    let leg_k_t = d_mul(
-        leg_k_t_discounted,
-        n_d2,
-        "pricing::chooser::price::leg_k_t",
-    )?;
+    let leg_k_t = d_mul(leg_k_t_discounted, n_d2, "pricing::chooser::price::leg_k_t")?;
     let leg_k_choice_discounted = d_mul(
         k.to_dec(),
         discount_choice,

src/pricing/compound.rs

@@ -340,13 +340,15 @@ fn price_compound(
     // overflow on either monetary product surfaces a tagged
     // `DecimalError::Overflow` instead of saturating silently before
     // the final `d_add` / `d_sub`.
-    let build_leg =
-        |base: Decimal, discount: Decimal, weight: Decimal, op_base: &'static str,
-         op_leg: &'static str|
-         -> Result<Decimal, PricingError> {
-            let discounted = d_mul(base, discount, op_base)?;
-            Ok(d_mul(discounted, weight, op_leg)?)
-        };
+    let build_leg = |base: Decimal,
+                     discount: Decimal,
+                     weight: Decimal,
+                     op_base: &'static str,
+                     op_leg: &'static str|
+     -> Result<Decimal, PricingError> {
+        let discounted = d_mul(base, discount, op_base)?;
+        Ok(d_mul(discounted, weight, op_leg)?)
+    };
 
     let price = if is_compound_call && is_underlying_call {
         // Call-on-Call

src/strategies/short_straddle.rs

@@ -843,23 +843,25 @@ impl Optimizable for ShortStraddle {
 impl Profit for ShortStraddle {
     fn calculate_profit_at(&self, price: &Positive) -> Result<Decimal, PricingError> {
         let price = Some(price);
+        // Compute each leg's P&L exactly once and reuse for both the
+        // `trace!` line and the checked `d_sum`, so the evaluator
+        // never observes two different snapshots of the same leg.
+        let call_pnl = self.short_call.pnl_at_expiration(&price)?;
+        let put_pnl = self.short_put.pnl_at_expiration(&price)?;
+        let total = d_sum(
+            &[call_pnl, put_pnl],
+            "strategies::short_straddle::profit_at",
+        )?;
         trace!(
             "Price: {:?} Strike: {} Call: {:.2} Strike: {} Put: {:.2} Profit: {:.2}",
             price,
             self.short_call.option.strike_price,
-            self.short_call.pnl_at_expiration(&price)?,
+            call_pnl,
             self.short_put.option.strike_price,
-            self.short_put.pnl_at_expiration(&price)?,
-            self.short_call.pnl_at_expiration(&price)?
-                + self.short_put.pnl_at_expiration(&price)?
+            put_pnl,
+            total
         );
-        Ok(d_sum(
-            &[
-                self.short_call.pnl_at_expiration(&price)?,
-                self.short_put.pnl_at_expiration(&price)?,
-            ],
-            "strategies::short_straddle::profit_at",
-        )?)
+        Ok(total)
     }
 }
 

src/strategies/short_strangle.rs

@@ -1133,22 +1133,25 @@ impl Optimizable for ShortStrangle {
 impl Profit for ShortStrangle {
     fn calculate_profit_at(&self, price: &Positive) -> Result<Decimal, PricingError> {
         let price = &Some(price);
+        // Compute each leg's P&L exactly once and reuse for both the
+        // `trace!` line and the checked `d_sum`, so the evaluator
+        // never observes two different snapshots of the same leg.
+        let call_pnl = self.short_call.pnl_at_expiration(price)?;
+        let put_pnl = self.short_put.pnl_at_expiration(price)?;
+        let total = d_sum(
+            &[call_pnl, put_pnl],
+            "strategies::short_strangle::profit_at",
+        )?;
         trace!(
             "Price: {:?} Strike: {} Call: {:.2} Strike: {} Put: {:.2} Profit: {:.2}",
             price,
             self.one_option().strike_price,
-            self.short_call.pnl_at_expiration(price)?,
+            call_pnl,
             self.short_put.option.strike_price,
-            self.short_put.pnl_at_expiration(price)?,
-            self.short_call.pnl_at_expiration(price)? + self.short_put.pnl_at_expiration(price)?
-        );
-        Ok(d_sum(
-            &[
-                self.short_call.pnl_at_expiration(price)?,
-                self.short_put.pnl_at_expiration(price)?,
-            ],
-            "strategies::short_strangle::profit_at",
-        )?)
+            put_pnl,
+            total
+        );
+        Ok(total)
     }
 }
 

src/volatility/utils.rs

@@ -165,16 +165,8 @@ pub fn ewma_volatility(
         // innovation term.
         let persistent = d_mul(lambda, variance, "volatility::ewma::persistent")?;
         let innovation_weight = d_sub(Decimal::ONE, lambda, "volatility::ewma::innovation_weight")?;
-        let return_sq = d_mul(
-            return_value,
-            return_value,
-            "volatility::ewma::return_sq",
-        )?;
-        let innovation = d_mul(
-            innovation_weight,
-            return_sq,
-            "volatility::ewma::innovation",
-        )?;
+        let return_sq = d_mul(return_value, return_value, "volatility::ewma::return_sq")?;
+        let innovation = d_mul(innovation_weight, return_sq, "volatility::ewma::innovation")?;
         variance = d_add(persistent, innovation, "volatility::ewma::variance")?;
         let std_dev = variance
             .sqrt()
@@ -353,15 +345,12 @@ pub fn garch_volatility(
     // the rejection (negative variance) onto a readable
     // `VolatilityError::NumericalFailure` instead of silently
     // clamping to zero.
-    let to_positive_variance = |value: Decimal,
-                                context: &'static str|
-     -> Result<Positive, VolatilityError> {
-        Positive::new_decimal(value).map_err(|_| VolatilityError::NumericalFailure {
-            reason: format!(
-                "garch_volatility: negative variance at {context} ({value})"
-            ),
-        })
-    };
+    let to_positive_variance =
+        |value: Decimal, context: &'static str| -> Result<Positive, VolatilityError> {
+            Positive::new_decimal(value).map_err(|_| VolatilityError::NumericalFailure {
+                reason: format!("garch_volatility: negative variance at {context} ({value})"),
+            })
+        };
 
     // Seed the recursion with `r_0^2` via `d_mul` so a saturating
     // squaring cannot feed a silently capped value into the GARCH
@@ -376,11 +365,7 @@ pub fn garch_volatility(
     for &return_value in &returns[1..] {
         // GARCH(1, 1) variance recursion:
         // v' = ω + α · r² + β · v_prev.
-        let return_sq = d_mul(
-            return_value,
-            return_value,
-            "volatility::garch::return_sq",
-        )?;
+        let return_sq = d_mul(return_value, return_value, "volatility::garch::return_sq")?;
         let alpha_r2 = d_mul(alpha, return_sq, "volatility::garch::alpha_r2")?;
         let beta_v = d_mul(beta, variance.to_dec(), "volatility::garch::beta_v")?;
         let persistent = d_add(omega, alpha_r2, "volatility::garch::omega_plus_alpha")?;