Skip to content

fix(detectors): stop RefusalOnlyAdversarial over-counting skipped multi-turn attempts - #2111

Open
Yigtwxx wants to merge 1 commit into
NVIDIA:mainfrom
Yigtwxx:fix/judge-refusalonly-outputs-alignment
Open

fix(detectors): stop RefusalOnlyAdversarial over-counting skipped multi-turn attempts#2111
Yigtwxx wants to merge 1 commit into
NVIDIA:mainfrom
Yigtwxx:fix/judge-refusalonly-outputs-alignment

Conversation

@Yigtwxx

@Yigtwxx Yigtwxx commented Aug 23, 2026

Copy link
Copy Markdown

judge.RefusalOnlyAdversarial.detect returns [None] * len(attempt.all_outputs) when it skips a non-adversarial attempt, but detector results are expected to line up with attempt.outputs. garak/evaluators/base.py states this inline, next to the index it uses:

messages.append(
    attempt.outputs[idx]
)  # this is an opinion about scope of detection; expects that detector_results aligns with attempt.outputs (not all_outputs)

all_outputs collects every assistant turn across the attempt's conversations; outputs collects the last assistant turn of each. They are equal only for single-turn attempts, so the fix is one line, all_outputs to outputs.

probes.fitd.FITD sets primary_detector = "judge.RefusalOnlyAdversarial" and is multi-turn, so its intermediate attempts marked is_adversarial: False are exactly the misaligning case. Each skipped attempt contributes one None per turn taken rather than one per output, inflating the nones tally _evaluate_one_detector reports and the length of detector_results written to report.jsonl. attempt.py documents detector_results as "a list of scores corresponding to each of the generator output strings in outputs".

This looks like a missed spot rather than a deliberate choice. #1415 ("remove usage of Attempt.all_outputs; update detectors to process only latest Attempt.outputs") swept the detectors in October 2025; RefusalOnlyAdversarial was added on 2025-10-10 with the FITD probe, inside the same window, and kept all_outputs. The sibling class JailbreakOnlyAdversarial, added later for GOAT, does the same job and already returns one None per attempt.outputs. In #1430 @leondz asked for concrete bugs in this area to be raised ahead of the wider architectural change.

After this change the only remaining all_outputs use under garak/detectors/ is in any.AnyOutput / any.AnyNonspaceOutput, where scoring every turn is the stated purpose of the class. Those are left alone. Grepping all_outputs under garak/detectors/ returns twenty hits, so that claim reads as false at a glance: only any.py:21 and any.py:34 are reads of the attempt.all_outputs property. The rest (base.py, misleading.py, propile.py, snowball.py) bind a local named all_outputs from attempt.outputs_for(...), which already returns latest-turn outputs — the name is misleading there, the behaviour is not.

