fix: fail arbitrage scans that cannot evaluate every grid point - #134
Merged
pandashark merged 1 commit intoAug 20, 2026
Conversation
Both scan helpers swallowed density failures and still returned Ok, so a report could not distinguish "no violations found" from "most points were never checked". A smile with non-positive total variance reported as arbitrage-free. scan_g was the sharper case: it only evaluates density at points already flagged by g(k) < -tol, so the skip discarded violations that had already been detected rather than merely leaving points unchecked. Both now return NumericalError naming the strike that failed, so Ok means the whole configured grid was covered. Models built through their validated constructors are unaffected — SSVI and eSSVI cannot reach the path at all, SABR has been clamped since the correction floor was added, and SVI needs its variance minimum to sit exactly on a grid point. The SABR doc claiming failed points are skipped was already stale before this change; it now states the guarantee. Refs PAN-135 (item 1 of 7).
pandashark
deleted the
richardamaya/pan-135-api-contract-gaps-from-roborev-api-design-review-4228
branch
August 20, 2026 02:36
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
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.
Closes item 1 of PAN-135. The other six findings stay open on the ticket.
Problem
scan_densityandscan_gboth didErr(_) => continueon a density failure and still returnedOk. A caller could not tell "every grid point was checked and none violated" from "most points failed to evaluate and were dropped".The crate's own test suite contained the proof:
is_arbitrage_free_skips_density_errorsbuilt a smile with negative total variance at every strike and asserted it reported arbitrage-free.scan_gwas the sharper case. It only evaluates density at points already flagged byg(k) < -BUTTERFLY_G_TOL, so the skip discarded violations that had already been detected rather than merely leaving points unchecked. Sincegatheral_greturnsNEG_INFINITYwhenw <= 0, a smile with negative variance everywhere flagged every point and then dropped every one.Change
Both helpers now propagate, wrapping the cause with the strike that failed:
Ok(ArbitrageReport)now means the whole configured grid was covered.Why not record scan coverage instead
The ticket offered that alternative.
ArbitrageReporthas public fields and no#[non_exhaustive], so addingpoints_attempted/points_evaluatedbreaks every struct literal — including the doctest inarbitrage.rsand the construction inspline.rs. That belongs with the other breaking items the review already deferred to v3.0.No type signatures change here, and
VolSurfErroralready has aNumericalErrorvariant, so no new variant either.Blast radius
Near-zero for models built through validated constructors:
w > 0for all paramsnewaccepts.hagan_implied_vol.min_variance == 0.0and a grid point landing exactly on the argmin.is_arbitrage_freeoverride already propagated with?.The path is reachable mainly by bypassing constructor validation, which is exactly what the inverted test does.
Tests
is_arbitrage_free_skips_density_errors→is_arbitrage_free_errors_on_density_failure, now assertingErrand that the message names a strike.scan_density_fails_when_a_grid_point_cannot_be_evaluatedcovering thescan_densityside, which SVI's g-function path never exercised.cargo docclean under-D warnings.Not in scope
SplineSmileoverridesis_arbitrage_freebut notis_arbitrage_free_with, so it scans absolute strike space over its knot range in one method and log-moneyness via the trait default in the other, ignoringArbitrageScanConfigin both. After this change the two agree on error semantics but still disagree on domain. Separate defect — aligning it changes which violations spline reports.