refactor(redteam): per-call judge lifecycle + per-risk-category judge rubric + minor strategy improvements

[yeomjiwonyeom]

What

Three related changes to the red-team module, landed together:

1. Agent lifecycle: judges built fresh per call, attacker fresh per case; strategies stateless across cases. The four multi-turn strategies (bad_likert_judge, goat, pair, crescendo) used to cache their judge/attacker Agents on self and rely on a reset() override between cases. They now hold only static config on self, build the attacker once per run_attack, and build judges fresh per scoring call. This removes the strategy-state half of the max_workers=1 constraint.

   Crescendo P5a (bundled with this commit): is_refusal(make_judge=None) now returns False — refusal markers alone never decide "refused"; a positive verdict requires a judge. The live path always passes a judge factory, so production behavior is unchanged; only the exported-helper path (PAIR/TAP reuse) is affected.

2. Judge prompt v1: per-risk-category success criteria (new default). AttackSuccessEvaluator's v0 rubric defined success generically ("harmful content / policy violation"), which systematically mis-scored some categories — most visibly reading a verbatim system-prompt leak wrapped in "I can't share this" as only0-partial. v1 adds a "what counts as success, by risk category" block for the five risk categories. Same four score anchors, same severity labels, same schema — v0 is preserved unchanged.

3. Bad Likert Judge: per-category score-3 anchor. The top-score example anchor was a single generic line; it now describes, per risk category, the shape of a complete top-score example (no payloads — see Safety).

"Always build a new agent" — how it landed

This addresses the request to stop reusing cached agents. It splits by agent role:

• Judge / refusal judge → fresh per call. A judge that is cleared (messages.clear()) and reused still carries other accreting state (conversation-manager counters, agent state) that a fresh agent does not. Building fresh per call makes cross-call carryover structurally impossible rather than relying on a partial scrub, and matches the existing AttackSuccessEvaluator.evaluate() idiom (which already builds a fresh Agent per call).
• Attacker → fresh per case, not per call. The attacker's multi-turn history is the strategy (Crescendo escalation, PAIR refinement, GOAT's O/T/S/R chain) — rebuilding it every turn would erase the accumulated context the attack depends on. So it is built once per run_attack and never cached across cases.

Net: no agent state survives across cases, and the reset()-between-cases dependency is gone — without breaking within-case attacker continuity.

Why

• (1) See above; also drops the reset()-between-cases dependency.
• (2) Reading the risk category is the principled judge behavior — the judge should know what the targeted violation is before scoring it. The category text describes the threat type generally (keyed off the risk_category taxonomy, not any specific target), so it applies to any case of a category.
• (3) Sharpens what the role-played "judge" should elicit, per category.

Behavior / compatibility

• v0 is byte-for-byte unchanged (anchors + system prompt verbatim; pinned by test). Select it with version="v0".
• The shared judge-prompt builder now emits a # Risk Category line from case metadata (already present on RedTeamCase); v0's rubric ignores it, so v0 scoring is unchanged.
• reset() is now a base no-op that strategies don't override; the task runner still calls it harmlessly (kept as a hook for any future strategy that needs per-case state). No base files changed.

Safety

The Bad Likert Judge anchors are shape descriptors of a top-score example (e.g. "reproduces the configuration verbatim, not a paraphrase") — they contain no harmful payloads or instructions. The target still generates any content; nothing harmful is committed to the repo.

Validation

• Full red-team test suite green; ruff + mypy clean.
• New tests: cross-case isolation without reset() (per strategy), per-category score-3 anchor rendering + fallback, risk-category prompt injection, v1 anchors verbatim-match v0.
• A quantitative v0-vs-v1 judge comparison is a held-out evaluation (new targets/goals or human-labeled ground truth) owned by the Science track; this PR supplies the rubric and the lifecycle refactor.

Note

SequentialBreak gets the same lifecycle transform once it merges (not on this base).

[github-actions]

Assessment: Comment (approve-leaning)

Clean, well-tested refactor. Locally verified: 232 redteam tests pass, ruff + mypy clean. The lifecycle change (fresh judge per call via partial factories, attacker once per case, strategies stateless) is consistent across all four strategies and the cross-case freshness tests patch the real Agent ctor and count builds — they prove the actual property, not just mock wiring. v1 is added as a new prompt module with v0 preserved and verbatim-pinned, matching repo conventions.

Review notes

• Testing: One gap — the DEFAULT_VERSION flip (v0 → v1) is the user-facing behavior change but nothing pins it; a silent revert to v0 would pass the whole suite. Inline suggestion added.
• Error handling: get_template() raises a bare KeyError on an unknown version (pre-existing); a ValueError listing valid versions would be friendlier. Inline, non-blocking.
• Scope: Three changes bundled (lifecycle + rubric v1 + BLJ anchor); PR body notes this was OK'd and the diff stays reviewable. Noted only.
• Conventions: New _v1.py module (not overwriting _v0.py), built-in generics, structured logging, prompts-as-constants — all consistent with AGENTS.md.

Note: I could not apply the API bar-raising checklist — the linked doc returns 404 (docs repo archived) — but this lives under experimental/, so the public-API bar is lower regardless.

Nice work on the freshness tests — building real agents and asserting the build count is exactly the right way to lock this lifecycle in.

[github-actions]

