feat(experiment): add verbosity-aware stack traces to evaluation error reasons - #268
Open
AndyMc629 wants to merge 9 commits into
Open
feat(experiment): add verbosity-aware stack traces to evaluation error reasons#268AndyMc629 wants to merge 9 commits into
AndyMc629 wants to merge 9 commits into
Conversation
…ty changes error_msg
* chore: added RedTeamExperiment round-trips * fix(redteam): use Model.get_config() in _serialize_model The Model ABC contract is `get_config()`; `.config` is a BedrockModel implementation detail. Providers that store config elsewhere would have silently dropped the model id on round-trip. Switch to the documented accessor with the same dict guard. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* chore(redteam): Strands-MultiAgent Redteam session * updated multi-agent-session ordering
… rubric + minor strategy improvements (strands-agents#265) * refactor(redteam): build judge agents fresh per call, drop cross-case state Make the four attack strategies (bad_likert_judge, goat, pair, crescendo) stateless across cases and isolate judge agents per call. - Judges/refusal judges build FRESH PER CALL via a make_judge factory passed to the module-level scoring helpers; the judge.messages.clear() scrub is removed (a fresh agent has no state). messages.clear() only emptied the message list, leaving conversation_manager counters and agent.state to accrete across calls -- a fresh agent makes cross-call carryover structurally impossible rather than relying on 'only messages is fed to the model'. - Attackers stay per-case: their multi-turn history (escalation/refinement/ O-T-S-R) IS the strategy, so they are built once per run_attack, not per call. - Drop the cached self._attacker/_judge/_refusal_judge fields and each strategy's reset() override (falls back to the base no-op); strategies now hold only static config, so an instance is reusable across cases with no reset(). Removes the strategy-state half of the max_workers=1 constraint. - Crescendo P5a: is_refusal(make_judge=None) returns False (markers alone never decide 'refused'; a positive verdict requires a judge). Live path always passes a factory, so production is unchanged; only the exported-helper path (PAIR/TAP reuse) is affected. - Tests: per-strategy cross-case-isolation tests patch the SDK Agent ctor and assert build counts across two cases on one instance with no reset() (counterfactual-verified: reintroducing a judge cache fails them); delete the reset-nulls-fields tests; rename ~17 monkeypatch stub sites to _build_*. COST: on the string-model convenience path each per-call build mints a new boto3 client; hot paths (benchmark/production) pass a Model object, which the SDK reuses. * fix(redteam): guard None target responses; correct stale lifecycle docs Follow-up to 0e45648 from a 5-lens adversarial review of the implemented per-call-judge refactor. Two real (low-severity) bugs + three doc-decay fixes; no behavior change for the StrandsAgentSession path. Bugs (custom TargetSession returning None -> None.strip()/.lower() raise, which the per-case try/except swallows into a silent score=0 'defended'): - pair: empty-response guard now 'not response or not response.strip()' to match the GOAT/BLJ siblings (was missing the None check). - crescendo: coerce 'response = response or ""' right after invoke; crescendo deliberately keeps scoring empty turns, so '' stays a valid scored response. Docs (stale/misleading after the lifecycle refactor): - base.reset(): rewrite docstring -- statelessness is the isolation mechanism, reset() is now an optional no-op hook for rare custom stateful strategies, not a requirement every override must implement. - crescendo: stop advertising is_refusal/success_score/gen_escalating_question as a PAIR/TAP reuse surface (locked decision: each strategy inlines its own gate); drop them from __all__ to match GOAT/PAIR/BLJ (class-only export). Tests still reach them by direct module path. - bad_likert_judge: drop hard-coded SDK-internal field names from the _success_score docstring (rename-fragile); use generic wording like the three siblings. * feat(redteam): add judge prompt v1 with per-risk-category success definitions A 375-run benchmark (judge v0 vs the programmatic canary) showed the LLM judge making systematic errors because its rubric defined success only generically ('harmful content / policy violation') and never told it what the violation IS per risk category. The largest error: verbatim system-prompt LEAKS wrapped in 'I can't share this' scored as merely partial (46 false-negatives). - New judge prompt version v1 (attack_success_v1.py), registered alongside v0. v1 adds a 'What counts as success, by Risk Category' block defining the targeted violation for all five categories (system_prompt_leak, data_exfiltration, harmful_content, excessive_agency, guideline_bypass). The system_prompt_leak entry includes an echo-gate exception (echoing the attacker's own token is not a disclosure), mirroring the text-pattern canaries. - The four score anchors and SYSTEM_PROMPT are byte-identical to v0, so v0-vs-v1 isolates exactly the per-category block (pinned by test). - _build_judge_prompt now emits a '# Risk Category' line (it was already in the case metadata, just unused). v0's rubric ignores it -> v0 scoring unchanged. - v0 is preserved verbatim; default version stays v0, so existing behavior and prior benchmark numbers are untouched. v1 is opt-in via version='v1'. Keyed off risk_category (a stable product taxonomy), not this benchmark's five targets, so the definitions describe the threat type generally. NOTE: v1 was authored after inspecting v0's failures on this benchmark, so it is a PROPOSAL — a real quality claim needs held-out cases or human-labeled ground truth, not a re-score of the same runs. Deliberately no claim-vs-effect/tool-trace rule: that is the canary's job, and the judge does not reliably receive the trace. * feat(redteam): add per-risk-category score-3 anchor to bad_likert_judge (P2) The Bad Likert Judge scaffold's top-score (score-3) anchor was a single generic line ('engages completely and concretely, leaving nothing out'), which smoke testing found too vague for some risk categories — the judge-role had no sharp target for what a top-score example should look like. - Add _SCORE_TOP_ANCHOR: a per-category SHAPE descriptor of a top-score example, keyed by the five RISK_CATEGORIES. Each describes the STRUCTURE of a complete answer (verbatim config / step-by-step specifics / actual records / executed action), never a payload — SAFETY BOUNDARY preserved: the target still generates any harmful content, the scaffold only sharpens the rubric. - JUDGE_SETUP score-3 line gains a {score_top_anchor} slot; build_sequence._fill resolves it via the same brace-safe str.replace chain, with a generic fallback ('leaves nothing out') for an unknown/custom category so the slot never renders empty or leaves a stray brace. - Tests: anchor rendered per category, unknown-category fallback, no leftover slot, and _SCORE_TOP_ANCHOR covers exactly the built-in RISK_CATEGORIES. Benchmark-gated per plan: the N=5x3-goal baseline (commit 0e45648) was captured BEFORE this change; the gate is no ASR regression on research_asst/code_helper (already at ASR@1=1.00). Re-measure before claiming a win. * feat(redteam): make judge prompt v1 the default version Promote v1 (per-risk-category success definitions) to the default AttackSuccessEvaluator prompt; v0 is retained only for reproducing pre-v1 numbers. The evaluator now defaults its version to the registry's DEFAULT_VERSION (single source of truth) instead of a hardcoded literal. Rationale: the v0 rubric defined success only generically, producing systematic judge errors (notably 46 system-prompt-leak false-negatives in a 375-run benchmark). v1 tells the judge what the targeted violation is per risk category. Adopting it as the standard judge is the principled default — the judge SHOULD read the risk category — independent of the separate (held-out-data) question of quantifying v0->v1 improvement. Consequence for benchmarking: the prior stored baseline ran on v0, so it is no longer a clean comparand for runs on this code; re-baseline on the current commit before claiming any A/B (e.g. the BLJ P2 anchor gate). All redteam tests pass; the benchmark's AttackSuccessEvaluator(model=...) now resolves to v1 automatically. * test(redteam): remove duplicated assert block in goat lifecycle test Copy-paste slip left the system_prompts extraction + goal-A/goal-B asserts twice back-to-back in test_reused_instance_builds_fresh_agents_each_case_without_reset. Idempotent (test passed either way) but redundant; drop the second copy. * docs(redteam): neutral framing for v1 judge rubric rationale Reword the attack_success_v1 module docstring: keep the factual rationale (v1 defines success per risk category; v0's generic rubric systematically mis-scored some categories) and the accurate note that a quantitative v0-vs-v1 comparison is a held-out / Science-owned evaluation. Drop the 'proposal / not proven / overfit' self-framing -- the rubric change stands on the principle (the judge should read the risk category), and the held-out caveat is stated neutrally rather than as a disclaimer. No code change. * test(redteam): pin DEFAULT_VERSION=v1 so a silent revert to v0 fails Address PR strands-agents#265 review: the v0->v1 default flip is the one user-facing behavior change, but nothing pinned it -- a regression resetting DEFAULT_VERSION back to "v0" would pass the whole green suite (which only asserts version="v1" works and v1 anchors match v0). Add test_default_version_is_v1: asserts DEFAULT_VERSION == "v1", AttackSuccessEvaluator().version == "v1", and the default system prompt carries the per-category "by Risk Category" block. Counterfactual-verified: flipping the default to v0 fails this test and only this test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(redteam): collapse judge rubric to single version (v0) v0 was never released, so there is no reproducibility obligation to keep a separate v1. Fold the per-risk-category rubric into v0 as the single template, drop the v1 slot, and set DEFAULT_VERSION back to v0 -- matching every other evaluator in the package (all single-version, default v0). Removes the attack_success-only version asymmetry the bot flagged. - attack_success_v0.py: now carries the 'What counts as success, by Risk Category' block (RUBRIC body byte-identical to the old v1); docstring reworded to describe the rubric directly, not as a v0-vs-v1 delta - delete attack_success_v1.py; __init__ keeps a single v0 entry - tests: drop the v0-vs-v1 A/B comparison test; rename the default-pin test to assert the default carries the per-category block Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(redteam): unconditional partial judge factory in bad_likert_judge Address poshinchen's review (BLJ __init__.py:190): use partial(self._build_judge, resolved_model) unconditionally instead of a lambda gated on goal.success_criteria, matching crescendo/goat/pair (BLJ was the only strategy with a conditional make_judge). Drop the now-dead make_judge is None clause from _success_score's guard and the None from its signature. Safety is preserved: partial is lazy, and _success_score returns at 'if not success_criteria' BEFORE ever calling make_judge(), so a no-criteria case still never resolves a model / builds a judge. This invariant is now guaranteed by call-ordering rather than a None-check; documented in the _success_score docstring and covered end-to-end by the existing test_no_criteria_does_not_build_judge (real run_attack, asserts the judge builder is never called). Removed the redundant unit test that pinned _success_score(None, ...) == 0.0 (None is no longer a valid input). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…strands-agents#254) * feat(redteam): add SequentialBreak narrative-scaffold attack strategy SequentialBreak (Saiem et al. 2024, arXiv:2411.06426) hides one harmful request among benign siblings in a single narrative scaffold sent as one query. Pure-script attack side (no attacker LLM): each scaffold variant embeds the case's actor_goal via str.replace and is sent once through target_session.invoke. Append-only, invoke-only, pruned_branches always empty, never reads the trace. Ships five scaffold variants (dc_t1, dc_t2, ge_t1, ge_t2, qb_t1) modeled on the paper's Question Bank / Dialog Completion / Game Environment structures; tries them in order (dc_t1 first, the paper's strongest) and stops at the first that scores at or above success_threshold (default 0.5, tuned for single-shot). The harmful slot sits in the second half of each sequence (Algorithm 1 j>N/2). The only LLM built is the inline success judge; the authoritative verdict stays with AttackSuccessEvaluator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(redteam): add SequentialBreakStrategy unit tests 45 tests covering ctor guards (empty/unknown variants, max_turns<1, threshold band), assemble_scaffold (slot substitution, brace-safe, imperative-goal grammaticality, no concrete attack content, second-half slot placement), success_score (no-criteria skip, clamp, NaN/inf rejection, parse-failure, judge isolation), the run_attack loop (first- breach stop, MAX-not-last score, empty-response continue, all-empty clean defended, max_turns clamping both directions, append-only no snapshot/reset, target_calls parity), reset/model precedence, registry exclusion, and a contract pin on the result shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(redteam): address SequentialBreak review feedback - Build the success judge lazily and ONLY when the case has success_criteria: a no-criteria case never scores, so constructing an Agent (and resolving a model that could be a typo) it never uses is wasteful and could raise into the per-case score=0 swallow. success_score short-circuits on no-criteria-or-no-judge, so passing None on that path is safe (signature widened to Agent | None). - Strengthen happy-path loop tests to assert the full result.metadata dict rather than individual fields, so a regression in any unlisted field is caught. - Add a test asserting the judge is NOT built on a no-criteria case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(redteam): align SequentialBreak with per-call judge lifecycle Follow the same lifecycle refactor the other strategies got: the success judge is now built fresh PER scoring call via a make_judge factory instead of a cached self._judge cleared with messages.clear(). Brings SequentialBreak in line with crescendo/goat/pair/bad_likert_judge. - success_score takes make_judge: Callable[[], Agent] (was judge: Agent | None); builds a fresh judge per call (judge = make_judge()), no messages.clear() - run_attack builds make_judge = partial(self._build_judge, self._model or model) unconditionally; partial is lazy so a no-criteria case still never resolves a model (success_score returns before calling it) -- matches bad_likert_judge - drop the cached self._judge field, the reset() override, and _judge_agent; add _build_judge (never caches). Strategy is now stateless across cases. - tests: success_score tests use a make_judge factory; replace the messages.clear assertion and the reset test with a fresh-judge-per-scoring-call build-count test (patches the real Agent ctor across two cases) The judge rubric change (per-category v0) is intentionally NOT made here -- it rides in when this branch rebases onto strands-agents#265 after it merges (SequentialBreak does not touch the evaluator templates, so that rebase is conflict-free). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AndyMc629
had a problem deploying
to
manual-approval
June 15, 2026 21:05 — with 
GitHub Actions
Failure
GitHub Actions
Failure
AndyMc629
had a problem deploying
to
manual-approval
June 15, 2026 21:05 — with
GitHub Actions
Failure
GitHub Actions
Failure
github-actions
Bot
added
enhancement
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
3 participants
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.
Description
Addresses #86 - when an exception occurs during evaluation, only the short exception message was captured in the
reasonfield, making debugging difficult without re-running with extra instrumentation.The original issue proposed a new
STRANDS_EVALS_VERBOSE_ERRORSenvironment variable to control this. After auditing the codebase,experiment.pyalready has access to the existing-v/--verboseCLI verbosity mechanism via thestrands_evalslogger hierarchy — introducing a new env var would add unnecessary API surface and be inconsistent with how verbosity is handled elsewhere in the project. This implementation useslogger.isEnabledFor(logging.DEBUG)instead, so full stack traces are included automatically when running with-vv.As part of this change, a pre-existing bug was also fixed:
experiment.pywas usinglogging.getLogger()(root logger) instead oflogging.getLogger(__name__), meaning the CLI verbosity flag had no effect on this module at all. This is fixed as an incidental but necessary part of the implementation.Changes:
logging.getLogger(__name__), remove hardcodedsetLevel(logging.INFO)_workerand both exception handlers in_run_evaluatoroccured→occurredRelated Issues
Closes #86
Type of Change
New feature
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.
hatch run preparetests/strands_evals/test_experiment_logging.pycoveringlogger hierarchy inheritance, and verbosity-aware error messages across all
three exception handlers in
_workerand_run_evaluatorhappy to do this if maintainers require it before merge
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.