test: centralise HF Hub skip guard to boost CI resiliency against 429 errors#1199
Merged
planetf1 merged 4 commits intoJun 4, 2026
Merged
Conversation
Module-scoped rd fixture now skips gracefully on 429 / network failures instead of erroring all dependent tests. Fixes intermittent CI failures reported in generative-computing#1198. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…errors Matches the skip pattern from test_richdocument.py — wraps LocalHFBackend construction in try/except so tests skip rather than error when the Hub is unavailable or rate-limiting. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Adds hf_skip() to test/conftest.py: a single contextlib.contextmanager
that catches (OSError, requests.exceptions.RequestException) — the two
exception families that cover HfHubHTTPError, LocalEntryNotFoundError,
and every transformers-wrapped Hub error — and converts them into a
clean pytest.skip rather than an ERROR.
Applies hf_skip() to all 11 fixtures (across 11 test files) that call
LocalHFBackend(), start_session("hf", ...), RichDocument.from_document_file(),
or obtain_io_yaml() without a guard. Retrofits the two pre-existing
hand-rolled guards in test_intrinsics_formatters.py to use the helper,
removing the now-redundant LocalEntryNotFoundError import from that file.
This does not address the underlying cause of 429 rate-limit responses
or other mitigations such as authenticated Hub access tokens.
Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
planetf1
changed the title
planetf1
marked this pull request as ready for review
June 4, 2026 10:31
Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
8 tasks
planetf1
enabled auto-merge
June 4, 2026 13:02
jakelorocco
approved these changes
Jun 4, 2026
planetf1
added a commit
to planetf1/mellea
that referenced
this pull request
Jun 19, 2026
…tes_and_returns The test patched fetch_intrinsic_metadata but resolve_adapter calls IntrinsicAdapter without a config_dict, causing __init__ to fall through to intrinsics.obtain_io_yaml() and make a real HF Hub request. Mock obtain_io_yaml and builtins.open to keep this unit test fully offline, consistent with the hf_skip() pattern from PR generative-computing#1199. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
planetf1
added a commit
to planetf1/mellea
that referenced
this pull request
Jun 23, 2026
…tes_and_returns The test patched fetch_intrinsic_metadata but resolve_adapter calls IntrinsicAdapter without a config_dict, causing __init__ to fall through to intrinsics.obtain_io_yaml() and make a real HF Hub request. Mock obtain_io_yaml and builtins.open to keep this unit test fully offline, consistent with the hf_skip() pattern from PR generative-computing#1199. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
AngeloDanducci
pushed a commit
to AngeloDanducci/mellea
that referenced
this pull request
Jun 23, 2026
…insic rewritten (Epic generative-computing#929 Phase 1) (generative-computing#1269) * feat(intrinsics): shim old adapters onto Adapter dataclass; rewrite call_intrinsic (Epic generative-computing#929 Phase 1) Old classes (IntrinsicAdapter, EmbeddedIntrinsicAdapter, CustomIntrinsicAdapter) now inherit from the new Adapter dataclass introduced in Phase 0 (PR generative-computing#1158). Each emits DeprecationWarning on construction and exposes an Identity triple, making isinstance(x, Adapter) return True alongside their existing type checks. call_intrinsic is rewritten to go through backend.resolve_adapter() and backend.adapter_scope() (no-op stub in Phase 1), removing the direct IntrinsicAdapter / EmbeddedIntrinsicAdapter branching from stdlib code. Requirement rerouting in OpenAIBackend and LocalHFBackend is updated to use Identity.capability for adapter lookup instead of isinstance + AdapterType checks. This removes the last internal callers that branched on the old class hierarchy and unblocks Phase 3 (issues generative-computing#1137, generative-computing#1138, generative-computing#1139, generative-computing#1140, generative-computing#1141). Closes generative-computing#1136 Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * test(intrinsics): fix stale patch + add resolve_adapter unit tests Fix test_huggingface_unit.py: remove dead patch of get_adapter_for_intrinsic (removed from huggingface.py imports in parent commit) and pre-populate _added_adapters with a proper Identity so the new capability-based lookup finds the adapter. Add two unit tests to test_shims.py covering the find-existing and no-model-id error paths of resolve_adapter. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * nit: drop line number from comment (stale-proof) Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(docs): align resolve_adapter Returns: type with annotation Docstring quality gate (--fail-on-quality) requires the Returns: prefix to match the return type annotation exactly. Annotation is _AdapterCore; docstring incorrectly said Adapter. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * refactor(adapters): extract _find_adapter helper; clarify shim docs Add AdapterMixin._find_adapter(capability, adapter_types) to centralise the capability-based lookup that was repeated inline four times across huggingface.py and openai.py. resolve_adapter's duplicate search loops also collapse to single _find_adapter calls. Clarify IntrinsicAdapter and EmbeddedIntrinsicAdapter docstrings: replace the bare identity/io_contract/weights Attributes entries with a .. note:: explaining these are Phase 1 internal scaffolding not meant for consumer use. Update test fixtures to wire _find_adapter through the real AdapterMixin implementation, and fix a misleading comment in test_huggingface_unit.py. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * docs(dev): update Phase 1 code example to use _find_adapter The inline next(...) example was superseded by the _find_adapter helper extracted in the previous commit. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(adapters): validate technology before warning; document _find_adapter ordering EmbeddedIntrinsicAdapter previously emitted DeprecationWarning before validating the technology argument, so an invalid call (e.g. technology='qlora') would both warn and raise. Validation now runs first. Add a Note: section to _find_adapter documenting that insertion-order is used and the Phase 0 preference-ordering semantics are not preserved; Phase 2 (issue generative-computing#1138) will revisit if multiple entries per capability become possible. Test updated: test_embedded_invalid_technology now asserts that no DeprecationWarning fires on an invalid-technology construction attempt. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(adapters): honour adapter type preference order; remove adapter_scope from call_intrinsic _find_adapter previously accepted set[str] and returned the first hit in _added_adapters insertion order, ignoring any caller-specified type preference. Change the signature to tuple[str, ...] and iterate preferred types first so aLoRA is returned before LoRA when both are registered for the same capability, matching Phase 0's get_adapter_for_intrinsic behaviour. The HF backend was also discarding the preference order at the call site by building a set from action.adapter_types before passing to _find_adapter. Both call sites in huggingface.py now pass a tuple so the preference flows end-to-end from call_intrinsic through to adapter selection. Remove the adapter_scope wrapper from call_intrinsic. The HF backend already handles adapter activation inside _generate_with_adapter_lock under the generation lock, immediately before generate_func is called. Activating at the call_intrinsic level would be outside that lock and could race with concurrent async requests. Add a comment explaining this so the pattern is clear for Phase 2 implementors. Add test_find_adapter_honours_type_preference_order to verify aLoRA wins over LoRA regardless of insertion order — the previous set-based implementation would have passed all prior tests since none registered both types for the same capability. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(adapters): fix openai.py call sites to pass tuple not set to _find_adapter Missed in the initial fix — openai.py had the same set[str] call sites as huggingface.py at lines 486 and 571. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * docs(dev): fix set-literal in requirement_aLoRA_rerouting Phase 1 example The "After (Phase 1)" snippet showed {"alora"} (set) but _find_adapter takes tuple[str, ...] for preference-ordered iteration. A set has no guaranteed order, defeating the aLoRA-before-LoRA preference the method provides. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * fix(adapters): raise NotImplementedError in _ShimWeightsBinding; document resolve_adapter contracts _ShimWeightsBinding methods were silent no-ops while _ShimIOContract already raised NotImplementedError. The asymmetry means Phase 2 lifecycle code accidentally calling WeightsBinding.activate/deactivate on an un-migrated shim would silently succeed rather than surface the gap. Raise consistently. Also adds three inline comments to resolve_adapter: - warns that warnings.catch_warnings() is not async/thread-safe (Phase 2 gap) - notes the embedded branch is only valid for backends whose add_adapter accepts the full Adapter type (not LocalHFBackend) - documents that the AdapterType.LORA default is intentional (mirrors the pre-Phase-1 _util.py default) and is Phase 2 scope to improve Adds test_resolve_adapter_lazy_creates_and_returns covering the previously untested lazy-creation branch of resolve_adapter. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * test(adapters): mock obtain_io_yaml in test_resolve_adapter_lazy_creates_and_returns The test patched fetch_intrinsic_metadata but resolve_adapter calls IntrinsicAdapter without a config_dict, causing __init__ to fall through to intrinsics.obtain_io_yaml() and make a real HF Hub request. Mock obtain_io_yaml and builtins.open to keep this unit test fully offline, consistent with the hf_skip() pattern from PR generative-computing#1199. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> * docs(adapters): update adapter.py docstrings to reflect Phase 1 shim changes - Rewrite module-level docstring to name resolve_adapter/_find_adapter as primary surface and LocalHFAdapter/IntrinsicAdapter/EmbeddedIntrinsicAdapter as deprecation shims; removes stale description of get_adapter_for_intrinsic - Expand catch_warnings race comment to name the concrete failure mode: concurrent first-time call_intrinsic calls interleaving catch_warnings contexts, causing DeprecationWarning to surface in user code - Make _ShimWeightsBinding NotImplementedError messages consistent with _ShimIOContract by including "Phase 2 (issue generative-computing#1138)" navigation context Addresses markstur review comments on PR generative-computing#1269. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com> --------- Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
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.
Summary
CI was failing intermittently when HuggingFace Hub returned HTTP 429 (rate limit) or was temporarily unreachable: fixtures that called
LocalHFBackend(),start_session("hf", ...),RichDocument.from_document_file(), orobtain_io_yaml()without a guard would raise an unhandledOSErrororHfHubHTTPError, erroring every dependent test in the module rather than skipping them cleanly. This masked unrelated failures and made CI noise harder to diagnose.This PR adds a single
hf_skip()context manager totest/conftest.pythat catches(OSError, requests.exceptions.RequestException)— the two exception families that cover every Hub failure mode (directHfHubHTTPError,LocalEntryNotFoundError, and transformers-internal wrappers that re-raise asOSError) — and converts them into a cleanpytest.skip. It then applies this guard to all 11 fixtures across the test suite that were previously unprotected, and retrofits the two hand-rolled guards already intest_intrinsics_formatters.pyto use the same helper, removing the now-redundantLocalEntryNotFoundErrorimport.This PR does not address the underlying cause of 429 responses — it makes no changes to production code, adds no retry logic, and provides no mitigation such as an authenticated Hub token. It purely ensures that Hub unavailability produces clean SKIPPED outcomes rather than ERROR noise.
Changed files (test-only)
test/conftest.pyhf_skip()context managertest/stdlib/components/docs/test_richdocument.pyhf_skip()test/stdlib/requirements/test_groundedness_requirement.pyhf_skip()test/stdlib/requirements/test_groundedness_requirement_e2e.pyhf_skip()test/stdlib/components/intrinsic/test_core.pyhf_skip()test/stdlib/components/intrinsic/test_guardian.pyhf_skip()test/stdlib/components/intrinsic/test_rag.pyhf_skip()(2 fixtures)test/backends/test_huggingface.pyhf_skip()(coversadd_adapterHub calls too)test/backends/test_huggingface_tools.pyhf_skip()test/telemetry/test_metrics_backend.pyhf_skip()alongside existing CI guardtest/stdlib/test_spans.pyhf_skip()test/formatters/granite/test_intrinsics_formatters.pyhf_skip(), removeLocalEntryNotFoundErrorimportTesting
Closes #1198