Skip to content

Fix FileDetector output/result misalignment (yield None for skipped outputs)#1883

Open
AUTHENSOR wants to merge 1 commit into
NVIDIA:mainfrom
AUTHENSOR:fix/filedetector-output-alignment
Open

Fix FileDetector output/result misalignment (yield None for skipped outputs)#1883
AUTHENSOR wants to merge 1 commit into
NVIDIA:mainfrom
AUTHENSOR:fix/filedetector-output-alignment

Conversation

@AUTHENSOR

Copy link
Copy Markdown

Fix FileDetector output/result misalignment (yield None for skipped outputs)

Summary

FileDetector.detect() used continue to skip missing/empty/non-file outputs,
which made detector_results shorter than attempt.outputs. The evaluator
then indexes attempt.outputs by position, so this misattributed later
detector scores to the wrong outputs
and silently dropped some from scoring.
The hitlog recorded incorrect output↔score pairings.

The fix yields None for skipped outputs, preserving length alignment. The
evaluator already counts None scores as nones (excluded from pass/fail), so
this is the correct downstream signal.

The bug

# garak/detectors/base.py, FileDetector.detect (before)
for local_filename in attempt.outputs:
    if not local_filename or not local_filename.text:
        continue                          # <- drops this output's result
    if not os.path.isfile(local_filename.text):
        logging.info("Skipping non-file path %s", local_filename)
        continue                          # <- drops this output's result
    else:
        test_result = self._test_file(local_filename.text)
        yield test_result if test_result is not None else 0.0

The evaluator consumes these results positionally:

# garak/evaluators/base.py, Evaluator.evaluate
for idx, score in enumerate(attempt.detector_results[detector]):
    ...
    messages.append(attempt.outputs[idx])   # <- indexes outputs by position

When FileDetector skips an output, the results list shifts — so outputs[idx]
no longer corresponds to the output that produced results[idx]. The code even
documents the assumption: "expects that detector_results aligns with
attempt.outputs"
.

Reproduction

import os, tempfile
from garak.detectors.base import FileDetector
from garak.attempt import Attempt, Message

class T(FileDetector):
    """test"""
    valid_format = "local filename"
    def _test_file(self, f): return 1.0 if f.endswith(".pkl") else 0.0

tmp = tempfile.mkdtemp()
pkl, txt = os.path.join(tmp,"a.pkl"), os.path.join(tmp,"a.txt")
open(pkl,"w").write("x"); open(txt,"w").write("x")

a = Attempt(prompt=Message(""))
a.notes["format"] = "local filename"
a.outputs = [Message(pkl), Message("/missing"), Message(txt)]

# Before fix: list(T().detect(a)) == [1.0, 0.0]   (len 2, NOT 3)
#   -> evaluator attributes 0.0 to outputs[1] ("/missing")  ← WRONG
#   -> outputs[2] (txt) is never scored                     ← DROPPED
# After fix:  list(T().detect(a)) == [1.0, None, 0.0]       (len 3, aligned)
#   -> evaluator counts None as a "none" (excluded), 0.0 correctly to outputs[2]

The fix

# after
for local_filename in attempt.outputs:
    if not local_filename or not local_filename.text:
        yield None
        continue
    if not os.path.isfile(local_filename.text):
        logging.info("Skipping non-file path %s", local_filename)
        yield None
        continue
    else:
        test_result = self._test_file(local_filename.text)
        yield test_result if test_result is not None else 0.0

Yielding None for skipped outputs keeps detector_results length-aligned
with attempt.outputs. The evaluator already handles None:

# garak/evaluators/base.py
if score is None:
    nones += 1          # excluded from pass/fail totals

Tests

Updated test_filedetector_nonexist (it previously asserted len(results) == 0
for skipped files — encoding the buggy behavior) and added 3 alignment tests to
tests/detectors/test_detectors_base.py:

  • Alignment regression: a missing file between two valid files yields
    None, so the valid files' scores stay aligned ([1.0, None, 0.0]).
  • Empty/None outputs: each yields None rather than being dropped.
  • All-valid sanity: when all outputs are valid files, scoring works.
$ python -m pytest tests/detectors/test_detectors_base.py -q
..............                                                           [100%]
14 passed in 0.05s

Impact

  • Affected detectors: all FileDetector subclasses — FileIsPickled,
    FileIsExecutable, PossiblePickleName in detectors/fileformats.py — and
    any third-party subclass.
  • Trigger: any attempt where at least one output is a missing file, empty,
    or non-file (e.g. a broken model-repo download, a non-file path).
  • Severity: high for integrity. A safety scanner whose hitlog attributes
    detector hits to the wrong prompts produces unreliable reports. Verified
    against the three built-in fileformat detectors.

Scope

Single-purpose: one fix in FileDetector.detect + tests. Diff: +94/−3 across
2 files. No changes to any probe, evaluator, or other detector class.

Notes

  • Verified the three real fileformat detectors (FileIsPickled,
    FileIsExecutable, PossiblePickleName) produce aligned results after the
    fix.
  • The evaluator's None handling predates this change — it was already designed
    to accept None scores (e.g. StringDetector yields None for None
    outputs). This fix makes FileDetector consistent with that contract.
  • No behavior change for the all-valid-files path (the common case).

…utputs)

FileDetector.detect() used `continue` to skip missing/empty/non-file outputs,
which made detector_results SHORTER than attempt.outputs. The evaluator then
indexed attempt.outputs by position (evaluators.base.Evaluator.evaluate:
`messages.append(attempt.outputs[idx])`), misattributing later detector scores
to the WRONG outputs and silently dropping some from scoring entirely. The
hitlog recorded incorrect output<->score pairings.

Concrete: with outputs [valid.pkl, /missing, valid.txt], FileDetector yielded
[1.0, 0.0] (dropping the missing file). The evaluator then attributed the 0.0
(valid.txt) to outputs[1] (/missing), and outputs[2] was never scored.

Fix: yield None for skipped outputs instead of `continue`, preserving
length alignment with attempt.outputs. The evaluator already counts None
scores as `nones` (excluded from pass/fail), so this is the correct signal.

The existing test_filedetector_nonexist asserted `len(results) == 0` for
skipped outputs -- this encoded the buggy behavior; updated to assert
length alignment + None for skipped.

Signed-off-by: John Kearney <johndanielkearney@gmail.com>
@leondz leondz self-assigned this Jul 6, 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.

2 participants