Fix: unreachable Conversation branch in Probe.probe() pre-translation notes - #2022
Conversation
…otes The third isinstance check repeated the Message condition, so a probe with Conversation prompts never recorded notes["pre_translation_prompt"]. Attempt.prompt_for() then returned the translated prompt even when asked for the probe language, which is what detectors.misleading and the other prompt_for(self.lang_spec) consumers read. The branch body also used turn.context, an attribute Turn has never had, so it would have raised AttributeError if it had ever been reachable. TreeSearchProbe._create_attempt already has the intended form of this branch. Match it, and add a regression test that runs Probe.probe() in translated mode with a Conversation prompt. Closes NVIDIA#2020 Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Aditya Singh <adisin650@gmail.com>
jmartin-tech
left a comment
There was a problem hiding this comment.
LGTM thanks @adityasingh2400, credit also goes to @feiiiiii5 for reporting the issue with details pointing to this solution.
|
Thanks for the review. Agreed on the credit, @feiiiiii5's report already had the unreachable branch pinned down, so the fix mostly followed from that. |
|
Thanks for picking this up so quickly, @adityasingh2400 — the fix matches my diagnosis exactly (dead |
|
Thanks @feiiiiii5, and the credit is yours. Your issue report had the diagnosis in it already, both the dead Agreed on the follow-up. I re-checked it against current
I have not opened the issue yet. Happy to write it up with the reproduction path if that is useful, or leave it to you since it is your find originally. Either way it is independent of this PR, which only touches the pre-translation notes branch. |
|
Thanks — I went ahead and opened the follow-up: #2026 with the empty-prompt-set repro and a suggested fix direction (return |
|
Please take it, it is your find and your issue write-up already has the fix direction in it. I am not opening PRs against garak right now, so it would only sit waiting on me. Re-checked #2026 against current Happy to review when you open it. |
|
Opened it: #2027 — early |
Probe.probe()recordsnotes["pre_translation_prompt"]so thatAttempt.prompt_for()can hand back the original-language prompt after forward translation. The thirdisinstancecheck repeated theMessagecondition instead of testing forConversation, so a probe withConversationprompts leftnotesasNoneandprompt_for()returned the translated prompt even when asked for the probe language. That is whatdetectors.misleadingand the otherprompt_for(self.lang_spec)consumers read, so with a langprovider configured they were scoring against the wrong text. The branch body also usedturn.context, an attributeTurnhas never had, so it would have raisedAttributeErrorif it had ever been reachable.TreeSearchProbe._create_attemptfurther down the same file already has the intended form of this branch, so this change bringsProbe.probe()back in line with it. The duplicate condition arrived in e2d3667. Thanks to @feiiiiii5 for the diagnosis in the issue.Closes #2020
Why this is not a duplicate
gh pr list --repo nvidia/garak --state open --search "2020 in:body"returns nothing related.pre_translation_promptreturns only closed PRs (improve prompt translation support for Conversation related types #1441, fix: encoding probes storing translated text inpre_translation_prompt#1483, probe: Base class for iterative probes and FITD as an example #1414, Configurable system prompt #1337, Feature: multilingual machine translation #943), none of which touch this branch.probes/base.py.Tests run
Python 3.12, macOS, CPU only.
Restoring
garak/probes/base.pytomainand rerunning the new test makes it fail onassert isinstance(notes_prompt, Conversation), so it is a real regression test rather than a restatement of current behaviour.Every failure above is pre-existing in my environment rather than caused by this change, and I checked each one by restoring
garak/probes/base.pytomainand confirming the same failure.tests/probes/test_probes.py: the 7 failures are alltest_probe_metadataon plugins whose optional dependencies I do not have locally, namelyprobes.audio.AudioAchillesHeel(wantssoundfileandlibrosa),probes.suffix.BEAST,probes.suffix.GCG, and the threeprobes.topic.Wordnet*entries. None of them reachProbe.probe().test_atkgen_probe_translation[probes.atkgen.Tox]: fails locally only becauseaccelerateis not installed.test_probe_prompt_translation[probes.sysprompt_extraction.SystemPromptExtraction]: fails withIndexError: list index out of rangeatgarak/probes/base.py:402, which is theisinstance(prompts[0], str)check at the top ofprobe(). Its prompt set comes back empty locally. This reproduces identically withgarak/probes/base.pyrestored tomain, so it is not related to this change, but flagging it in case it is news.black --check garak/probes/base.pyalso flags one pre-existing line further down the file, thelogging.debugcall in the intent-filteringprobe()override. That flag is present on an unmodified checkout ofmaintoo, so I left it alone rather than widen this PR.Reverting only the
garak/probes/base.pyhunk makes the new test fail withassert None is not Noneonnotes["pre_translation_prompt"], so it is a real regression test rather than a restatement of current behaviour.AI assistance
AI assistance was used for this change. I used Claude to trace the call path from
Probe.probe()throughAttempt.prompt_for()to the detector consumers, and to draft the fix and the regression test. I reviewed every changed line, confirmed againstTreeSearchProbe._create_attemptand against theTurndataclass thatcontentis the correct attribute, and ran the tests above locally.