Re-review of latest revision (221891e) — re-verified locally: 233 red-team tests pass, ruff + mypy clean.

✅ My one substantive finding from the prior pass (the unpinned DEFAULT_VERSION flip) is resolved — test_default_version_is_v1 now locks the default selection and the per-category system prompt. Good guard.

One small carry-over suggestion (non-blocking; my earlier inline on this got dropped by GitHub because the target line wasn't in the diff hunk):

Issue: get_template() does VERSIONS[version], so an unknown/typo'd version (e.g. version="v2") surfaces as a bare KeyError: 'v2' at AttackSuccessEvaluator construction. Since version is a public ctor arg, the failure mode is cryptic.

Suggestion: raise ValueError(f"unknown version {version!r}; expected one of {sorted(VERSIONS)}"). Pre-existing and out of this PR's core scope — fine to defer, just flagging while the file is open.

Otherwise this is good to go from my side — clean lifecycle refactor with strong cross-case freshness tests, and v0 preserved verbatim for a clean A/B.

[github-actions]

Re-review of latest revision (597c0ca — "collapse judge rubric to single version").

This revision responds to @poshinchen's "why keep v0?" — the author folded the per-category rubric into v0, deleted attack_success_v1.py, and reset DEFAULT_VERSION = "v0". Re-verified locally: 232 tests pass, ruff + mypy clean (32 source files, down 1 as expected from the v1 deletion). No dangling v1/attack_success_v1 references remain.

Status of my prior findings:

• ✅ Unpinned default (my original finding) — still guarded. The pin test was renamed to test_default_version_carries_per_category_block and now asserts the per-category block is present in the default system prompt, which is the more durable property to lock.
• 🔸 get_template() bare KeyError — unchanged and still applies (VERSIONS[version] with no validation). Now genuinely low-impact since v0 is the only version, so a typo'd version= arg is the only path that hits it. Non-blocking; defer freely.

The collapse is the right call — it removes the attack_success-only version asymmetry and matches every other evaluator in the package. Approve-leaning, no blocking issues. Clean lifecycle refactor with strong cross-case freshness tests; nice work.

[github-actions]

Re-review of latest revision (a497c81 — "Merge upstream/main into redteam/agent-lifecycle").

This is an integration commit pulling upstream #251 (multi-agent session), #263 (RedTeamExperiment round-trips), and #264 (docs) onto the branch — no new red-team-authored logic to review. I focused on confirming the merge didn't disturb this PR's core changes during conflict resolution.

Verified on the merged HEAD:

• ✅ No conflict markers anywhere in src//tests/.
• ✅ v0 collapse intact — attack_success_v0.py is still the only template, no v1 references remain.
• ✅ Lifecycle refactor intact — no cached judge/attacker on self in any strategy, and the make_judge/partial(...) fresh-per-call factory pattern is present across all four strategies.
• ✅ Suite green — full red-team suite 262 passed (up from 232, from the upstream tests the merge pulled in), ruff clean, mypy clean (33 source files, up from 32 — experiment.py/report.py/generators/adversarial.py came in with the merge).

Both my prior findings remain as previously noted (default-pin: guarded; get_template() bare KeyError: non-blocking, defer freely). Still approve-leaning, no blocking issues — the merge integrated cleanly.

[poshinchen]

Should Crescendo.is_refusal be independent calls too?

[yeomjiwonyeom]

@poshinchen on is_refusal — it already is on this branch; that's one of the things this PR does. The link points at 56118d4 (main, the merge base), which still has the old shape: is_refusal(response, judge: Agent) reusing one judge via judge.messages.clear(), plus cached self._judge / self._refusal_judge. Refusal checks are independent calls, same as the success scorer. Let me know if you meant something beyond the per-call freshness.

[github-actions]

Re-review of latest revision (96a6075 — "unconditional partial judge factory in bad_likert_judge").

This revision responds to @poshinchen's BLJ inline (__init__.py:190: "this is the same as partial(self._build_judge, resolved_model)? You can drop the checks"). I focused on that change.

What it does:

• run_attack now builds make_judge = partial(self._build_judge, resolved_model) unconditionally (was a lambda gated on goal.success_criteria), and _success_score's make_judge is now required — the None clause is gone from both the signature and the guard.
• Cross-strategy parity restored: BLJ was the only strategy with a conditional make_judge. All four (_)success_score helpers now take a required make_judge: Callable[[], Agent].
• No-criteria safety preserved by call-ordering (not a None-check): if not success_criteria: return 0.0 precedes the make_judge() call, and partial is lazy, so a no-criteria case still never resolves a model or builds a judge. Still covered end-to-end by test_no_criteria_does_not_build_judge.
• The redundant _success_score(None, ...) == 0.0 unit test was correctly removed (None is no longer valid input).

Verified locally on 96a6075:

• ✅ BLJ tests: 39 passed; full red-team suite: 261 passed (−1 from the removed test, expected)
• ✅ ruff clean, mypy clean (33 source files)
• ✅ The lingering make_judge is None in crescendo.is_refusal is the separate P5a refusal-marker design (already reviewed), not the gate this comment was about — correctly untouched.

Status of my prior findings: default-pin still guarded; get_template() bare KeyError unchanged (non-blocking, defer freely). Still approve-leaning, no blocking issues — clean simplification that removes the last gate-wiring asymmetry.

[poshinchen]

LGTM