fix(interactive): do_probe drops a first-time probe name and can crash with UnboundLocalError#1965
Open
chuenchen309 wants to merge 1 commit into
Open
Conversation
Collaborator
|
DCO signoff must be in the commit message |
…h with UnboundLocalError Two bugs in GarakCommands.do_probe (garak/interactive.py): 1. The probe-name branch only covers "overwrite an existing probe" (`if args.probe and self._cmd.probe`) and "neither given" (`elif not args.probe and not self._cmd.probe`). The case of a probe name given for the FIRST TIME (self._cmd.probe was falsy, args.probe is truthy) matches neither branch, so self._cmd.probe is silently never set -- `probe myprobe` run as the very first `probe` command in a session does nothing. 2. If loading the generator raises ImportError or AttributeError, the except blocks only log/print and fall through -- `generator` was never assigned, so the next line, `harness.run(generator, ...)`, raises UnboundLocalError instead of the intended graceful message. Fix: add an explicit `elif args.probe: self._cmd.probe = args.probe` branch for the first-time-set case, and `return` after each except block so a failed generator load stops the command instead of falling through to code that needs a `generator` that was never created. Added tests/test_interactive.py (no prior test coverage existed for this module): calls GarakCommands.do_probe.__wrapped__ directly (bypassing the @cmd2.with_argparser decorator, so no full cmd2.Cmd registration or garak config is needed), with ThresholdEvaluator and ProbewiseHarness mocked out to isolate the control-flow bug from unrelated config-loading. TDD-verified red/green: reverting only interactive.py reproduces both symptoms exactly (probe stays None; UnboundLocalError on 'generator'); reapplying passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>
chuenchen309
force-pushed
the
fix/interactive-do-probe-argparser-bugs
branch
from
July 15, 2026 14:18
0d2be91 to
75f28a1
Compare
Contributor
Author
|
Thanks @jmartin-tech — added the DCO sign-off; the commit now carries |
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.
Two bugs in
GarakCommands.do_probe(garak/interactive.py):if args.probe and self._cmd.probe) and "neither given" (elif not args.probe and not self._cmd.probe). The case of a probe name given for the first time (self._cmd.probewas falsy,args.probeis truthy) matches neither branch, soself._cmd.probeis silently never set —probe myproberun as the very firstprobecommand in a session does nothing.ImportErrororAttributeError, theexceptblocks only log/print and fall through —generatorwas never assigned, so the next line,harness.run(generator, ...), raisesUnboundLocalErrorinstead of the intended graceful message.Fix
Add an explicit
elif args.probe: self._cmd.probe = args.probebranch for the first-time-set case, andreturnafter eachexceptblock so a failed generator load stops the command instead of falling through to code that needs ageneratorthat was never created.Verification
tests/test_interactive.py(no prior test coverage existed for this module): callsGarakCommands.do_probe.__wrapped__directly (bypassing the@cmd2.with_argparserdecorator, so no fullcmd2.Cmdregistration or garak config is needed), withThresholdEvaluatorandProbewiseHarnessmocked out to isolate the control-flow bug from unrelated config-loading concerns.interactive.pyreproduces both symptoms exactly (probe staysNone;UnboundLocalErrorongenerator); reapplying passes.black --checkclean on both changed files.do_probe, but only for an f-string→%slogging conversion and a trailingreturn None, not either of these two logic bugs — not a duplicate, though a trivial rebase may be needed if it merges first.garak -t <target_type> -n <model_name>— not run: this is a unit-level fix and I have no live target configured; verified via the tests above rather than ticking a box I didn't exercise.I used AI assistance (Claude) to help investigate and draft this fix, but I personally constructed the repro, reviewed the root cause, and reviewed the final diff before submitting.
Signed-off-by: Andrew Chen 48723787+chuenchen309@users.noreply.github.com