Skip to content

FIX: reject a non-finite threshold that silently scores every response False - #2704

Merged
Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
feiiiiii5:fix/threshold-non-finite
Sep 18, 2026
Merged

Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
feiiiiii5:fix/threshold-non-finite

Conversation

@feiiiiii5

@feiiiiii5 fei (feiiiiii5) commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

FIX: reject a non-finite threshold that silently scores every response False

Closes #2703

Description

FloatScaleThresholdScorer accepted threshold=float("nan"): the constructor guard checks the range
but not finiteness, and nan <= 0 and nan > 1 are both False. The NaN then reaches the comparison
at pyrit/score/true_false/float_scale_threshold_scorer.py:198, where every value >= nan is
False, so each response is recorded as score_value='False' — and line 209 stamps that verdict
status=ScoreStatus.COMPLETE.

That is the bad case #2613 describes for a red-teaming tool, and worse here: the converter family
silently wrote a corrupted artifact, whereas this silently persists a definitive negative verdict. A
run configured this way reports "the target did not violate the objective" for content the threshold
never judged, and nothing in the stored score says so.

It contradicts the constructor's own contract (:60, :65 "Raises ValueError: If the threshold is
not between 0 and 1"), and the distinction the same method draws a few lines above: an aggregate with
no value returns ScoreStatus.UNDETERMINED rather than a negative (:174-195). An unreadable
threshold can't reach that branch, so the failure degrades into a real-looking False.

Fix: guard finiteness with the idiom this repo already applies to every other (0, 1] parameter —
audio_white_noise_converter.py:54, audio_volume_converter.py:55, audio_speed_converter.py:54,
image_resizing_converter.py:50, image_color_saturation_converter.py:51, cli/pyrit_scan.py:163
(the #2560 / #2566 / #2613 family). FloatScaleThresholdScorer was the remaining outlier. The raised
message is unchanged, so no previously accepted or previously rejected value changes behaviour.

No shipped default passes NaN (every in-repo call site uses a literal, e.g.
setup/initializers/scorers.py:328); the reachable sources are caller-supplied — a threshold computed
from a calibration sample (numpy.mean([]), violations / total with total == 0), or a hand-written
scenario config, where json.loads("NaN") and YAML .nan both yield a clean float. Nothing between
the config and the persisted verdict objects, which is what this guard adds.

Tests and Documentation

Reproduced on both trees with the same code; the base run used a detached worktree at 66d77e4c with
source untouched, so nothing was reverted in place. Full reproduction and measured output in #2703.

  • base, new tests only: 1 failed, 7 passed, 28 deselected — the single failure is
    test_init_rejects_non_finite_or_outside_unit_range_threshold[nan] (DID NOT RAISE ValueError);
    inf, -inf, 0.0, -0.5, 1.5 already raised, so the test bites exactly this defect
  • base, full parallel suite (pytest -n 4 --dist=loadfile tests/unit, same test file present,
    source at 66d77e4c): 1 failed, 18061 passed, 10 skipped — that one failure is the same
    [nan] case and nothing else in the suite changes
  • after the fix: tests/unit/score/test_float_scale_threshold_scorer.py → 36 passed
  • tests/unit/score → 1917 passed; tests/unit/executor → 1194 passed, 33 skipped
  • fixed tree, same full parallel command with coverage: 18062 passed, 10 skipped, 0 failed in
    114s — the same total as the base run above, with the [nan] case the only difference
  • --cov-fail-under=78 → coverage 95.03%; diff_cover --fail-under=90 → the two changed source
    lines at 100% (0 missing)
  • pre-commit run --files <both files> → ruff format, ruff check, ty, async-suffix and the rest
    all Passed

CI-equivalent environment. I also reproduced CI's dev_all leg (uv sync --extra all, then
pytest -n 4 --dist=loadfile tests/unit): 18061 passed, 10 skipped, 1 failed, and the only
failure was tests/unit/datasets/test_comic_jailbreak_dataset.py::test_fetch_dataset_missing_goal_raises,
which is unrelated to this change. Tracking that one honestly across six full-suite runs: it failed
twice on the fix branch (default env once, dev_all once) and never on a detached base worktree run
with the same command and venv (2/2 clean apart from the expected [nan] case); it passes in
isolation on both trees, and tests/unit/datasets alone passes serially (4678) and under -n 4
(4678). So it looks like an ordering-dependent flake under xdist rather than something this diff
causes — I could not prove it either way from these samples, so I am flagging it instead of
claiming it green.

The two new tests are offline and parametrized on both sides of the boundary (reject nan, inf,
-inf, 0.0, -0.5, 1.5; accept 0.0001, 1.0). This constructor had no pytest.raises
coverage at all before, so its documented ValueError was untested.

Scope: one guard condition, two docstring sentences, and the tests. Four other comparisons read a
caller-supplied float the same way (promptgen/fuzzer/fuzzer.py:1125,
analytics/text_matching.py:110, analytics/conversation_analytics.py:79,
output/scorer/pretty.py:58) but return an in-memory boolean or a colour instead of persisting a
COMPLETE score, so they are a separate concern — happy to follow up. A NaN score value is not a separate leak here: Score's own validation rejects it
(Float scale scorers must have a score value between 0 and 1. Got nan, checked on main), so the
threshold side was the only unguarded entry point in this path.

Developed with AI assistance (Claude), reviewed line by line against the code paths above; the
commands and outputs quoted are the ones I ran on the commits named.

FloatScaleThresholdScorer validated the threshold range but not its
finiteness: `nan <= 0 or nan > 1` is False, so a NaN threshold was
accepted, and since every comparison against NaN is False the scorer
recorded each response as score_value='False' with status COMPLETE - a
definitive "no violation" for content it never judged.

Guard finiteness with the same idiom the repo already uses for every
other (0,1] value (e.g. AudioWhiteNoiseConverter), and add the
constructor's first validation tests.
Aligns the arg description with the guard that now rejects non-finite
values, and renames the parametrized case so it says which two reasons
are covered.
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Sep 18, 2026
Merged via the queue into microsoft:main with commit 1d0052f Sep 18, 2026
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FloatScaleThresholdScorer accepts a NaN threshold and then records every response as a complete False verdict

2 participants