Skip to content

refactor(intrinsics): resolve the adapter's output contract from the adapter, not a parallel argument - #1556

Draft
planetf1 wants to merge 5 commits into
generative-computing:mainfrom
planetf1:issue-1516
Draft

refactor(intrinsics): resolve the adapter's output contract from the adapter, not a parallel argument#1556
planetf1 wants to merge 5 commits into
generative-computing:mainfrom
planetf1:issue-1516

Conversation

@planetf1

@planetf1 planetf1 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

The problem, in plain terms

Mellea's adapter functions (LoRA/aLoRA capabilities like answerability
checking, hallucination detection, guardrails) each produce output in their
own JSON shape, and something has to check that shape before the caller trusts
it. Before this PR, which adapter actually ran and what shape its output
should be
were decided in two unconnected places: the backend resolved the
adapter, then the high-level helper (check_answerability, guardian_check,
...) separately supplied a validator built from its own module-level constant.
Nothing enforced that the two matched. Passing the wrong validator for the
adapter that actually ran was possible and silent — a caller could think it
was checking one capability's output shape while actually resolving another
capability's adapter, or vice versa.

The impact was concentrated on core.py's three helpers
(check_certainty, requirement_check, find_context_attributions):
check_certainty and find_context_attributions had no validation at all
(raw json.loads, so a malformed or schema-drifted response failed with a
bare KeyError/IndexError far from the cause), and requirement_check
carried its own hand-rolled copy of the score-range check that a second,
independent copy in mellea/stdlib/requirements/requirement.py also carried
— two places that had to be kept in sync by hand and weren't.

Where this sits in Epic #929

Epic #929 (Adapter Function Lifecycle) restructures how Mellea composes
adapters — an Adapter is Identity + IOContract + WeightsBinding,
where IOContract varies by capability and WeightsBinding varies by
deployment. Discussion #1486 agreed that split explicitly; #1516 is Phase 2's
work on the IOContract (capability) axis only. The WeightsBinding
(deployment) axis is deliberately untouched here — activation/apply_activation
code is not part of this PR, by design, per that discussion.

This PR blocks #1144 (Phase 4 — removing the shim adapter classes
IntrinsicAdapter/EmbeddedIntrinsicAdapter entirely), since that removal
needs the shims to already carry real contracts rather than the Phase 1
_ShimIOContract placeholder. It also blocks #1358. Fixes #1516 itself.

What changed

Commits 1–2: the core refactor

  • mellea/backends/adapters/io_contracts.py (new): a single,
    capability-keyed registry of IOContract instances, keyed by the same
    catalog name passed to both call_intrinsic() and resolve_adapter(), and
    exported as mellea.backends.adapters.get_io_contract. Its exhaustiveness
    over the adapter-function catalog is enforced at import time (mirroring the
    existing duplicate-effective_capability check in catalog.py).
  • call_intrinsic() no longer takes an io_contract= argument. It calls
    .io_contract.parse() on the adapter resolve_adapter() actually returns
    — the validator now travels with the adapter, so a mismatch between the two
    is no longer expressible.
  • IntrinsicAdapter/EmbeddedIntrinsicAdapter (the shims
    resolve_adapter() constructs) now look up their contract in the registry
    instead of the _ShimIOContract placeholder, which is now unreachable
    everywhere and deleted.
  • The ten pre-existing module-level Adapter constants in rag.py/guardian.py
    keep their identity and weights as before, but now read io_contract
    from the same registry instead of declaring their own instance.
  • core.py's three helpers each gained a declared contract for the first
    time. requirement_check's hand-rolled score-range validation moved into
    _RequirementCheckContract; find_context_attributions reads its
    now-list-wrapped result via ["items"].

Commits 3–4: independent code review, then fixes

Before merging, I ran an independent 3-reviewer panel (exhaustive / pragmatic
/ pattern-matching perspectives) against the diff. Two reviewers
independently found, from different angles, that the PR's own "single source
of truth" claim was false for the one capability it had done the most work
on: requirement_check_to_bool() (mellea/stdlib/requirements/requirement.py)
— the other production consumer of requirement-check output, used by
ALoraRequirement — still hand-rolled the exact validation this PR had just
consolidated elsewhere, with a comment pointing at code the PR had deleted.
Fixed: it now delegates to get_io_contract("requirement-check").parse(...).

