probes: factor up local prompt-cap logic into probes.base - #2085
probes: factor up local prompt-cap logic into probes.base#2085VishnuR23 wants to merge 1 commit into
Conversation
|
This looks like it only tests three probes instead of all that support capping - why? |
jmartin-tech
left a comment
There was a problem hiding this comment.
The goal of the issue being targeted here was to consolidate and ensure all probes use a shared application of the pruning process as provided by a base class.
For the probes addressed here setting self.prompts then calling self._prune_data(self.soft_probe_prompt_cap) should be sufficient to get the same result, why add a new function that implements the pruning without awareness of triggers?
Note by calling self._prune_data() this also offers probe authors a single location to override the pruning process when they need to preform a revised pattern for pruning. For instance, this would allow for badchars._downsample_prompts() to simply be renamed _prune_data() with a minor signature adjustment and shift that outlier.
Another consolidation task that could be considered part of the reported issue is that follow_prompt_cap could become a probes.base.DEFAULT_PARAM evaluated in _prune_data() ensuring all probes have an ability to enable or disable pruning in a common way and shifting the burden from each probe that currently supports the option to the base class.
Probes capped their prompts in several different ways: some called _prune_data, some hand-rolled the same random.sample truncation, and latentinjection carried a verbatim copy of the base implementation. The copies had drifted, and probes whose copy compared against the cap without a None check raised TypeError when soft_probe_prompt_cap was unset. Make _prune_data the single place pruning happens. It now defaults cap to soft_probe_prompt_cap and evaluates follow_prompt_cap itself, so probes no longer guard the call, and _prune_cap gives overrides the same guard. follow_prompt_cap becomes a Probe DEFAULT_PARAM, so every probe carries the switch rather than each declaring it. encoding, sysprompt_extraction, dra, web_injection and latentinjection now assign their prompts and prune, rather than capping a local list. badchars._downsample_prompts becomes a _prune_data override, which is what its category-balanced pruning always was. encoding gains an override too, keeping _prompt_intents aligned the way IntentProbe keeps prompt_intents. divergence still uses the cap as a generation target rather than pruning after the fact, so it is left alone. Tests discover pruning probes from the plugin registry instead of naming a few: 77 probes across 16 modules are now checked for honouring the cap, building with no cap set, and keeping triggers aligned after pruning. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Vishnu Rajeev <19866703+VishnuR23@users.noreply.github.com>
46a86e8 to
f814875
Compare
|
Thanks both — reworked to follow the design you described. Force-pushed. @jmartin-tech, you were right that a second function was the wrong shape.
Sweeping for the rest turned up more than the three I first found.
@leondz — fair hit on the test coverage. Tests no longer name probes; they discover every probe that prunes by walking the plugin registry and each class's MRO. That is 77 probes across 16 modules, each checked for honouring the cap, building with no cap set, and keeping triggers aligned after pruning. That coverage also showed the null-cap crash was much wider than the two probes I originally reported — running the new tests against Verification:
|
Addresses #1546.
Why this is not already addressed
Probe._prune_datacovers probes that capself.promptsin place, and plenty use it. But probes that build a prompt list locally and then assign it still hand-roll the cap. There were four such sites:probes/encoding.py(EncodingMixin.__init__)probes/sysprompt_extraction.py(_generate_attempts)probes/dra.py(twice, with an identical comment)The copies had already drifted, and not harmlessly.
dra.pyandbadchars.pyguard withsoft_probe_prompt_cap is not None;encoding.pyandsysprompt_extraction.pycompared against it directly, so a config with no cap set crashed them:That is the case for factoring up: with the policy in four places, two of them were wrong.
What this changes
Adds
Probe._cap_prompt_list(prompts)next to_prune_data— same policy, but for the "returns a list" shape rather than the in-place one — and routes the four sites through it.Nonehandling now lives in one place, so both crashes go away by construction.badchars._downsample_promptsis deliberately left alone: it preserves category balance and its docstring already explains why it differs.Removing the last
randomuse fromencoding.pyorphaned itsimport random, so that goes too.Not duplicating existing work
#1546has no linked PR and no assignee. No open PR touches the cap logic in these files. I commented on the issue with this plan before writing code.AI assistance disclosure
Written with AI assistance (Claude). I reviewed every changed line, reproduced both crashes before fixing them, and confirmed the new tests fail without the fix.
Verification
drapasses either way — it already had the guard, which is the point.)python -m pytest tests/probes/test_probes.py tests/probes/test_probes_encoding.py tests/probes/test_probes_sysprompt_extraction.py tests/probes/test_probes_dra.py tests/probes/test_probes_badcharacters.py— 1466 passed, 1 skippedfollow_prompt_cap = Falsebadcharscategory-balanced downsampling is untouched and its tests still pass_cap_prompt_listdocstring states when to use it versus_prune_dataTwo unrelated failures show up locally in
test_probes.py(probes.audio.AudioAchillesHeel,probes.sata.MLM). They fail identically on unmodifiedmainin my environment — missingaudioextras and NLTK data — and are not touched by this change.Tested on macOS, Python 3.13.7.