FEAT add AgentThreatRulesScorer (ATR taxonomy scorer)

[eeee2345]

Adds AgentThreatRulesScorer, the scorer half of #1702 (the dataset loader landed in #1715).

What it does

• A deterministic TrueFalseScorer that evaluates text against the open Agent Threat Rules (ATR) ruleset via the pyatr engine.
• Returns True when at least one rule at or above a configurable min_severity matches; attaches matched rule ids, ATR category, and max severity as score metadata.
• Mirrors SubStringScorer's shape (true_false base, _score_piece_async, _build_identifier, min_severity validation).

Dependency

• pyatr (>=0.2.6, which bundles the ATR ruleset) is an optional dependency, imported lazily with a clear ImportError. The unit test uses pytest.importorskip("pyatr"); if you'd like CI to exercise it, pyatr can be added to the test extras — happy to wire that into whichever group you prefer.

Pairs with the _AgentThreatRulesDataset loader: the dataset supplies ATR-derived adversarial prompts, and this scorer detects whether a response trips an ATR rule.

[eeee2345]

Freshened this onto the latest main — it is up to date and mergeable now. Ready for review when convenient.

[adrian-gavrila]

Thanks for picking this up! Overall it looks good, but there are a couple of things that should be addressed, primarily around things actually being excercised in CI and following some style guidelines.

[adrian-gavrila]

top = hits[0] relies on pyatr returning matches pre-sorted critical-first (per the comment on line 108). That ordering is an undocumented pyatr internal, so if a future release changes or drops the sort, max_severity and the highest-severity value in the description silently go wrong while every test still passes.

We already own a severity ranking in _SEVERITY_ORDER, so we can pick the top match ourselves instead of trusting the engine's order:

top = max(hits, key=lambda m: _SEVERITY_ORDER.get((m.severity or "").lower(), 0))

Separately, there's a casing inconsistency: the filter on line 109 lowercases severity before comparing, but line 122 stores top.severity raw. If pyatr ever emits mixed case, the stored max_severity won't match what the filter normalized.

[adrian-gavrila]

This importorskip is at module scope, so it gates the whole file, and pyatr isn't declared anywhere in pyproject.toml. CI never installs it, so all four tests are skipped on every run. That's also why the broken assertion in test_atr_scorer_passes_benign below was never caught.

Two things to fix it:

1. Wire pyatr>=0.2.6 into an optional extra and the all group so CI installs it and the live tests actually run.
2. Move test_atr_scorer_rejects_invalid_min_severity out from behind this module-level skip. It only asserts that a bad min_severity raises ValueError, which happens before the pyatr import in __init__, so it doesn't need the engine and shouldn't be gated.

[adrian-gavrila]

Nit, optional and low likelihood: _build_identifier omits rules_dir, so two scorers loading different custom rulesets share an eval_hash (and stored scorer_class_identifier). It only bites if you pass a custom rules_dir and use the scorer-eval metrics harness, so the bundled-ruleset path is never affected. Cheap to close though, and consistent with how SubStringScorer includes substring in its identifier:

Suggested change

	"min_severity": self._min_severity,
	"min_severity": self._min_severity,
	"rules_dir": self._rules_dir,

[adrian-gavrila]

