docs(lib,readme): refresh crate-level docs + mermaid for 0.16.x (0.16.5)#395
docs(lib,readme): refresh crate-level docs + mermaid for 0.16.x (0.16.5)#395joaquinbejar merged 1 commit intomainfrom
Conversation
The crate-level rustdoc in `src/lib.rs` predated the 0.16 quality
hardening. Refresh it so the top-level narrative, the mermaid
diagrams, and the example invocations match the shipped library:
- New *Quality & Discipline (0.16.x)* section listing the invariants
that are now enforced crate-wide: checked `Decimal` helpers
(`d_add` / `d_sub` / `d_mul` / `d_div` / `d_sum` /
`d_sum_iter`), `finite_decimal` guard and `NonFinite` error
variants at every `f64` boundary, `NonZeroUsize` step/simulation
counts, `#![deny(clippy::indexing_slicing)]` and
`#![deny(missing_docs, rustdoc::broken_intra_doc_links)]` with
scoped escapes, `#[tracing::instrument]` on public hot paths,
`utils::deterministic_rng` for seeded Monte-Carlo tests, and the
pricing-identity regression tests.
- New *Arithmetic-Error Cascade* mermaid diagram showing the
`d_*` helpers + `finite_decimal` feeding
`DecimalError::Overflow` / `*::NonFinite` and then cascading
through `#[from]` into the domain error enums.
- New *Observability* mermaid diagram listing the five instrumented
public hot paths (`black_scholes`,
`monte_carlo_option_pricing`, `price_binomial`,
`implied_volatility`, `Optimizable::{get_best_ratio,get_best_area}`).
- Testing section updated to the actual current counts (3760 lib +
205 doctests) and mentions the deterministic-RNG helper plus the
identity regression tests under `tests/unit/pricing/identities_test.rs`.
- Examples section rewritten for the shipped layout of sub-crates
under `examples/` with the correct `--manifest-path=` invocation,
plus a note on the hourly demo grid for simulation-heavy binaries.
- Version markers bumped to 0.16.5; prerequisites note 2024 edition
/ 1.85 toolchain; feature list documents `plotly` /
`static_export` / `async`.
Regenerated README via `cargo readme`. `cargo test --doc --all-features`
— 205 pass, 0 fail on the bumped crate.
There was a problem hiding this comment.
Pull request overview
Documentation refresh to align crate-level rustdoc (src/lib.rs) and the generated README.md with the 0.16.x “quality hardening” narrative, plus a version bump to 0.16.5 and a corresponding changelog entry.
Changes:
- Updated crate-level docs and README with new “Quality & Discipline (0.16.x)” section plus mermaid diagrams (arithmetic error cascade + observability).
- Refreshed testing/exmaples sections to reflect current counts and the sub-crate examples layout.
- Bumped crate version to 0.16.5 and added a 0.16.5 changelog entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/lib.rs | Adds/updates rustdoc sections, mermaid diagrams, version markers, and updated examples/testing narrative. |
| README.md | Regenerated README to mirror crate-level docs changes and version markers. |
| Cargo.toml | Version bump to 0.16.5. |
| CHANGELOG.md | Adds 0.16.5 entry and updates compare links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Structured tracing.** `#[tracing::instrument]` on the public | ||
| hot paths: `pricing::black_scholes`, | ||
| `pricing::monte_carlo_option_pricing`, | ||
| `pricing::price_binomial`, `volatility::implied_volatility`, | ||
| and the strategy optimizer entry points | ||
| `get_best_ratio` / `get_best_area`. No `println!` / `eprintln!` | ||
| / `dbg!` / `log::` anywhere in `src/`. |
There was a problem hiding this comment.
The docs say the hot paths are annotated with #[tracing::instrument], but the codebase uses #[instrument] (via use tracing::instrument;) on these functions. Please update the docs to match the actual attribute form (or mention both if you want to allow either style).
| Enable a subscriber in the consumer crate (the library itself never | ||
| installs one) to surface structured spans: |
There was a problem hiding this comment.
This claims the library “never installs” a tracing subscriber, but optionstratlib::utils::setup_logger / setup_logger_with_level do call tracing::subscriber::set_global_default(...). Consider rewording to “does not install one by default” and optionally point readers to the provided setup helper.
| Enable a subscriber in the consumer crate (the library itself never | |
| installs one) to surface structured spans: | |
| Enable a subscriber in the consumer crate (the library itself does not | |
| install one by default; you can also use | |
| `optionstratlib::utils::setup_logger` or `setup_logger_with_level`) to | |
| surface structured spans: |
| - **Non-finite `f64` guards.** Every `f64 → Decimal` boundary inside | ||
| pricing, Greeks, volatility, and simulation is wrapped with | ||
| `finite_decimal(..)` and surfaces a domain-specific | ||
| `NonFinite { context, value }` variant | ||
| (`PricingError`, `GreeksError`, `VolatilityError`, | ||
| `SimulationError`) instead of collapsing silently to |
There was a problem hiding this comment.
The text states that every f64 → Decimal boundary is wrapped with finite_decimal(..) and no longer collapses silently to Decimal::ZERO, but there are still production call sites using Decimal::from_f64(..).unwrap_or_default() / unwrap() (e.g., Options::payoff* and chain time-to-expiry conversions). Please either narrow this claim (scope it to specific modules/kernels) or update remaining boundaries to use finite_decimal(..) + *Error::NonFinite so the invariant is accurate.
| - **Non-finite `f64` guards.** Every `f64 → Decimal` boundary inside | |
| pricing, Greeks, volatility, and simulation is wrapped with | |
| `finite_decimal(..)` and surfaces a domain-specific | |
| `NonFinite { context, value }` variant | |
| (`PricingError`, `GreeksError`, `VolatilityError`, | |
| `SimulationError`) instead of collapsing silently to | |
| - **Non-finite `f64` guards.** Hardened `f64 → Decimal` boundaries in | |
| pricing, Greeks, volatility, and simulation route through | |
| `finite_decimal(..)` and surface a domain-specific | |
| `NonFinite { context, value }` variant | |
| (`PricingError`, `GreeksError`, `VolatilityError`, | |
| `SimulationError`) rather than silently collapsing to |
| BS["pricing::black_scholes\n(strike, style, side)"] | ||
| MC["pricing::monte_carlo_option_pricing\n(steps, simulations, strike, style, side)"] | ||
| BI["pricing::price_binomial\n(strike, asset, steps, style, side)"] | ||
| IV["volatility::implied_volatility\n(market_price, strike, max_iterations)"] |
There was a problem hiding this comment.
In the Observability mermaid, volatility::implied_volatility is shown as taking (market_price, strike, max_iterations), but the actual signature takes (market_price, &mut Options, max_iterations) (strike is only an instrumented field derived from options.strike_price). Please adjust the diagram label to avoid implying a different public API.
| IV["volatility::implied_volatility\n(market_price, strike, max_iterations)"] | |
| IV["volatility::implied_volatility\n(market_price, &mut Options, max_iterations)"] |
| //! - **Structured tracing.** `#[tracing::instrument]` on the public | ||
| //! hot paths: `pricing::black_scholes`, | ||
| //! `pricing::monte_carlo_option_pricing`, | ||
| //! `pricing::price_binomial`, `volatility::implied_volatility`, | ||
| //! and the strategy optimizer entry points | ||
| //! `get_best_ratio` / `get_best_area`. No `println!` / `eprintln!` | ||
| //! / `dbg!` / `log::` anywhere in `src/`. |
There was a problem hiding this comment.
The docs say the hot paths are annotated with #[tracing::instrument], but the codebase uses #[instrument] (via use tracing::instrument;) on these functions. Please update the docs to match the actual attribute form (or mention both if you want to allow either style).
| //! Enable a subscriber in the consumer crate (the library itself never | ||
| //! installs one) to surface structured spans: |
There was a problem hiding this comment.
This claims the library “never installs” a tracing subscriber, but optionstratlib::utils::setup_logger / setup_logger_with_level do call tracing::subscriber::set_global_default(...). Consider rewording to “does not install one by default” and optionally point readers to the provided setup helper.
| //! Enable a subscriber in the consumer crate (the library itself never | |
| //! installs one) to surface structured spans: | |
| //! Enable a subscriber in the consumer crate to surface structured spans. | |
| //! The library does not install one by default, though you can use | |
| //! `optionstratlib::utils::setup_logger` or | |
| //! `optionstratlib::utils::setup_logger_with_level` if desired. |
| //! - **Non-finite `f64` guards.** Every `f64 → Decimal` boundary inside | ||
| //! pricing, Greeks, volatility, and simulation is wrapped with | ||
| //! `finite_decimal(..)` and surfaces a domain-specific | ||
| //! `NonFinite { context, value }` variant | ||
| //! (`PricingError`, `GreeksError`, `VolatilityError`, | ||
| //! `SimulationError`) instead of collapsing silently to | ||
| //! `Decimal::ZERO`. |
There was a problem hiding this comment.
The text states that every f64 → Decimal boundary is wrapped with finite_decimal(..) and no longer collapses silently to Decimal::ZERO, but there are still production call sites using Decimal::from_f64(..).unwrap_or_default() / unwrap() (e.g., Options::payoff* and chain time-to-expiry conversions). Please either narrow this claim (scope it to specific modules/kernels) or update remaining boundaries to use finite_decimal(..) + *Error::NonFinite so the invariant is accurate.
| //! - **Non-finite `f64` guards.** Every `f64 → Decimal` boundary inside | |
| //! pricing, Greeks, volatility, and simulation is wrapped with | |
| //! `finite_decimal(..)` and surfaces a domain-specific | |
| //! `NonFinite { context, value }` variant | |
| //! (`PricingError`, `GreeksError`, `VolatilityError`, | |
| //! `SimulationError`) instead of collapsing silently to | |
| //! `Decimal::ZERO`. | |
| //! - **Non-finite `f64` guards.** Hardened kernels inside pricing, | |
| //! Greeks, volatility, and simulation route `f64 → Decimal` | |
| //! conversions through `finite_decimal(..)` and surface a | |
| //! domain-specific `NonFinite { context, value }` variant | |
| //! (`PricingError`, `GreeksError`, `VolatilityError`, | |
| //! `SimulationError`) instead of collapsing silently to | |
| //! `Decimal::ZERO`; this guarantee applies to the migrated kernels, | |
| //! not yet to every remaining production call site crate-wide. |
| //! BS["pricing::black_scholes\n(strike, style, side)"] | ||
| //! MC["pricing::monte_carlo_option_pricing\n(steps, simulations, strike, style, side)"] | ||
| //! BI["pricing::price_binomial\n(strike, asset, steps, style, side)"] | ||
| //! IV["volatility::implied_volatility\n(market_price, strike, max_iterations)"] |
There was a problem hiding this comment.
In the Observability mermaid, volatility::implied_volatility is shown as taking (market_price, strike, max_iterations), but the actual signature takes (market_price, &mut Options, max_iterations) (strike is only an instrumented field derived from options.strike_price). Please adjust the diagram label to avoid implying a different public API.
| //! IV["volatility::implied_volatility\n(market_price, strike, max_iterations)"] | |
| //! IV["volatility::implied_volatility\n(market_price, &mut Options, max_iterations)"] |
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Summary
Documentation-only refresh of `src/lib.rs` and `README.md` to match the 0.16 quality hardening.
Regenerated `README.md` via `cargo readme`. `CHANGELOG.md` updated for 0.16.5. Cargo.toml bumped to 0.16.5.
Test plan