Skip to content

fix: remove drift detection from intrinsic tests and pin tests to SHAs#1196

Merged
jakelorocco merged 4 commits into
generative-computing:mainfrom
jakelorocco:fix/ha-intrinsic-downloads
Jun 5, 2026
Merged

fix: remove drift detection from intrinsic tests and pin tests to SHAs#1196
jakelorocco merged 4 commits into
generative-computing:mainfrom
jakelorocco:fix/ha-intrinsic-downloads

Conversation

@jakelorocco
Copy link
Copy Markdown
Contributor

@jakelorocco jakelorocco commented Jun 3, 2026

Pull Request

Issue

Fixes N/A

Description

Removes drift detection from each individual adapter. Instead, pins each repo to a single sha to reduce downloads and improve caching. This SHA is retrieved from the SHA used in our catalog pinning.

Changes the adapter change test to work off the repo SHA. Nightlies may fail more often but it will keep us up to date.

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

  • AI coding assistants used

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.

  • Component
  • Requirement
  • Sampling Strategy
  • Tool

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.

Signed-off-by: Jake LoRocco <jake.lorocco@ibm.com>
Assisted-by: CLAUDE:OPUS
@github-actions github-actions Bot added the bug Something isn't working label Jun 3, 2026
@jakelorocco jakelorocco marked this pull request as ready for review June 3, 2026 20:38
@jakelorocco jakelorocco requested a review from a team as a code owner June 3, 2026 20:38
Signed-off-by: Jake LoRocco <jake.lorocco@ibm.com>
@jakelorocco jakelorocco force-pushed the fix/ha-intrinsic-downloads branch from 19cb7e3 to d72d408 Compare June 3, 2026 20:41
@ajbozarth
Copy link
Copy Markdown
Contributor

Quick review from Claude found a few items worth noting but nothing blocking:


A few optional follow-ups, nothing blocking:

  1. The test now imports _RAG_SHA / _CORE_R1_SHA from mellea/backends/adapters/catalog.py. Since they're underscore-prefixed they're technically private — consider promoting them to public names, or adding a comment in the
    catalog noting the cross-module use so a future rename doesn't silently break this test.

  2. Worth filing a follow-up issue to add ibm-granite/granitelib-rag-gpt-oss-r1.0 to the catalog, so its SHA lives in one place rather than being pinned locally here.

  3. Quick sanity check: the new drift signal fires on any commit to repo main, not just the adapter subpath the old check tracked. The PR description's "nightlies may fail more often" is probably understating it — an unrelated
    README edit in any of the three repos will trip it. Fine if that's the intended tradeoff, just want to confirm.

  4. Minor: the revision property docstring largely restates what _REPO_PINNED_SHAS already conveys; could be trimmed.

@ajbozarth
Copy link
Copy Markdown
Contributor

It looks like the CI is hitting the same issue on all the PRs again:

=========================== short test summary info ============================
ERROR test/stdlib/components/docs/test_richdocument.py::test_richdocument_basics - huggingface_hub.errors.LocalEntryNotFoundError: An error happened while trying to locate the files on the Hub and we cannot find the appropriate snapshot folder for the specified revision on the local disk. Please check your internet connection and try again.
ERROR test/stdlib/components/docs/test_richdocument.py::test_richdocument_markdown - huggingface_hub.errors.LocalEntryNotFoundError: An error happened while trying to locate the files on the Hub and we cannot find the appropriate snapshot folder for the specified revision on the local disk. Please check your internet connection and try again.
ERROR test/stdlib/components/docs/test_richdocument.py::test_richdocument_save - huggingface_hub.errors.LocalEntryNotFoundError: An error happened while trying to locate the files on the Hub and we cannot find the appropriate snapshot folder for the specified revision on the local disk. Please check your internet connection and try again.
ERROR test/stdlib/components/docs/test_richdocument.py::test_table - huggingface_hub.errors.LocalEntryNotFoundError: An error happened while trying to locate the files on the Hub and we cannot find the appropriate snapshot folder for the specified revision on the local disk. Please check your internet connection and try again.
ERROR test/stdlib/requirements/test_groundedness_requirement.py::test_groundedness_requirement_allow_partial_support_parameter - OSError: We couldn't connect to 'https://huggingface.co/' to load the files, and couldn't find them in the cached files.
Check your internet connection or see how to run the library in offline mode at 'https://huggingface.co/docs/transformers/installation#offline-mode'.
= 2423 passed, 230 skipped, 21 deselected, 1 xfailed, 1 xpassed, 521 warnings, 5 errors in 1036.27s (0:17:16) =

I think these just need the same fix as @planetf1 made in #1195

Copy link
Copy Markdown
Contributor

@ajbozarth ajbozarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an approval to unblock merge since my previous review is non-blocking and the CI failure is flakey and probably better to solve in another PR

(@planetf1 you may to take a look at the CI failure point above in the morning)

@planetf1
Copy link
Copy Markdown
Contributor

planetf1 commented Jun 4, 2026

@ajbozarth taking a look

@planetf1
Copy link
Copy Markdown
Contributor

planetf1 commented Jun 4, 2026

Thanks @ajbozarth — opened #1199 which centralises the skip guard into a single hf_skip() context manager and applies it across all 11 unprotected fixtures, including the two you flagged. Should sort it out once merged.

@jakelorocco jakelorocco requested a review from planetf1 June 4, 2026 19:11
Comment thread test/formatters/granite/test_intrinsics_formatters.py Outdated
Co-authored-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Signed-off-by: Jake LoRocco <jake.lorocco@ibm.com>
@jakelorocco jakelorocco force-pushed the fix/ha-intrinsic-downloads branch from a6c5138 to 80ad675 Compare June 5, 2026 13:01
@jakelorocco jakelorocco requested a review from planetf1 June 5, 2026 13:02
@jakelorocco
Copy link
Copy Markdown
Contributor Author

@planetf1, applied the change. I will need your review to get this merged since it requires an intrinsics maintainer.

@jakelorocco jakelorocco enabled auto-merge June 5, 2026 13:02
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jakelorocco jakelorocco added this pull request to the merge queue Jun 5, 2026
Merged via the queue into generative-computing:main with commit 67ab9b1 Jun 5, 2026
7 checks passed
@jakelorocco jakelorocco deleted the fix/ha-intrinsic-downloads branch June 5, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants