fix(stage4): fail loudly on out-of-range and non-finite immunogenicity_score (LRF-1) - #292
Merged
Merged
Conversation
immunogenicity_score had no range check between a model's predict_proba output and everything downstream that treats it as a [0, 1] probability - calibration, thresholding, the ranked CSV, and (once promoted) api/main.py's response model. A model loaded with a ranking objective returns raw margins outside [0, 1] instead of a probability, and nothing caught it: models/xgb_50feature_integrated.joblib is an XGBClassifier with objective='rank:pairwise', and a mode-50 sweep was observed writing mean_score=-4.38 while scripts/batch_experiment_runner.py recorded the trial as status=SUCCESS. Fails loudly instead, raising RuntimeError naming the model path and the observed range, at the single point every scoring branch converges (before calibration, before the ranked CSV is written). Ruling: hard fail, not warn-and-clip - converts today's silent wrong value into an immediate, unambiguous failure rather than a value nothing downstream can tell apart from a genuine low-confidence score. scripts/batch_experiment_runner.py already catches and records status=FAILED: RuntimeError with the message in notes for any pipeline exception, so this needed no caller-side change to turn the false SUCCESS into an honest FAILED - verified by reading that except block directly rather than assumed. Not a copy-paste of api/main.py's guard, which the register that raised this item described as ready-made: that is a Pydantic Field(ge=0.0, le=1.0) on an API response model, not on this array write, so the bound transfers but the mechanism does not. The PyTorch path is unaffected by construction (_load_pytorch_model applies a sigmoid, which cannot exceed [0, 1]) - confirmed by reading _load_pytorch_model directly. New regression test uses a _FakeRankingModel mirroring the ranking objective's raw-margin output and confirms the joblib path now raises; the existing parametrized test_joblib_branch_scores (4 feature-layout cases, all in-range) continues to pass unmodified, confirming no regression on the legitimate path. Signed-off-by: Gavin Borges <gavinmborges1104@gmail.com>
Resolves the CHANGELOG.md conflict in the [Unreleased] 'Fixed' section, where both sides appended entries: LRF-1's stage-4 range guard on this branch, and the D7 provenance-digest, B3 Zenodo-checksum, branch-protection, B1 leave-one-out and fuzzing.yml entries that landed on main via PR #288 and #291. Both blocks are kept; nothing is dropped. Verified after resolution: the branch differs from main by exactly the three LRF-1 files (CHANGELOG.md, functions/stage4_immunogenicity_scoring.py, tests/test_stage4_scoring.py), +75 lines and no deletions, so no content that arrived on main was lost in the merge. No conflict markers and no banned non-ASCII characters remain in CHANGELOG.md. Signed-off-by: Gavin Borges <gavinmborges1104@gmail.com>
…a false attribution Three defects found by an adversarial review of this branch, two of them in the work this branch had already published. 1. The guard passed NaN. `(x < 0) | (x > 1)` is False for NaN, so a NaN score flowed straight through calibration, thresholding and ranking - as undetectable downstream as the negative margin the guard exists to stop. Verified reachable rather than assumed: `_load_pytorch_model` divides by `scaler_scale + 1e-10` with no finiteness check, and sigmoid(NaN) is NaN. The predicate is now `~np.isfinite(...) | (x < 0.0) | (x > 1.0)`, and the message reports the non-finite count separately from the observed finite range, so `min()`/`max()` are never formatted over a NaN. 2. The CHANGELOG entry misattributed a characterisation to the register that raised this item. It said `api/main.py`'s guard was one 'the register ... described as ready-made'. The register's LRF-1 row says no such thing - it prescribes only 'a range assertion at the write boundary, not a retrain', and never mentions `api/main.py`. The phrase occurs once in the whole tree, in a session planning document that was itself correcting it. This is instance #5's exact shape from `.claude/rules/third-party-claims.md`: inherited from a planning doc and published without re-verification. The clause is removed rather than reattributed. 3. The same entry compressed the register's measured range '-4.38 to -4.20 across 40 rows' to a bare '-4.38', which rule 5 of that same file names as the compression hazard. It now states the range, cites the ledger it came from, and replaces the unsourced 'status=SUCCESS' assertion with the mechanism that is checkable in tracked code: the runner sets SUCCESS on the ranked CSV existing, with no check on the values inside it. Tests: the single mixed fixture pinned neither half of the range check - it held 15 margins below 0 and one above 1, so either clause alone still fired and both mutants survived. It is now parametrized onto each side separately (shift -5.0 and +100.0), with an in-test assertion that the fixture has not drifted off its side, plus new cases for NaN and for exact 0.0/1.0 being accepted. Each of the three clauses is now individually pinned. 41 passed in test_stage4_scoring.py, 52 in the adjacent stage-4/pipeline/API/CLI suites; ruff and mypy clean. Signed-off-by: Gavin Borges <gavinmborges1104@gmail.com>
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 LRF-1. Gavin ruled hard-fail on the failure mode; this implements it, and then a three-lens adversarial review of this branch found three further defects, two of them in the work this branch had already published. All are fixed here.
The defect
immunogenicity_scoreis documented and consumed downstream as a[0, 1]probability - calibration, thresholding, the ranked CSV. Nothing enforced it.models/xgb_50feature_integrated.joblibis anXGBClassifierwithobjective='rank:pairwise', whosepredict_probareturns raw margins, not probabilities. Verified directly: it loads withn_features_in_=50and returns column-1 values from -5.70 to 2.54. Inresults/local_test_sweep_2026-08-21/merged_leaderboard.csva mode-50 sweep recordsmean_scorebetween -4.38 and -4.20 across 40 rows (5 seeds x 8 panels), against rf+mode50's normal 0.686 to 0.702.That run was still recorded as a success. The mechanism is checkable in tracked code rather than only in that ledger:
scripts/batch_experiment_runner.pysetsstatus = "SUCCESS"on the existence of a ranked CSV alone, with no check on the values inside it.The fix
A range guard at the single point every scoring branch in
functions/stage4_immunogenicity_scoring.pyconverges - after the joblib, PyTorch, prototype and degenerate-constant branches, and before calibration, thresholding and the ranked CSV write. Out-of-range and non-finite values raiseRuntimeErrornaming the model path and the observed range.No caller-side change was needed:
batch_experiment_runner.py'sexcept Exceptionis broad andpipeline.py'srun_pipelinecarries notryof its own, so the error propagates and the falseSUCCESSbecomes an honestFAILED: RuntimeErrorwith the message innotes.What the adversarial review changed
1. The guard passed
NaNsilently.(x < 0) | (x > 1)isFalseforNaN, so aNaNscore flowed straight through to ranking - as undetectable downstream as the negative margin the guard exists to stop. Verified reachable rather than assumed:_load_pytorch_modeldivides byscaler_scale + 1e-10with no finiteness check, andsigmoid(NaN)isNaN. The predicate is now~np.isfinite(...) | (x < 0.0) | (x > 1.0), and the message reports the non-finite count separately from the observed finite range somin()/max()are never formatted over aNaN.2. A false attribution in the CHANGELOG entry, retracted. It said
api/main.py's guard was one "the register that raised this item described as ready-made". The register's LRF-1 row says no such thing - it prescribes only "a range assertion at the write boundary, not a retrain", and never mentionsapi/main.py. That phrase occurs once in the entire tree, in a session planning document that was itself correcting it. This is instance #5's exact shape from.claude/rules/third-party-claims.md: inherited from a planning doc and published without re-verification. The clause is removed rather than reattributed.3. A compression hazard, corrected. The entry carried a bare
-4.38where the register measured-4.38 to -4.20 across 40 rows- rule 5 of that same file. It now states the range, cites the ledger, and replaces the unsourcedstatus=SUCCESSassertion with the tracked-code mechanism above.Tests
The original single test pinned neither half of the range check: its fixture is mixed (15 margins below 0, one above 1), so either clause alone still fired and both mutants survived. Caught by a mutation matrix, not by inspection.
shift=-5.0fully below 0,shift=+100.0fully above 1), each with an in-test assertion that the fixture has not drifted off its side.test_joblib_branch_raises_on_nan_score.test_joblib_branch_accepts_exact_bounds- exact0.0and1.0are valid probabilities, pinning the strict</>against a<=/>=mutant.Each of the three clauses is now individually pinned: deleting any one fails its test.
_FakeRankingModelwas also made faithful - it now returns rows summing to1.0as the real ranker does, which is precisely what makes the bad output look probability-shaped on casual inspection.Verification
tests/test_stage4_scoring.py: 41 passed.ruffclean,mypyclean.Behaviour change, stated plainly
This turns a silent wrong value into a loud crash on a scoring path. That is the intended ruling. The mode-50 path that today reports
status=SUCCESSwhile writingmean_score = -4.38will now fail that trial explicitly. No certified or public number is affected -feature_mode=50is documented "Experimental".