refactor: M1 final sweep — eliminate remaining .unwrap()/.expect() (#321) by joaquinbejar · Pull Request #355 · joaquinbejar/OptionStratLib

joaquinbejar · 2026-04-17T16:59:09Z

Summary

Closes M1 — Panic-Free Core. This is the seventh and final PR in the
panic-free sweep (after #347, #348, #350, #352, #353, #354). It removes
every remaining production .unwrap() / .expect() from src/ —
~76 sites across 25 files spanning model/, pnl/, simulation/,
risk/, geometrics/, volatility/, utils/, visualization/,
metrics/, lib.rs, and assorted doc-comment examples.

After this PR, the acceptance grep returns 0 matches across all of src/**/*.rs:

for f in $(find src -name '*.rs'); do
  awk '/#\[cfg\(test\)\]/{exit} {print}' "$f" | grep -nE '\.unwrap\(\)|\.expect\('
done

Per-pattern transformations

Pattern	Replacement	Rationale
`///` example `.unwrap()`	`# fn main() -> Result<...>` shim + `?`	Doc-only
`Decimal::sqrt().unwrap()` in Result fn	`.ok_or_else(\|\| <Module>Error::...)?`	Bubble error
`Decimal::sqrt().unwrap()` in non-Result closure	`.unwrap_or_else(\|\| { warn!(...); Decimal::ZERO })`	Operand non-negative; overflow only
`partial_cmp().unwrap()` in sort/min_by	`.unwrap_or(std::cmp::Ordering::Equal)`	NaN guard
`Mutex.lock().unwrap()`	`.unwrap_or_else(\|e\| e.into_inner())`	Poison recovery
`colors.get(i % len).unwrap()` (mod-bounded)	`match { Some(c) => c, None => ... }`	Index proven in-bounds
`Normal::new(0.0, 1.0).expect(...)`	`match { Ok(n) => n, Err(_) => unreachable!() }`	Hardcoded params; never fires
`start.unwrap()` after `is_some()`	`.is_some_and(\|s\| ...)` restructure
`set_global_default(...).expect(...)` (logger)	silent no-op via `.is_ok()` check	Library must not hard-fail when wrapped by a binary that already configured tracing
`step.last().unwrap()` in `Simulator::get_last_*`	`.filter_map(\|s\| s.last())`	Skip empty walks rather than panic
`to_i32().unwrap()` (PnLRange::new_decimal)	sig change to `Result<Self, DecimalError>`	Surface conversion failure

Public-surface notes

PnLRange::new_decimal changes return type from Self to
Result<Self, DecimalError>. Breaking minor — only caller is the
doc-comment, no in-tree breakage.
Simulator::get_last_steps / get_last_values change behaviour:
empty random walks are silently dropped (via filter_map) instead
of panicking. Returned vector may be shorter than self.len().
Documented in ///.
setup_logger / setup_logger_with_level no longer panic when a
global tracing subscriber is already installed — they silently
no-op, matching the tracing-subscriber::fmt::try_init semantics
and the CLAUDE.md "library code must NOT install a global
subscriber" rule.

Test plan

Final M1 acceptance grep returns 0 across all of src/**/*.rs.
cargo clippy --all-targets --all-features --workspace -- -D warnings
cargo fmt --all --check
cargo test --all-features --workspace (3724 lib + 416 integration + plotly)
cargo build --release
make pre-push (fix + fmt + lint-fix + test + readme + doc)
test_step_next_with_weeks regression: kept exact Decimal arithmetic for units_per_year(Week) after an f64 literal lost ~1 ulp of precision.

Commit layout

5 atomic per-bucket commits:

8e3af240 — doc-comment cleanup (~36 sites across 16 files)
b955dc24 — model/ + pnl/ panic-free (5 files)
ddf17da3 — simulation/ + risk/ + geometrics/ panic-free (6 files)
1bc97a38 — utils/ + volatility/ + visualization/ panic-free (7 files)
305e2aac — rustfmt cleanup + Week-timeframe precision fix

Closes #321
Closes M1 milestone.

Sweep doc-only sites across lib.rs, strategies/{mod, bull_put_spread}, model/{position, leg/mod, utils}, visualization/mod, volatility/{mod, utils}, metrics/{risk/dollar_gamma, temporal/theta, stress/time_decay, liquidity/{volume_profile, open_interest, bid_ask_spread}}, and utils/others. Each example wrapped in `# fn main() -> Result<(), Box<dyn Error>>` shim with `?` propagation, or partial_cmp().unwrap() replaced with unwrap_or(Ordering::Equal) for the non-runnable `ignore` blocks. No behaviour change — documentation cleanup only.

- src/model/profit_range.rs: hoist `lower_bound`/`upper_bound` into local bindings before the format!() so the InvalidPriceRange branch no longer .unwrap()s. - src/model/option.rs: replace `Positive::new_decimal(mid_vol) .expect(...)` in the IV binary-search loop with `.map_err` to `OptionsError::OtherError` — the bound is structurally non-negative so this is a defensive guard rather than an expected error path. - src/model/decimal.rs::decimal_normal_sample: use `match` with `unreachable!()` on the Err arm of `Normal::new(0.0, 1.0)` — the parameters are hardcoded and statrs accepts mean=0/std=1. - src/pnl/model.rs::PnLRange::new_decimal: change return type from `Self` to `Result<Self, DecimalError>` so out-of-range bounds surface as a typed error rather than panicking. Doc updated; only caller is the doc-comment, no breakage. - src/pnl/metrics.rs: switch the `FILE_LOCKS` and per-file lock acquisitions to `unwrap_or_else(|e| e.into_inner())` for mutex-poison recovery — a panic in one writer no longer permanently blocks subsequent writes to the same path.

- src/simulation/traits.rs::heston: hoist sqrt(dt) out of the loop and surface every Decimal::sqrt failure as `SimulationError::other`. The enclosing fn already returns Result so `?` propagates cleanly. - src/simulation/simulator.rs::get_last_steps + ::get_last_values: switch from `step.last().unwrap()` to `filter_map(|s| s.last())` so empty walks are silently dropped from the result rather than panicking. Doc updated to reflect the new contract. - src/simulation/steps/x.rs::next + ::previous: the enclosing fns already return `Result<_, SimulationError>`, so `?` lands directly on `self.datetime.get_days()`. - src/simulation/steps/step.rs::get_graph_x_in_days_left: visualization helper falls back to `Positive::ZERO` with a `tracing::warn!` instead of panicking — keeps the graph rendering path crash-free. - src/risk/span.rs::calculate_margin: replace nested `partial_cmp().unwrap()` + outer `.unwrap()` with NaN-safe ordering fallback + ZERO fallback for the impossibly-empty risk array, emitting a `tracing::warn!` if the invariant is ever breached. - src/risk/span.rs::calculate_scenario_loss: BS pricing failures fall back to ZERO P&L for that scenario with a warn — conservative behaviour vs. poisoning the whole margin calculation. - src/geometrics/operations/axis.rs::merge_indexes: replace 4 sequential `.first().unwrap()` / `.last().unwrap()` calls with a single `let-else` destructure that falls back to `vec![]` if the invariant ever breaks (the match guard above guarantees both vectors are non-empty, so the else arm is structurally unreachable).

Final-sweep cleanup, closing M1 — Panic-Free Core. All non-test code under src/ now passes the acceptance grep with zero matches. - src/volatility/utils.rs: surface every Decimal::sqrt + Decimal::from_usize + Decimal::from_f64 failure as VolatilityError via the existing From<&str>/From<String> impls; covers constant_volatility, ewma_volatility, simulate_heston_volatility. - src/utils/others.rs: replace TOLERANCE.to_f64().unwrap() in approx_equal with a precomputed TOLERANCE_F64 const, and process_combination.lock().unwrap() with poison-recovery unwrap_or_else(|e| e.into_inner()). - src/utils/time.rs: precompute units_per_year(Week) as pos_or_panic!(52.142857142857142857142857143) (= 365/7) so the runtime fallible Positive::new_decimal call is gone; switch NaiveTime::from_hms_opt(18,30,0).unwrap() to a match with an unreachable!() Err arm. - src/utils/logger.rs: setup_logger / setup_logger_with_level now silently no-op when set_global_default returns Err (a global subscriber is already installed) instead of panicking. Aligns with CLAUDE.md "library code must not install a global subscriber" — these helpers remain opt-in but no longer hard-fail when wrapped by a binary that already configured tracing. - src/utils/csv.rs: replace `start.is_some() && date < start.unwrap()` with `start.is_some_and(|s| date < s)` (and same for end). - src/visualization/utils.rs: 3 colors.get(idx % len).unwrap() sites switched to a `match { Some(c) => c, None => return None }` so the proven-in-bounds index can no longer panic. - src/constants.rs: TOLERANCE marked #[allow(dead_code)] now that utils/others uses the precomputed f64 form.

- Rustfmt cosmetic cleanup across the panic-free refactor (line breaks in long .lock().unwrap_or_else() chains, etc.). - src/utils/time.rs::units_per_year(Week): revert from f64 literal back to exact `Decimal` arithmetic via `Positive::new_decimal(dec!(365.0) / dec!(7.0))` with `match` + `unreachable!()` fallback. The f64 form lost ~1 ulp of precision and broke the strict round-trip assertion in `test_step_next_with_weeks` (Week->Day->Week chain). - src/utils/others.rs: hoist TOLERANCE_F64 const above the `approx_equal` doc-comment so the doc attaches to the function and not the const. - README.md regen from `cargo readme`.

Copilot

Pull request overview

Final sweep to remove remaining production .unwrap() / .expect() calls across the crate, aligning core library code with the M1 “panic-free” milestone while preserving behavior via error propagation, poison recovery, and documented fallbacks.

Changes:

Replaced .unwrap() / .expect() in core logic with Result propagation (?), ok_or_else, and poison-recovering Mutex locking.
Updated doctests/doc examples to compile without panicking (wrapping with fn main() -> Result<...> and using ?).
Adjusted a few behaviors to avoid panics (e.g., skipping empty walks, logger no-op when subscriber already set).

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/volatility/utils.rs	Replaces unwraps in volatility routines (len conversions, sqrt, f64/Decimal conversions) and updates doc examples to use `?`.
src/volatility/mod.rs	Updates module-level docs to avoid `.unwrap()` in examples.
src/visualization/utils.rs	Removes `.unwrap()` from color lookup via `?` propagation.
src/visualization/mod.rs	Updates visualization docs/examples to avoid `.unwrap()`/panic cleanup.
src/utils/time.rs	Replaces `.expect()`/`.unwrap()` with `unreachable!()`-guarded matches for structurally valid constants.
src/utils/others.rs	Removes fallible `Decimal::to_f64().unwrap()` from hot path and adds poison recovery for a shared `Mutex`.
src/utils/logger.rs	Avoids panicking when a global tracing subscriber is already installed.
src/utils/csv.rs	Replaces `is_some()+unwrap()` with `is_some_and(...)`.
src/strategies/mod.rs	Updates strategy docs/examples to avoid `.expect()` in constructors.
src/strategies/bull_put_spread.rs	Updates doctest to propagate errors instead of `.expect()`.
src/simulation/traits.rs	Eliminates unwraps in Heston path generation by checking `sqrt()` results and returning typed errors.
src/simulation/steps/x.rs	Propagates `get_days()` errors instead of unwrapping.
src/simulation/steps/step.rs	Avoids panicking in graphing helper by logging + returning a fallback value.
src/simulation/simulator.rs	Avoids panics on empty walks by skipping empties in “last step/value” helpers and updates docs accordingly.
src/risk/span.rs	Removes unwraps in scenario aggregation and BS pricing calls by adding fallback behavior + logging.
src/pnl/model.rs	Makes `PnLRange::new_decimal` fallible to surface Decimal→i32 conversion failures.
src/pnl/metrics.rs	Adds poison recovery for global/per-file locks used during metric writes.
src/model/utils.rs	Updates doctest to use `?` instead of `Positive::new(...).unwrap()`.
src/model/profit_range.rs	Removes `unwrap()` usage when formatting invalid range errors.
src/model/position.rs	Updates doctests to propagate errors instead of unwrapping.
src/model/option.rs	Replaces `.expect()` on `Positive::new_decimal` with error mapping in IV loop.
src/model/leg/mod.rs	Updates docs to propagate errors (`?`) instead of `unwrap()`.
src/model/decimal.rs	Replaces `Normal::new(...).expect(...)` with `match` + `unreachable!()` for provably-valid params.
src/metrics/temporal/theta.rs	Guards `partial_cmp` unwrap in docs/tests with `unwrap_or(Ordering::Equal)`.
src/metrics/stress/time_decay.rs	Guards `partial_cmp` unwrap in docs/tests with `unwrap_or(Ordering::Equal)`.
src/metrics/risk/dollar_gamma.rs	Updates example to avoid `unwrap()` on max selection by returning an error on empty.
src/metrics/liquidity/volume_profile.rs	Guards `partial_cmp` unwrap in docs/tests with `unwrap_or(Ordering::Equal)`.
src/metrics/liquidity/open_interest.rs	Guards `partial_cmp` unwrap in docs/tests with `unwrap_or(Ordering::Equal)`.
src/metrics/liquidity/bid_ask_spread.rs	Guards `partial_cmp` unwrap in docs/tests with `unwrap_or(Ordering::Equal)`.
src/lib.rs	Updates crate-level docs/examples to propagate errors instead of unwrapping.
src/geometrics/operations/axis.rs	Replaces `.first()/.last().unwrap()` with a `let-else` guard for defensive safety.
src/constants.rs	Adds `#[allow(dead_code)]` to tolerate unused `TOLERANCE` after refactor.
README.md	Updates README examples to remove `.unwrap()`/`.expect()` usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

joaquinbejar · 2026-04-17T17:20:03Z

    implied_volatility,
-).expect("valid custom strategy");
+)?;