This file currently fails ruff check under the repo config (pyrit/score/** isn't DOC/D-exempt), so CI will be red until these are addressed. I ran it locally; the failures are:

• UP045 (x5): Optional[...] should be X | None (lines 4, 42, 43, 45, 94, 119). This is the modern union syntax the codebase standardized on; SubStringScorer, which this file mirrors, already uses it.
• D213 (x3): multi-line docstring summary should start on the second line (lines 19, 47, 95), matching SubStringScorer's docstring style.
• DOC501 (x2): __init__ raises ValueError (bad min_severity) and ImportError (missing pyatr), but neither is in a Raises: section.
• DOC201: _score_piece_async describes its return in prose but has no Returns: section.

ruff check --fix auto-resolves the UP045 and D213; the Raises: and Returns: sections need to be added by hand.

[adrian-gavrila]

Two assertion issues here, both of which only surface once this file actually runs in CI (see the coverage comment at the top of the file):

• Line 34: assert scores[0].score_metadata is None can never pass. The Score model has a mode="before" validator that coerces None to {}, so on the benign path this is effectively assert {} is None. I confirmed it by installing pyatr locally and running the module: 1 failed, 3 passed. Assert the no-match contract you actually want, e.g. not scores[0].score_metadata or == {}.
• Line 23: assert scores[0].score_metadata is not None is vacuous for the same reason (metadata is always a dict), so it never tests anything. The next line (["matched_rule_ids"]) is the real check, so this one can go.

[eeee2345]

Thanks for the thorough review, Adrian — all addressed in the latest push:

• Severity selection no longer relies on pyatr's ordering: the scorer now sorts hits by severity explicitly before picking max_severity.
• Wired pyatr>=0.2.6 as an optional atr extra and into the all group so CI installs it and the live tests actually run (kept out of dev per the functional-deps convention).
• Moved test_atr_scorer_rejects_invalid_min_severity out from behind the module-level skip — it asserts the ValueError that's raised before the pyatr import, so it no longer needs the engine. The three engine tests are now gated individually with a skipif marker.
• Fixed the assertions: benign now checks == {} (the model validator coerces None), and dropped the vacuous is-not-None line.
• _build_identifier now includes rules_dir so custom rulesets don't share an eval_hash.
• ruff clean: Optional[...] -> X | None, plus the Raises/Returns/D213 docstring fixes.

Verified locally: ruff passes, and against pyatr 0.2.6 the injection string trips 5 rules (top severity critical) while the benign string trips none, so the severity-floor tests hold. Ready for another look when you have a moment.

[eeee2345]

Thanks for the thorough review — all points addressed (pushed in 6010cb0).

CI exercising the scorer (the root cause): wired pyatr>=0.2.6 into the optional extra and the all group, so CI installs it and the three live tests run instead of being skipped. Moved test_atr_scorer_rejects_invalid_min_severity out from behind the engine gate (it only asserts the ValueError raised before the pyatr import) and switched the remaining three to a per-test requires_pyatr skipif.

Test assertions: the benign path now asserts score_metadata == {} (the no-match contract) instead of is None; dropped the vacuous is not None in the injection test, leaving the ["matched_rule_ids"] check as the real one.

Severity robustness + casing: the filtered hits are now sorted by our own _SEVERITY_ORDER (critical-first) and we take the top, instead of trusting pyatr's internal ordering. max_severity (and the description) now store the lowercased value the filter/sort compares against, so it stays correct even if pyatr ever emits mixed case.

Style: ruff check passes locally now — Optional[...] → X | None, D213 summaries, and the missing Raises: / Returns: sections added. _build_identifier now includes rules_dir, matching how SubStringScorer includes substring.

The checks here are still gated (action_required) on the fork PR — would you mind approving/re-running the workflows so the now-installed pyatr tests run? Happy to tweak anything further.

[adrian-gavrila]

Thanks for addressing those! A couple of things lingering now that we have added this as an optional dependency.

[adrian-gavrila]

ruff format --check fails on this file, so the pre-commit CI job will go red (the ruff-format hook runs on changed files; ruff check passing masked it since it's a separate hook). The hand-wrapped 3 lines fit under 120 chars, so the formatter wants them collapsed to one line.

Good spot to also adopt the house idiom while fixing it: importlib.util.find_spec is the only such use in tests/. Other optional-dep tests define an is_X_installed() helper (e.g. is_opencv_installed() in test_video_scorer.py). A one-line version fixes both at once:

requires_pyatr = pytest.mark.skipif(not is_pyatr_installed(), reason="pyatr is not installed")

Unrelated housekeeping: pyatr was added to the pyproject extras but uv.lock wasn't regenerated -> uv lock + commit so the lockfile matches.

[adrian-gavrila]

This message and the class docstring (:27-28) both tell users pip install pyatr, but this PR adds an atr extra to pyproject, so the idiomatic install is pip install pyrit[atr] (matches pip install pyrit[opencv] etc. in the other optional-dep scorers). Worth updating both spots.

Minor: peers catch ModuleNotFoundError here rather than the broader ImportError. ImportError would also swallow an import failure from inside pyatr itself and misreport it as "not installed", so ModuleNotFoundError is a touch more precise.

[eeee2345]

Thanks Adrian. All addressed:

• ruff format: collapsed the skipif to one line and moved to an is_pyatr_installed() helper, matching is_opencv_installed() in test_video_scorer.py. ruff check and ruff format --check are both clean now.
• Install hint: the ImportError message and the class docstring both say pip install pyrit[atr] now.
• Narrowed the import guard to ModuleNotFoundError so a failure from inside pyatr isn't misreported as "not installed".
• uv.lock: regenerated with uv lock, pyatr 0.2.6 is in it.

With pyatr wired into the atr extra and the all group, the four tests run instead of skipping. I ran them locally against pyatr 0.2.6 and all four pass.

[adrian-gavrila]

Looks great! Thank you for following up on the comments

[eeee2345]

Pushed a one-line fix (a91bcb4) for the only failing check — the ty pre-commit hook on macOS. It flagged a bare dict annotation:

error[missing-type-argument]: Missing type arguments for generic class `dict`
  --> pyrit/score/true_false/agent_threat_rules_scorer.py:135

The local metadata is now annotated dict[str, str | int | float] | None, matching Score.score_metadata's own type (no new import). ruff, the unit tests, and coverage were already green — this was the sole blocker.

The new runs are sitting in action_required pending workflow approval. Could a maintainer kick off a re-run when convenient? Thanks!

[eeee2345]

Thanks for the thorough review and the approval, @adrian-gavrila — the CI-exercise and style points made the PR better. Is there anything left on your side before it can merge, or is it good to go?

@romanlutz, since you'd asked about the scorer over on #1702 — this is it, approved and ready whenever you'd like to bring it in.

[eeee2345]

@adrian-gavrila — heads up that I force-pushed this branch. It had drifted 102 commits behind main and was conflicting, so it could not merge despite your approval on 2026-06-16, and it has been sitting since.

Rebuilt on current main rather than merged. The conflicts were all in pyproject.toml, uv.lock and the score package exports, which churn frequently upstream — a merge commit would have made the diff harder to re-read than a clean rebuild.

The content you approved is unchanged. The diff against main is still 5 files, +245/-1, matching the original exactly:

• the scorer and its test were carried over untouched
• the export line was reapplied with a three-way merge, not rewritten
• pyproject.toml took the same five lines (the atr extra plus the entry under all)
• uv.lock was regenerated rather than merged, which is the only substantive difference — pyatr now resolves to 0.2.7 instead of 0.2.6

Tests pass locally (4 passed).

If the force-push invalidated your approval on GitHub's side, could you re-approve when you have a moment? Happy to walk through the rebuild if you would rather verify it independently first.