fix(derivations): evaluate/validate never raise; non-finite results are violations; construction validates - #126
Merged
Conversation
…re violations; construction validates ## Summary - **What:** honour the module's never-raises contract and stop silent wrong answers in derived variables (transforms + binning). - **Why:** an adversarial pass (36,000 fuzzed evaluations) found pandas raising through evaluate() on ±inf inputs, colliding range labels, duplicate/null custom labels and string parameters; non-finite outputs passing with n_invalid=0; z-score of a constant never triggering on_invalid; more bins than distinct values with no message; labels spelled "NaN"/"Infinity" corrupted on round trip. Coverage was 92% but no test fed any of these inputs. - **Scope:** derivations.py (construction validation, evaluation, labelling, serialization); tests/test_derivations_robustness.py (56 tests incl. a seeded fuzz); CHANGELOG. ## Contract / Invariants (must remain true) - Finite inputs with distinct labels: byte-identical edges, labels, values (existing 73 derivation/bin-extreme tests pass unchanged; pinned again in the new file). - evaluate()/validate() signatures and EvalResult/ValidationResult shapes unchanged. - Bishop gate 280/280 (derivations are not on that path). ## Behavior Changes (explicit) - ±inf: transforms -> violation; bins -> excluded from fit, NaN, counted, message. - Every non-finite transform output is a violation (was: only power overflow). - zscore with undefined sigma -> every present value is a violation. - n capped at distinct-value count with a message; ordinal >5 bins says so; arcsin hint. - Construction rejects: bool/non-integral/<=0 n; non-finite or non-numeric shift, exponent, breaks; unknown on_invalid; empty/null/duplicate explicit labels. numpy integers/floats are accepted and normalised to Python int/float. - to_dict JSON-safe for numpy; from_dict decodes float tags only under numeric keys; missing id -> ValidationError. - Non-numeric (datetime/categorical) source -> all-NA result with message, no raise. ## Tests - tests/test_derivations_robustness.py: construction validation (parametrised), serialization round trips through json.dumps, non-finite transforms, bin infinities, tiny-range labels, near-constant sd, n cap, ordinal fallback, non-numeric sources, finite-data-unchanged pins, seeded fuzz (400 series x 63 specs, marked slow) plus a 40-series quick slice, and the ProcessBehavior remove/replace/free-function paths. ## Manual Verification - pytest tests/: 2421 passed, 10 skipped; ruff clean; mypy derivations.py clean - validation/e2e_bishop_report.py: exit 0 - Ad-hoc harnesses: 34,800 evaluations, zero raises, zero inf outputs, zero finite inputs dropped from a fitted bin
10 tasks
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.
Summary
evaluateandvalidatenever raise on routine data, and stop the silent wrong answers found alongside.evaluate()on ±inf inputs ("bins must increase monotonically"), on range labels that collide at six significant digits ("labels must be unique"), on duplicate or null explicit labels, and on string parameters reaching numpy. It also found non-finite transform outputs passing withn_invalid=0, a z-score of a constant column that never triggeredon_invalid, more bins than distinct values with no message, and custom labels spelled "NaN" or "Infinity" turned into floats on round trip. Line coverage was 92 percent, but no test fed any of these inputs. The feature is young and lightly used, so fixing now is cheap.derivations.py(construction validation, evaluation, labelling, serialization); a new test file with 56 tests including a permanent seeded fuzz; CHANGELOG.Contract / invariants
evaluate/validatesignatures andEvalResult/ValidationResultshapes unchanged.fittedfor a degenerate bin now always carriesn_bins: 0,edges: [],labels: [].Behaviour changes
n_invalidwith the message "k non-finite value(s) cannot be binned".on_invalid='error'names the derivation atformulateinstead of the study failing later with "No valid response values".'ordinal'above five bins says it fell back to numbered bins;arcsinon values in (1, 100] hints "divide by 100".na positive integer (numpy integers accepted and normalised, booleans rejected);shift,exponent,breaksfinite numbers;on_invalidin {'error', 'na'}; explicit labels non-empty, null-free, unique. AllValidationError, all before any data is touched.to_dictconverts numpy scalars and arrays;from_dictdecodes theInfinity/NaNtags only under numeric keys, so a label spelled "NaN" stays a string; a missingidis aValidationError.Methodology
Tests
tests/test_derivations_robustness.py(56 tests):TestConstructionValidation— every rejected shape, parametrised; numpynaccepted and normalised; direct construction shares the gate.TestSerialization—json.dumpsround trip with numpy inputs; label tags stay strings; non-finite numbers round-trip under numeric keys; missing id.TestTransformNonFinite— inf in or out flagged for every function; constant z-score is a violation andformulatenames it; arcsin hint; finite data unchanged.TestBinRobustness— infinities leave the fit and are counted for every method; only-infinities reported not raised; tiny-range labels distinct; near-constantsdfits; bin cap; ordinal message; non-numeric sources; finite edges pinned.test_evaluate_never_raises_and_never_returns_infinity— 400 random series × 63 specs (markedslow), plus a 40-series quick slice in the default selection.TestProcessBehaviorDerivationErrors— the remove/replace error paths and free functions that had no test.pytest tests/— 2421 passed, 10 skippedruff check .— cleanmypy processbehavior/derivations.py— cleanGolden masters untouched; validator exit 0
Notes
derivation_ui.previewsurfacesn_invalidandmessageautomatically, so the new bin messages appear in the live preview. The preview's warning text forn_invalidwas written for transforms ("outside this function's domain" plus the on_invalid radio); with bins now reporting non-finite counts that wording should be split. Not in this PR; belongs with the next app pin bump.🤖 Generated with Claude Code
https://claude.ai/code/session_01BKphCi1LC1hjGYteWEK9JV