Right, the # fn main() hidden line gets stripped by cargo readme so the snippet broke when copy-pasted. Switched to a visible fn main() -> Result<...> wrapper in aa8b592 so both rustdoc and the README work.

joaquinbejar · 2026-04-17T17:25:33Z

+            let color = colors.get(idx % colors.len())?;
            Some(color.to_string())
        }
        ColorScheme::Custom(list) => list.get(idx % list.len()).cloned(),


Real bug, good catch. Added an if list.is_empty() guard returning None in aa8b592 so Custom(Vec::new()) no longer hits the idx % 0 panic.

joaquinbejar · 2026-04-17T17:27:35Z

+        // BS pricing failure here would skew margin to ZERO for that
+        // scenario; we log and continue rather than poison the whole
+        // margin calculation with a panic.
+        let current_price = option.calculate_price_black_scholes().unwrap_or_else(|e| {
+            tracing::warn!("calculate_scenario_loss: current BS price failed: {e}; using 0");
+            Decimal::ZERO
+        });


Agreed, ZERO would mask systematic pricing failures and underestimate margin. Changed calculate_margin, calculate_risk_array and calculate_scenario_loss to Result<_, PricingError> in aa8b592 so BS errors propagate via ? rather than silently flatten the risk surface.

joaquinbejar · 2026-04-17T17:30:03Z

