From cadc9df5313cfdd5f61de0c24ec87e323c2952ee Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Fri, 21 Aug 2026 10:25:49 +0000 Subject: [PATCH 01/10] chore(adapters): split EmbeddedIntrinsicAdapter's io_contract/weights construction onto separate lines Prep step for Epic #929 Phase 2: #1516 and #1142 both need to change one of io_contract/weights on the same call and the same docstring paragraph. This splits each into its own local variable/bullet, separated by blank lines, so the two PRs no longer touch adjacent lines and can merge in either order. No behavior change. Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 9 ++++--- mellea/backends/adapters/adapter.py | 37 ++++++++++++++++++----------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 5351d12f60..64c4e3bc66 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -9,9 +9,12 @@ - :class:`IOContract` — ABC for prompt building and output parsing - :class:`WeightsBinding` — pluggable ABC for weights lifecycle management -Also provides :class:`LocalFileBinding`, two stub :class:`WeightsBinding` -subclasses (:class:`EmbeddedBinding`, :class:`ServerMediatedBinding`), and -:class:`AdapterSchemaMismatchError`. +Also provides: + +- :class:`LocalFileBinding` +- :class:`EmbeddedBinding` — stub :class:`WeightsBinding` subclass +- :class:`ServerMediatedBinding` — stub :class:`WeightsBinding` subclass +- :class:`AdapterSchemaMismatchError` Note: The existing :class:`~mellea.backends.adapters.adapter.Adapter` ABC in diff --git a/mellea/backends/adapters/adapter.py b/mellea/backends/adapters/adapter.py index 0e5bb2e3f5..69e2ddc6f7 100644 --- a/mellea/backends/adapters/adapter.py +++ b/mellea/backends/adapters/adapter.py @@ -915,12 +915,18 @@ class EmbeddedIntrinsicAdapter(_AdapterCore): config (dict): Parsed I/O transformation configuration. technology (str): `"lora"` or `"alora"`. - .. note:: - `identity`, `io_contract`, and `weights` are Phase 1 internal scaffolding - populated in `__init__` to satisfy the new :class:`~mellea.backends.adapters.Adapter` - protocol. They are not meaningful consumer-facing attributes; `io_contract` and - `weights` raise :exc:`NotImplementedError` and will be replaced in Phase 2 - (issues #1137, #1142). + Note: + `identity`, `io_contract`, and `weights` are internal scaffolding + populated in `__init__` to satisfy the `Adapter` protocol; they are + not meaningful consumer-facing attributes. + + - `identity`: always a real value. + + - `io_contract`: Phase 1 `_ShimIOContract` placeholder; raises + `NotImplementedError` until issue #1516 replaces it. + + - `weights`: Phase 1 `_ShimWeightsBinding` placeholder; raises + `NotImplementedError` until issue #1142 replaces it. """ def __setattr__(self, name: str, value: object) -> None: @@ -958,15 +964,18 @@ def __init__(self, intrinsic_name: str, config: dict, technology: str = "lora"): # Populate the new Adapter triple so isinstance(self, _AdapterCore) holds. # technology is validated above; cast to the Literal type mypy expects. + identity = Identity( + name=intrinsic_name, + adapter_type=cast(Literal["lora", "alora"], technology), + capability=intrinsic_name, + ) + + io_contract = _ShimIOContract() + + weights = _ShimWeightsBinding() + _AdapterCore.__init__( - self, - identity=Identity( - name=intrinsic_name, - adapter_type=cast(Literal["lora", "alora"], technology), - capability=intrinsic_name, - ), - io_contract=_ShimIOContract(), - weights=_ShimWeightsBinding(), + self, identity=identity, io_contract=io_contract, weights=weights ) @staticmethod From 747d17318d758a57f8071ced6838201ad6290387 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 18 Aug 2026 12:58:50 +0100 Subject: [PATCH 02/10] test(adapters): add canonical per-adapter-function output-contract registry Introduces mellea/backends/adapters/io_contracts.py: a capability-keyed registry of IOContract instances, keyed by the same catalog name passed to call_intrinsic() and resolve_adapter(). Moves _ListContract (from rag.py) next to the existing _DictContract in _core.py, adds the guardian-specific contracts, and adds _RequirementCheckContract to consolidate the score-range validation core.requirement_check() previously hand-rolled after each call. This is the single source of truth a later commit wires resolve_adapter() and the intrinsic helpers to consume, instead of each declaring its own IOContract instance that could silently drift from the other's. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 59 ++++ mellea/backends/adapters/io_contracts.py | 265 ++++++++++++++++++ .../test_adapters/test_io_contracts.py | 139 +++++++++ 3 files changed, 463 insertions(+) create mode 100644 mellea/backends/adapters/io_contracts.py create mode 100644 test/backends/test_adapters/test_io_contracts.py diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 64c4e3bc66..7e85ff764f 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -189,6 +189,65 @@ def parse(self, raw: str) -> dict[str, object]: return data +class _ListContract(IOContract): + """Validate list-of-dicts adapter output and wrap it under key `"items"`. + + Each item in the list is checked for the declared required keys. The + validated list is returned wrapped in `{"items": [...]}` so that + :func:`~mellea.stdlib.components.intrinsic._util.call_intrinsic` can always + return a plain `dict`. + + Args: + name: Adapter capability name; included in + :class:`~mellea.backends.adapters.AdapterSchemaMismatchError` messages. + required_item_keys: Keys that must be present in every item dict. + """ + + def __init__(self, name: str, required_item_keys: frozenset[str]) -> None: + self._name = name + self._required_item_keys = required_item_keys + + def build_prompt(self, **_kwargs: object) -> Component: + raise NotImplementedError( + "build_prompt is not used in Phase 1; implemented in Phase 2." + ) + + def parse(self, raw: str) -> dict[str, object]: + """Parse and validate a list-of-dicts adapter output. + + Args: + raw (str): Raw JSON string from the model. + + Returns: + dict[str, object]: `{"items": [list of validated dicts]}`. + An empty list parses to `{"items": []}`. + + Raises: + ValueError: When *raw* is not valid JSON, is not a JSON array, or + contains a non-object element. + AdapterSchemaMismatchError: When any item is missing a required key. + """ + data = json.loads(raw) + if not isinstance(data, list): + raise ValueError( + f"Adapter '{self._name}' output must be a JSON array, " + f"got {type(data).__name__}." + ) + for item in data: + if not isinstance(item, dict): + raise ValueError( + f"Adapter '{self._name}' output array must contain only JSON " + f"objects, got a {type(item).__name__} element." + ) + observed = frozenset(item.keys()) + missing = self._required_item_keys - observed + if missing: + raise AdapterSchemaMismatchError( + self._name, observed, self._required_item_keys + ) + return {"items": data} + + class WeightsBinding(abc.ABC): """Abstract lifecycle interface for adapter weights. diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py new file mode 100644 index 0000000000..326b3c6792 --- /dev/null +++ b/mellea/backends/adapters/io_contracts.py @@ -0,0 +1,265 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Canonical output contracts for adapter functions (Epic #929 Phase 2, issue #1516). + +Before this module existed, an adapter function's output contract travelled to +:func:`~mellea.stdlib.components.intrinsic._util.call_intrinsic` as a caller-supplied +argument, built from a module-level constant in `rag.py` or `guardian.py`, while the +adapter actually resolved by :meth:`~mellea.backends.adapters.AdapterMixin.resolve_adapter` +carried an unrelated placeholder contract. Nothing tied the two together, so passing the +wrong constant was a silent mismatch. + +:data:`_INTRINSIC_IO_CONTRACTS` is the single source of truth instead: it is keyed by the +adapter function's catalog name (:attr:`~mellea.backends.adapters.catalog.IntrinsicsCatalogEntry.name`, +e.g. `"guardian-core"` — the same string passed to `call_intrinsic` and +`resolve_adapter`), and both the shim adapters in `adapter.py` (via :func:`get_io_contract`) +and the high-level helpers in `mellea.stdlib.components.intrinsic` read from it. Declaring +a capability's contract anywhere else reintroduces the parallel-argument problem this +module exists to close. +""" + +import json +import math + +from ...core import Component +from ._core import AdapterSchemaMismatchError, IOContract, _DictContract, _ListContract + + +class _PolicyGuardrailsContract(IOContract): + """Validate policy-guardrails adapter output: exactly one of `label` or `score`. + + The adapter returns either `{"label": "Yes"|"No"|"Ambiguous"}` or + `{"score": "Yes"|"No"|"Ambiguous"}` — never both, never neither. + """ + + def build_prompt(self, **_kwargs: object) -> Component: + raise NotImplementedError( + "build_prompt is not used in Phase 1; implemented in Phase 2." + ) + + def parse(self, raw: str) -> dict[str, object]: + """Parse and validate policy-guardrails output. + + Args: + raw (str): Raw JSON string from the model. + + Returns: + dict[str, object]: Parsed output dict with exactly one of `label` or `score`. + + Raises: + ValueError: When *raw* is not valid JSON or is not a JSON object. + AdapterSchemaMismatchError: When neither or both of `label` / `score` are present. + """ + data = json.loads(raw) + if not isinstance(data, dict): + raise ValueError( + f"Adapter 'policy-guardrails' output must be a JSON object, " + f"got {type(data).__name__}." + ) + has_label = "label" in data + has_score = "score" in data + if not has_label and not has_score: + raise AdapterSchemaMismatchError( + "policy-guardrails", + frozenset(data.keys()), + frozenset({"label", "score"}), + ) + if has_label and has_score: + raise AdapterSchemaMismatchError( + "policy-guardrails", + frozenset(data.keys()), + frozenset({"label", "score"}), + ) + return data + + +class _GuardianCheckContract(IOContract): + """Validate guardian-core adapter output: `{"guardian": {"score": }}`. + + Checks that the outer `guardian` key is present and that it contains + a nested `score` key. + """ + + def build_prompt(self, **_kwargs: object) -> Component: + raise NotImplementedError( + "build_prompt is not used in Phase 1; implemented in Phase 2." + ) + + def parse(self, raw: str) -> dict[str, object]: + """Parse and validate guardian-core output. + + Args: + raw (str): Raw JSON string from the model. + + Returns: + dict[str, object]: Parsed output dict containing `{"guardian": {"score": ...}}`. + + Raises: + ValueError: When *raw* is not valid JSON or is not a JSON object. + AdapterSchemaMismatchError: When `guardian` key is absent or `guardian.score` + is absent. + """ + data = json.loads(raw) + if not isinstance(data, dict): + raise ValueError( + f"Adapter 'guardian-core' output must be a JSON object, " + f"got {type(data).__name__}." + ) + if "guardian" not in data: + raise AdapterSchemaMismatchError( + "guardian-core", frozenset(data.keys()), frozenset({"guardian"}) + ) + guardian_val = data["guardian"] + if not isinstance(guardian_val, dict) or "score" not in guardian_val: + raise AdapterSchemaMismatchError( + "guardian-core", + frozenset(guardian_val.keys()) + if isinstance(guardian_val, dict) + else frozenset(data.keys()), + frozenset({"score"}), + ) + return data + + +class _RequirementCheckContract(IOContract): + """Validate requirement-check output: `{"requirement_check": {"score": <0.0-1.0 float>}}`. + + Consolidates the score-range validation `requirement_check()` (in + `mellea.stdlib.components.intrinsic.core`) previously hand-rolled after each call — + mirroring `requirement_check_to_bool()` in `requirement.py` — into a declared contract, + per issue #1516. + """ + + def build_prompt(self, **_kwargs: object) -> Component: + raise NotImplementedError( + "build_prompt is not used in Phase 1; implemented in Phase 2." + ) + + def parse(self, raw: str) -> dict[str, object]: + """Parse and validate requirement-check output. + + Args: + raw (str): Raw JSON string from the model. + + Returns: + dict[str, object]: Parsed output dict containing + `{"requirement_check": {"score": }}`. + + Raises: + ValueError: When *raw* is not valid JSON or is not a JSON object. + AdapterSchemaMismatchError: When `requirement_check` is absent or not a + dict, or when its `score` is absent, not a finite number, is a `bool`, + or falls outside the closed range `[0.0, 1.0]`. + """ + data = json.loads(raw) + if not isinstance(data, dict): + raise ValueError( + f"Adapter 'requirement-check' output must be a JSON object, " + f"got {type(data).__name__}." + ) + req_check = data.get("requirement_check") + if not isinstance(req_check, dict): + raise AdapterSchemaMismatchError( + "requirement-check", + frozenset(data.keys()), + frozenset({"requirement_check"}), + ) + score = req_check.get("score") + if ( + not isinstance(score, (int, float)) + or isinstance(score, bool) # bool subclasses int; exclude it explicitly + or not math.isfinite(score) + or not 0.0 <= score <= 1.0 + ): + raise AdapterSchemaMismatchError( + "requirement-check", frozenset(req_check.keys()), frozenset({"score"}) + ) + return data + + +_INTRINSIC_IO_CONTRACTS: dict[str, IOContract] = { + "answerability": _DictContract("answerability", frozenset({"answerability"})), + "query_rewrite": _DictContract("query_rewrite", frozenset({"rewritten_question"})), + "query_clarification": _DictContract( + "query_clarification", frozenset({"clarification"}) + ), + "citations": _ListContract( + "citations", + frozenset( + { + "response_begin", + "response_end", + "response_text", + "citation_doc_id", + "citation_begin", + "citation_end", + "citation_text", + } + ), + ), + "context_relevance": _DictContract( + "context_relevance", frozenset({"context_relevance"}) + ), + "hallucination_detection": _ListContract( + "hallucination_detection", + frozenset( + { + "response_begin", + "response_end", + "response_text", + "faithfulness", + "explanation", + } + ), + ), + "policy-guardrails": _PolicyGuardrailsContract(), + "guardian-core": _GuardianCheckContract(), + "factuality-detection": _DictContract("factuality-detection", frozenset({"score"})), + "factuality-correction": _DictContract( + "factuality-correction", frozenset({"correction"}) + ), + "uncertainty": _DictContract("uncertainty", frozenset({"certainty"})), + "requirement-check": _RequirementCheckContract(), + "context-attribution": _ListContract( + "context-attribution", + frozenset( + { + "response_begin", + "response_end", + "response_text", + "attribution_doc_id", + "attribution_msg_index", + "attribution_begin", + "attribution_end", + "attribution_text", + } + ), + ), +} +"""Canonical output contract for every catalogued adapter function, keyed by its +catalog `name` (see module docstring). Kept exhaustive over +:func:`~mellea.backends.adapters.catalog.known_intrinsic_names` by +`test/backends/test_adapters/test_io_contracts.py`.""" + + +def get_io_contract(name: str) -> IOContract: + """Return the canonical output contract for an adapter function. + + Args: + name (str): Catalog name of the adapter function (e.g. `"answerability"`), + i.e. the same string passed to + :func:`~mellea.stdlib.components.intrinsic._util.call_intrinsic` and + :meth:`~mellea.backends.adapters.AdapterMixin.resolve_adapter`. + + Returns: + IOContract: The contract declared in :data:`_INTRINSIC_IO_CONTRACTS` for + `name`. Adapter functions outside the catalog (e.g. one registered + through the deprecated `CustomIntrinsicAdapter`) fall back to a + permissive dict contract with no required keys, since Mellea has no + declared schema for them. + """ + contract = _INTRINSIC_IO_CONTRACTS.get(name) + if contract is not None: + return contract + return _DictContract(name, frozenset()) diff --git a/test/backends/test_adapters/test_io_contracts.py b/test/backends/test_adapters/test_io_contracts.py new file mode 100644 index 0000000000..0a0c4bd0fc --- /dev/null +++ b/test/backends/test_adapters/test_io_contracts.py @@ -0,0 +1,139 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the canonical adapter-function output-contract registry (issue #1516). + +Covers the registry's completeness/exhaustiveness invariant, the fallback contract +for names outside the catalog, and the `_RequirementCheckContract` parsing logic that +replaced `core.requirement_check`'s hand-rolled post-call validation. +""" + +import json +import math + +import pytest + +from mellea.backends.adapters import AdapterSchemaMismatchError +from mellea.backends.adapters.catalog import known_intrinsic_names +from mellea.backends.adapters.io_contracts import ( + _INTRINSIC_IO_CONTRACTS, + get_io_contract, +) + +# --------------------------------------------------------------------------- +# Registry completeness +# --------------------------------------------------------------------------- + + +def test_registry_covers_every_catalog_name(): + """Every catalogued adapter function must have a declared contract. + + Regression guard for the acceptance criterion that no adapter reachable + through `resolve_adapter` carries an unimplemented placeholder contract. + """ + missing = set(known_intrinsic_names()) - set(_INTRINSIC_IO_CONTRACTS) + assert missing == set() + + +def test_get_io_contract_returns_declared_instance_for_known_names(): + for name in known_intrinsic_names(): + assert get_io_contract(name) is _INTRINSIC_IO_CONTRACTS[name] + + +def test_get_io_contract_falls_back_permissively_for_unknown_names(): + """A name outside the catalog (e.g. a `CustomIntrinsicAdapter`) must not raise.""" + contract = get_io_contract("some-custom-user-adapter") + result = contract.parse(json.dumps({"anything": "goes"})) + assert result == {"anything": "goes"} + + +def test_get_io_contract_fallback_still_rejects_non_dict_output(): + contract = get_io_contract("some-custom-user-adapter") + with pytest.raises(ValueError, match="must be a JSON object"): + contract.parse(json.dumps(["not", "a", "dict"])) + + +# --------------------------------------------------------------------------- +# _RequirementCheckContract — replaces core.requirement_check's hand-rolled validation +# --------------------------------------------------------------------------- + + +@pytest.fixture +def contract(): + return _INTRINSIC_IO_CONTRACTS["requirement-check"] + + +def test_valid_score_returned(contract): + result = contract.parse(json.dumps({"requirement_check": {"score": 0.8}})) + assert result["requirement_check"]["score"] == pytest.approx(0.8) # type: ignore[index] + + +def test_missing_requirement_check_key_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"other_field": 0.9})) + + +def test_requirement_check_not_a_dict_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": None})) + + +def test_requirement_check_list_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": []})) + + +def test_missing_score_key_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"other_key": 0.9}})) + + +def test_null_score_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": None}})) + + +def test_string_score_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": "0.9"}})) + + +def test_bool_score_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": True}})) + + +def test_nan_score_raises(contract): + # json.dumps(nan) emits the non-standard `NaN` token, which json.loads accepts. + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": math.nan}})) + + +def test_inf_score_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": math.inf}})) + + +def test_score_above_range_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": 1.5}})) + + +def test_score_below_range_raises(contract): + with pytest.raises(AdapterSchemaMismatchError): + contract.parse(json.dumps({"requirement_check": {"score": -0.1}})) + + +def test_boundary_score_zero(contract): + result = contract.parse(json.dumps({"requirement_check": {"score": 0.0}})) + assert result["requirement_check"]["score"] == pytest.approx(0.0) # type: ignore[index] + + +def test_boundary_score_one(contract): + result = contract.parse(json.dumps({"requirement_check": {"score": 1.0}})) + assert result["requirement_check"]["score"] == pytest.approx(1.0) # type: ignore[index] + + +def test_requirement_check_rejects_non_dict_output(contract): + with pytest.raises(ValueError, match="must be a JSON object"): + contract.parse(json.dumps(["not", "a", "dict"])) From bc017a29373147f0ce773bf9e983d29f2cfa949c Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 18 Aug 2026 12:59:36 +0100 Subject: [PATCH 03/10] refactor(intrinsics): resolve the output contract from the adapter, not a parallel argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit call_intrinsic() resolved the adapter and discarded it, taking the output contract instead as a separate io_contract= argument that each caller supplied from its own module-level Adapter constant. Nothing tied the two together, so a caller could pass a contract that didn't match the adapter resolve_adapter() actually returned. call_intrinsic() now keeps the adapter resolve_adapter() returns and calls its own io_contract.parse() on the raw output; the io_contract= parameter is gone, so a mismatched pair is no longer expressible. IntrinsicAdapter and EmbeddedIntrinsicAdapter (the shims resolve_adapter() constructs) now look up their contract in the io_contracts registry instead of the _ShimIOContract placeholder, which is now unreachable and removed. The ten module-level Adapter constants in rag.py/guardian.py keep their identity and weights as before, and read io_contract from that same registry rather than declaring their own instance. core.py's three helpers (check_certainty, requirement_check, find_context_attributions) previously had no declared contract at all — the first two skipped validation (raw json.loads), and requirement_check hand-rolled its own score-range check after the call. All three now have a declared contract in the registry; requirement_check's hand-rolled validation is replaced by _RequirementCheckContract, and find_context_attributions reads its now-list-wrapped result via ["items"]. Weights binding is untouched — this stays entirely on the io_contract axis (the design discussion in #1486 that split the two). The ten constants' placeholder LocalFileBinding() and the backed-out Adapter.__post_init__ cross-check remain future work, as noted in _core.py. Verified against real granite-4.1-3b weights: test/stdlib/components/ intrinsic/test_core.py, test_rag.py, and test_guardian.py (qualitative, GPU-gated) all pass on this run — 3 passed + 2 pre-existing xfails (non-deterministic attribution count, tracked separately) for core.py, 14/14 for rag.py, 6/6 for guardian.py. No test constructs a resolve_adapter() result carrying the old placeholder contract; test_io_contracts.py's registry-completeness test guards that going forward. Fixes #1516 Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 7 +- mellea/backends/adapters/adapter.py | 40 ++--- mellea/stdlib/components/intrinsic/_util.py | 39 +++-- mellea/stdlib/components/intrinsic/core.py | 81 +++++++--- .../stdlib/components/intrinsic/guardian.py | 141 ++---------------- mellea/stdlib/components/intrinsic/rag.py | 123 ++------------- .../components/intrinsic/test_core_schema.py | 116 +++++++------- .../intrinsic/test_guardian_deprecation.py | 4 +- .../intrinsic/test_guardian_documents.py | 6 +- .../intrinsic/test_guardian_io_contract.py | 4 +- .../components/intrinsic/test_util_unit.py | 37 +++++ 11 files changed, 222 insertions(+), 376 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 7e85ff764f..83c853d2cb 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -720,5 +720,8 @@ class Adapter: # and `guardian.py` pair an `alora` identity with a bare, deliberately # unconfigured `LocalFileBinding()` that defaults to LoRA. Every catalogue # entry supports both types, so those are placeholders rather than genuine - # conflicts, and the check fired on "not configured yet". Enforce it once - # #1516 gives those constants real bindings. + # conflicts, and the check fired on "not configured yet". #1516 gave those + # constants real `io_contract`s (the capability axis) but deliberately left + # `weights` untouched — the alora-vs-lora binding question is the deployment + # axis, out of #1516's scope and still open. Enforce this check once a + # follow-up gives those constants real bindings. diff --git a/mellea/backends/adapters/adapter.py b/mellea/backends/adapters/adapter.py index 69e2ddc6f7..74e5d1bd04 100644 --- a/mellea/backends/adapters/adapter.py +++ b/mellea/backends/adapters/adapter.py @@ -34,11 +34,11 @@ Adapter as _AdapterCore, AdapterSchemaMismatchError, Identity, - IOContract, LocalFileBinding, WeightsBinding, ) from .catalog import AdapterType, fetch_intrinsic_metadata +from .io_contracts import get_io_contract class Adapter(abc.ABC): @@ -96,20 +96,6 @@ def get_local_hf_path(self, base_model_name: str) -> str: ... -class _ShimIOContract(IOContract): - """Phase 1 placeholder; Phase 2 (issue #1137) implements real I/O.""" - - def build_prompt(self, **kwargs: object): # type: ignore[override] - raise NotImplementedError( - "Phase 2 (issue #1137) — IOContract not yet implemented" - ) - - def parse(self, raw: str) -> dict[str, object]: - raise NotImplementedError( - "Phase 2 (issue #1137) — IOContract not yet implemented" - ) - - class _ShimWeightsBinding(WeightsBinding): """Phase 1 placeholder; Phase 2 (see epic #929) wires in real lifecycle.""" @@ -171,12 +157,13 @@ class IntrinsicAdapter(LocalHFAdapter, _AdapterCore): adapter_type (AdapterType): The adapter type (`LORA` or `ALORA`). config (dict): Parsed I/O transformation configuration for the adapter function. - .. note:: - `identity`, `io_contract`, and `weights` are Phase 1 internal scaffolding - populated in `__init__` to satisfy the new :class:`~mellea.backends.adapters.Adapter` - protocol. They are not meaningful consumer-facing attributes; `io_contract` and - `weights` raise :exc:`NotImplementedError` and will be replaced in Phase 2 - (issues #1137, #1141). + Note: + `identity`, `io_contract`, and `weights` are internal scaffolding populated + in `__init__` to satisfy the :class:`~mellea.backends.adapters.Adapter` + protocol; they are not meaningful consumer-facing attributes. `io_contract` + is the real, declared contract for `intrinsic_name` (issue #1516); `weights` + remains the Phase 1 `_ShimWeightsBinding` placeholder and raises + `NotImplementedError` until Phase 2 (issue #1141) replaces it. """ def __setattr__(self, name: str, value: object) -> None: @@ -256,6 +243,9 @@ def __init__( self.config: dict = config_dict # Populate the new Adapter triple so isinstance(self, _AdapterCore) holds. + # io_contract comes from the same registry resolve_adapter() consults + # (see issue #1516), not a placeholder. weights stays the Phase 2 + # _ShimWeightsBinding placeholder; that axis is #1141/#1142. _AdapterCore.__init__( self, identity=Identity( @@ -265,7 +255,7 @@ def __init__( else "lora", capability=intrinsic_name, ), - io_contract=_ShimIOContract(), + io_contract=get_io_contract(intrinsic_name), weights=_ShimWeightsBinding(), ) @@ -922,8 +912,8 @@ class EmbeddedIntrinsicAdapter(_AdapterCore): - `identity`: always a real value. - - `io_contract`: Phase 1 `_ShimIOContract` placeholder; raises - `NotImplementedError` until issue #1516 replaces it. + - `io_contract`: the real, declared contract for `intrinsic_name` + (issue #1516); no longer a placeholder. - `weights`: Phase 1 `_ShimWeightsBinding` placeholder; raises `NotImplementedError` until issue #1142 replaces it. @@ -970,7 +960,7 @@ def __init__(self, intrinsic_name: str, config: dict, technology: str = "lora"): capability=intrinsic_name, ) - io_contract = _ShimIOContract() + io_contract = get_io_contract(intrinsic_name) weights = _ShimWeightsBinding() diff --git a/mellea/stdlib/components/intrinsic/_util.py b/mellea/stdlib/components/intrinsic/_util.py index 3d939a5603..35054dba47 100644 --- a/mellea/stdlib/components/intrinsic/_util.py +++ b/mellea/stdlib/components/intrinsic/_util.py @@ -3,12 +3,11 @@ """Shared utilities for intrinsic convenience wrappers.""" -import json from typing import cast from ....backends import ModelOption from ....backends._options import resolve_model_options -from ....backends.adapters import AdapterMixin, AdapterType, IOContract +from ....backends.adapters import AdapterMixin, AdapterType from ....core import Backend from ....stdlib import functional as mfuncs from ...components import Document @@ -176,18 +175,16 @@ def call_intrinsic( /, kwargs: dict | None = None, model_options: dict | None = None, - io_contract: IOContract | None = None, ) -> dict[str, object]: - """Invoke an adapter function via the backend, returning parsed and optionally validated JSON output. + """Invoke an adapter function via the backend, returning parsed and validated JSON output. - Uses `AdapterMixin.resolve_adapter` to find or lazily register the adapter, - then executes via `mfuncs.act`. - - When *io_contract* is provided its :meth:`~mellea.backends.adapters.IOContract.parse` - method is called on the raw output string before returning. The contract validates - required fields and raises :class:`~mellea.backends.adapters.AdapterSchemaMismatchError` - on contract-breaking deltas; forward-compatible additions (extra optional fields) do - not raise. When *io_contract* is `None` the raw `json.loads` result is returned. + Uses `AdapterMixin.resolve_adapter` to find or lazily register the adapter, then + executes via `mfuncs.act`. The resolved adapter's own + `IOContract.parse` method is called on the raw output string before returning — the + output contract always travels with the adapter that produced it, rather than as a + separate argument a caller could mismatch. The contract validates required fields and + raises `AdapterSchemaMismatchError` on contract-breaking deltas; forward-compatible + additions (extra optional fields) do not raise. Args: intrinsic_name (str): Capability name of the adapter function @@ -199,23 +196,23 @@ def call_intrinsic( model_options (dict | None): Model options that override defaults. Adapter functions default to `TEMPERATURE: 0.0` for deterministic output; pass `TEMPERATURE` here to override it. - io_contract (IOContract | None): Output contract to validate and parse the - raw model output. When `None`, `json.loads` is used directly. Returns: - dict[str, object]: Parsed (and optionally validated) JSON output from the adapter function. + dict[str, object]: Parsed and validated JSON output from the adapter function. Raises: ValueError: When *context* forwards no history to the model (e.g. a `SimpleContext` was passed), or when the model output is `None` or is not valid JSON. - AdapterSchemaMismatchError: When *io_contract* is provided and the - model output is missing a required field. + AdapterSchemaMismatchError: When the model output is missing a field required + by the resolved adapter's output contract. """ _assert_context_forwards_history(intrinsic_name, context) - # Ensure the adapter is registered; resolve_adapter creates it if absent. - backend.resolve_adapter(intrinsic_name) + # Resolve (finding or lazily registering) the adapter now, rather than merely + # ensuring it is registered and discarding the result: its io_contract is what + # parses the raw output below. + adapter = backend.resolve_adapter(intrinsic_name) # Adapter activation is the backend's responsibility — the HF backend acquires # its generation lock and sets the active adapter inside _generate_with_adapter_lock, @@ -247,6 +244,4 @@ def call_intrinsic( result_str = model_output_thunk.value if result_str is None: raise ValueError("Model output is None.") - if io_contract is not None: - return io_contract.parse(result_str) - return json.loads(result_str) + return adapter.io_contract.parse(result_str) diff --git a/mellea/stdlib/components/intrinsic/core.py b/mellea/stdlib/components/intrinsic/core.py index e2feeddad8..274f39a898 100644 --- a/mellea/stdlib/components/intrinsic/core.py +++ b/mellea/stdlib/components/intrinsic/core.py @@ -4,15 +4,42 @@ """Adapter functions for core model capabilities.""" import collections.abc -import math from typing import cast -from ....backends.adapters import AdapterMixin, AdapterSchemaMismatchError +from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding +from ....backends.adapters.io_contracts import get_io_contract from ...components import Document, Message from ...context import ChatContext from ..docs.document import _coerce_to_documents from ._util import _resolve_response, call_intrinsic +# --------------------------------------------------------------------------- +# Module-level Adapter constants (one per helper) +# +# io_contract is read from the same registry `resolve_adapter()` uses +# (mellea.backends.adapters.io_contracts) rather than declared here — a second, +# independent declaration is exactly the parallel-argument problem issue #1516 +# closes. +# --------------------------------------------------------------------------- + +_UNCERTAINTY_ADAPTER = Adapter( + identity=Identity("uncertainty", "alora", capability="uncertainty"), + io_contract=get_io_contract("uncertainty"), + weights=LocalFileBinding(), +) + +_REQUIREMENT_CHECK_ADAPTER = Adapter( + identity=Identity("requirement-check", "alora", capability="requirement_check"), + io_contract=get_io_contract("requirement-check"), + weights=LocalFileBinding(), +) + +_CONTEXT_ATTRIBUTION_ADAPTER = Adapter( + identity=Identity("context-attribution", "alora", capability="context_attribution"), + io_contract=get_io_contract("context-attribution"), + weights=LocalFileBinding(), +) + def check_certainty( context: ChatContext, backend: AdapterMixin, model_options: dict | None = None @@ -23,6 +50,9 @@ def check_certainty( assistant's response to a user's question. The context should end with a user question followed by an assistant answer. + Output contract — required key: `certainty`. Missing the key raises + `AdapterSchemaMismatchError`; extra optional keys do not raise (forward-compatible). + Args: context: Chat context containing user question and assistant answer. backend: Backend instance that supports LoRA/aLoRA adapters. @@ -32,6 +62,11 @@ def check_certainty( Returns: Certainty score as a float (higher = more certain). + + Raises: + ValueError: When the model output is not valid JSON. + AdapterSchemaMismatchError: When the model output is missing the required + `certainty` field. """ result_json = call_intrinsic( "uncertainty", context, backend, model_options=model_options @@ -52,6 +87,10 @@ def requirement_check( `io.yaml` `instruction` template via `IntrinsicsRewriter`, which appends the formatted evaluation prompt as a new user message. + Output contract — required shape: `{"requirement_check": {"score": }}`, + with `score` a finite number (not a `bool`) in the closed range `[0.0, 1.0]`. + Any deviation raises `AdapterSchemaMismatchError`. + Args: context: Chat context containing user question and assistant answer. backend: Backend instance that supports LoRA/aLoRA adapters. @@ -64,6 +103,7 @@ def requirement_check( Score as a float between 0.0 and 1.0 (higher = more likely satisfied). Raises: + ValueError: When the model output is not valid JSON. AdapterSchemaMismatchError: If the adapter output does not match the expected `{"requirement_check": {"score": }}` contract, or if the score is not a finite number in the range 0.0-1.0. @@ -75,27 +115,9 @@ def requirement_check( kwargs={"requirement": requirement}, model_options=model_options, ) - # Mirrors the validation in requirement_check_to_bool() in requirement.py; Phase 2 will consolidate via IOContract. - req_check = result_json.get("requirement_check") - if not isinstance(req_check, dict): - raise AdapterSchemaMismatchError( - name="requirement-check", - observed_keys=frozenset(result_json.keys()), - expected_keys=frozenset({"requirement_check"}), - ) - score = req_check.get("score") - if ( - not isinstance(score, (int, float)) - or isinstance(score, bool) # bool subclasses int; exclude it explicitly - or not math.isfinite(score) - or not 0.0 <= score <= 1.0 - ): - raise AdapterSchemaMismatchError( - name="requirement-check", - observed_keys=frozenset(req_check.keys()), - expected_keys=frozenset({"score"}), - ) - return score + return cast( + float, cast(dict[str, object], result_json["requirement_check"])["score"] + ) def find_context_attributions( @@ -111,6 +133,12 @@ def find_context_attributions( documents that were most important to the LLM in generating each sentence in the assistant response. + Output contract — each record must contain: `response_begin`, `response_end`, + `response_text`, `attribution_doc_id`, `attribution_msg_index`, + `attribution_begin`, `attribution_end`, `attribution_text`. A record missing + any of these keys raises `AdapterSchemaMismatchError`; extra optional keys do + not raise (forward-compatible). + Args: response (str | None): Assistant response. When `None`, extracted from the last assistant output in `context`. @@ -133,6 +161,11 @@ def find_context_attributions( `attribution_begin`, `attribution_end`, and `attribution_text`. Begin and end offsets are character offsets into their respective UTF-8 strings. + + Raises: + ValueError: When the model output is not valid JSON. + AdapterSchemaMismatchError: When any record in the output is missing a + required field. """ response, context = _resolve_response(response, context) result_json = call_intrinsic( @@ -147,4 +180,4 @@ def find_context_attributions( backend, model_options=model_options, ) - return cast(list[dict], result_json) + return cast(list[dict], result_json["items"]) diff --git a/mellea/stdlib/components/intrinsic/guardian.py b/mellea/stdlib/components/intrinsic/guardian.py index 8730e94960..4707fa97a5 100644 --- a/mellea/stdlib/components/intrinsic/guardian.py +++ b/mellea/stdlib/components/intrinsic/guardian.py @@ -13,20 +13,11 @@ """ import collections.abc -import json import warnings from typing import cast -from ....backends.adapters import ( - Adapter, - AdapterMixin, - AdapterSchemaMismatchError, - Identity, - IOContract, - LocalFileBinding, -) -from ....backends.adapters._core import _DictContract -from ....core import Component +from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding +from ....backends.adapters.io_contracts import get_io_contract from ....core.utils import MelleaLogger from ...components import Document from ...context import ChatContext @@ -42,120 +33,24 @@ """Mapping used by the deprecated `target_role` path of `guardian_check`.""" -# --------------------------------------------------------------------------- -# IOContract implementations -# --------------------------------------------------------------------------- - - -class _PolicyGuardrailsContract(IOContract): - """Validate policy-guardrails adapter output: exactly one of `label` or `score`. - - The adapter returns either `{"label": "Yes"|"No"|"Ambiguous"}` or - `{"score": "Yes"|"No"|"Ambiguous"}` — never both, never neither. - """ - - def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." - ) - - def parse(self, raw: str) -> dict[str, object]: - """Parse and validate policy-guardrails output. - - Args: - raw (str): Raw JSON string from the model. - - Returns: - dict[str, object]: Parsed output dict with exactly one of `label` or `score`. - - Raises: - ValueError: When *raw* is not valid JSON or is not a JSON object. - AdapterSchemaMismatchError: When neither or both of `label` / `score` are present. - """ - data = json.loads(raw) - if not isinstance(data, dict): - raise ValueError( - f"Adapter 'policy-guardrails' output must be a JSON object, " - f"got {type(data).__name__}." - ) - has_label = "label" in data - has_score = "score" in data - if not has_label and not has_score: - raise AdapterSchemaMismatchError( - "policy-guardrails", - frozenset(data.keys()), - frozenset({"label", "score"}), - ) - if has_label and has_score: - raise AdapterSchemaMismatchError( - "policy-guardrails", - frozenset(data.keys()), - frozenset({"label", "score"}), - ) - return data - - -class _GuardianCheckContract(IOContract): - """Validate guardian-core adapter output: `{"guardian": {"score": }}`. - - Checks that the outer `guardian` key is present and that it contains - a nested `score` key. - """ - - def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." - ) - - def parse(self, raw: str) -> dict[str, object]: - """Parse and validate guardian-core output. - - Args: - raw (str): Raw JSON string from the model. - - Returns: - dict[str, object]: Parsed output dict containing `{"guardian": {"score": ...}}`. - - Raises: - ValueError: When *raw* is not valid JSON or is not a JSON object. - AdapterSchemaMismatchError: When `guardian` key is absent or `guardian.score` - is absent. - """ - data = json.loads(raw) - if not isinstance(data, dict): - raise ValueError( - f"Adapter 'guardian-core' output must be a JSON object, " - f"got {type(data).__name__}." - ) - if "guardian" not in data: - raise AdapterSchemaMismatchError( - "guardian-core", frozenset(data.keys()), frozenset({"guardian"}) - ) - guardian_val = data["guardian"] - if not isinstance(guardian_val, dict) or "score" not in guardian_val: - raise AdapterSchemaMismatchError( - "guardian-core", - frozenset(guardian_val.keys()) - if isinstance(guardian_val, dict) - else frozenset(data.keys()), - frozenset({"score"}), - ) - return data - - # --------------------------------------------------------------------------- # Module-level Adapter constants (one per helper) +# +# io_contract is read from the same registry `resolve_adapter()` uses +# (mellea.backends.adapters.io_contracts) rather than declared here — a second, +# independent declaration is exactly the parallel-argument problem issue #1516 +# closes. See test_guardian_io_contract.py for contract-enforcement tests. # --------------------------------------------------------------------------- _POLICY_GUARDRAILS_ADAPTER = Adapter( identity=Identity("policy-guardrails", "alora", capability="policy_guardrails"), - io_contract=_PolicyGuardrailsContract(), + io_contract=get_io_contract("policy-guardrails"), weights=LocalFileBinding(), ) _GUARDIAN_CHECK_ADAPTER = Adapter( identity=Identity("guardian-core", "alora", capability="guardian_core"), - io_contract=_GuardianCheckContract(), + io_contract=get_io_contract("guardian-core"), weights=LocalFileBinding(), ) @@ -163,7 +58,7 @@ def parse(self, raw: str) -> dict[str, object]: identity=Identity( "factuality-detection", "alora", capability="factuality_detection" ), - io_contract=_DictContract("factuality-detection", frozenset({"score"})), + io_contract=get_io_contract("factuality-detection"), weights=LocalFileBinding(), ) @@ -171,7 +66,7 @@ def parse(self, raw: str) -> dict[str, object]: identity=Identity( "factuality-correction", "alora", capability="factuality_correction" ), - io_contract=_DictContract("factuality-correction", frozenset({"correction"})), + io_contract=get_io_contract("factuality-correction"), weights=LocalFileBinding(), ) @@ -212,7 +107,6 @@ def policy_guardrails( backend, kwargs={"policy_text": policy_text}, model_options=model_options, - io_contract=_POLICY_GUARDRAILS_ADAPTER.io_contract, ) if "label" in result_json: @@ -412,7 +306,6 @@ def guardian_check( backend, kwargs={"criteria": criteria_text, "scoring_schema": scoring_schema_text}, model_options=model_options, - io_contract=_GUARDIAN_CHECK_ADAPTER.io_contract, ) return cast(float, cast(dict[str, object], result_json["guardian"])["score"]) @@ -469,11 +362,7 @@ def factuality_detection( if documents is not None: context = _inject_documents(context, documents) result_json = call_intrinsic( - "factuality-detection", - context, - backend, - model_options=model_options, - io_contract=_FACTUALITY_DETECTION_ADAPTER.io_contract, + "factuality-detection", context, backend, model_options=model_options ) return cast(str, result_json["score"]) @@ -530,11 +419,7 @@ def factuality_correction( if documents is not None: context = _inject_documents(context, documents) result_json = call_intrinsic( - "factuality-correction", - context, - backend, - model_options=model_options, - io_contract=_FACTUALITY_CORRECTION_ADAPTER.io_contract, + "factuality-correction", context, backend, model_options=model_options ) return cast(str, result_json["correction"]) diff --git a/mellea/stdlib/components/intrinsic/rag.py b/mellea/stdlib/components/intrinsic/rag.py index 92fde76dac..5581f0444d 100644 --- a/mellea/stdlib/components/intrinsic/rag.py +++ b/mellea/stdlib/components/intrinsic/rag.py @@ -4,133 +4,53 @@ """Adapter functions related to retrieval-augmented generation.""" import collections.abc -import json import warnings from typing import cast -from ....backends.adapters import ( - Adapter, - AdapterMixin, - AdapterSchemaMismatchError, - Identity, - IOContract, - LocalFileBinding, -) -from ....backends.adapters._core import _DictContract -from ....core import Component +from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding +from ....backends.adapters.io_contracts import get_io_contract from ...components import Document from ...context import ChatContext from ..chat import Message from ..docs.document import _coerce_to_document, _coerce_to_documents from ._util import _resolve_question, _resolve_response, call_intrinsic -# --------------------------------------------------------------------------- -# IOContract implementations -# --------------------------------------------------------------------------- - - -class _ListContract(IOContract): - """Validate list-of-dicts adapter output and wrap it under key `"items"`. - - Each item in the list is checked for the declared required keys. The - validated list is returned wrapped in `{"items": [...]}` so that - :func:`call_intrinsic` can always return a plain `dict`. - - Args: - name: Adapter capability name; included in - :class:`~mellea.backends.adapters.AdapterSchemaMismatchError` messages. - required_item_keys: Keys that must be present in every item dict. - """ - - def __init__(self, name: str, required_item_keys: frozenset[str]) -> None: - self._name = name - self._required_item_keys = required_item_keys - - def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." - ) - - def parse(self, raw: str) -> dict[str, object]: - """Parse and validate a list-of-dicts adapter output. - - Args: - raw (str): Raw JSON string from the model. - - Returns: - dict[str, object]: `{"items": [list of validated dicts]}`. - An empty list parses to `{"items": []}`. - - Raises: - ValueError: When *raw* is not valid JSON, is not a JSON array, or - contains a non-object element. - AdapterSchemaMismatchError: When any item is missing a required key. - """ - data = json.loads(raw) - if not isinstance(data, list): - raise ValueError( - f"Adapter '{self._name}' output must be a JSON array, " - f"got {type(data).__name__}." - ) - for item in data: - if not isinstance(item, dict): - raise ValueError( - f"Adapter '{self._name}' output array must contain only JSON " - f"objects, got a {type(item).__name__} element." - ) - observed = frozenset(item.keys()) - missing = self._required_item_keys - observed - if missing: - raise AdapterSchemaMismatchError( - self._name, observed, self._required_item_keys - ) - return {"items": data} - - # --------------------------------------------------------------------------- # Module-level Adapter constants (one per helper) +# +# io_contract is read from the same registry `resolve_adapter()` uses +# (mellea.backends.adapters.io_contracts) rather than declared here — a second, +# independent declaration is exactly the parallel-argument problem issue #1516 +# closes. See test_rag_contracts.py for contract-enforcement tests. # --------------------------------------------------------------------------- _ANSWERABILITY_ADAPTER = Adapter( identity=Identity("answerability", "alora", capability="answerability"), - io_contract=_DictContract("answerability", frozenset({"answerability"})), + io_contract=get_io_contract("answerability"), weights=LocalFileBinding(), ) _QUERY_REWRITE_ADAPTER = Adapter( identity=Identity("query_rewrite", "alora", capability="query_rewrite"), - io_contract=_DictContract("query_rewrite", frozenset({"rewritten_question"})), + io_contract=get_io_contract("query_rewrite"), weights=LocalFileBinding(), ) _QUERY_CLARIFY_ADAPTER = Adapter( identity=Identity("query_clarification", "alora", capability="query_clarification"), - io_contract=_DictContract("query_clarification", frozenset({"clarification"})), + io_contract=get_io_contract("query_clarification"), weights=LocalFileBinding(), ) _CITATIONS_ADAPTER = Adapter( identity=Identity("citations", "alora", capability="citations"), - io_contract=_ListContract( - "citations", - frozenset( - { - "response_begin", - "response_end", - "response_text", - "citation_doc_id", - "citation_begin", - "citation_end", - "citation_text", - } - ), - ), + io_contract=get_io_contract("citations"), weights=LocalFileBinding(), ) _CONTEXT_RELEVANCE_ADAPTER = Adapter( identity=Identity("context_relevance", "alora", capability="context_relevance"), - io_contract=_DictContract("context_relevance", frozenset({"context_relevance"})), + io_contract=get_io_contract("context_relevance"), weights=LocalFileBinding(), ) @@ -138,18 +58,7 @@ def parse(self, raw: str) -> dict[str, object]: identity=Identity( "hallucination_detection", "alora", capability="hallucination_detection" ), - io_contract=_ListContract( - "hallucination_detection", - frozenset( - { - "response_begin", - "response_end", - "response_text", - "faithfulness", - "explanation", - } - ), - ), + io_contract=get_io_contract("hallucination_detection"), weights=LocalFileBinding(), ) @@ -205,7 +114,6 @@ def check_answerability( Message("user", question, documents=_coerce_to_documents(documents)) ), backend, - io_contract=_ANSWERABILITY_ADAPTER.io_contract, model_options=model_options, ) return cast(str, result["answerability"]) @@ -250,7 +158,6 @@ def rewrite_question( "query_rewrite", context.add(Message("user", question)), backend, - io_contract=_QUERY_REWRITE_ADAPTER.io_contract, model_options=model_options, ) return cast(str, result["rewritten_question"]) @@ -303,7 +210,6 @@ def clarify_query( Message("user", question, documents=_coerce_to_documents(documents)) ), backend, - io_contract=_QUERY_CLARIFY_ADAPTER.io_contract, model_options=model_options, ) return cast(str, result["clarification"]) @@ -367,7 +273,6 @@ def find_citations( ) ), backend, - io_contract=_CITATIONS_ADAPTER.io_contract, model_options=model_options, ) return cast(list[dict], result["items"]) @@ -436,7 +341,6 @@ def check_context_relevance( context.add(Message("user", question)), backend, kwargs={"document_content": document.text}, - io_contract=_CONTEXT_RELEVANCE_ADAPTER.io_contract, model_options=model_options, ) return cast(str, result["context_relevance"]) @@ -496,7 +400,6 @@ def flag_hallucinated_content( Message("assistant", response, documents=_coerce_to_documents(documents)) ), backend, - io_contract=_HALLUCINATION_ADAPTER.io_contract, model_options=model_options, ) return cast(list[dict], result["items"]) diff --git a/test/stdlib/components/intrinsic/test_core_schema.py b/test/stdlib/components/intrinsic/test_core_schema.py index fa6607be4a..daf568eea4 100644 --- a/test/stdlib/components/intrinsic/test_core_schema.py +++ b/test/stdlib/components/intrinsic/test_core_schema.py @@ -1,93 +1,99 @@ # Copyright IBM Corp. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for requirement_check schema validation in core.py. - -These tests mock call_intrinsic so they run without a GPU or HF backend. +"""Integration-style unit tests for `core.requirement_check`'s schema validation. + +Issue #1516 replaced the hand-rolled score-range validation `requirement_check` used +to run after `call_intrinsic` with the `requirement-check` capability's declared +`IOContract`, obtained from the adapter `resolve_adapter` returns rather than passed +as a parallel argument. These tests exercise that real path — `call_intrinsic`, +`resolve_adapter`, and `IOContract.parse` all run — by mocking only `mfuncs.act` (the +actual model call), not `call_intrinsic` itself. + +Score-range edge cases (NaN, bool, out-of-range, ...) are covered directly against +the contract in `test/backends/test_adapters/test_io_contracts.py`; this file checks +that `core.requirement_check` is wired to that contract end-to-end. """ -import math -from unittest.mock import patch +import json +from unittest.mock import MagicMock import pytest from mellea.backends.adapters import AdapterSchemaMismatchError -from mellea.stdlib.components.intrinsic import core +from mellea.stdlib.components import Message +from mellea.stdlib.components.intrinsic import _util, core +from mellea.stdlib.components.intrinsic.core import _REQUIREMENT_CHECK_ADAPTER from mellea.stdlib.context import ChatContext -_CTX = ChatContext() -_BACKEND = object() _REQUIREMENT = "must be polite" -_PATCH = "mellea.stdlib.components.intrinsic.core.call_intrinsic" - - -def _call(result_json: dict) -> float: - with patch(_PATCH, return_value=result_json): - return core.requirement_check(_CTX, _BACKEND, _REQUIREMENT) # type: ignore[arg-type] - - -def test_valid_score_returned(): - assert _call({"requirement_check": {"score": 0.8}}) == pytest.approx(0.8) - - -def test_missing_requirement_check_key_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"other_field": 0.9}) -def test_null_requirement_check_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": None}) - - -def test_list_requirement_check_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": []}) +def _call(result_dict: dict, monkeypatch) -> float: + backend = MagicMock() + backend.resolve_adapter.return_value = _REQUIREMENT_CHECK_ADAPTER + def fake_act(_intrinsic, context, _backend, *, model_options=None, **_kwargs): + thunk = MagicMock() + thunk.is_computed.return_value = True + thunk.value = json.dumps(result_dict) + return thunk, context -def test_missing_score_key_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"other_key": 0.9}}) + monkeypatch.setattr(_util.mfuncs, "act", fake_act) + context = ChatContext().add(Message("user", "hi")) + return core.requirement_check(context, backend, _REQUIREMENT) -def test_null_score_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": None}}) +def test_valid_score_returned(monkeypatch): + assert _call({"requirement_check": {"score": 0.8}}, monkeypatch) == pytest.approx( + 0.8 + ) -def test_string_score_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": "0.9"}}) +def test_boundary_score_zero(monkeypatch): + assert _call({"requirement_check": {"score": 0.0}}, monkeypatch) == pytest.approx( + 0.0 + ) -def test_bool_score_raises(): - with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": True}}) +def test_boundary_score_one(monkeypatch): + assert _call({"requirement_check": {"score": 1.0}}, monkeypatch) == pytest.approx( + 1.0 + ) -def test_nan_score_raises(): +def test_missing_requirement_check_key_raises(monkeypatch): with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": math.nan}}) + _call({"other_field": 0.9}, monkeypatch) -def test_inf_score_raises(): +def test_missing_score_key_raises(monkeypatch): with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": math.inf}}) + _call({"requirement_check": {"other_key": 0.9}}, monkeypatch) -def test_score_above_range_raises(): +def test_score_above_range_raises(monkeypatch): with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": 1.5}}) + _call({"requirement_check": {"score": 1.5}}, monkeypatch) -def test_score_below_range_raises(): +def test_score_below_range_raises(monkeypatch): with pytest.raises(AdapterSchemaMismatchError): - _call({"requirement_check": {"score": -0.1}}) + _call({"requirement_check": {"score": -0.1}}, monkeypatch) -def test_boundary_score_zero(): - assert _call({"requirement_check": {"score": 0.0}}) == pytest.approx(0.0) +def test_requirement_check_resolves_adapter_by_name(monkeypatch): + """core.requirement_check must resolve the `requirement-check` capability.""" + backend = MagicMock() + backend.resolve_adapter.return_value = _REQUIREMENT_CHECK_ADAPTER + def fake_act(_intrinsic, context, _backend, *, model_options=None, **_kwargs): + thunk = MagicMock() + thunk.is_computed.return_value = True + thunk.value = json.dumps({"requirement_check": {"score": 0.5}}) + return thunk, context -def test_boundary_score_one(): - assert _call({"requirement_check": {"score": 1.0}}) == pytest.approx(1.0) + monkeypatch.setattr(_util.mfuncs, "act", fake_act) + context = ChatContext().add(Message("user", "hi")) + core.requirement_check(context, backend, _REQUIREMENT) + backend.resolve_adapter.assert_called_once_with("requirement-check") diff --git a/test/stdlib/components/intrinsic/test_guardian_deprecation.py b/test/stdlib/components/intrinsic/test_guardian_deprecation.py index 59a2cadef8..1984343d7a 100644 --- a/test/stdlib/components/intrinsic/test_guardian_deprecation.py +++ b/test/stdlib/components/intrinsic/test_guardian_deprecation.py @@ -21,9 +21,7 @@ def capture_kwargs(monkeypatch): """Replace call_intrinsic with a spy that returns a stub yes=1.0 result.""" captured: dict = {} - def fake_call_intrinsic( - name, context, backend, /, kwargs=None, model_options=None, io_contract=None - ): + def fake_call_intrinsic(name, context, backend, /, kwargs=None, model_options=None): captured["name"] = name captured["kwargs"] = kwargs return {"guardian": {"score": 1.0}} diff --git a/test/stdlib/components/intrinsic/test_guardian_documents.py b/test/stdlib/components/intrinsic/test_guardian_documents.py index 9201082a6f..5e12463cc0 100644 --- a/test/stdlib/components/intrinsic/test_guardian_documents.py +++ b/test/stdlib/components/intrinsic/test_guardian_documents.py @@ -20,9 +20,7 @@ def capture_intrinsic(monkeypatch): """Spy that replaces call_intrinsic and captures what it receives.""" captured: dict = {} - def fake_call_intrinsic( - name, context, backend, /, kwargs=None, model_options=None, io_contract=None - ): + def fake_call_intrinsic(name, context, backend, /, kwargs=None, model_options=None): captured["name"] = name captured["context"] = context return {"score": "yes", "correction": "corrected"} @@ -215,7 +213,7 @@ def _make_capture(monkeypatch, result: dict): """Patch call_intrinsic to capture (name, model_options) and return result.""" calls: list[tuple] = [] - def _fake(name, ctx, backend, /, kwargs=None, model_options=None, io_contract=None): + def _fake(name, ctx, backend, /, kwargs=None, model_options=None): calls.append((name, model_options)) return result diff --git a/test/stdlib/components/intrinsic/test_guardian_io_contract.py b/test/stdlib/components/intrinsic/test_guardian_io_contract.py index e230539ba9..12153d13be 100644 --- a/test/stdlib/components/intrinsic/test_guardian_io_contract.py +++ b/test/stdlib/components/intrinsic/test_guardian_io_contract.py @@ -207,9 +207,7 @@ def test_factuality_correction_error_mentions_adapter_name() -> None: def test_policy_guardrails_score_branch(monkeypatch) -> None: """policy_guardrails returns the `score` value when the adapter omits `label`.""" - def fake_call_intrinsic( - name, context, backend, /, kwargs=None, model_options=None, io_contract=None - ): + def fake_call_intrinsic(name, context, backend, /, kwargs=None, model_options=None): return {"score": "No"} monkeypatch.setattr(guardian, "call_intrinsic", fake_call_intrinsic) diff --git a/test/stdlib/components/intrinsic/test_util_unit.py b/test/stdlib/components/intrinsic/test_util_unit.py index 666c01bb39..801a512681 100644 --- a/test/stdlib/components/intrinsic/test_util_unit.py +++ b/test/stdlib/components/intrinsic/test_util_unit.py @@ -107,3 +107,40 @@ def test_call_intrinsic_rejects_empty_chat_context(monkeypatch): backend.resolve_adapter.assert_not_called() assert calls == [] + + +def test_call_intrinsic_parses_via_resolved_adapters_io_contract(monkeypatch): + """The output contract must come from resolve_adapter's return value, not a + parallel argument (issue #1516).""" + calls: list[dict | None] = [] + monkeypatch.setattr(_util.mfuncs, "act", _fake_act_capturing(calls)) + + resolved_adapter = MagicMock() + resolved_adapter.io_contract.parse.return_value = {"parsed": "by-resolved-adapter"} + backend = MagicMock() + backend.resolve_adapter.return_value = resolved_adapter + context = ChatContext().add(Message("user", "hi")) + + result = _util.call_intrinsic("answerability", context, backend) + + resolved_adapter.io_contract.parse.assert_called_once_with( + json.dumps({"result": "ok"}) + ) + assert result == {"parsed": "by-resolved-adapter"} + + +def test_call_intrinsic_no_longer_accepts_io_contract_kwarg(monkeypatch): + """A caller can no longer pass a separate, possibly-mismatched io_contract.""" + calls: list[dict | None] = [] + monkeypatch.setattr(_util.mfuncs, "act", _fake_act_capturing(calls)) + + backend = MagicMock() + context = ChatContext().add(Message("user", "hi")) + + with pytest.raises(TypeError, match="io_contract"): + _util.call_intrinsic( + "answerability", + context, + backend, + io_contract=MagicMock(), # type: ignore[call-arg] + ) From ea46af212eba53a6c42550c8b8c3d091c9d97b0e Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 18 Aug 2026 14:22:48 +0100 Subject: [PATCH 04/10] =?UTF-8?q?refactor(intrinsics):=20address=20code=20?= =?UTF-8?q?review=20=E2=80=94=20export=20registry,=20drop=20dead=20constan?= =?UTF-8?q?ts,=20fix=20real=20duplicate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent reviewers found the same gap from different angles: the registry this PR introduces to close the parallel-declaration problem still had one. mellea/stdlib/requirements/requirement.py's requirement_check_to_bool() hand-rolled the exact score-range validation just consolidated into _RequirementCheckContract, with a comment pointing at code this PR deleted from core.py. It now delegates to get_io_contract("requirement-check").parse(), which is a strict improvement on its undocumented AttributeError-on-non-dict failure mode (now the documented ValueError the contract raises). Also, per review: - Export get_io_contract from mellea.backends.adapters.__init__ (and __all__), matching the sibling adapter-package imports. Unexported, it was invisible to the docs pipeline (io_contracts.mdx was pruned as "not imported by __init__.py") and to the AGENTS.md-mandated docstring quality gate, despite being the function the module's own docstring designates as the mandatory entry point. - Delete _UNCERTAINTY_ADAPTER and _CONTEXT_ATTRIBUTION_ADAPTER from core.py: nothing referenced them — their only purpose (supplying io_contract= to call_intrinsic) was exactly what the prior commit removed. Keep _REQUIREMENT_CHECK_ADAPTER, which test_core_schema.py uses as a resolve_adapter() stub. - Add a regression test per shim class (test_shims.py) asserting IntrinsicAdapter/EmbeddedIntrinsicAdapter carry the real registry contract, not a placeholder. Without it, reverting get_io_contract(intrinsic_name) back to a stub would have passed every existing test in the file. Verified by temporarily reintroducing a stub object in place of get_io_contract(): both new tests failed as expected, then passed again once reverted. - Enforce the registry's exhaustiveness over known_intrinsic_names() at import time in io_contracts.py, mirroring the existing duplicate-effective_capability check in catalog.py, rather than relying solely on a test. - Correct test_core_schema.py's module docstring, which claimed resolve_adapter itself runs; only its stubbed return value does. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/__init__.py | 2 + mellea/backends/adapters/io_contracts.py | 12 +++++- mellea/stdlib/components/intrinsic/core.py | 33 +++++++-------- .../stdlib/components/intrinsic/guardian.py | 9 +++- mellea/stdlib/components/intrinsic/rag.py | 9 +++- mellea/stdlib/requirements/requirement.py | 42 ++++++------------- test/backends/test_adapters/test_shims.py | 33 ++++++++++++++- .../components/intrinsic/test_core_schema.py | 8 ++-- 8 files changed, 90 insertions(+), 58 deletions(-) diff --git a/mellea/backends/adapters/__init__.py b/mellea/backends/adapters/__init__.py index 3055e77e27..932f3f55b0 100644 --- a/mellea/backends/adapters/__init__.py +++ b/mellea/backends/adapters/__init__.py @@ -25,6 +25,7 @@ ) from .capabilities import KNOWN_CAPABILITIES from .catalog import validate_revision +from .io_contracts import get_io_contract __all__ = [ "KNOWN_CAPABILITIES", @@ -44,5 +45,6 @@ "WeightsBinding", "fetch_intrinsic_metadata", "get_adapter_for_intrinsic", + "get_io_contract", "validate_revision", ] diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index 326b3c6792..ae61e35312 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -24,6 +24,7 @@ from ...core import Component from ._core import AdapterSchemaMismatchError, IOContract, _DictContract, _ListContract +from .catalog import known_intrinsic_names class _PolicyGuardrailsContract(IOContract): @@ -240,7 +241,16 @@ def parse(self, raw: str) -> dict[str, object]: """Canonical output contract for every catalogued adapter function, keyed by its catalog `name` (see module docstring). Kept exhaustive over :func:`~mellea.backends.adapters.catalog.known_intrinsic_names` by -`test/backends/test_adapters/test_io_contracts.py`.""" +`test/backends/test_adapters/test_io_contracts.py`, and enforced below at import +time — mirroring the duplicate-`effective_capability` check in `catalog.py`.""" + +_missing_contracts = set(known_intrinsic_names()) - set(_INTRINSIC_IO_CONTRACTS) +if _missing_contracts: + raise ValueError( + f"Catalogued adapter functions with no declared IOContract: " + f"{_missing_contracts}" + ) +del _missing_contracts def get_io_contract(name: str) -> IOContract: diff --git a/mellea/stdlib/components/intrinsic/core.py b/mellea/stdlib/components/intrinsic/core.py index 274f39a898..062ae04ff0 100644 --- a/mellea/stdlib/components/intrinsic/core.py +++ b/mellea/stdlib/components/intrinsic/core.py @@ -6,40 +6,35 @@ import collections.abc from typing import cast -from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding -from ....backends.adapters.io_contracts import get_io_contract +from ....backends.adapters import ( + Adapter, + AdapterMixin, + Identity, + LocalFileBinding, + get_io_contract, +) from ...components import Document, Message from ...context import ChatContext from ..docs.document import _coerce_to_documents from ._util import _resolve_response, call_intrinsic # --------------------------------------------------------------------------- -# Module-level Adapter constants (one per helper) -# -# io_contract is read from the same registry `resolve_adapter()` uses -# (mellea.backends.adapters.io_contracts) rather than declared here — a second, -# independent declaration is exactly the parallel-argument problem issue #1516 +# _REQUIREMENT_CHECK_ADAPTER: a test-visible handle onto the `requirement-check` +# capability's canonical Adapter, used by test_core_schema.py to stub +# `resolve_adapter`'s return value. `check_certainty` and `find_context_attributions` +# have no equivalent constant — nothing outside a test needs one, since their +# io_contract is looked up by resolve_adapter() at call time +# (mellea.backends.adapters.io_contracts), never declared here. Declaring one +# unused by any caller is exactly the parallel-declaration problem issue #1516 # closes. # --------------------------------------------------------------------------- -_UNCERTAINTY_ADAPTER = Adapter( - identity=Identity("uncertainty", "alora", capability="uncertainty"), - io_contract=get_io_contract("uncertainty"), - weights=LocalFileBinding(), -) - _REQUIREMENT_CHECK_ADAPTER = Adapter( identity=Identity("requirement-check", "alora", capability="requirement_check"), io_contract=get_io_contract("requirement-check"), weights=LocalFileBinding(), ) -_CONTEXT_ATTRIBUTION_ADAPTER = Adapter( - identity=Identity("context-attribution", "alora", capability="context_attribution"), - io_contract=get_io_contract("context-attribution"), - weights=LocalFileBinding(), -) - def check_certainty( context: ChatContext, backend: AdapterMixin, model_options: dict | None = None diff --git a/mellea/stdlib/components/intrinsic/guardian.py b/mellea/stdlib/components/intrinsic/guardian.py index 4707fa97a5..8d74303cbd 100644 --- a/mellea/stdlib/components/intrinsic/guardian.py +++ b/mellea/stdlib/components/intrinsic/guardian.py @@ -16,8 +16,13 @@ import warnings from typing import cast -from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding -from ....backends.adapters.io_contracts import get_io_contract +from ....backends.adapters import ( + Adapter, + AdapterMixin, + Identity, + LocalFileBinding, + get_io_contract, +) from ....core.utils import MelleaLogger from ...components import Document from ...context import ChatContext diff --git a/mellea/stdlib/components/intrinsic/rag.py b/mellea/stdlib/components/intrinsic/rag.py index 5581f0444d..7fbb95abeb 100644 --- a/mellea/stdlib/components/intrinsic/rag.py +++ b/mellea/stdlib/components/intrinsic/rag.py @@ -7,8 +7,13 @@ import warnings from typing import cast -from ....backends.adapters import Adapter, AdapterMixin, Identity, LocalFileBinding -from ....backends.adapters.io_contracts import get_io_contract +from ....backends.adapters import ( + Adapter, + AdapterMixin, + Identity, + LocalFileBinding, + get_io_contract, +) from ...components import Document from ...context import ChatContext from ..chat import Message diff --git a/mellea/stdlib/requirements/requirement.py b/mellea/stdlib/requirements/requirement.py index e102880f99..fc072f2aed 100644 --- a/mellea/stdlib/requirements/requirement.py +++ b/mellea/stdlib/requirements/requirement.py @@ -3,12 +3,10 @@ """Requirements are a special type of Component used as input to the "validate" step in Instruct/Validate/Repair design patterns.""" -import json -import math from collections.abc import Callable -from typing import Any, overload +from typing import Any, cast, overload -from ...backends.adapters import AdapterSchemaMismatchError +from ...backends.adapters import get_io_contract from ...core import ( CBlock, Context, @@ -46,38 +44,22 @@ def requirement_check_to_bool(x: CBlock | ModelOutputThunk | str) -> bool: Raises: json.JSONDecodeError: If `x` is not valid JSON. + ValueError: If the parsed JSON is not an object (e.g. a list, string, or + number). AdapterSchemaMismatchError: If the parsed output does not contain the expected `requirement_check.score` structure, or if the score is not a finite number in the range 0.0-1.0. Callers that previously treated `False` as "requirement not met" must now catch this error separately. """ - output = str(x) - req_dict: dict[str, Any] = json.loads(output) - - # Mirrors the validation in requirement_check() in core.py; Phase 2 will consolidate via IOContract. - req_check = req_dict.get("requirement_check", None) - if not isinstance(req_check, dict): - raise AdapterSchemaMismatchError( - name="requirement-check", - observed_keys=frozenset(req_dict.keys()), - expected_keys=frozenset({"requirement_check"}), - ) - - score = req_check.get("score", None) - if ( - not isinstance(score, (int, float)) - or isinstance(score, bool) # bool subclasses int; exclude it explicitly - or not math.isfinite(score) - or not 0.0 <= score <= 1.0 - ): - raise AdapterSchemaMismatchError( - name="requirement-check", - observed_keys=frozenset(req_check.keys()), - expected_keys=frozenset({"score"}), - ) - - return score > 0.5 + # Delegates to the same `requirement-check` IOContract resolve_adapter() hands + # back to call_intrinsic() — see mellea.backends.adapters.io_contracts. A second, + # independent validation here is exactly the parallel-declaration problem #1516 + # closes; ALoraRequirement (below) is the other production consumer of this + # capability's output alongside core.requirement_check(). + parsed = get_io_contract("requirement-check").parse(str(x)) + score = cast(dict[str, Any], parsed["requirement_check"])["score"] + return cast(float, score) > 0.5 class ALoraRequirement(Requirement, Intrinsic): diff --git a/test/backends/test_adapters/test_shims.py b/test/backends/test_adapters/test_shims.py index 98801a75da..457810dc0a 100644 --- a/test/backends/test_adapters/test_shims.py +++ b/test/backends/test_adapters/test_shims.py @@ -15,7 +15,12 @@ import pytest -from mellea.backends.adapters import Adapter, EmbeddedIntrinsicAdapter, IntrinsicAdapter +from mellea.backends.adapters import ( + Adapter, + EmbeddedIntrinsicAdapter, + IntrinsicAdapter, + get_io_contract, +) from mellea.backends.adapters._core import Identity, LocalFileBinding from mellea.backends.adapters.adapter import AdapterMixin from mellea.backends.adapters.catalog import AdapterType, IntrinsicsCatalogEntry @@ -92,6 +97,21 @@ def test_embedded_identity_populated(): assert adapter.identity.adapter_type == "alora" +def test_embedded_carries_the_declared_io_contract_not_a_shim(): + """resolve_adapter()'s shim path must carry a real contract (issue #1516). + + Regression guard: without this, reverting `get_io_contract(intrinsic_name)` + back to the Phase 1 `_ShimIOContract()` placeholder would pass every other + test in this file, since none of them inspect `.io_contract`. + """ + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + adapter = EmbeddedIntrinsicAdapter( + "answerability", config={}, technology="alora" + ) + assert adapter.io_contract is get_io_contract("answerability") + + def test_embedded_identity_lora_technology(): with warnings.catch_warnings(): warnings.simplefilter("ignore", DeprecationWarning) @@ -170,6 +190,17 @@ def test_intrinsic_identity_populated(): assert adapter.identity.adapter_type == "alora" +def test_intrinsic_carries_the_declared_io_contract_not_a_shim(): + """resolve_adapter()'s shim path must carry a real contract (issue #1516). + + Regression guard: without this, reverting `get_io_contract(intrinsic_name)` + back to the Phase 1 `_ShimIOContract()` placeholder would pass every other + test in this file, since none of them inspect `.io_contract`. + """ + adapter = _make_intrinsic_adapter("answerability") + assert adapter.io_contract is get_io_contract("answerability") + + def test_intrinsic_identity_lora_adapter_type(): with ( patch( diff --git a/test/stdlib/components/intrinsic/test_core_schema.py b/test/stdlib/components/intrinsic/test_core_schema.py index daf568eea4..400f1fba02 100644 --- a/test/stdlib/components/intrinsic/test_core_schema.py +++ b/test/stdlib/components/intrinsic/test_core_schema.py @@ -6,9 +6,11 @@ Issue #1516 replaced the hand-rolled score-range validation `requirement_check` used to run after `call_intrinsic` with the `requirement-check` capability's declared `IOContract`, obtained from the adapter `resolve_adapter` returns rather than passed -as a parallel argument. These tests exercise that real path — `call_intrinsic`, -`resolve_adapter`, and `IOContract.parse` all run — by mocking only `mfuncs.act` (the -actual model call), not `call_intrinsic` itself. +as a parallel argument. These tests exercise that real path — `call_intrinsic` and +`IOContract.parse` both run against a stubbed `resolve_adapter` return value — by +mocking `mfuncs.act` (the actual model call) and `backend.resolve_adapter` (a bare +`MagicMock`), not `call_intrinsic` itself. `resolve_adapter`'s own lazy-registration +and catalogue-lookup logic is exercised separately in `test/backends/test_adapters/`. Score-range edge cases (NaN, bool, out-of-range, ...) are covered directly against the contract in `test/backends/test_adapters/test_io_contracts.py`; this file checks From 25c4118f3c4e40c5496fa1b2ac95e0a591825a9c Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 18 Aug 2026 16:12:38 +0100 Subject: [PATCH 05/10] refactor(intrinsics): apply remaining review suggestions and nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review response commit — the lower-severity items all three reviewers raised, applied where they were cheap and genuinely useful: - adapter.py: convert the remaining `.. deprecated::` RST directives on IntrinsicAdapter/EmbeddedIntrinsicAdapter to Google-style `Deprecated:` sections, matching the `Note:` conversion already done on the same docstrings. (CustomIntrinsicAdapter's directive is untouched by this diff and left alone.) - _core.py / io_contracts.py: replace the stale "not used in Phase 1; implemented in Phase 2" build_prompt placeholder message — self-contradictory now that this module *is* the Phase 2 work — with an accurate description of the current state. Updated the one test asserting on the old wording. - _core.py: module docstring now names _ListContract and explains the generic-vs-capability-specific split with io_contracts.py. - io_contracts.py: get_io_contract's docstring now states its keys are catalog `name`s, not `effective_capability` tokens, and corrects "permissive" to mean permissive about which keys are present, not about the JSON shape. Added the matching inline comment on the fallback return. - io_contracts.py: comment distinguishing the two AdapterSchemaMismatchError raise sites in _PolicyGuardrailsContract (neither key present vs. both). _RequirementCheckContract's docstring now names both production consumers it consolidates (core.py and requirement.py) with full paths. - test_io_contracts.py: new non-GPU test feeding the recorded context-attribution model output (test/stdlib/components/intrinsic/testdata) through the real contract. The GPU-gated equivalent is xfail(strict=False) for unrelated non-determinism, so it gives no CI signal on schema drift; this closes that gap without a GPU. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 12 +++++- mellea/backends/adapters/adapter.py | 4 +- mellea/backends/adapters/io_contracts.py | 38 +++++++++++++------ .../backends/test_adapters/test_core_types.py | 2 +- .../test_adapters/test_io_contracts.py | 30 +++++++++++++++ 5 files changed, 69 insertions(+), 17 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 83c853d2cb..a54349724a 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -15,6 +15,12 @@ - :class:`EmbeddedBinding` — stub :class:`WeightsBinding` subclass - :class:`ServerMediatedBinding` — stub :class:`WeightsBinding` subclass - :class:`AdapterSchemaMismatchError` +- :class:`_DictContract`, :class:`_ListContract` — generic, capability-agnostic + :class:`IOContract` implementations that validate required keys on a JSON + object or a JSON array of objects, respectively. Capability-*specific* + contracts (e.g. the guardian adapters' nested-key shapes) live in + :mod:`~mellea.backends.adapters.io_contracts` instead, alongside the + registry that maps every catalogued adapter function to its contract. Note: The existing :class:`~mellea.backends.adapters.adapter.Adapter` ABC in @@ -160,7 +166,8 @@ def __init__(self, name: str, required_keys: frozenset[str]) -> None: def build_prompt(self, **_kwargs: object) -> Component: raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." ) def parse(self, raw: str) -> dict[str, object]: @@ -209,7 +216,8 @@ def __init__(self, name: str, required_item_keys: frozenset[str]) -> None: def build_prompt(self, **_kwargs: object) -> Component: raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." ) def parse(self, raw: str) -> dict[str, object]: diff --git a/mellea/backends/adapters/adapter.py b/mellea/backends/adapters/adapter.py index 74e5d1bd04..146b04e084 100644 --- a/mellea/backends/adapters/adapter.py +++ b/mellea/backends/adapters/adapter.py @@ -123,7 +123,7 @@ def release(self) -> None: class IntrinsicAdapter(LocalHFAdapter, _AdapterCore): """Deprecated shim for adapters that implement adapter functions. - .. deprecated:: + Deprecated: Use :class:`~mellea.backends.adapters.Adapter` directly. `IntrinsicAdapter` will be removed in a future release (Epic #929, issue #1144). @@ -881,7 +881,7 @@ def _find_adapter( class EmbeddedIntrinsicAdapter(_AdapterCore): """Deprecated shim for adapter functions embedded in a Granite Switch model. - .. deprecated:: + Deprecated: Use :class:`~mellea.backends.adapters.Adapter` directly. `EmbeddedIntrinsicAdapter` will be removed in a future release (Epic #929, issue #1144). diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index ae61e35312..5d83ba0746 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -36,7 +36,8 @@ class _PolicyGuardrailsContract(IOContract): def build_prompt(self, **_kwargs: object) -> Component: raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." ) def parse(self, raw: str) -> dict[str, object]: @@ -60,6 +61,10 @@ def parse(self, raw: str) -> dict[str, object]: ) has_label = "label" in data has_score = "score" in data + # Both branches raise the same exception shape; `observed_keys` (all of + # `data`'s keys) is what distinguishes them when printed — neither + # "label" nor "score" appears for the missing-both case, and both do + # for the has-both case. if not has_label and not has_score: raise AdapterSchemaMismatchError( "policy-guardrails", @@ -84,7 +89,8 @@ class _GuardianCheckContract(IOContract): def build_prompt(self, **_kwargs: object) -> Component: raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." ) def parse(self, raw: str) -> dict[str, object]: @@ -126,15 +132,17 @@ def parse(self, raw: str) -> dict[str, object]: class _RequirementCheckContract(IOContract): """Validate requirement-check output: `{"requirement_check": {"score": <0.0-1.0 float>}}`. - Consolidates the score-range validation `requirement_check()` (in - `mellea.stdlib.components.intrinsic.core`) previously hand-rolled after each call — - mirroring `requirement_check_to_bool()` in `requirement.py` — into a declared contract, - per issue #1516. + Consolidates the score-range validation that both production consumers of this + capability's output — `core.requirement_check()` + (`mellea/stdlib/components/intrinsic/core.py`) and `requirement_check_to_bool()` + (`mellea/stdlib/requirements/requirement.py`) — used to hand-roll independently, + into one declared contract, per issue #1516. """ def build_prompt(self, **_kwargs: object) -> Component: raise NotImplementedError( - "build_prompt is not used in Phase 1; implemented in Phase 2." + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." ) def parse(self, raw: str) -> dict[str, object]: @@ -257,19 +265,25 @@ def get_io_contract(name: str) -> IOContract: """Return the canonical output contract for an adapter function. Args: - name (str): Catalog name of the adapter function (e.g. `"answerability"`), - i.e. the same string passed to + name (str): Catalog name of the adapter function (e.g. `"answerability"`, + `"guardian-core"`) — :attr:`~mellea.backends.adapters.catalog.IntrinsicsCatalogEntry.name`, + never :attr:`~mellea.backends.adapters.catalog.IntrinsicsCatalogEntry.effective_capability`. + This is the same string passed to :func:`~mellea.stdlib.components.intrinsic._util.call_intrinsic` and :meth:`~mellea.backends.adapters.AdapterMixin.resolve_adapter`. Returns: IOContract: The contract declared in :data:`_INTRINSIC_IO_CONTRACTS` for `name`. Adapter functions outside the catalog (e.g. one registered - through the deprecated `CustomIntrinsicAdapter`) fall back to a - permissive dict contract with no required keys, since Mellea has no - declared schema for them. + through the deprecated `CustomIntrinsicAdapter`) fall back to a dict + contract with no required keys — permissive about *which* keys are + present, but still requires the parsed JSON to be an object; a + top-level array or scalar still raises `ValueError`, since Mellea has + no declared schema for these adapter functions to relax that far. """ contract = _INTRINSIC_IO_CONTRACTS.get(name) if contract is not None: return contract + # Custom/user-defined adapters outside the catalog have no declared schema; + # accept any JSON object rather than rejecting on an arbitrary key set. return _DictContract(name, frozenset()) diff --git a/test/backends/test_adapters/test_core_types.py b/test/backends/test_adapters/test_core_types.py index d3751d891f..5d3557a3dd 100644 --- a/test/backends/test_adapters/test_core_types.py +++ b/test/backends/test_adapters/test_core_types.py @@ -281,5 +281,5 @@ def test_dict_contract_reports_all_missing_multi_key(): def test_dict_contract_build_prompt_not_implemented(): contract = _DictContract("answerability", frozenset({"answerability"})) - with pytest.raises(NotImplementedError, match="Phase 1"): + with pytest.raises(NotImplementedError, match="build_prompt is not implemented"): contract.build_prompt() diff --git a/test/backends/test_adapters/test_io_contracts.py b/test/backends/test_adapters/test_io_contracts.py index 0a0c4bd0fc..76061324da 100644 --- a/test/backends/test_adapters/test_io_contracts.py +++ b/test/backends/test_adapters/test_io_contracts.py @@ -10,6 +10,7 @@ import json import math +import pathlib import pytest @@ -20,6 +21,14 @@ get_io_contract, ) +_INTRINSIC_TESTDATA = ( + pathlib.Path(__file__).resolve().parents[2] + / "stdlib" + / "components" + / "intrinsic" + / "testdata" +) + # --------------------------------------------------------------------------- # Registry completeness # --------------------------------------------------------------------------- @@ -137,3 +146,24 @@ def test_boundary_score_one(contract): def test_requirement_check_rejects_non_dict_output(contract): with pytest.raises(ValueError, match="must be a JSON object"): contract.parse(json.dumps(["not", "a", "dict"])) + + +# --------------------------------------------------------------------------- +# context-attribution — recorded model output against the real _ListContract +# +# The GPU-gated equivalents (test_find_context_attributions and +# test_find_context_attributions_resolve in test_core.py) are xfail(strict=False) +# for unrelated non-determinism, so they give no CI signal on schema drift. This +# feeds the exact recorded output through the contract without a GPU. +# --------------------------------------------------------------------------- + + +def test_context_attribution_contract_accepts_recorded_model_output(): + fixture = _INTRINSIC_TESTDATA / "output_json" / "context-attribution.json" + completion = json.loads(fixture.read_text(encoding="utf-8")) + raw = completion["choices"][0]["message"]["content"] + + result = _INTRINSIC_IO_CONTRACTS["context-attribution"].parse(raw) + + assert len(result["items"]) == 7 # type: ignore[arg-type] + assert result["items"][0]["attribution_msg_index"] is None # type: ignore[index] From 1363fec3a47789d61ffdb08acb8bd0bc15b7b1ca Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Wed, 19 Aug 2026 21:06:54 +0100 Subject: [PATCH 06/10] test(intrinsics): add contract-wiring tests and fix docstring accuracy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the multi-party review findings on this PR (issue #1516): - test_core_contracts.py: CI-runnable wiring tests for check_certainty and find_context_attributions — the two helpers that moved to registry-contract validation (plus the items unwrap) with only GPU-gated qualitative/xfail coverage before - test_requirement.py: cover the newly-typed ValueError for non-object JSON in requirement_check_to_bool (was an undocumented AttributeError) - test_io_contracts.py: guard the reverse direction of the registry exhaustiveness invariant (orphan keys) - core.py: the new Raises: ValueError entries were narrower than the contracts' actual raise paths (wrong top-level shape is also ValueError) - io_contracts.py: the string literal after the registry assignment was a dead expression — dicts have no docstring; make it a comment - test_rag_contracts.py / test_guardian_io_contract.py: the contracts no longer live in rag.py/guardian.py (Phase 1 -> io_contracts.py, #1516) - rag.py / guardian.py / core.py: state consistently that the per-helper Adapter constants are weights scaffolding for #1141/#1142, not a second contract source Assisted-by: opencode Signed-off-by: Nigel Jones --- mellea/backends/adapters/io_contracts.py | 10 +- mellea/stdlib/components/intrinsic/core.py | 15 ++- .../stdlib/components/intrinsic/guardian.py | 4 + mellea/stdlib/components/intrinsic/rag.py | 4 + .../test_adapters/test_io_contracts.py | 11 ++ .../intrinsic/test_core_contracts.py | 109 ++++++++++++++++++ .../intrinsic/test_guardian_io_contract.py | 7 +- .../intrinsic/test_rag_contracts.py | 6 +- .../components/intrinsic/test_util_unit.py | 7 +- test/stdlib/requirements/test_requirement.py | 10 ++ 10 files changed, 167 insertions(+), 16 deletions(-) create mode 100644 test/stdlib/components/intrinsic/test_core_contracts.py diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index 5d83ba0746..793cb3e740 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -246,11 +246,11 @@ def parse(self, raw: str) -> dict[str, object]: ), ), } -"""Canonical output contract for every catalogued adapter function, keyed by its -catalog `name` (see module docstring). Kept exhaustive over -:func:`~mellea.backends.adapters.catalog.known_intrinsic_names` by -`test/backends/test_adapters/test_io_contracts.py`, and enforced below at import -time — mirroring the duplicate-`effective_capability` check in `catalog.py`.""" +# Canonical output contract for every catalogued adapter function, keyed by its +# catalog `name` (see module docstring). Kept exhaustive over +# known_intrinsic_names() by test/backends/test_adapters/test_io_contracts.py, +# and enforced below at import time — mirroring the +# duplicate-`effective_capability` check in catalog.py. _missing_contracts = set(known_intrinsic_names()) - set(_INTRINSIC_IO_CONTRACTS) if _missing_contracts: diff --git a/mellea/stdlib/components/intrinsic/core.py b/mellea/stdlib/components/intrinsic/core.py index 062ae04ff0..bc2277f772 100644 --- a/mellea/stdlib/components/intrinsic/core.py +++ b/mellea/stdlib/components/intrinsic/core.py @@ -25,8 +25,10 @@ # have no equivalent constant — nothing outside a test needs one, since their # io_contract is looked up by resolve_adapter() at call time # (mellea.backends.adapters.io_contracts), never declared here. Declaring one -# unused by any caller is exactly the parallel-declaration problem issue #1516 -# closes. +# unused by any caller would be exactly the parallel-declaration problem +# issue #1516 closes; the `rag.py`/`guardian.py` sibling constants are kept +# only as per-helper weights scaffolding for #1141/#1142, never as a second +# source of their contracts. # --------------------------------------------------------------------------- _REQUIREMENT_CHECK_ADAPTER = Adapter( @@ -59,7 +61,8 @@ def check_certainty( Certainty score as a float (higher = more certain). Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When the model output is missing the required `certainty` field. """ @@ -98,7 +101,8 @@ def requirement_check( Score as a float between 0.0 and 1.0 (higher = more likely satisfied). Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: If the adapter output does not match the expected `{"requirement_check": {"score": }}` contract, or if the score is not a finite number in the range 0.0-1.0. @@ -158,7 +162,8 @@ def find_context_attributions( UTF-8 strings. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON, is not a + JSON array, or contains a non-object element. AdapterSchemaMismatchError: When any record in the output is missing a required field. """ diff --git a/mellea/stdlib/components/intrinsic/guardian.py b/mellea/stdlib/components/intrinsic/guardian.py index 8d74303cbd..68cb6cb887 100644 --- a/mellea/stdlib/components/intrinsic/guardian.py +++ b/mellea/stdlib/components/intrinsic/guardian.py @@ -45,6 +45,10 @@ # (mellea.backends.adapters.io_contracts) rather than declared here — a second, # independent declaration is exactly the parallel-argument problem issue #1516 # closes. See test_guardian_io_contract.py for contract-enforcement tests. +# +# The helper bodies no longer reference these constants: the contract travels +# with the adapter resolve_adapter() returns. They remain as the per-helper +# Adapter handles that #1141/#1142 will fill with real weights bindings. # --------------------------------------------------------------------------- _POLICY_GUARDRAILS_ADAPTER = Adapter( diff --git a/mellea/stdlib/components/intrinsic/rag.py b/mellea/stdlib/components/intrinsic/rag.py index 7fbb95abeb..737a6692c8 100644 --- a/mellea/stdlib/components/intrinsic/rag.py +++ b/mellea/stdlib/components/intrinsic/rag.py @@ -27,6 +27,10 @@ # (mellea.backends.adapters.io_contracts) rather than declared here — a second, # independent declaration is exactly the parallel-argument problem issue #1516 # closes. See test_rag_contracts.py for contract-enforcement tests. +# +# The helper bodies no longer reference these constants: the contract travels +# with the adapter resolve_adapter() returns. They remain as the per-helper +# Adapter handles that #1141/#1142 will fill with real weights bindings. # --------------------------------------------------------------------------- _ANSWERABILITY_ADAPTER = Adapter( diff --git a/test/backends/test_adapters/test_io_contracts.py b/test/backends/test_adapters/test_io_contracts.py index 76061324da..587d063c5c 100644 --- a/test/backends/test_adapters/test_io_contracts.py +++ b/test/backends/test_adapters/test_io_contracts.py @@ -44,6 +44,17 @@ def test_registry_covers_every_catalog_name(): assert missing == set() +def test_registry_has_no_orphan_keys(): + """A contract keyed to a name absent from the catalog would be dead code. + + Guards the reverse direction of the completeness invariant: an orphan key + would be served by `get_io_contract` ahead of the permissive fallback if a + later adapter ever registered under that name. + """ + orphans = set(_INTRINSIC_IO_CONTRACTS) - set(known_intrinsic_names()) + assert orphans == set() + + def test_get_io_contract_returns_declared_instance_for_known_names(): for name in known_intrinsic_names(): assert get_io_contract(name) is _INTRINSIC_IO_CONTRACTS[name] diff --git a/test/stdlib/components/intrinsic/test_core_contracts.py b/test/stdlib/components/intrinsic/test_core_contracts.py new file mode 100644 index 0000000000..fc0c07c9ce --- /dev/null +++ b/test/stdlib/components/intrinsic/test_core_contracts.py @@ -0,0 +1,109 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Wiring tests for check_certainty and find_context_attributions (issue #1516). + +Both helpers moved from raw `json.loads` to registry-contract validation when +`call_intrinsic` started parsing via the resolved adapter's contract, and +`find_context_attributions` additionally changed return shape (unwrap of +`"items"`). Their only other tests are GPU-gated qualitative/xfail tests in +`test_core.py`, which give no CI signal on that wiring — these stub +`mfuncs.act` and `backend.resolve_adapter` (the `test_core_schema.py` pattern) +and exercise the helper boundary itself. +""" + +import json +import pathlib +from unittest.mock import MagicMock + +import pytest + +from mellea.backends.adapters import AdapterSchemaMismatchError, get_io_contract +from mellea.stdlib.components import Message +from mellea.stdlib.components.intrinsic import _util, core +from mellea.stdlib.context import ChatContext + + +def _stub_model_call(raw_output: str, monkeypatch, intrinsic_name: str) -> MagicMock: + """Replace `mfuncs.act` to return *raw_output* and stub `resolve_adapter` + to carry *intrinsic_name*'s registry contract. Returns the backend mock.""" + backend = MagicMock() + backend.resolve_adapter.return_value = MagicMock( + io_contract=get_io_contract(intrinsic_name) + ) + + def fake_act(_intrinsic, context, _backend, *, model_options=None, **_kwargs): + thunk = MagicMock() + thunk.is_computed.return_value = True + thunk.value = raw_output + return thunk, context + + monkeypatch.setattr(_util.mfuncs, "act", fake_act) + return backend + + +def test_check_certainty_resolves_and_parses_via_registry_contract(monkeypatch): + """check_certainty must resolve `uncertainty` and parse its contract output.""" + backend = _stub_model_call( + json.dumps({"certainty": 0.9}), monkeypatch, "uncertainty" + ) + context = ChatContext().add(Message("user", "hi")) + + assert core.check_certainty(context, backend) == pytest.approx(0.9) + backend.resolve_adapter.assert_called_once_with("uncertainty") + + +def test_check_certainty_missing_certainty_key_raises(monkeypatch): + """A missing `certainty` key must raise AdapterSchemaMismatchError.""" + backend = _stub_model_call( + json.dumps({"wrong_key": 0.9}), monkeypatch, "uncertainty" + ) + context = ChatContext().add(Message("user", "hi")) + + with pytest.raises(AdapterSchemaMismatchError): + core.check_certainty(context, backend) + + +def test_check_certainty_rejects_non_object_output(monkeypatch): + """A top-level JSON array is a ValueError, not a schema mismatch.""" + backend = _stub_model_call(json.dumps([0.9]), monkeypatch, "uncertainty") + context = ChatContext().add(Message("user", "hi")) + + with pytest.raises(ValueError, match="must be a JSON object"): + core.check_certainty(context, backend) + + +def test_find_context_attributions_returns_items_from_recorded_output(monkeypatch): + """The `result_json["items"]` unwrap must survive the helper boundary.""" + fixture = ( + pathlib.Path(__file__).resolve().parent + / "testdata" + / "output_json" + / "context-attribution.json" + ) + completion = json.loads(fixture.read_text(encoding="utf-8")) + raw = completion["choices"][0]["message"]["content"] + + backend = _stub_model_call(raw, monkeypatch, "context-attribution") + context = ChatContext().add(Message("user", "hi")) + + result = core.find_context_attributions( + "The answer is 42.", ["A document."], context, backend + ) + + assert len(result) == 7 + assert result[0]["attribution_msg_index"] is None + backend.resolve_adapter.assert_called_once_with("context-attribution") + + +def test_find_context_attributions_rejects_non_array_output(monkeypatch): + """A top-level JSON object is a ValueError for the list contract.""" + backend = _stub_model_call( + json.dumps({"not": "a list"}), monkeypatch, "context-attribution" + ) + context = ChatContext().add(Message("user", "hi")) + + with pytest.raises(ValueError, match="must be a JSON array"): + core.find_context_attributions( + "The answer is 42.", ["A document."], context, backend + ) diff --git a/test/stdlib/components/intrinsic/test_guardian_io_contract.py b/test/stdlib/components/intrinsic/test_guardian_io_contract.py index 12153d13be..53274550d3 100644 --- a/test/stdlib/components/intrinsic/test_guardian_io_contract.py +++ b/test/stdlib/components/intrinsic/test_guardian_io_contract.py @@ -1,9 +1,12 @@ # Copyright IBM Corp. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for IOContract validation in guardian.py (Epic #929 Phase 1). +"""Unit tests for the guardian adapter functions' output contracts (Epic #929, +#1516). -Tests the `parse()` method of each IOContract subclass directly — no backend, +The contracts are declared in `mellea.backends.adapters.io_contracts` and +carried by the `guardian.py` adapter constants via `get_io_contract`. +Tests the `parse()` method of each contract directly — no backend, no GPU, no model download required. Two tests per helper: - `test__contract_enforced` — output missing a required field raises diff --git a/test/stdlib/components/intrinsic/test_rag_contracts.py b/test/stdlib/components/intrinsic/test_rag_contracts.py index aaa2af32f7..0ae1691046 100644 --- a/test/stdlib/components/intrinsic/test_rag_contracts.py +++ b/test/stdlib/components/intrinsic/test_rag_contracts.py @@ -1,9 +1,11 @@ # Copyright IBM Corp. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for IOContract validation in rag.py (Epic #929 Phase 1). +"""Unit tests for the rag adapter functions' output contracts (Epic #929, #1516). -Tests the `parse()` method of each IOContract subclass directly — no backend, +The contracts are declared in `mellea.backends.adapters.io_contracts` and +carried by the `rag.py` adapter constants via `get_io_contract`. +Tests the `parse()` method of each contract directly — no backend, no GPU, no model download required. Two tests per helper: - `test__contract_enforced` — output missing a required field raises diff --git a/test/stdlib/components/intrinsic/test_util_unit.py b/test/stdlib/components/intrinsic/test_util_unit.py index 801a512681..93c5d65acc 100644 --- a/test/stdlib/components/intrinsic/test_util_unit.py +++ b/test/stdlib/components/intrinsic/test_util_unit.py @@ -1,11 +1,14 @@ # Copyright IBM Corp. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for `call_intrinsic`'s model_options resolution. +"""Unit tests for `call_intrinsic`'s model_options resolution and contract +wiring. Exercises the model_options precedence without a real backend or model — guards against the PR #972 bug class (caller-supplied model_options silently -discarded behind a hardcoded default) resurfacing. +discarded behind a hardcoded default) resurfacing. Also covers the issue #1516 +change: the output contract is taken from the adapter `resolve_adapter()` +returns, and `call_intrinsic` no longer accepts an `io_contract` kwarg. """ import json diff --git a/test/stdlib/requirements/test_requirement.py b/test/stdlib/requirements/test_requirement.py index 8f5e79e856..f8d4116896 100644 --- a/test/stdlib/requirements/test_requirement.py +++ b/test/stdlib/requirements/test_requirement.py @@ -127,6 +127,16 @@ def test_requirement_check_to_bool_invalid_json(): requirement_check_to_bool("not json") +def test_requirement_check_to_bool_non_object_raises(): + """A top-level JSON array or scalar is a ValueError, not a schema mismatch. + + The replaced code raised an undocumented AttributeError here instead + (`list.get` on the parsed result). + """ + with pytest.raises(ValueError, match="must be a JSON object"): + requirement_check_to_bool("[1, 2]") + + def test_requirement_check_to_bool_nan_score_raises(): """NaN would silently evaluate as False without the finiteness guard.""" with pytest.raises(AdapterSchemaMismatchError): From cb45a3467a2f909921d5cfa6160d7d0fb423f855 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Thu, 20 Aug 2026 12:25:40 +0100 Subject: [PATCH 07/10] refactor(intrinsics): drop dead per-helper Adapter constants, consolidate build_prompt message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continues the multi-party review follow-ups on this PR (issue #1516): - rag.py / guardian.py: drop the ten module-level Adapter constants. Their only purpose (feeding io_contract= to call_intrinsic) was removed by the earlier commits, nothing in production references them, and the #1141/#1142 weights work builds on the binding classes (PR #1454 merged, PR #1559 open) rather than on these constants — whose underscore capability axis would never match _find_adapter's name-keyed scan anyway. test_rag_contracts.py / test_guardian_io_contract.py now look contracts up via get_io_contract(), exercising the registry directly. - _core.py NOTE(#1516): re-pointed at the remaining placeholder constructions (the core.py _REQUIREMENT_CHECK_ADAPTER test stub and the shims' _ShimWeightsBinding). - core.py: comment no longer assumes the rag/guardian sibling constants are kept. - build_prompt NotImplementedError message consolidated into the _BUILD_PROMPT_NOT_IMPLEMENTED constant (was five copies). - _util.py: call_intrinsic's Raises ValueError entry now covers well-formed JSON with a contract-rejected top-level shape. Assisted-by: opencode Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 34 ++++--- mellea/backends/adapters/io_contracts.py | 23 ++--- mellea/stdlib/components/intrinsic/_util.py | 5 +- mellea/stdlib/components/intrinsic/core.py | 5 +- .../stdlib/components/intrinsic/guardian.py | 50 +--------- mellea/stdlib/components/intrinsic/rag.py | 60 +----------- .../intrinsic/test_guardian_io_contract.py | 94 ++++++++++--------- .../intrinsic/test_rag_contracts.py | 79 ++++++++-------- 8 files changed, 123 insertions(+), 227 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index a54349724a..7592be928e 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -53,6 +53,11 @@ "{cls} is a Phase 0 stub; implementation lands in Epic #929 Phase 2." ) +_BUILD_PROMPT_NOT_IMPLEMENTED = ( + "build_prompt is not implemented; request construction still goes " + "through the legacy formatter/rewriter path, not IOContract." +) + class AdapterSchemaMismatchError(Exception): """Raised by :meth:`IOContract.parse` when output cannot satisfy the declared contract. @@ -165,10 +170,7 @@ def __init__(self, name: str, required_keys: frozenset[str]) -> None: self._required_keys = required_keys def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not implemented; request construction still goes " - "through the legacy formatter/rewriter path, not IOContract." - ) + raise NotImplementedError(_BUILD_PROMPT_NOT_IMPLEMENTED) def parse(self, raw: str) -> dict[str, object]: """Parse and validate dict-shaped adapter output. @@ -215,10 +217,7 @@ def __init__(self, name: str, required_item_keys: frozenset[str]) -> None: self._required_item_keys = required_item_keys def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not implemented; request construction still goes " - "through the legacy formatter/rewriter path, not IOContract." - ) + raise NotImplementedError(_BUILD_PROMPT_NOT_IMPLEMENTED) def parse(self, raw: str) -> dict[str, object]: """Parse and validate a list-of-dicts adapter output. @@ -723,13 +722,12 @@ class Adapter: # right invariant — the two feed different lookup paths (registration and the # verbs key on the binding's `qualified_name`; `_find_adapter` scans on the # identity) and both return `None` on a miss, so a disagreement surfaces as - # "adapter not found" far from its cause. But it cannot be enforced yet: the - # ten module-level `Adapter` constants in `stdlib/components/intrinsic/rag.py` - # and `guardian.py` pair an `alora` identity with a bare, deliberately - # unconfigured `LocalFileBinding()` that defaults to LoRA. Every catalogue - # entry supports both types, so those are placeholders rather than genuine - # conflicts, and the check fired on "not configured yet". #1516 gave those - # constants real `io_contract`s (the capability axis) but deliberately left - # `weights` untouched — the alora-vs-lora binding question is the deployment - # axis, out of #1516's scope and still open. Enforce this check once a - # follow-up gives those constants real bindings. + # "adapter not found" far from its cause. But it cannot be enforced yet: + # in-tree constructions still pair an `alora` identity with a placeholder + # binding — the `_REQUIREMENT_CHECK_ADAPTER` test stub in + # `stdlib/components/intrinsic/core.py` (a bare, deliberately unconfigured + # `LocalFileBinding()` that defaults to LoRA) and the deprecated shims' + # `_ShimWeightsBinding`. Every catalogue entry supports both types, so those + # are placeholders rather than genuine conflicts, and the check fired on + # "not configured yet". Enforce the check once those constructions carry + # real bindings (the shims retire in #1144). diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index 793cb3e740..93cd96e0e3 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -23,7 +23,13 @@ import math from ...core import Component -from ._core import AdapterSchemaMismatchError, IOContract, _DictContract, _ListContract +from ._core import ( + _BUILD_PROMPT_NOT_IMPLEMENTED, + AdapterSchemaMismatchError, + IOContract, + _DictContract, + _ListContract, +) from .catalog import known_intrinsic_names @@ -35,10 +41,7 @@ class _PolicyGuardrailsContract(IOContract): """ def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not implemented; request construction still goes " - "through the legacy formatter/rewriter path, not IOContract." - ) + raise NotImplementedError(_BUILD_PROMPT_NOT_IMPLEMENTED) def parse(self, raw: str) -> dict[str, object]: """Parse and validate policy-guardrails output. @@ -88,10 +91,7 @@ class _GuardianCheckContract(IOContract): """ def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not implemented; request construction still goes " - "through the legacy formatter/rewriter path, not IOContract." - ) + raise NotImplementedError(_BUILD_PROMPT_NOT_IMPLEMENTED) def parse(self, raw: str) -> dict[str, object]: """Parse and validate guardian-core output. @@ -140,10 +140,7 @@ class _RequirementCheckContract(IOContract): """ def build_prompt(self, **_kwargs: object) -> Component: - raise NotImplementedError( - "build_prompt is not implemented; request construction still goes " - "through the legacy formatter/rewriter path, not IOContract." - ) + raise NotImplementedError(_BUILD_PROMPT_NOT_IMPLEMENTED) def parse(self, raw: str) -> dict[str, object]: """Parse and validate requirement-check output. diff --git a/mellea/stdlib/components/intrinsic/_util.py b/mellea/stdlib/components/intrinsic/_util.py index 35054dba47..e4cd972329 100644 --- a/mellea/stdlib/components/intrinsic/_util.py +++ b/mellea/stdlib/components/intrinsic/_util.py @@ -202,8 +202,9 @@ def call_intrinsic( Raises: ValueError: When *context* forwards no history to the model (e.g. a - `SimpleContext` was passed), or when the model output is `None` or - is not valid JSON. + `SimpleContext` was passed), when the model output is `None` or is + not valid JSON, or when well-formed JSON has a top-level shape the + resolved adapter's contract rejects. AdapterSchemaMismatchError: When the model output is missing a field required by the resolved adapter's output contract. """ diff --git a/mellea/stdlib/components/intrinsic/core.py b/mellea/stdlib/components/intrinsic/core.py index bc2277f772..1d2d62626a 100644 --- a/mellea/stdlib/components/intrinsic/core.py +++ b/mellea/stdlib/components/intrinsic/core.py @@ -26,9 +26,8 @@ # io_contract is looked up by resolve_adapter() at call time # (mellea.backends.adapters.io_contracts), never declared here. Declaring one # unused by any caller would be exactly the parallel-declaration problem -# issue #1516 closes; the `rag.py`/`guardian.py` sibling constants are kept -# only as per-helper weights scaffolding for #1141/#1142, never as a second -# source of their contracts. +# issue #1516 closes — the `rag.py`/`guardian.py` sibling constants carried +# the same test-only status and were dropped for the same reason. # --------------------------------------------------------------------------- _REQUIREMENT_CHECK_ADAPTER = Adapter( diff --git a/mellea/stdlib/components/intrinsic/guardian.py b/mellea/stdlib/components/intrinsic/guardian.py index 68cb6cb887..5fa7bce514 100644 --- a/mellea/stdlib/components/intrinsic/guardian.py +++ b/mellea/stdlib/components/intrinsic/guardian.py @@ -16,13 +16,7 @@ import warnings from typing import cast -from ....backends.adapters import ( - Adapter, - AdapterMixin, - Identity, - LocalFileBinding, - get_io_contract, -) +from ....backends.adapters import AdapterMixin from ....core.utils import MelleaLogger from ...components import Document from ...context import ChatContext @@ -38,48 +32,6 @@ """Mapping used by the deprecated `target_role` path of `guardian_check`.""" -# --------------------------------------------------------------------------- -# Module-level Adapter constants (one per helper) -# -# io_contract is read from the same registry `resolve_adapter()` uses -# (mellea.backends.adapters.io_contracts) rather than declared here — a second, -# independent declaration is exactly the parallel-argument problem issue #1516 -# closes. See test_guardian_io_contract.py for contract-enforcement tests. -# -# The helper bodies no longer reference these constants: the contract travels -# with the adapter resolve_adapter() returns. They remain as the per-helper -# Adapter handles that #1141/#1142 will fill with real weights bindings. -# --------------------------------------------------------------------------- - -_POLICY_GUARDRAILS_ADAPTER = Adapter( - identity=Identity("policy-guardrails", "alora", capability="policy_guardrails"), - io_contract=get_io_contract("policy-guardrails"), - weights=LocalFileBinding(), -) - -_GUARDIAN_CHECK_ADAPTER = Adapter( - identity=Identity("guardian-core", "alora", capability="guardian_core"), - io_contract=get_io_contract("guardian-core"), - weights=LocalFileBinding(), -) - -_FACTUALITY_DETECTION_ADAPTER = Adapter( - identity=Identity( - "factuality-detection", "alora", capability="factuality_detection" - ), - io_contract=get_io_contract("factuality-detection"), - weights=LocalFileBinding(), -) - -_FACTUALITY_CORRECTION_ADAPTER = Adapter( - identity=Identity( - "factuality-correction", "alora", capability="factuality_correction" - ), - io_contract=get_io_contract("factuality-correction"), - weights=LocalFileBinding(), -) - - def policy_guardrails( context: ChatContext, backend: AdapterMixin, diff --git a/mellea/stdlib/components/intrinsic/rag.py b/mellea/stdlib/components/intrinsic/rag.py index 737a6692c8..9866bb7322 100644 --- a/mellea/stdlib/components/intrinsic/rag.py +++ b/mellea/stdlib/components/intrinsic/rag.py @@ -7,71 +7,13 @@ import warnings from typing import cast -from ....backends.adapters import ( - Adapter, - AdapterMixin, - Identity, - LocalFileBinding, - get_io_contract, -) +from ....backends.adapters import AdapterMixin from ...components import Document from ...context import ChatContext from ..chat import Message from ..docs.document import _coerce_to_document, _coerce_to_documents from ._util import _resolve_question, _resolve_response, call_intrinsic -# --------------------------------------------------------------------------- -# Module-level Adapter constants (one per helper) -# -# io_contract is read from the same registry `resolve_adapter()` uses -# (mellea.backends.adapters.io_contracts) rather than declared here — a second, -# independent declaration is exactly the parallel-argument problem issue #1516 -# closes. See test_rag_contracts.py for contract-enforcement tests. -# -# The helper bodies no longer reference these constants: the contract travels -# with the adapter resolve_adapter() returns. They remain as the per-helper -# Adapter handles that #1141/#1142 will fill with real weights bindings. -# --------------------------------------------------------------------------- - -_ANSWERABILITY_ADAPTER = Adapter( - identity=Identity("answerability", "alora", capability="answerability"), - io_contract=get_io_contract("answerability"), - weights=LocalFileBinding(), -) - -_QUERY_REWRITE_ADAPTER = Adapter( - identity=Identity("query_rewrite", "alora", capability="query_rewrite"), - io_contract=get_io_contract("query_rewrite"), - weights=LocalFileBinding(), -) - -_QUERY_CLARIFY_ADAPTER = Adapter( - identity=Identity("query_clarification", "alora", capability="query_clarification"), - io_contract=get_io_contract("query_clarification"), - weights=LocalFileBinding(), -) - -_CITATIONS_ADAPTER = Adapter( - identity=Identity("citations", "alora", capability="citations"), - io_contract=get_io_contract("citations"), - weights=LocalFileBinding(), -) - -_CONTEXT_RELEVANCE_ADAPTER = Adapter( - identity=Identity("context_relevance", "alora", capability="context_relevance"), - io_contract=get_io_contract("context_relevance"), - weights=LocalFileBinding(), -) - -_HALLUCINATION_ADAPTER = Adapter( - identity=Identity( - "hallucination_detection", "alora", capability="hallucination_detection" - ), - io_contract=get_io_contract("hallucination_detection"), - weights=LocalFileBinding(), -) - - # --------------------------------------------------------------------------- # High-level helper functions # --------------------------------------------------------------------------- diff --git a/test/stdlib/components/intrinsic/test_guardian_io_contract.py b/test/stdlib/components/intrinsic/test_guardian_io_contract.py index 53274550d3..db4fb92a03 100644 --- a/test/stdlib/components/intrinsic/test_guardian_io_contract.py +++ b/test/stdlib/components/intrinsic/test_guardian_io_contract.py @@ -5,9 +5,9 @@ #1516). The contracts are declared in `mellea.backends.adapters.io_contracts` and -carried by the `guardian.py` adapter constants via `get_io_contract`. -Tests the `parse()` method of each contract directly — no backend, -no GPU, no model download required. Two tests per helper: +looked up by catalog name via `get_io_contract`. Tests the `parse()` method +of each contract directly — no backend, no GPU, no model download required. +Two tests per helper: - `test__contract_enforced` — output missing a required field raises :class:`~mellea.backends.adapters.AdapterSchemaMismatchError`. @@ -20,14 +20,12 @@ import pytest -from mellea.backends.adapters import AdapterMixin, AdapterSchemaMismatchError -from mellea.stdlib.components.intrinsic import guardian -from mellea.stdlib.components.intrinsic.guardian import ( - _FACTUALITY_CORRECTION_ADAPTER, - _FACTUALITY_DETECTION_ADAPTER, - _GUARDIAN_CHECK_ADAPTER, - _POLICY_GUARDRAILS_ADAPTER, +from mellea.backends.adapters import ( + AdapterMixin, + AdapterSchemaMismatchError, + get_io_contract, ) +from mellea.stdlib.components.intrinsic import guardian from mellea.stdlib.context import ChatContext # --------------------------------------------------------------------------- @@ -36,8 +34,9 @@ def test_policy_guardrails_contract_enforced_neither_key() -> None: + contract = get_io_contract("policy-guardrails") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _POLICY_GUARDRAILS_ADAPTER.io_contract.parse(json.dumps({"wrong_key": "value"})) + contract.parse(json.dumps({"wrong_key": "value"})) err = exc_info.value assert err.name == "policy-guardrails" assert "label" in err.expected_keys @@ -45,31 +44,29 @@ def test_policy_guardrails_contract_enforced_neither_key() -> None: def test_policy_guardrails_contract_enforced_both_keys() -> None: + contract = get_io_contract("policy-guardrails") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _POLICY_GUARDRAILS_ADAPTER.io_contract.parse( - json.dumps({"label": "Yes", "score": "Yes"}) - ) + contract.parse(json.dumps({"label": "Yes", "score": "Yes"})) err = exc_info.value assert err.name == "policy-guardrails" def test_policy_guardrails_forward_compat_label() -> None: - result = _POLICY_GUARDRAILS_ADAPTER.io_contract.parse( - json.dumps({"label": "Yes", "extra": "ignored"}) - ) + contract = get_io_contract("policy-guardrails") + result = contract.parse(json.dumps({"label": "Yes", "extra": "ignored"})) assert result["label"] == "Yes" def test_policy_guardrails_forward_compat_score() -> None: - result = _POLICY_GUARDRAILS_ADAPTER.io_contract.parse( - json.dumps({"score": "No", "extra": "ignored"}) - ) + contract = get_io_contract("policy-guardrails") + result = contract.parse(json.dumps({"score": "No", "extra": "ignored"})) assert result["score"] == "No" def test_policy_guardrails_rejects_non_dict() -> None: + contract = get_io_contract("policy-guardrails") with pytest.raises(ValueError, match="must be a JSON object"): - _POLICY_GUARDRAILS_ADAPTER.io_contract.parse(json.dumps(["not", "a", "dict"])) + contract.parse(json.dumps(["not", "a", "dict"])) # --------------------------------------------------------------------------- @@ -78,26 +75,27 @@ def test_policy_guardrails_rejects_non_dict() -> None: def test_guardian_check_contract_enforced_missing_guardian_key() -> None: + contract = get_io_contract("guardian-core") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _GUARDIAN_CHECK_ADAPTER.io_contract.parse(json.dumps({"wrong_key": 0.5})) + contract.parse(json.dumps({"wrong_key": 0.5})) err = exc_info.value assert err.name == "guardian-core" assert "guardian" in err.expected_keys def test_guardian_check_contract_enforced_missing_score_in_guardian() -> None: + contract = get_io_contract("guardian-core") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _GUARDIAN_CHECK_ADAPTER.io_contract.parse( - json.dumps({"guardian": {"wrong_key": 0.5}}) - ) + contract.parse(json.dumps({"guardian": {"wrong_key": 0.5}})) err = exc_info.value assert err.name == "guardian-core" assert "score" in err.expected_keys def test_guardian_check_contract_enforced_guardian_not_dict() -> None: + contract = get_io_contract("guardian-core") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _GUARDIAN_CHECK_ADAPTER.io_contract.parse(json.dumps({"guardian": 0.8})) + contract.parse(json.dumps({"guardian": 0.8})) err = exc_info.value assert err.name == "guardian-core" assert "score" in err.expected_keys @@ -105,7 +103,8 @@ def test_guardian_check_contract_enforced_guardian_not_dict() -> None: def test_guardian_check_forward_compat() -> None: - result = _GUARDIAN_CHECK_ADAPTER.io_contract.parse( + contract = get_io_contract("guardian-core") + result = contract.parse( json.dumps({"guardian": {"score": 0.9}, "extra": "ignored"}) ) assert isinstance(result["guardian"], dict) @@ -113,8 +112,9 @@ def test_guardian_check_forward_compat() -> None: def test_guardian_check_rejects_non_dict() -> None: + contract = get_io_contract("guardian-core") with pytest.raises(ValueError, match="must be a JSON object"): - _GUARDIAN_CHECK_ADAPTER.io_contract.parse(json.dumps([0.5])) + contract.parse(json.dumps([0.5])) # --------------------------------------------------------------------------- @@ -123,25 +123,24 @@ def test_guardian_check_rejects_non_dict() -> None: def test_factuality_detection_contract_enforced() -> None: + contract = get_io_contract("factuality-detection") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _FACTUALITY_DETECTION_ADAPTER.io_contract.parse( - json.dumps({"wrong_key": "yes"}) - ) + contract.parse(json.dumps({"wrong_key": "yes"})) err = exc_info.value assert err.name == "factuality-detection" assert "score" in err.expected_keys def test_factuality_detection_forward_compat() -> None: - result = _FACTUALITY_DETECTION_ADAPTER.io_contract.parse( - json.dumps({"score": "yes", "confidence": 0.9}) - ) + contract = get_io_contract("factuality-detection") + result = contract.parse(json.dumps({"score": "yes", "confidence": 0.9})) assert result["score"] == "yes" def test_factuality_detection_rejects_non_dict() -> None: + contract = get_io_contract("factuality-detection") with pytest.raises(ValueError, match="must be a JSON object"): - _FACTUALITY_DETECTION_ADAPTER.io_contract.parse(json.dumps("yes")) + contract.parse(json.dumps("yes")) # --------------------------------------------------------------------------- @@ -150,27 +149,26 @@ def test_factuality_detection_rejects_non_dict() -> None: def test_factuality_correction_contract_enforced() -> None: + contract = get_io_contract("factuality-correction") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _FACTUALITY_CORRECTION_ADAPTER.io_contract.parse( - json.dumps({"wrong_key": "corrected text"}) - ) + contract.parse(json.dumps({"wrong_key": "corrected text"})) err = exc_info.value assert err.name == "factuality-correction" assert "correction" in err.expected_keys def test_factuality_correction_forward_compat() -> None: - result = _FACTUALITY_CORRECTION_ADAPTER.io_contract.parse( + contract = get_io_contract("factuality-correction") + result = contract.parse( json.dumps({"correction": "The correct answer is 42.", "score": 0.95}) ) assert result["correction"] == "The correct answer is 42." def test_factuality_correction_rejects_non_dict() -> None: + contract = get_io_contract("factuality-correction") with pytest.raises(ValueError, match="must be a JSON object"): - _FACTUALITY_CORRECTION_ADAPTER.io_contract.parse( - json.dumps(["not", "a", "dict"]) - ) + contract.parse(json.dumps(["not", "a", "dict"])) # --------------------------------------------------------------------------- @@ -179,26 +177,30 @@ def test_factuality_correction_rejects_non_dict() -> None: def test_policy_guardrails_error_mentions_adapter_name() -> None: + contract = get_io_contract("policy-guardrails") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _POLICY_GUARDRAILS_ADAPTER.io_contract.parse(json.dumps({})) + contract.parse(json.dumps({})) assert exc_info.value.name == "policy-guardrails" def test_guardian_check_error_mentions_adapter_name() -> None: + contract = get_io_contract("guardian-core") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _GUARDIAN_CHECK_ADAPTER.io_contract.parse(json.dumps({})) + contract.parse(json.dumps({})) assert exc_info.value.name == "guardian-core" def test_factuality_detection_error_mentions_adapter_name() -> None: + contract = get_io_contract("factuality-detection") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _FACTUALITY_DETECTION_ADAPTER.io_contract.parse(json.dumps({})) + contract.parse(json.dumps({})) assert exc_info.value.name == "factuality-detection" def test_factuality_correction_error_mentions_adapter_name() -> None: + contract = get_io_contract("factuality-correction") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _FACTUALITY_CORRECTION_ADAPTER.io_contract.parse(json.dumps({})) + contract.parse(json.dumps({})) assert exc_info.value.name == "factuality-correction" diff --git a/test/stdlib/components/intrinsic/test_rag_contracts.py b/test/stdlib/components/intrinsic/test_rag_contracts.py index 0ae1691046..0b1b6676d1 100644 --- a/test/stdlib/components/intrinsic/test_rag_contracts.py +++ b/test/stdlib/components/intrinsic/test_rag_contracts.py @@ -4,9 +4,9 @@ """Unit tests for the rag adapter functions' output contracts (Epic #929, #1516). The contracts are declared in `mellea.backends.adapters.io_contracts` and -carried by the `rag.py` adapter constants via `get_io_contract`. -Tests the `parse()` method of each contract directly — no backend, -no GPU, no model download required. Two tests per helper: +looked up by catalog name via `get_io_contract`. Tests the `parse()` method +of each contract directly — no backend, no GPU, no model download required. +Two tests per helper: - `test__contract_enforced` — output missing a required field raises :class:`~mellea.backends.adapters.AdapterSchemaMismatchError`. @@ -18,15 +18,7 @@ import pytest -from mellea.backends.adapters import AdapterSchemaMismatchError -from mellea.stdlib.components.intrinsic.rag import ( - _ANSWERABILITY_ADAPTER, - _CITATIONS_ADAPTER, - _CONTEXT_RELEVANCE_ADAPTER, - _HALLUCINATION_ADAPTER, - _QUERY_CLARIFY_ADAPTER, - _QUERY_REWRITE_ADAPTER, -) +from mellea.backends.adapters import AdapterSchemaMismatchError, get_io_contract # --------------------------------------------------------------------------- # check_answerability @@ -34,15 +26,17 @@ def test_check_answerability_contract_enforced() -> None: + contract = get_io_contract("answerability") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _ANSWERABILITY_ADAPTER.io_contract.parse(json.dumps({"wrong_key": "value"})) + contract.parse(json.dumps({"wrong_key": "value"})) err = exc_info.value assert err.name == "answerability" assert "answerability" in err.expected_keys def test_check_answerability_forward_compat() -> None: - result = _ANSWERABILITY_ADAPTER.io_contract.parse( + contract = get_io_contract("answerability") + result = contract.parse( json.dumps({"answerability": "answerable", "extra": "ignored"}) ) assert result["answerability"] == "answerable" @@ -54,15 +48,17 @@ def test_check_answerability_forward_compat() -> None: def test_rewrite_question_contract_enforced() -> None: + contract = get_io_contract("query_rewrite") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _QUERY_REWRITE_ADAPTER.io_contract.parse(json.dumps({"wrong_key": "value"})) + contract.parse(json.dumps({"wrong_key": "value"})) err = exc_info.value assert err.name == "query_rewrite" assert "rewritten_question" in err.expected_keys def test_rewrite_question_forward_compat() -> None: - result = _QUERY_REWRITE_ADAPTER.io_contract.parse( + contract = get_io_contract("query_rewrite") + result = contract.parse( json.dumps({"rewritten_question": "new query?", "confidence": 0.9}) ) assert result["rewritten_question"] == "new query?" @@ -74,17 +70,17 @@ def test_rewrite_question_forward_compat() -> None: def test_clarify_query_contract_enforced() -> None: + contract = get_io_contract("query_clarification") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _QUERY_CLARIFY_ADAPTER.io_contract.parse(json.dumps({"wrong_key": "value"})) + contract.parse(json.dumps({"wrong_key": "value"})) err = exc_info.value assert err.name == "query_clarification" assert "clarification" in err.expected_keys def test_clarify_query_forward_compat() -> None: - result = _QUERY_CLARIFY_ADAPTER.io_contract.parse( - json.dumps({"clarification": "CLEAR", "score": 1.0}) - ) + contract = get_io_contract("query_clarification") + result = contract.parse(json.dumps({"clarification": "CLEAR", "score": 1.0})) assert result["clarification"] == "CLEAR" @@ -106,8 +102,9 @@ def test_clarify_query_forward_compat() -> None: def test_find_citations_contract_enforced() -> None: bad_item = {k: v for k, v in _GOOD_CITATION.items() if k != "citation_doc_id"} + contract = get_io_contract("citations") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _CITATIONS_ADAPTER.io_contract.parse(json.dumps([bad_item])) + contract.parse(json.dumps([bad_item])) err = exc_info.value assert err.name == "citations" assert "citation_doc_id" in err.expected_keys @@ -115,7 +112,8 @@ def test_find_citations_contract_enforced() -> None: def test_find_citations_forward_compat() -> None: extra_item = {**_GOOD_CITATION, "extra_field": "ignored"} - result = _CITATIONS_ADAPTER.io_contract.parse(json.dumps([extra_item])) + contract = get_io_contract("citations") + result = contract.parse(json.dumps([extra_item])) assert result["items"][0]["citation_doc_id"] == "0" # type: ignore[index] @@ -125,17 +123,17 @@ def test_find_citations_forward_compat() -> None: def test_check_context_relevance_contract_enforced() -> None: + contract = get_io_contract("context_relevance") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _CONTEXT_RELEVANCE_ADAPTER.io_contract.parse(json.dumps({"wrong_key": "value"})) + contract.parse(json.dumps({"wrong_key": "value"})) err = exc_info.value assert err.name == "context_relevance" assert "context_relevance" in err.expected_keys def test_check_context_relevance_forward_compat() -> None: - result = _CONTEXT_RELEVANCE_ADAPTER.io_contract.parse( - json.dumps({"context_relevance": "relevant", "score": 0.8}) - ) + contract = get_io_contract("context_relevance") + result = contract.parse(json.dumps({"context_relevance": "relevant", "score": 0.8})) assert result["context_relevance"] == "relevant" @@ -155,8 +153,9 @@ def test_check_context_relevance_forward_compat() -> None: def test_flag_hallucinated_content_contract_enforced() -> None: bad_item = {k: v for k, v in _GOOD_SPAN.items() if k != "explanation"} + contract = get_io_contract("hallucination_detection") with pytest.raises(AdapterSchemaMismatchError) as exc_info: - _HALLUCINATION_ADAPTER.io_contract.parse(json.dumps([bad_item])) + contract.parse(json.dumps([bad_item])) err = exc_info.value assert err.name == "hallucination_detection" assert "explanation" in err.expected_keys @@ -164,7 +163,8 @@ def test_flag_hallucinated_content_contract_enforced() -> None: def test_flag_hallucinated_content_forward_compat() -> None: extra_item = {**_GOOD_SPAN, "extra_field": "ignored"} - result = _HALLUCINATION_ADAPTER.io_contract.parse(json.dumps([extra_item])) + contract = get_io_contract("hallucination_detection") + result = contract.parse(json.dumps([extra_item])) assert result["items"][0]["faithfulness"] == "faithful" # type: ignore[index] @@ -174,12 +174,14 @@ def test_flag_hallucinated_content_forward_compat() -> None: def test_find_citations_empty_list() -> None: - result = _CITATIONS_ADAPTER.io_contract.parse(json.dumps([])) + contract = get_io_contract("citations") + result = contract.parse(json.dumps([])) assert result == {"items": []} def test_flag_hallucinated_content_empty_list() -> None: - result = _HALLUCINATION_ADAPTER.io_contract.parse(json.dumps([])) + contract = get_io_contract("hallucination_detection") + result = contract.parse(json.dumps([])) assert result == {"items": []} @@ -189,27 +191,30 @@ def test_flag_hallucinated_content_empty_list() -> None: def test_dict_contract_rejects_non_dict() -> None: + contract = get_io_contract("answerability") with pytest.raises(ValueError, match="must be a JSON object"): - _ANSWERABILITY_ADAPTER.io_contract.parse(json.dumps(["not", "a", "dict"])) + contract.parse(json.dumps(["not", "a", "dict"])) def test_dict_contract_error_mentions_adapter_name() -> None: + contract = get_io_contract("answerability") with pytest.raises(ValueError, match="answerability"): - _ANSWERABILITY_ADAPTER.io_contract.parse(json.dumps(42)) + contract.parse(json.dumps(42)) def test_list_contract_rejects_non_list() -> None: + contract = get_io_contract("citations") with pytest.raises(ValueError, match="must be a JSON array"): - _CITATIONS_ADAPTER.io_contract.parse(json.dumps({"not": "a list"})) + contract.parse(json.dumps({"not": "a list"})) def test_list_contract_rejects_non_dict_element() -> None: + contract = get_io_contract("citations") with pytest.raises(ValueError, match="must contain only JSON objects"): - _CITATIONS_ADAPTER.io_contract.parse(json.dumps(["string_element"])) + contract.parse(json.dumps(["string_element"])) def test_list_contract_rejects_non_dict_element_after_valid_item() -> None: + contract = get_io_contract("citations") with pytest.raises(ValueError, match="must contain only JSON objects"): - _CITATIONS_ADAPTER.io_contract.parse( - json.dumps([_GOOD_CITATION, "string_element"]) - ) + contract.parse(json.dumps([_GOOD_CITATION, "string_element"])) From 156594bc2caee5176b931737814c980f15385ec6 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Thu, 20 Aug 2026 13:19:25 +0100 Subject: [PATCH 08/10] docs(intrinsics): document registered-adapter contract precedence call_intrinsic now parses via the resolved adapter's io_contract, so a user-registered Adapter for a catalog name takes precedence over the io_contracts registry for parsing. State that in the registry's module docstring and in call_intrinsic, where the single-source-of-truth claim otherwise overreaches for the registered-adapter path. Assisted-by: opencode Signed-off-by: Nigel Jones --- mellea/backends/adapters/io_contracts.py | 4 ++++ mellea/stdlib/components/intrinsic/_util.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index 93cd96e0e3..47ff978255 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -17,6 +17,10 @@ and the high-level helpers in `mellea.stdlib.components.intrinsic` read from it. Declaring a capability's contract anywhere else reintroduces the parallel-argument problem this module exists to close. + +An adapter a user registers with a backend takes precedence over the registry for its +own contract: `call_intrinsic` parses with whatever contract the resolved adapter +carries, and only adapters Mellea constructs itself (the lazy shims) get theirs from here. """ import json diff --git a/mellea/stdlib/components/intrinsic/_util.py b/mellea/stdlib/components/intrinsic/_util.py index e4cd972329..c2c3fcbc08 100644 --- a/mellea/stdlib/components/intrinsic/_util.py +++ b/mellea/stdlib/components/intrinsic/_util.py @@ -182,7 +182,10 @@ def call_intrinsic( executes via `mfuncs.act`. The resolved adapter's own `IOContract.parse` method is called on the raw output string before returning — the output contract always travels with the adapter that produced it, rather than as a - separate argument a caller could mismatch. The contract validates required fields and + separate argument a caller could mismatch. For adapters Mellea constructs (the lazy + shims) that contract comes from the `mellea.backends.adapters.io_contracts` registry; + for an adapter a user registered under the same name, the user's own `io_contract` + wins. The contract validates required fields and raises `AdapterSchemaMismatchError` on contract-breaking deltas; forward-compatible additions (extra optional fields) do not raise. From 320498ca793c221e832def265a62dc4da52d0b62 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Thu, 20 Aug 2026 17:38:05 +0100 Subject: [PATCH 09/10] =?UTF-8?q?docs(intrinsics):=20fix=20increment=20rev?= =?UTF-8?q?iew=20findings=20=E2=80=94=20stale=20docs,=20unguarded=20stub,?= =?UTF-8?q?=20missing=20negative=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes out the 3-party review of the follow-up increment (b88b17df..50ff6a59): - io_contracts.py: the module docstring still described the constant carrying model the increment deleted (and contradicted the precedence paragraph three lines below); re-point at the shims and the core.py test stub as the construction-time readers - _core.py: NOTE(#1516) claimed the shims would make the backed-out type-agreement check fire; _ShimWeightsBinding has no adapter_type to compare and shim identities track the configured type — split the two cases - test_core_schema.py: pin _REQUIREMENT_CHECK_ADAPTER to the registry instance (the is get_io_contract(...) guard test_shims.py established was not extended to the one non-shim construction that survived) - test_io_contracts.py: negative test for the context-attribution contract's required item keys (every sibling contract has one; shrinking the frozenset used to pass the whole suite) - rag.py / guardian.py: complete the Raises ValueError accuracy pass the increment started on core.py/_util.py — dict contracts are not a JSON object, list contracts are not a JSON array / non-object element - _util.py: the new Raises clause now covers the array element case, and says 'same capability' (the field _find_adapter actually matches) - small wording fixes: 'only other tests' (docs/examples e2e runs also exist), missing relative pronoun in two docstrings, and the issue reference form in the two rewritten test headers Assisted-by: opencode Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 17 +++++++++-------- mellea/backends/adapters/io_contracts.py | 10 ++++++---- mellea/stdlib/components/intrinsic/_util.py | 9 +++++---- mellea/stdlib/components/intrinsic/guardian.py | 11 +++++++---- mellea/stdlib/components/intrinsic/rag.py | 18 ++++++++++++------ .../test_adapters/test_io_contracts.py | 12 ++++++++++++ .../intrinsic/test_core_contracts.py | 8 ++++---- .../components/intrinsic/test_core_schema.py | 11 +++++++++-- .../intrinsic/test_guardian_io_contract.py | 2 +- .../components/intrinsic/test_rag_contracts.py | 2 +- .../components/intrinsic/test_util_unit.py | 2 +- 11 files changed, 67 insertions(+), 35 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 7592be928e..926fc0591b 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -723,11 +723,12 @@ class Adapter: # verbs key on the binding's `qualified_name`; `_find_adapter` scans on the # identity) and both return `None` on a miss, so a disagreement surfaces as # "adapter not found" far from its cause. But it cannot be enforced yet: - # in-tree constructions still pair an `alora` identity with a placeholder - # binding — the `_REQUIREMENT_CHECK_ADAPTER` test stub in - # `stdlib/components/intrinsic/core.py` (a bare, deliberately unconfigured - # `LocalFileBinding()` that defaults to LoRA) and the deprecated shims' - # `_ShimWeightsBinding`. Every catalogue entry supports both types, so those - # are placeholders rather than genuine conflicts, and the check fired on - # "not configured yet". Enforce the check once those constructions carry - # real bindings (the shims retire in #1144). + # the `_REQUIREMENT_CHECK_ADAPTER` test stub in + # `stdlib/components/intrinsic/core.py` pairs an `alora` identity with a + # bare, deliberately unconfigured `LocalFileBinding()` that defaults to + # LoRA (every catalogue entry supports both types, so a placeholder rather + # than a genuine conflict — the check fired on "not configured yet"), and + # the deprecated shims carry a `_ShimWeightsBinding` with no `adapter_type` + # to compare at all (their identity tracks the configured type). Enforce + # the check once those constructions carry real, typed bindings (the shims + # retire in #1144). diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index 47ff978255..abdb91ff71 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -13,10 +13,12 @@ :data:`_INTRINSIC_IO_CONTRACTS` is the single source of truth instead: it is keyed by the adapter function's catalog name (:attr:`~mellea.backends.adapters.catalog.IntrinsicsCatalogEntry.name`, e.g. `"guardian-core"` — the same string passed to `call_intrinsic` and -`resolve_adapter`), and both the shim adapters in `adapter.py` (via :func:`get_io_contract`) -and the high-level helpers in `mellea.stdlib.components.intrinsic` read from it. Declaring -a capability's contract anywhere else reintroduces the parallel-argument problem this -module exists to close. +`resolve_adapter`). The shim adapters in `adapter.py` and the +`_REQUIREMENT_CHECK_ADAPTER` test stub in `mellea/stdlib/components/intrinsic/core.py` +read from it at construction (via :func:`get_io_contract`); the high-level helpers +reach it indirectly, through the adapter `resolve_adapter` returns. Declaring a +capability's contract anywhere else reintroduces the parallel-argument problem +this module exists to close. An adapter a user registers with a backend takes precedence over the registry for its own contract: `call_intrinsic` parses with whatever contract the resolved adapter diff --git a/mellea/stdlib/components/intrinsic/_util.py b/mellea/stdlib/components/intrinsic/_util.py index c2c3fcbc08..0b10349543 100644 --- a/mellea/stdlib/components/intrinsic/_util.py +++ b/mellea/stdlib/components/intrinsic/_util.py @@ -184,8 +184,8 @@ def call_intrinsic( output contract always travels with the adapter that produced it, rather than as a separate argument a caller could mismatch. For adapters Mellea constructs (the lazy shims) that contract comes from the `mellea.backends.adapters.io_contracts` registry; - for an adapter a user registered under the same name, the user's own `io_contract` - wins. The contract validates required fields and + for an adapter a user registered under the same capability, the user's own + `io_contract` wins. The contract validates required fields and raises `AdapterSchemaMismatchError` on contract-breaking deltas; forward-compatible additions (extra optional fields) do not raise. @@ -206,8 +206,9 @@ def call_intrinsic( Raises: ValueError: When *context* forwards no history to the model (e.g. a `SimpleContext` was passed), when the model output is `None` or is - not valid JSON, or when well-formed JSON has a top-level shape the - resolved adapter's contract rejects. + not valid JSON, or when well-formed JSON is rejected for shape by + the resolved adapter's contract (wrong top-level type, or a + non-object element of an array-shaped contract). AdapterSchemaMismatchError: When the model output is missing a field required by the resolved adapter's output contract. """ diff --git a/mellea/stdlib/components/intrinsic/guardian.py b/mellea/stdlib/components/intrinsic/guardian.py index 5fa7bce514..9822336049 100644 --- a/mellea/stdlib/components/intrinsic/guardian.py +++ b/mellea/stdlib/components/intrinsic/guardian.py @@ -59,7 +59,8 @@ def policy_guardrails( Compliance label as `"Yes"`, `"No"`, or `"Ambiguous"` (`"Yes"` = compliant). Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When neither or both of `label` / `score` are present. """ result_json = call_intrinsic( @@ -221,7 +222,7 @@ def guardian_check( Raises: TypeError: When both `scoring_schema` and `target_role` are provided. ValueError: When `target_role` is not `"user"` or `"assistant"`, or - when the model output is not valid JSON. + when the model output is not valid JSON or is not a JSON object. AdapterSchemaMismatchError: When the model output is missing the required `guardian` key or its nested `score` key. """ @@ -316,7 +317,8 @@ def factuality_detection( Raises: ValueError: If `documents` is provided but the last assistant response cannot be extracted (empty context, non-assistant last turn, or - uncomputed response). Also raised when the model output is not valid JSON. + uncomputed response). Also raised when the model output is not valid + JSON, or is not a JSON object. AdapterSchemaMismatchError: When the model output is missing the required `score` field. """ @@ -373,7 +375,8 @@ def factuality_correction( Raises: ValueError: If `documents` is provided but the last assistant response cannot be extracted (empty context, non-assistant last turn, or - uncomputed response). Also raised when the model output is not valid JSON. + uncomputed response). Also raised when the model output is not valid + JSON, or is not a JSON object. AdapterSchemaMismatchError: When the model output is missing the required `correction` field. """ diff --git a/mellea/stdlib/components/intrinsic/rag.py b/mellea/stdlib/components/intrinsic/rag.py index 9866bb7322..0232480e5f 100644 --- a/mellea/stdlib/components/intrinsic/rag.py +++ b/mellea/stdlib/components/intrinsic/rag.py @@ -54,7 +54,8 @@ def check_answerability( A string value of either `"answerable"` or `"unanswerable"`. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When the model output is missing the required `answerability` field. """ @@ -100,7 +101,8 @@ def rewrite_question( Rewritten version of `question`. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When the model output is missing the required `rewritten_question` field. """ @@ -150,7 +152,8 @@ def clarify_query( the string `"CLEAR"` if no clarification is needed. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When the model output is missing the required `clarification` field. """ @@ -209,7 +212,8 @@ def find_citations( character offsets into their respective UTF-8 strings. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON, is not a + JSON array, or contains a non-object element. AdapterSchemaMismatchError: When any record in the output is missing a required field. """ @@ -273,7 +277,8 @@ def check_context_relevance( `"relevant"`, `"irrelevant"`, or `"partially relevant"`. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON or is not a + JSON object. AdapterSchemaMismatchError: When the model output is missing the required `context_relevance` field. """ @@ -340,7 +345,8 @@ def flag_hallucinated_content( `response_text`, `faithfulness`, `explanation`. Raises: - ValueError: When the model output is not valid JSON. + ValueError: When the model output is not valid JSON, is not a + JSON array, or contains a non-object element. AdapterSchemaMismatchError: When any record in the output is missing a required field. """ diff --git a/test/backends/test_adapters/test_io_contracts.py b/test/backends/test_adapters/test_io_contracts.py index 587d063c5c..3d6d5031c2 100644 --- a/test/backends/test_adapters/test_io_contracts.py +++ b/test/backends/test_adapters/test_io_contracts.py @@ -178,3 +178,15 @@ def test_context_attribution_contract_accepts_recorded_model_output(): assert len(result["items"]) == 7 # type: ignore[arg-type] assert result["items"][0]["attribution_msg_index"] is None # type: ignore[index] + + +def test_context_attribution_contract_rejects_missing_item_key(): + """A record missing a required item key raises AdapterSchemaMismatchError.""" + fixture = _INTRINSIC_TESTDATA / "output_json" / "context-attribution.json" + completion = json.loads(fixture.read_text(encoding="utf-8")) + raw = completion["choices"][0]["message"]["content"] + items = json.loads(raw) + del items[0]["attribution_text"] + + with pytest.raises(AdapterSchemaMismatchError): + _INTRINSIC_IO_CONTRACTS["context-attribution"].parse(json.dumps(items)) diff --git a/test/stdlib/components/intrinsic/test_core_contracts.py b/test/stdlib/components/intrinsic/test_core_contracts.py index fc0c07c9ce..1a1b8fec06 100644 --- a/test/stdlib/components/intrinsic/test_core_contracts.py +++ b/test/stdlib/components/intrinsic/test_core_contracts.py @@ -6,10 +6,10 @@ Both helpers moved from raw `json.loads` to registry-contract validation when `call_intrinsic` started parsing via the resolved adapter's contract, and `find_context_attributions` additionally changed return shape (unwrap of -`"items"`). Their only other tests are GPU-gated qualitative/xfail tests in -`test_core.py`, which give no CI signal on that wiring — these stub -`mfuncs.act` and `backend.resolve_adapter` (the `test_core_schema.py` pattern) -and exercise the helper boundary itself. +`"items"`). Their other tests — GPU-gated qualitative/xfail tests in +`test_core.py` plus opt-in e2e runs in `docs/examples` — give no CI signal on +that wiring, so these stub `mfuncs.act` and `backend.resolve_adapter` (the +`test_core_schema.py` pattern) and exercise the helper boundary itself. """ import json diff --git a/test/stdlib/components/intrinsic/test_core_schema.py b/test/stdlib/components/intrinsic/test_core_schema.py index 400f1fba02..a7f1da92c8 100644 --- a/test/stdlib/components/intrinsic/test_core_schema.py +++ b/test/stdlib/components/intrinsic/test_core_schema.py @@ -5,7 +5,7 @@ Issue #1516 replaced the hand-rolled score-range validation `requirement_check` used to run after `call_intrinsic` with the `requirement-check` capability's declared -`IOContract`, obtained from the adapter `resolve_adapter` returns rather than passed +`IOContract`, obtained from the adapter that `resolve_adapter()` returns rather than passed as a parallel argument. These tests exercise that real path — `call_intrinsic` and `IOContract.parse` both run against a stubbed `resolve_adapter` return value — by mocking `mfuncs.act` (the actual model call) and `backend.resolve_adapter` (a bare @@ -22,7 +22,7 @@ import pytest -from mellea.backends.adapters import AdapterSchemaMismatchError +from mellea.backends.adapters import AdapterSchemaMismatchError, get_io_contract from mellea.stdlib.components import Message from mellea.stdlib.components.intrinsic import _util, core from mellea.stdlib.components.intrinsic.core import _REQUIREMENT_CHECK_ADAPTER @@ -99,3 +99,10 @@ def fake_act(_intrinsic, context, _backend, *, model_options=None, **_kwargs): context = ChatContext().add(Message("user", "hi")) core.requirement_check(context, backend, _REQUIREMENT) backend.resolve_adapter.assert_called_once_with("requirement-check") + + +def test_requirement_check_adapter_carries_registry_contract(): + """The stub must carry the registry's declared contract, not a local copy.""" + assert _REQUIREMENT_CHECK_ADAPTER.io_contract is get_io_contract( + "requirement-check" + ) diff --git a/test/stdlib/components/intrinsic/test_guardian_io_contract.py b/test/stdlib/components/intrinsic/test_guardian_io_contract.py index db4fb92a03..5b194c02fb 100644 --- a/test/stdlib/components/intrinsic/test_guardian_io_contract.py +++ b/test/stdlib/components/intrinsic/test_guardian_io_contract.py @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 """Unit tests for the guardian adapter functions' output contracts (Epic #929, -#1516). +issue #1516). The contracts are declared in `mellea.backends.adapters.io_contracts` and looked up by catalog name via `get_io_contract`. Tests the `parse()` method diff --git a/test/stdlib/components/intrinsic/test_rag_contracts.py b/test/stdlib/components/intrinsic/test_rag_contracts.py index 0b1b6676d1..79026b1123 100644 --- a/test/stdlib/components/intrinsic/test_rag_contracts.py +++ b/test/stdlib/components/intrinsic/test_rag_contracts.py @@ -1,7 +1,7 @@ # Copyright IBM Corp. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for the rag adapter functions' output contracts (Epic #929, #1516). +"""Unit tests for the rag adapter functions' output contracts (Epic #929, issue #1516). The contracts are declared in `mellea.backends.adapters.io_contracts` and looked up by catalog name via `get_io_contract`. Tests the `parse()` method diff --git a/test/stdlib/components/intrinsic/test_util_unit.py b/test/stdlib/components/intrinsic/test_util_unit.py index 93c5d65acc..1bdadd1860 100644 --- a/test/stdlib/components/intrinsic/test_util_unit.py +++ b/test/stdlib/components/intrinsic/test_util_unit.py @@ -7,7 +7,7 @@ Exercises the model_options precedence without a real backend or model — guards against the PR #972 bug class (caller-supplied model_options silently discarded behind a hardcoded default) resurfacing. Also covers the issue #1516 -change: the output contract is taken from the adapter `resolve_adapter()` +change: the output contract is taken from the adapter that `resolve_adapter()` returns, and `call_intrinsic` no longer accepts an `io_contract` kwarg. """ From 40f95ca7995b9a59db8a4b0013f9d6cfe450fe06 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Fri, 21 Aug 2026 15:44:47 +0100 Subject: [PATCH 10/10] fix(intrinsics): address remaining contract review feedback Move the requirement-check adapter stub into its test, scope registry invariants to built-in catalogue entries, and distinguish policy-guardrails exclusivity errors.\n\nAssisted-by: Codex Signed-off-by: Nigel Jones --- mellea/backends/adapters/_core.py | 24 +++++---- mellea/backends/adapters/io_contracts.py | 30 +++++------- mellea/stdlib/components/intrinsic/core.py | 26 +--------- .../backends/test_adapters/test_core_types.py | 8 ++- .../test_adapters/test_io_contracts.py | 49 +++++++++++++++++-- .../components/intrinsic/test_core_schema.py | 19 ++++++- 6 files changed, 96 insertions(+), 60 deletions(-) diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 926fc0591b..0552f5bfdc 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -66,24 +66,33 @@ class AdapterSchemaMismatchError(Exception): name (str): Name of the adapter whose contract was violated. observed_keys (frozenset[str]): Keys present in the observed output. expected_keys (frozenset[str]): Keys required by the contract. + reason (str | None): Capability-specific explanation of the mismatch. """ def __init__( - self, name: str, observed_keys: frozenset[str], expected_keys: frozenset[str] + self, + name: str, + observed_keys: frozenset[str], + expected_keys: frozenset[str], + reason: str | None = None, ) -> None: self.name = name self.observed_keys = observed_keys self.expected_keys = expected_keys - # Pass the structured fields (not the formatted message) to Exception so - # that ``self.args`` round-trips through ``pickle`` / ``copy`` — the default - # ``Exception.__reduce__`` reconstructs by calling ``cls(*self.args)``. + # Preserve the existing three-item ``args`` shape for callers and + # cross-version pickle compatibility. ``reason`` lives in instance state, + # which pickle restores after calling this constructor with ``args``. + self.reason = reason super().__init__(name, observed_keys, expected_keys) def __str__(self) -> str: - return ( + message = ( f"Adapter '{self.name}' output cannot satisfy declared contract. " f"Observed keys: {self.observed_keys}; expected: {self.expected_keys}." ) + if self.reason is not None: + message += f" Reason: {self.reason}." + return message @dataclass(frozen=True) @@ -723,11 +732,6 @@ class Adapter: # verbs key on the binding's `qualified_name`; `_find_adapter` scans on the # identity) and both return `None` on a miss, so a disagreement surfaces as # "adapter not found" far from its cause. But it cannot be enforced yet: - # the `_REQUIREMENT_CHECK_ADAPTER` test stub in - # `stdlib/components/intrinsic/core.py` pairs an `alora` identity with a - # bare, deliberately unconfigured `LocalFileBinding()` that defaults to - # LoRA (every catalogue entry supports both types, so a placeholder rather - # than a genuine conflict — the check fired on "not configured yet"), and # the deprecated shims carry a `_ShimWeightsBinding` with no `adapter_type` # to compare at all (their identity tracks the configured type). Enforce # the check once those constructions carry real, typed bindings (the shims diff --git a/mellea/backends/adapters/io_contracts.py b/mellea/backends/adapters/io_contracts.py index abdb91ff71..91ff2c59cd 100644 --- a/mellea/backends/adapters/io_contracts.py +++ b/mellea/backends/adapters/io_contracts.py @@ -13,12 +13,10 @@ :data:`_INTRINSIC_IO_CONTRACTS` is the single source of truth instead: it is keyed by the adapter function's catalog name (:attr:`~mellea.backends.adapters.catalog.IntrinsicsCatalogEntry.name`, e.g. `"guardian-core"` — the same string passed to `call_intrinsic` and -`resolve_adapter`). The shim adapters in `adapter.py` and the -`_REQUIREMENT_CHECK_ADAPTER` test stub in `mellea/stdlib/components/intrinsic/core.py` -read from it at construction (via :func:`get_io_contract`); the high-level helpers -reach it indirectly, through the adapter `resolve_adapter` returns. Declaring a -capability's contract anywhere else reintroduces the parallel-argument problem -this module exists to close. +`resolve_adapter`). The shim adapters in `adapter.py` read from it at construction +(via :func:`get_io_contract`); the high-level helpers reach it indirectly, through +the adapter `resolve_adapter` returns. Declaring a capability's contract anywhere +else reintroduces the parallel-argument problem this module exists to close. An adapter a user registers with a backend takes precedence over the registry for its own contract: `call_intrinsic` parses with whatever contract the resolved adapter @@ -70,21 +68,19 @@ def parse(self, raw: str) -> dict[str, object]: ) has_label = "label" in data has_score = "score" in data - # Both branches raise the same exception shape; `observed_keys` (all of - # `data`'s keys) is what distinguishes them when printed — neither - # "label" nor "score" appears for the missing-both case, and both do - # for the has-both case. if not has_label and not has_score: raise AdapterSchemaMismatchError( "policy-guardrails", frozenset(data.keys()), frozenset({"label", "score"}), + "neither `label` nor `score` was present", ) if has_label and has_score: raise AdapterSchemaMismatchError( "policy-guardrails", frozenset(data.keys()), frozenset({"label", "score"}), + "both `label` and `score` were present", ) return data @@ -249,16 +245,16 @@ def parse(self, raw: str) -> dict[str, object]: ), ), } -# Canonical output contract for every catalogued adapter function, keyed by its -# catalog `name` (see module docstring). Kept exhaustive over -# known_intrinsic_names() by test/backends/test_adapters/test_io_contracts.py, -# and enforced below at import time — mirroring the +# Canonical output contract for every built-in adapter function, keyed by its +# catalog `name` (see module docstring). Capture the built-in names before +# `CustomIntrinsicAdapter` can extend the global catalogue at runtime, then keep +# the registry exhaustive over that fixed set at import time — mirroring the # duplicate-`effective_capability` check in catalog.py. - -_missing_contracts = set(known_intrinsic_names()) - set(_INTRINSIC_IO_CONTRACTS) +_BUILTIN_INTRINSIC_NAMES = frozenset(known_intrinsic_names()) +_missing_contracts = _BUILTIN_INTRINSIC_NAMES - set(_INTRINSIC_IO_CONTRACTS) if _missing_contracts: raise ValueError( - f"Catalogued adapter functions with no declared IOContract: " + f"Built-in catalogued adapter functions with no declared IOContract: " f"{_missing_contracts}" ) del _missing_contracts diff --git a/mellea/stdlib/components/intrinsic/core.py b/mellea/stdlib/components/intrinsic/core.py index 1d2d62626a..bd75f4a3e9 100644 --- a/mellea/stdlib/components/intrinsic/core.py +++ b/mellea/stdlib/components/intrinsic/core.py @@ -6,36 +6,12 @@ import collections.abc from typing import cast -from ....backends.adapters import ( - Adapter, - AdapterMixin, - Identity, - LocalFileBinding, - get_io_contract, -) +from ....backends.adapters import AdapterMixin from ...components import Document, Message from ...context import ChatContext from ..docs.document import _coerce_to_documents from ._util import _resolve_response, call_intrinsic -# --------------------------------------------------------------------------- -# _REQUIREMENT_CHECK_ADAPTER: a test-visible handle onto the `requirement-check` -# capability's canonical Adapter, used by test_core_schema.py to stub -# `resolve_adapter`'s return value. `check_certainty` and `find_context_attributions` -# have no equivalent constant — nothing outside a test needs one, since their -# io_contract is looked up by resolve_adapter() at call time -# (mellea.backends.adapters.io_contracts), never declared here. Declaring one -# unused by any caller would be exactly the parallel-declaration problem -# issue #1516 closes — the `rag.py`/`guardian.py` sibling constants carried -# the same test-only status and were dropped for the same reason. -# --------------------------------------------------------------------------- - -_REQUIREMENT_CHECK_ADAPTER = Adapter( - identity=Identity("requirement-check", "alora", capability="requirement_check"), - io_contract=get_io_contract("requirement-check"), - weights=LocalFileBinding(), -) - def check_certainty( context: ChatContext, backend: AdapterMixin, model_options: dict | None = None diff --git a/test/backends/test_adapters/test_core_types.py b/test/backends/test_adapters/test_core_types.py index 5d3557a3dd..72fe311ac3 100644 --- a/test/backends/test_adapters/test_core_types.py +++ b/test/backends/test_adapters/test_core_types.py @@ -143,6 +143,7 @@ def test_adapter_schema_mismatch_error_format(): assert err.name == "answerability" assert err.observed_keys == observed assert err.expected_keys == expected + assert err.reason is None msg = str(err) assert "answerability" in msg assert "Observed keys:" in msg @@ -153,7 +154,10 @@ def test_adapter_schema_mismatch_error_pickles(): observed = frozenset({"key_a"}) expected = frozenset({"key_b"}) err = AdapterSchemaMismatchError( - name="answerability", observed_keys=observed, expected_keys=expected + name="answerability", + observed_keys=observed, + expected_keys=expected, + reason="schema changed", ) restored = pickle.loads(pickle.dumps(err)) @@ -162,6 +166,8 @@ def test_adapter_schema_mismatch_error_pickles(): assert restored.name == "answerability" assert restored.observed_keys == observed assert restored.expected_keys == expected + assert restored.reason == "schema changed" + assert restored.args == ("answerability", observed, expected) assert str(restored) == str(err) diff --git a/test/backends/test_adapters/test_io_contracts.py b/test/backends/test_adapters/test_io_contracts.py index 3d6d5031c2..b02a694418 100644 --- a/test/backends/test_adapters/test_io_contracts.py +++ b/test/backends/test_adapters/test_io_contracts.py @@ -14,9 +14,13 @@ import pytest -from mellea.backends.adapters import AdapterSchemaMismatchError -from mellea.backends.adapters.catalog import known_intrinsic_names +from mellea.backends.adapters import AdapterSchemaMismatchError, catalog +from mellea.backends.adapters.catalog import ( + IntrinsicsCatalogEntry, + known_intrinsic_names, +) from mellea.backends.adapters.io_contracts import ( + _BUILTIN_INTRINSIC_NAMES, _INTRINSIC_IO_CONTRACTS, get_io_contract, ) @@ -40,7 +44,7 @@ def test_registry_covers_every_catalog_name(): Regression guard for the acceptance criterion that no adapter reachable through `resolve_adapter` carries an unimplemented placeholder contract. """ - missing = set(known_intrinsic_names()) - set(_INTRINSIC_IO_CONTRACTS) + missing = _BUILTIN_INTRINSIC_NAMES - set(_INTRINSIC_IO_CONTRACTS) assert missing == set() @@ -51,15 +55,32 @@ def test_registry_has_no_orphan_keys(): would be served by `get_io_contract` ahead of the permissive fallback if a later adapter ever registered under that name. """ - orphans = set(_INTRINSIC_IO_CONTRACTS) - set(known_intrinsic_names()) + orphans = set(_INTRINSIC_IO_CONTRACTS) - _BUILTIN_INTRINSIC_NAMES assert orphans == set() def test_get_io_contract_returns_declared_instance_for_known_names(): - for name in known_intrinsic_names(): + for name in _BUILTIN_INTRINSIC_NAMES: assert get_io_contract(name) is _INTRINSIC_IO_CONTRACTS[name] +def test_registry_invariant_ignores_runtime_custom_catalogue_entry(monkeypatch): + """A custom adapter must not make the built-in registry checks order-dependent.""" + custom_name = "custom-user-adapter" + custom_entry = IntrinsicsCatalogEntry( + name=custom_name, repo_id="example/custom-user-adapter", revision="main" + ) + monkeypatch.setattr( + catalog, + "_INTRINSICS_CATALOG", + {**catalog._INTRINSICS_CATALOG, custom_name: custom_entry}, + ) + + assert custom_name in known_intrinsic_names() + assert custom_name not in _BUILTIN_INTRINSIC_NAMES + assert _BUILTIN_INTRINSIC_NAMES - set(_INTRINSIC_IO_CONTRACTS) == set() + + def test_get_io_contract_falls_back_permissively_for_unknown_names(): """A name outside the catalog (e.g. a `CustomIntrinsicAdapter`) must not raise.""" contract = get_io_contract("some-custom-user-adapter") @@ -190,3 +211,21 @@ def test_context_attribution_contract_rejects_missing_item_key(): with pytest.raises(AdapterSchemaMismatchError): _INTRINSIC_IO_CONTRACTS["context-attribution"].parse(json.dumps(items)) + + +@pytest.mark.parametrize( + ("output", "reason"), + [ + ({"other": "value"}, "neither `label` nor `score` was present"), + ({"label": "Yes", "score": "No"}, "both `label` and `score` were present"), + ], +) +def test_policy_guardrails_contract_distinguishes_exclusivity_failures(output, reason): + """Neither-key and both-key output errors must identify the failed condition.""" + contract = _INTRINSIC_IO_CONTRACTS["policy-guardrails"] + + with pytest.raises(AdapterSchemaMismatchError) as exc_info: + contract.parse(json.dumps(output)) + + assert exc_info.value.reason == reason + assert reason in str(exc_info.value) diff --git a/test/stdlib/components/intrinsic/test_core_schema.py b/test/stdlib/components/intrinsic/test_core_schema.py index a7f1da92c8..8cb0fcfd5f 100644 --- a/test/stdlib/components/intrinsic/test_core_schema.py +++ b/test/stdlib/components/intrinsic/test_core_schema.py @@ -22,13 +22,23 @@ import pytest -from mellea.backends.adapters import AdapterSchemaMismatchError, get_io_contract +from mellea.backends.adapters import ( + Adapter, + AdapterSchemaMismatchError, + Identity, + LocalFileBinding, + get_io_contract, +) from mellea.stdlib.components import Message from mellea.stdlib.components.intrinsic import _util, core -from mellea.stdlib.components.intrinsic.core import _REQUIREMENT_CHECK_ADAPTER from mellea.stdlib.context import ChatContext _REQUIREMENT = "must be polite" +_REQUIREMENT_CHECK_ADAPTER = Adapter( + identity=Identity("requirement-check", "alora", capability="requirement_check"), + io_contract=get_io_contract("requirement-check"), + weights=LocalFileBinding(), +) def _call(result_dict: dict, monkeypatch) -> float: @@ -106,3 +116,8 @@ def test_requirement_check_adapter_carries_registry_contract(): assert _REQUIREMENT_CHECK_ADAPTER.io_contract is get_io_contract( "requirement-check" ) + + +def test_core_module_does_not_define_requirement_check_adapter(): + """The adapter stub belongs to this test, not production core code.""" + assert not hasattr(core, "_REQUIREMENT_CHECK_ADAPTER")