Checked open PRs before opening this one: fifteen touch garak/detectors/judge.py (#2013, #1832, #1898, #1970, #1773, #1885, #1717 and #1688 among them) and none modifies RefusalOnlyAdversarial.detect. No open PR or open issue references this misalignment; #1430, which describes the general indexing mismatch, was closed as completed by #1415.

Verification

  • Added TestRefusalOnlyAdversarial::test_non_adversarial_returns_one_result_per_output, which builds an attempt holding two assistant turns and one output, asserts that multi-turn precondition, and checks the skip path returns one result per output without calling the judge model. It mirrors the existing TestJailbreakOnlyAdversarial, whose single-turn attempts cannot catch this.
  • Verify the test fails without the fix. It was written first and run against unmodified code:
$ python -m pytest tests/detectors/test_detectors_judge.py::TestRefusalOnlyAdversarial -q
>       assert res == [None] * len(a.outputs)
E       assert [None, None] == [None]
E         Left contains one more item: None
1 failed in 2.40s
  • Run the tests and ensure they pass python -m pytest tests/
5691 passed, 105 skipped in 1091.24s (0:18:11)
  • Verify the thing does not do what it should not. The adversarial branch is untouched and still delegates to Refusal.detect; only the length of the skip branch's list changes.
  • No new dependencies, and garak/resources/plugin_cache.json is not included.

Environment: Windows 11, Python 3.11. black --config pyproject.toml --check already flags both touched files on main, over lines this PR does not go near; the lines added here are black-clean and the pre-existing formatting is left as it is.

This change was made with AI assistance (Claude Code): it found the inconsistency, wrote the failing test first, applied the one-line fix and ran the suites quoted above. I reviewed every changed line, confirmed the all_outputs / outputs divergence and the #1415 history myself, and ran the tests locally.

…t.outputs

Detector results are expected to line up with attempt.outputs, as stated in
evaluators/base.py. When RefusalOnlyAdversarial skipped a non-adversarial
attempt it returned one None per assistant turn held by the attempt instead,
so a multi-turn attempt was scored with more results than it has outputs.

probes.fitd.FITD carries several assistant turns per attempt and names this
detector as its primary detector, so every skipped intermediate attempt
inflated the evaluator's None count.

NVIDIA#1415 removed Attempt.all_outputs use from the detectors; this class landed
in the same window and kept it. Its sibling JailbreakOnlyAdversarial already
returns one None per attempt.outputs.

Co-authored-by: Claude <noreply@anthropic.com>

Signed-off-by: Yigtwxx <yigiterdogan023@gmail.com>
@leondz

leondz commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

What

Please amend title

@Yigtwxx Yigtwxx changed the title fix(detectors): align RefusalOnlyAdversarial skip results with attempt.outputs fix(detectors): stop RefusalOnlyAdversarial over-counting skipped multi-turn attempts Aug 23, 2026
@Yigtwxx

Yigtwxx commented Aug 23, 2026

Copy link
Copy Markdown
Author

Sorry, that was opaque. Amended.

The old title described the change; the new one describes the defect: when RefusalOnlyAdversarial skips a non-adversarial attempt it emits one None per assistant turn rather than one per output, so multi-turn attempts (probes.fitd.FITD) are counted with more results than they have outputs.

Happy to reword again if you had something else in mind, or to trim the description down.

@Yigtwxx

Yigtwxx commented Aug 23, 2026

Copy link
Copy Markdown
Author

Re-read your comment: you were pointing at the What heading, not just the title. My mistake.

The description now follows PULL_REQUEST_TEMPLATE.md instead: what the change does as prose, then ## Verification. The invented headings are gone.

@MohammedAlkindi

Copy link
Copy Markdown
Contributor

Confirmed on Windows 11 / Python 3.13 against main at 3845757.

Reproduced your fail-before by taking this branch and reverting only judge.py:

>       assert res == [None] * len(a.outputs)
E       assert [None, None] == [None]
E         Left contains one more item: None

Byte-identical to what you reported on 3.11, and the test passes on the branch as written.

Your completeness claim holds too, though it reads as false at a glance. Grepping all_outputs under garak/detectors/ returns around twenty hits, but most are a local variable assigned from outputs_for(...). Only three are attempt.all_outputs property reads: two in any.py, and judge.py:171 which this removes. Worth knowing before someone greps and doubts it.

@Yigtwxx

Yigtwxx commented Aug 24, 2026

Copy link
Copy Markdown
Author

Thanks for the independent repro — good to have it confirmed on 3.13 as well as 3.11.

On the grep: agreed, and worth spelling out. Of the 20 all_outputs hits under garak/detectors/, only any.py:21 and any.py:34 are reads of the attempt.all_outputs property. The rest (base.py, misleading.py, propile.py, snowball.py) bind a local named all_outputs from attempt.outputs_for(...), which already returns latest-turn outputs — same shape as attempt.outputs, just language-filtered. So the name is misleading there, but the behaviour is not.

I've added a note to the PR description so the next person greping this doesn't have to re-derive it.

@jmartin-tech jmartin-tech self-assigned this Sep 3, 2026
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.

4 participants