refactor(intrinsics): migrate rag.py to new Adapter types (Epic #929 Phase 1 Wave 3)#1321
Conversation
- Add model_options: dict | None = None keyword-only param to all four helpers (policy_guardrails, guardian_check, factuality_detection, factuality_correction); forwarded to call_intrinsic unchanged - Two unit tests verifying model_options forwarding - Fix example comments: ctx_no_doc -> ctx (phantom variable), clarify documents= as an alternative to the context-based approach Addresses AC gap: model_options= was missing from all four helpers despite being listed in the issue and present in sibling rag.py (generative-computing#1321). Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…ative-computing#929 Phase 1) (generative-computing#1137) All six RAG helpers migrated to the new Adapter dataclass per the per-helper migration template in PR generative-computing#1080 §13: - Add _DictContract and _ListContract IOContract subclasses for all six helpers. parse() raises AdapterSchemaMismatchError on contract-breaking deltas (missing required field); forward-compatible additions (extra optional fields) do not raise. - Create six module-level Adapter constants, one per helper. - Add model_options: dict | None = None keyword argument to every helper. - Extend call_intrinsic in _util.py with an io_contract parameter; when provided, io_contract.parse(result_str) replaces json.loads for combined parse+validate. - find_citations and flag_hallucinated_content extract the validated list from the parse result["items"] wrapper key; public return types are unchanged. Tests: 12 new unit tests in test_rag_contracts.py — two per helper (contract_enforced / forward_compat) — run without GPU or backend. Closes generative-computing#1137 Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
- Add ValueError to Raises: sections in all six RAG helpers and
call_intrinsic; json.JSONDecodeError propagates before schema checks.
- Document that an empty list parses to {"items": []} in _ListContract.
- Remove generative-computing#1138 issue refs from build_prompt NotImplementedError strings
(project convention: no #NNN refs in inline code).
- Tighten call_intrinsic one-line summary to say "optionally validated".
- Add two empty-list edge-case tests (citations, hallucination) to
test_rag_contracts.py; 14 tests pass.
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…unction' in rag.py Five backend: docstring entries in clarify_query, find_citations (×2), check_context_relevance, and flag_hallucinated_content still used the old 'intrinsic' terminology. Aligned with the agreed Granite Switch glossary (issue generative-computing#1192 / PR generative-computing#1256); matches the prose already updated in upstream/main for the other helper files. Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
- Add `-> dict[str, object]` return annotation to `call_intrinsic` so callers have a concrete type to work against - Replace `# type: ignore[return-value]` at all six rag.py call sites with `cast(str, ...)` / `cast(list[dict], ...)` — documents the adapter contract rather than silencing the checker - Raise `ValueError` (not `AdapterSchemaMismatchError`) when parsed JSON is the wrong type entirely (not a dict / not a list); reserve `AdapterSchemaMismatchError` for missing-key violations where its observed/expected-keys vocabulary actually makes sense - Update `Raises:` docstrings in `_DictContract` and `_ListContract` to match Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Four new unit tests verify the behaviour introduced in the preceding commit: dict-shaped contracts reject non-dicts, list-shaped contracts reject non-lists and non-dict list elements, and the error messages name the adapter. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Rebasing onto upstream/main introduced a conflict at the import block; resolving it dropped `import warnings`, which is needed for the `check_context_relevance` deprecation warning added in upstream generative-computing#1339. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
5d6f96f to
fbb206b
Compare
|
HPC results (job Intrinsic tests —
All green on CUDA (A100, BlueVela) and locally (MPS, Apple Silicon). |
…call_intrinsic + add mixed-list test Addresses two post-review suggestions from PR generative-computing#1321: - `call_intrinsic` Raises: section now documents that `AdapterSchemaMismatchError` is propagated when an `io_contract` is provided and the model output violates the declared contract. Keeps the docstring accurate ahead of the quality gate. - Add `test_list_contract_rejects_non_dict_element_after_valid_item` to cover the full loop body of `_ListContract.parse` (previously only a single-element list was tested; this adds a valid-then-invalid mixed list to exercise the per-item check on non-first elements). 19 tests pass. Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code
…object] return type Adding -> dict[str, object] to call_intrinsic in _util.py (this PR) made mypy correctly flag return sites in guardian.py and core.py where a subscript value (typed object) was being returned as str or float. Fixes CI quality gate failure. Apply the same cast() pattern already used in rag.py: - guardian.py: cast(str, ...) on policy_guardrails label/score returns; cast(float, cast(dict[str, object], ...)["score"]) on guardian_check; cast(str, ...) on factuality_detection and factuality_correction returns. - core.py: cast(float, ...) on check_certainty and requirement_check returns; cast(list[dict], ...) on find_context_attributions (adapter emits a JSON array). Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code
TestingLocal (macOS/MPS) — full non-qualitative suite: Result: 1837 passed, 10 skipped, 1 error (pre-existing #1344 — MPS/docling float64), 0 failures. BlueVela (LSF, GPU) — intrinsics scope (job 1691399): Result: 274 passed, 2 skipped, 2 xfailed, 0 failures. Run time: ~11 min on a single A100. |
|
The |
d02dc63
…w-only discovery (generative-computing#1139) (generative-computing#1323) * refactor(intrinsics): guardian.py whole-file migration + documents= kw-only discovery (generative-computing#1139) Closes generative-computing#1139, closes generative-computing#1094 **`guardian.py` (whole-file migration)** - `policy_guardrails`: migrate `:param`/`:return:` reST docstring to Google-style `Args:`/`Returns:`; tighten `label xor score` guard to raise `ValueError` instead of bare `Exception`. - `factuality_detection` / `factuality_correction`: add `documents=` keyword-only argument (`Iterable[str | Document] | None = None`). When provided, the last assistant turn is extracted from the context and re-added as a `Message` with the supplied documents attached, matching the pattern established by `flag_hallucinated_content` in `rag.py`. Works for both session-generated contexts (last element is a `ModelOutputThunk`) and manually-constructed contexts (last element is a `Message`). Raises `ValueError` when `documents=` is provided but the context does not end with an assistant turn. - Add private helper `_inject_documents` to encapsulate the extraction + reconstruction logic. - Migrate docstrings for both functions to Google style; add `Raises:` sections. - New imports: `collections.abc`, `typing.cast`, `Document`, `_coerce_to_documents`, `Message`. **`creating_a_new_type_of_session.py` (closes generative-computing#1094)** - Remove deprecated `GuardianCheck` / `GuardianRisk` imports. - Replace `requirements=[GuardianCheck(...), ...]` + `self.validate()` with direct `guardian.guardian_check()` calls in `chat()`. - Switch backend from `OllamaModelBackend` to `LocalHFBackend(IBM_GRANITE_4_MICRO_3B)`, which carries the embedded guardian-core adapter; same model used by the existing guardian e2e tests. - Update pytest marker: `ollama` → `huggingface`; add `slow`. **Tests** - `test_guardian.py`: `_read_guardian_input` now returns `(ChatContext, list[Document])`, loading reference documents from `extra_body.documents` in the fixture JSON. Both factuality tests pass documents to the functions under test. - `test_guardian_documents.py` (new): 11 unit tests covering the no-documents path, the documents= injection path, correct intrinsic names, string-to-Document coercion, preservation of preceding turns, and all `ValueError` cases — no model required. Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code * fix(intrinsics): address code review findings on guardian documents= PR - Fix _inject_documents to raise a distinct ValueError when the last assistant turn exists but has not been computed yet (uncomputed thunk), rather than misleadingly saying 'not an assistant response' - Drop redundant _coerce_to_documents pre-call: Message.__init__ already coerces, so pass documents directly; removes private-API import - Remove spurious # type: ignore[assignment] on Message.content access - Clarify docstrings on factuality_detection and factuality_correction: reword 'in precedence order' to 'in any of these ways' and note that documents= replaces the last assistant turn's docs only - Add unit tests covering the ModelOutputThunk (session-generated) path and the uncomputed-thunk error case (4 new tests, 15 total) Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Assisted-by: Claude Code * test(intrinsics): add policy_guardrails XOR tests; narrow example backend type - Add two unit tests for policy_guardrails ValueError paths: both label+score present ("found both") and neither present ("found neither") - Narrow ChatCheckingSession.__init__ to backend: AdapterMixin; retain type: ignore[arg-type] at call site because MelleaSession.backend property is typed Backend Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(intrinsics): add model_options= to guardian helpers; update examples - Add model_options: dict | None = None keyword-only param to all four helpers (policy_guardrails, guardian_check, factuality_detection, factuality_correction); forwarded to call_intrinsic unchanged - Two unit tests verifying model_options forwarding - Fix example comments: ctx_no_doc -> ctx (phantom variable), clarify documents= as an alternative to the context-based approach Addresses AC gap: model_options= was missing from all four helpers despite being listed in the issue and present in sibling rag.py (generative-computing#1321). Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(examples): correct phantom ctx_no_doc variable in factuality example comments The documents= kwarg examples referenced ctx_no_doc which doesn't exist in either file; replace with the actual ctx variable and clarify the comment wording. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(intrinsics): address jakelorocco review comments on guardian PR - Hoist result-type comments above result= assignment in factuality examples - Update policy_guardrails docstring to reflect LoRA or aLoRA adapter Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * refactor(intrinsics): extract _extract_last_response shared helper _inject_documents and _resolve_response shared the same thunk-extraction and context-rewind logic. Extract _extract_last_response into _util.py and delegate both functions to it. _extract_last_response also handles manually-added assistant Messages (a case _resolve_response did not cover), so find_citations, flag_hallucinated_content, and find_context_attributions now accept manually-constructed contexts where previously they would raise. Addresses code-review feedback from jakelorocco on PR generative-computing#1323. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * docs(guardian): document image-drop behaviour and enumerate Raises triggers _inject_documents rebuilds the assistant turn from extracted text only; any images on the original turn are silently dropped — add a note in the Returns section. Broaden the Raises: prose in factuality_detection and factuality_correction to enumerate the three triggers (empty context, non-assistant last turn, uncomputed response) rather than vague "cannot be extracted". Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * refactor(guardian): replace RST markup with Google-style in intrinsic/ docstrings Replace :class:, :func:, :meth:, :data: cross-reference roles and double-backtick ``x`` literals throughout guardian.py and _util.py with plain single-backtick identifiers, matching the style already used in core.py and rag.py in the same package. No logic changes; docstring-only. Closes out the intrinsic/ portion of generative-computing#1336. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> --------- Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> Co-authored-by: AngeloDanducci <angelo.danducci.ii@ibm.com>
Summary
This is Epic #929 Phase 1 Wave 3 — whole-file migration of
mellea/stdlib/components/intrinsic/rag.pyto the newAdapter(identity, io_contract, weights)dataclass system introduced in Phase 1.A (PR #1269, merged 2026-06-23).Epic #929 is a multi-phase effort to fix Adapter Function Lifecycle & Consistency in Mellea. The phases are:
Adapterdataclass,IOContractABC,Identity,WeightsBindingtypescall_intrinsicrefactor +_ShimIOContractshims; merged ✅build_promptimplementation (currently stubs raisingNotImplementedError)What this PR changes
rag.py— all six RAG adapter functions fully migrated:IOContractsubclasses:_DictContract(single-dict output) and_ListContract(list-of-dicts output)._ListContract.parse()wraps the validated list in{"items": [...]}to satisfy thedict[str, object]return type ofIOContract.parse(); helper functions unwrap it before returning — public return types are unchanged.parse()raisesAdapterSchemaMismatchErrorwhen a required field is missing; raisesValueErrorwhen the JSON output is the wrong type entirely (not a dict / not a list). This replaces the previous silentKeyError; callers that catchKeyErrorshould update toAdapterSchemaMismatchError.parse()propagatesValueError/json.JSONDecodeErrorfromjson.loadswhen the model output is not valid JSON.Adapterconstants, one per helper, each withIdentity,_*Contract, andLocalFileBinding().model_options: dict | None = Noneadded as a keyword-only argument to all six helpers (previously absent).backend:parameter descriptions.cast(str, ...)/cast(list[dict], ...)rather than# type: ignore[return-value]._util.py—call_intrinsicextended withio_contract: IOContract | None = None(keyword-only, defaultedNone). When provided, replaces the rawjson.loadscall withio_contract.parse()for combined parse+validate. All existing callers (core.py,guardian.py) pass noio_contractand are unaffected. The function now carries an explicit-> dict[str, object]return annotation.test_rag_contracts.py(new) — 18 pure unit tests (no GPU, no backend):contract_enforced(missing required field →AdapterSchemaMismatchError) andforward_compat(extra field → no raise)find_citations,flag_hallucinated_content)ValueErrorwhen JSON is not a dict, not a list, or contains a non-dict list elementOutput contract verification
Each contract was verified against the current
granitelib-rag-r1.0weights README before writing the tests:check_answerabilityanswerabilityrewrite_questionrewritten_questionclarify_queryclarificationfind_citationsresponse_begin,response_end,response_text,citation_doc_id,citation_begin,citation_end,citation_textcheck_context_relevancecontext_relevanceflag_hallucinated_contentresponse_begin,response_end,response_text,faithfulness,explanationExamples review
All six
docs/examples/intrinsics/examples that call these helpers (answerability.py,citations.py,context_relevance.py,hallucination_detection.py,query_clarification.py,query_rewrite.py) were reviewed. All existing calls pass the migrated parameters positionally or by name, and none rely on amodel_optionsposition — the keyword-only addition is non-breaking. All examples are gated with# pytest: huggingface, e2e(or have no pytest marker and are silently skipped) and therefore do not run in unit CI.Relationship to other open PRs
requirement_check_to_bool,core.py— refactor(intrinsics): requirement_check migration — loud schema-mismatch, no more silent False (Epic #929 Phase 1) #1138): Wave 3 parallel sibling; touches different file, no conflict.guardian.py— refactor(intrinsics): guardian.py whole-file migration + documents= kw-only + auto-context discovery (Epic #929 Phase 1) #1139): Wave 3 parallel sibling; touches different file, no conflict.docs/examplesterminology sweep — chore(terminology): complete prose sweep in docs/examples/ and Python docstrings (#1192 follow-up) #1279): touchesdocs/examples/intrinsics/README.mdand Python docstring files, but NOTrag.pyor_util.py. No conflict..parsedproperty /format=overloads — bug: structured output via act() silently returns JSON string, not Pydantic instance #1273/chore: thread format= through act()/instruct() return type overloads so structured output is cast-free #1274): unrelated to this file.Ordering: this PR has no blocking relationship to #1320 or #1323 — all three Wave 3 PRs are independent and can merge in any order.
Related issues
Adapter,IOContract,LocalFileBindingAPI)Test plan
uv run pytest test/stdlib/components/intrinsic/test_rag_contracts.py -v— 18/18 passuv run pytest test/stdlib/components/intrinsic/ -m "not qualitative"— all passuv run pytest test/ -m "not qualitative"(full local suite) — 1837 passed, 0 failures (1 pre-existing error: test_richdocument errors on Apple Silicon: docling layout pipeline fails on MPS (float64 unsupported) #1344)uv run ruff format . && uv run ruff check .— cleandocs/examples/intrinsics/examples reviewed — backward-compatible, no changes requiredcode-checks3.11/3.12/3.13) — all pass (3.13 rerun needed:httpx.ReadTimeoutflake, tracked test: backend tracing tests flake with httpx.ReadTimeout on loaded CI runners #1318)