Repository navigation
Conversation
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>
Requirement PR ChecklistUse this checklist when adding or modifying requirements in Base Class
Validation Logic
Integration
|
ajbozarth
left a comment
There was a problem hiding this comment.
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.
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>
|
@ajbozarth Done. I dropped the |
ajbozarth
left a comment
There was a problem hiding this comment.
LGTM, though I'd recommend another reviewers eye before merge
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-runm.instruct()by hand. That isn't true. A plainRequirementwith avalidation_fnalready feeds a failed Guardian score intoRepairTemplateStrategy, we just never documented it. Meanwhile the oldGuardianCheckclass 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/, andGuardianCheckandGuardianRiskare gone. Code that importsmellea.stdlib.requirements.safety.guardianwill now fail with aModuleNotFoundError. This is a breaking change, so the commit is marked!with aBREAKING 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:Then pass it as
requirements=[harm_check(0.2)]withRepairTemplateStrategy(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 thanGuardianCheck(thinking=True)was, and the docs say so.What changed
mellea/stdlib/requirements/safety/(the package,GuardianCheck,GuardianRisk, and their unit test).docs/docs/how-to/safety-guardrails.md, including a short migration note fromGuardianRiskvalues toCRITERIA_BANKkeys.docs/examples/safety/repair_with_guardian.py(marked# pytest: huggingface, e2e) and a README beside it.docs/docs/observability/metrics.md. It said adapter functions can't emit requirement metrics, which is wrong once they are wrapped in aRequirement. Both counters are labelled with the class name, and onlymellea.requirement.failurescarries thereason, so the docs now say to keep reasons short and free of per-request values like scores.GuardianCheck/GuardianRiskreferences 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
Requirementsubclass backed byguardian_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 whenscore < threshold) is easy to resurrect, and I'm happy to have that argument on #1071.I used
Refsrather thanFixesfor #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
ibm-granite/granite-4.1-3band the Guardian adapter. With threshold0.05Guardian scored0.00026, so it passed on the first attempt. With threshold0.0(which can't be met, sincescore < 0.0is false for any probability) all three attempts failed, the reason was fed back between attempts, and the first attempt was selected. The example asserts thatvalidation_fnactually 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 istest_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 melleaclean, andruff,codespell,markdownlint-cli2and the SPDX hook pass on every changed file via pre-commit.The branch was reviewed three times by an independent model. The first pass caught that my first version of the example used the default
SimpleContextand never called Guardian while still exiting 0. That is why the example now setsctx=ChatContext()and asserts.Testing
Attribution
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.
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.