+        let scenario_price = scenario_option
+            .calculate_price_black_scholes()
+            .unwrap_or_else(|e| {
+                tracing::warn!("calculate_scenario_loss: scenario BS price failed: {e}; using 0");
+                Decimal::ZERO
+            });


Same fix applied in aa8b592 — scenario pricing now propagates the error rather than falling back to 0.

+                    let one_minus_rho_sq_sqrt =
+                        (Decimal::ONE - rho * rho).sqrt().ok_or_else(|| {
+                            SimulationError::other(
+                                "Heston: sqrt(1 - rho^2) failed (rho out of range or overflow)",
+                            )


 // Calculate option price using Black-Scholes
-let price = option.calculate_price_black_scholes().unwrap();
+let price = option.calculate_price_black_scholes()?;
 tracing::info!("Option price: ${:.2}", price);

 // Calculate Greeks for risk management
-let delta = option.delta().unwrap();
-let gamma = option.gamma().unwrap();
-let theta = option.theta().unwrap();
-let vega = option.vega().unwrap();
-let vanna = option.vanna().unwrap();
-let vomma = option.vomma().unwrap();
-let veta = option.veta().unwrap();
-let charm = option.charm().unwrap();
-let color = option.color().unwrap();
+let delta = option.delta()?;
+let gamma = option.gamma()?;
+let theta = option.theta()?;
+let vega = option.vega()?;
+let vanna = option.vanna()?;
+let vomma = option.vomma()?;
+let veta = option.veta()?;
+let charm = option.charm()?;
+let color = option.color()?;


codecov · 2026-04-17T17:09:11Z

Codecov Report

❌ Patch coverage is 64.60177% with 40 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/pnl/model.rs	0.00%	14 Missing ⚠️
src/volatility/utils.rs	65.21%	8 Missing ⚠️
src/simulation/traits.rs	69.23%	4 Missing ⚠️
src/geometrics/operations/axis.rs	71.42%	2 Missing ⚠️
src/risk/span.rs	85.71%	2 Missing ⚠️
src/simulation/steps/step.rs	33.33%	2 Missing ⚠️
src/utils/logger.rs	50.00%	2 Missing ⚠️
src/utils/time.rs	75.00%	2 Missing ⚠️
src/model/option.rs	66.66%	1 Missing ⚠️
src/model/profit_range.rs	75.00%	1 Missing ⚠️
... and 2 more

Files with missing lines	Coverage Δ
src/lib.rs	`100.00% <ø> (ø)`
src/model/decimal.rs	`93.10% <100.00%> (ø)`
src/model/position.rs	`82.13% <ø> (ø)`
src/model/utils.rs	`91.17% <ø> (ø)`
src/pnl/metrics.rs	`98.43% <100.00%> (ø)`
src/simulation/steps/x.rs	`91.37% <100.00%> (ø)`
src/strategies/bull_put_spread.rs	`78.65% <ø> (ø)`
src/utils/csv.rs	`89.09% <100.00%> (ø)`
src/utils/others.rs	`100.00% <100.00%> (ø)`
src/model/option.rs	`57.32% <66.66%> (-0.05%)`	⬇️
... and 11 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…e margin, hoisted Heston sqrt - src/lib.rs: README's Basic Option Pricing and Custom Strategy examples now use a visible `fn main() -> Result<...>` wrapper (instead of `# fn main()` hidden lines) so the snippets compile when copy-pasted out of the regenerated README.md. - src/visualization/utils.rs::pick_color: guard `ColorScheme::Custom` against `Vec::new()` (the previous `idx % list.len()` would panic with division by zero). - src/risk/span.rs: change `calculate_margin`, `calculate_scenario_loss` and `calculate_risk_array` from `Decimal` / `Vec<Decimal>` returns to `Result<_, PricingError>`, propagating BS pricing errors via `?` instead of falling back to ZERO. Falling back to ZERO would otherwise silently underestimate margin requirements for systematic pricing failures (Copilot review on PR #355). Internal test updated to `?` as well. - src/risk/mod.rs: doc-examples of `calculate_margin` updated to use `?` propagation through a visible `fn main()` wrapper. - src/simulation/traits.rs::heston: hoist `sqrt(1 - rho^2)` out of the per-step loop — it depends only on `rho` (constant for the path) so the recomputation was wasted work.

joaquinbejar added 5 commits April 17, 2026 18:08

joaquinbejar self-assigned this Apr 17, 2026

joaquinbejar requested a review from Copilot April 17, 2026 17:02

Copilot started reviewing on behalf of joaquinbejar April 17, 2026 17:02 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

joaquinbejar merged commit 9b83e85 into main Apr 17, 2026
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: M1 final sweep — eliminate remaining .unwrap()/.expect() (#321)#355

refactor: M1 final sweep — eliminate remaining .unwrap()/.expect() (#321)#355
joaquinbejar merged 6 commits intomainfrom
issue-321-panic-free-final-sweep

joaquinbejar commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

joaquinbejar Apr 17, 2026

Uh oh!

joaquinbejar Apr 17, 2026

Uh oh!

joaquinbejar Apr 17, 2026

Uh oh!

joaquinbejar Apr 17, 2026

Uh oh!

codecov bot commented Apr 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

joaquinbejar commented Apr 17, 2026

Summary

Per-pattern transformations

Public-surface notes

Test plan

Commit layout

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

joaquinbejar Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

joaquinbejar Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

joaquinbejar Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

joaquinbejar Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Apr 17, 2026 •

edited

Loading