Also fixed from review:

  • get_io_contract wasn't exported from mellea.backends.adapters.__init__,
    so the docs pipeline pruned its page and the AGENTS.md-mandated docstring
    quality gate never saw it, despite being the function the module's own
    docstring designates as the mandatory entry point. Now exported.
  • Two newly-added Adapter constants in core.py
    (_UNCERTAINTY_ADAPTER, _CONTEXT_ATTRIBUTION_ADAPTER) were dead code —
    nothing referenced them once io_contract= was removed from call_intrinsic
    call sites. Deleted; kept _REQUIREMENT_CHECK_ADAPTER, which a test uses as
    a resolve_adapter() stub.
  • No test asserted that the shim adapters actually carry the real registry
    contract rather than a placeholder — every other test would still pass if
    that wiring were reverted. Added one regression test per shim class;
    verified each fails against a reintroduced placeholder and passes against
    the real fix.
  • A round of lower-severity suggestions and nits from all three reviewers:
    stale RST directives left over from an earlier docstring pass, a
    self-contradictory "not used in Phase 1; implemented in Phase 2" placeholder
    message inside the Phase-2 module itself, ambiguous file references in
    docstrings, a fallback-contract docstring that undersold what it actually
    guards against, and a non-GPU unit test exercising the recorded
    context-attribution model output against the real contract (its GPU-gated
    equivalent is xfail(strict=False) for unrelated non-determinism, so it
    gave no CI signal on schema drift).

Behaviour changes worth flagging

Both are validation additions, not regressions, but they change what a
caller catches:

  • check_certainty previously returned a plain KeyError for a response
    missing certainty; it now raises AdapterSchemaMismatchError.
  • requirement_check_to_bool() previously raised an undocumented
    AttributeError for non-object JSON input (e.g. a bare JSON array); it now
    raises the documented ValueError its contract's parse() raises for the
    same input.

Deliberately out of scope

Per the #1486 capability/deployment split: the ten pre-existing constants'
placeholder LocalFileBinding() weights, and the Adapter.__post_init__
identity/weights cross-check that was tried and backed out during #1454
both are deployment-axis concerns, noted as future work in _core.py.
Activation/apply_activation code is untouched throughout.

Verification against real weights

Ran the qualitative, GPU-gated suites against ibm-granite/granite-4.1-3b
(Apple Silicon MPS, cached weights) rather than relying on mocks alone —
covering all 13 catalogued adapter functions' contracts against real model
output:

  • test/stdlib/components/intrinsic/test_core.py — 3 passed, 2 xfailed
    (pre-existing, tracked non-determinism in attribution count — not a schema
    failure; confirmed with --runxfail that the failure is a plain
    list-equality AssertionError, not AdapterSchemaMismatchError).
  • test/stdlib/components/intrinsic/test_rag.py — 14/14 passed.
  • test/stdlib/components/intrinsic/test_guardian.py — 6/6 passed.

Test plan

  • test/backends/test_adapters/test_io_contracts.py (new) — registry
    completeness against known_intrinsic_names(), the permissive-about-keys
    fallback for names outside the catalog (e.g. CustomIntrinsicAdapter),
    _RequirementCheckContract's score-range validation, and the recorded
    context-attribution fixture parsed without a GPU.
  • test/backends/test_adapters/test_shims.py — new: IntrinsicAdapter
    and EmbeddedIntrinsicAdapter carry the real registry contract, not a
    placeholder (verified to fail on a reintroduced placeholder, pass on the fix).
  • Existing test_rag_contracts.py / test_guardian_io_contract.py
    unchanged, still exercise the same contract objects (now sourced from the
    registry) directly.
  • test_core_schema.py — rewritten to exercise the real
    call_intrinsicIOContract.parse path against a stubbed
    resolve_adapter return value, since the validation it tests no longer
    lives in core.py.
  • test_util_unit.py — new: call_intrinsic parses via the resolved
    adapter's io_contract, and no longer accepts an io_contract= kwarg.
  • Existing test_requirement.py (requirement_check_to_bool) — every
    pre-existing case passes unchanged against the delegated implementation.
  • uv run pytest -m "not qualitative" — full suite, 3800+ passed / 5
    failed (all in test_python_tool.py, pre-existing and unrelated — Docker
    isn't available in this environment).
  • ruff format --check ., ruff check ., uv run mypy . — all clean.
  • Docstring quality gate
    (tooling/docs-autogen/audit_coverage.py --quality --fail-on-quality --threshold 100)
    — 100% coverage, 0 issues.

Fixes #1516

…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>
@github-actions github-actions Bot added the enhancement New feature or request label Aug 18, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(intrinsics): resolve the adapter's output contract from the adapter, not a parallel argument (Epic #929 Phase 2)

1 participant