Skip to content

refactor(safety)!: remove deprecated GuardianCheck, document Guardian repair loops - #1804

Open
planetf1 wants to merge 2 commits into
generative-computing:mainfrom
planetf1:issue-1071
Open

planetf1 wants to merge 2 commits into
generative-computing:mainfrom
planetf1:issue-1071

Conversation

@planetf1

Copy link
Copy Markdown
Contributor

Pull Request

Thanks for sending a pull request! Please make sure you read the contributing guidelines.

Issue

Refs #1071, #1351. This is deliberately narrower than #1071, see "Scope" below.

Description

What you see today. If you want a Guardian safety check to take part in a repair loop, the how-to guide tells you it can't: call guardian_check() after generation and re-run m.instruct() by hand. That isn't true. A plain Requirement with a validation_fn already feeds a failed Guardian score into RepairTemplateStrategy, we just never documented it. Meanwhile the old GuardianCheck class is still importable, deprecated since v0.4, and on the Guardian 4.1 chat template it loses its criteria without raising anything (#1351).

What you get after this PR. The how-to has a short "Guardian checks inside a repair loop" section showing the pattern, there is a runnable example under docs/examples/safety/, and GuardianCheck and GuardianRisk are gone. Code that imports mellea.stdlib.requirements.safety.guardian will now fail with a ModuleNotFoundError. This is a breaking change, so the commit is marked ! with a BREAKING CHANGE: footer. Neither name was in any __all__, so only direct module imports are affected.

The replacement is about twenty lines. Wrap the call in a Requirement:

def harm_check(threshold: float) -> Requirement:
    def validate(ctx: Context) -> ValidationResult:
        if not isinstance(ctx, ChatContext):
            raise TypeError("Guardian checks need a ChatContext; create the session with ctx=ChatContext()")
        score = guardian.guardian_check(ctx, backend, criteria="harm")
        return ValidationResult(
            score < threshold,
            reason=f"Guardian flagged harm at or above threshold {threshold}",
        )

    return Requirement("The response must not be harmful", validation_fn=validate, check_only=True)

Then pass it as requirements=[harm_check(0.2)] with RepairTemplateStrategy(loop_budget=3). Guardian returns a score and no reasoning text, so the repair feedback names the criteria and threshold rather than explaining what went wrong. That is less targeted than GuardianCheck(thinking=True) was, and the docs say so.

What changed

  • Removed mellea/stdlib/requirements/safety/ (the package, GuardianCheck, GuardianRisk, and their unit test).
  • Rewrote the repair-loop section of docs/docs/how-to/safety-guardrails.md, including a short migration note from GuardianRisk values to CRITERIA_BANK keys.
  • Added docs/examples/safety/repair_with_guardian.py (marked # pytest: huggingface, e2e) and a README beside it.
  • Corrected docs/docs/observability/metrics.md. It said adapter functions can't emit requirement metrics, which is wrong once they are wrapped in a Requirement. Both counters are labelled with the class name, and only mellea.requirement.failures carries the reason, so the docs now say to keep reasons short and free of per-request values like scores.
  • Removed the stale GuardianCheck/GuardianRisk references in the glossary, the contributing-docs link table, the troubleshooting page, tutorial 04, the docs checker allowlist, and the examples indexes. Versioned docs are frozen snapshots and are untouched.

Scope

Issue #1071 asks for a new Requirement subclass backed by guardian_check(). I left that out on purpose. The closure above covers the use case today, and a Guardian-only class would be API we'd likely replace once the adapter and validation-result work settles (#1680, #1720 touch the same ground). A single score-and-threshold requirement over any adapter function looks like a better shape than one class per capability. I'd rather not freeze a name for it now. If you disagree, the drafted design (threshold required, no default; pass when score < threshold) is easy to resurrect, and I'm happy to have that argument on #1071.

I used Refs rather than Fixes for #1071 for that reason. Whether that issue closes, narrows, or stays open is a call for the maintainers.

policy_guardrails() returns a label, not a score, so it needs its own decision about what "Ambiguous" means and isn't covered here.

Verification

  • Ran the new example for real on Apple Silicon with ibm-granite/granite-4.1-3b and the Guardian adapter. With threshold 0.05 Guardian scored 0.00026, so it passed on the first attempt. With threshold 0.0 (which can't be met, since score < 0.0 is false for any probability) all three attempts failed, the reason was fed back between attempts, and the first attempt was selected. The example asserts that validation_fn actually ran, so a wrong context type fails loudly instead of passing quietly.
  • uv run pytest test/ -m "not qualitative": 5328 passed, 205 skipped, 1 failed. The failure is test_intrinsics_formatters.py::test_adapter_versions_unchanged, the pinned adapter SHA check. It fails the same way with this change stashed.
  • uv run pytest test/test_example_collection.py: 20 passed, so the new example is collected from its # pytest: marker.
  • uv run mypy mellea clean, and ruff, codespell, markdownlint-cli2 and the SPDX hook pass on every changed file via pre-commit.
  • The example needs a GPU or Apple Silicon, so CI will only collect it, not run it.
  • No new library code, so no new unit tests. The only code deleted is the class and its own test.

The branch was reviewed three times by an independent model. The first pass caught that my first version of the example used the default SimpleContext and never called Guardian while still exiting 0. That is why the example now sets ctx=ChatContext() and asserts.

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

  • AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

  • Component
  • Requirement
  • Sampling Strategy
  • Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

Removes the deprecated `GuardianCheck` requirement class and `GuardianRisk` enum along with their unit test, and clears stale references across the documentation and examples. Replaces the workaround paragraph in the safety how-to guide with a `Requirement` + `validation_fn` pattern that calls `guardian_check()` and feeds the score and threshold into `RepairTemplateStrategy`.

Adds `docs/examples/safety/repair_with_guardian.py` (#pytest: huggingface, e2e) demonstrating the two-stage pattern against a local backend: a realistic threshold that clears on attempt 1, and a zero-tolerance threshold that exercises the full repair loop.

BREAKING CHANGE: `mellea.stdlib.requirements.safety.guardian` (and its parent package `mellea.stdlib.requirements.safety`) has been removed. Use `guardian_check()` from `mellea.stdlib.components.intrinsic.guardian`, wrapped in a `Requirement` with `validation_fn` for repair loops.

Refs generative-computing#1071
Refs generative-computing#1351

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1
planetf1 requested a review from a team as a code owner November 10, 2026 14:03
@github-actions github-actions Bot added the enhancement New feature or request label Nov 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This comment is managed by a bot. Editing it is fine — checking off boxes, adding notes — but please leave the HTML comment marker on the first line alone, otherwise checklist updates will break.

Requirement PR Checklist

Use this checklist when adding or modifying requirements in mellea/stdlib/requirements/.

Base Class

  • Extends appropriate base class:
    • Requirement - standard requirement
    • ALoraRequirement - uses specialized Intrinsic/Adapter for generation-based validation

Validation Logic

  • validation_fn defined (if using Python-based validation)
    • re-usable functionality within the validation_fn should be separated out into mellea/stdlib/tools/
  • validate returns a ValidationResult with
    • a thunk and context if using a backend to generate
    • a specific reason and score when possible

Integration

  • Requirement exported in mellea/stdlib/requirements/__init__.py or, if you are adding a library of requirements, from your sub-module

@ajbozarth ajbozarth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some feedback from Claude

docs/metrics/coverage-current.json still lists mellea.stdlib.requirements.safety.guardian (around line 1584). #1337 removed its stale entries from this snapshot when it deleted classes, so this PR should probably drop the entry and update total_symbols / coverage_percentage too.

Comment thread docs/docs/how-to/safety-guardrails.md Resolved
Comment thread docs/docs/observability/metrics.md Resolved
Use loop_budget=2 in the greedy-decoding snippet, document subclassing
Requirement for a per-check metric label, and drop the removed
safety.guardian module from the coverage snapshot.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1

Copy link
Copy Markdown
Contributor Author

@ajbozarth Done. I dropped the safety.guardian entry and updated total_symbols to 2019 and coverage_percentage to 15.5, the same way #1337 did it.

@planetf1 planetf1 self-assigned this Nov 11, 2026

@ajbozarth ajbozarth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though I'd recommend another reviewers eye before merge

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants