refactor(intrinsics): resolve the adapter's output contract from the adapter, not a parallel argument - #1556
Merged
Merged
Conversation
jakelorocco
reviewed
Aug 20, 2026
Comment on lines
+258
to
+264
| _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 |
Contributor
There was a problem hiding this comment.
Won't this cause a missing contract to prevent importing mellea? Maybe that's okay but I feel like a warning might be better? Or just enforcing this for existing adapters through a test?
Contributor
Author
There was a problem hiding this comment.
I think it is still useful to keep this strict: a missing contract for a built-in adapter would otherwise fall back to the undeclared generic contract. Custom adapters remain covered by the fallback path.
| # 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)) |
Contributor
There was a problem hiding this comment.
Should we extract references to "requirement-check" here and in the backends into a constant?
Contributor
Author
There was a problem hiding this comment.
This is the catalogue key, and lookup is already done by literals, so I do not think a constant adds much here.
… construction onto separate lines Prep step for Epic generative-computing#929 Phase 2: generative-computing#1516 and generative-computing#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 <jonesn@uk.ibm.com>
…gistry 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 <jonesn@uk.ibm.com>
…ot a parallel argument 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 generative-computing#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 generative-computing#1516 Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…d constants, fix real duplicate
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 <jonesn@uk.ibm.com>
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 <jonesn@uk.ibm.com>
Applies the multi-party review findings on this PR (issue generative-computing#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, generative-computing#1516) - rag.py / guardian.py / core.py: state consistently that the per-helper Adapter constants are weights scaffolding for generative-computing#1141/generative-computing#1142, not a second contract source Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…date build_prompt message Continues the multi-party review follow-ups on this PR (issue generative-computing#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 generative-computing#1141/generative-computing#1142 weights work builds on the binding classes (PR generative-computing#1454 merged, PR generative-computing#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(generative-computing#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 <jonesn@uk.ibm.com>
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 <jonesn@uk.ibm.com>
…ed stub, missing negative test Closes out the 3-party review of the follow-up increment (b88b17d..50ff6a5): - 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(generative-computing#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 <jonesn@uk.ibm.com>
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 <jonesn@uk.ibm.com>
jakelorocco
approved these changes
Aug 24, 2026
Merged
via the queue into
generative-computing:main
with commit Aug 25, 2026
8d353c6
11 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Issue
Fixes #1516.
Description
Adapter functions were resolved separately from the output contract used to parse their response. Callers supplied a parallel
io_contractargument, so nothing prevented a capability from being paired with the wrong schema; core helpers also bypassed contract validation entirely.This PR makes the resolved adapter the source of its output contract. It registers real contracts on adapters, removes the parallel argument and per-helper constants, and gives the core intrinsic helpers the same validated contracts as the RAG and Guardian helpers.
Where this fits
This is Phase 2 work for Epic #929 and implements the output-contract axis from design discussion #1486. It runs independently of #1142, #1465, #1466, and #1528, and is a prerequisite for the shim-removal and tutorial work in #1144. Schema versioning remains with #1111.
What changed
resolve_adapter().io_contractparameter and redundant caller-side adapter constants.check_certainty,requirement_check, and context-attribution helpers.requirement_check's duplicated post-call validation with its contract.Testing
Focused adapter and intrinsic tests, Ruff, mypy, and documentation checks have been run locally.
Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.
NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.