Skip to content

Template-fit residuals + HF retry wrapper + pytest in CI#13

Open
Jay-Bailey wants to merge 2 commits into
mainfrom
fix/template-fit-residuals
Open

Template-fit residuals + HF retry wrapper + pytest in CI#13
Jay-Bailey wants to merge 2 commits into
mainfrom
fix/template-fit-residuals

Conversation

@Jay-Bailey

@Jay-Bailey Jay-Bailey commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

This PR contains

Description

Started as three small fixes for stale upstream-shaped bits the post-merge audit caught. Grew to also include a port of inspect_evals's HuggingFace retry wrapper (so the investigate-dataset / create-eval skills can recommend it accurately) and wiring pytest into CI as an enforced check.

Section 1: Template-fit residuals (3 commits worth, in 1)

  • `make test` was broken. The recipe used a `test` extra and `test_py311_or_lower` / `test_py312_or_higher` uv groups that don't exist in the template's `pyproject.toml`. Single-eval forks don't need the multi-Python-version dep matrix that inspect_evals carries, and CI already uses plain `uv sync --frozen`. Recipe is now `uv run pytest $(TEST_ARGS)`.

  • investigate-dataset skill pointed at unavailable wrapper. The rate-limit bullet referenced `inspect_evals.utils.huggingface` for built-in retry, which isn't installed for template users. (Section 2 below makes that reference newly accurate.)

  • Autolint dependencies check had hardcoded `inspect_evals` self-import filter. `tools/run_autolint/checks/dependencies.py:250` hardcoded `if imp != "inspect_evals":`. Dead code in template context (package name is contributor-chosen). Dropped; the existing `local_modules` check below handles the legitimate case.

Section 2: Port the HF retry wrapper

`src/utils/huggingface.py` previously had only schema-assertion helpers. The full retry-aware wrappers from `inspect_evals.utils.huggingface` are now ported in: `hf_dataset`, `load_dataset`, `snapshot_download`, `hf_hub_download`, plus lazy-imported wrappers for `sentence_transformer`, `transformers_pipeline`, `auto_model_for_sequence_classification`, `auto_tokenizer`. Each retries on HF rate-limits (429), bad gateway (502), `LocalEntryNotFoundError`, and the file-not-found / value-error patterns that show up in the datasets library when the cache is missing. Each also requires a `revision=` keyword for reproducibility.

Adaptations:

  • Telemetry hooks (`record_backoff`, `record_hf_call`) stubbed as no-ops; the template doesn't need registry-scale call tracking.
  • File restructured into "schema assertions" + "retry-aware wrappers" sections; both kept.
  • Runtime deps gain `backoff`, `datasets`, `huggingface_hub` (with a comment explaining how to drop them if an eval doesn't use HF). Dev deps gain `types-requests`. Mypy override added for `transformers` / `sentence_transformers` since they're lazy-imported and only needed if an eval opts in.

`investigate-dataset/SKILL.md` and `create-eval/SKILL.md` both updated to recommend `from utils.huggingface import hf_dataset` over the bare `inspect_ai.dataset` import.

Section 3: Pytest in CI

CI didn't run tests. Added a `tests` job to `checks.yml`: matrix across 3.11/3.12/3.13, fail-fast: false, runs `uv run pytest -q` (slow tests excluded by the conftest's `--runslow` gate). New `ENFORCE_TESTS` toggle in `tools/enforcement.config` (default-enforced, sits alongside ruff/mypy). `tools/run_checks.sh` also got a Pytest step so `make check` covers it locally. README's enforcement toggle list updated.

The matrix doesn't need a per-Python-version dep-group split: the template's dep surface is uniform across 3.11-3.13, so `uv sync --frozen` works cleanly per matrix leg from the single `dev` group.

Test plan

  • `make check` — 11/11 pass (was 10; new Pytest step covers the existing test suite)
  • `make test` — 81 passed, 1 skipped (slow)
  • `from utils.huggingface import hf_dataset, snapshot_download, …` imports cleanly
  • Mypy clean on `src tests`
  • Ruff format + lint clean
  • `uv sync --frozen` resolves; `uv lock` no-op on the committed lockfile
  • CI on this PR: validates the new `tests` job and the ENFORCE_TESTS wiring across the 3.11/3.12/3.13 matrix

Checklist

  • Are you adding a new eval?

  • Does this change affect existing eval(s)?

  • Does this change affect how future contributors write or submit evaluations (e.g. new required fields, changed tooling, updated conventions)?

    • If yes, has the relevant documentation been updated? Yes — README "Checks and enforcement" lists the new `ENFORCE_TESTS` toggle; investigate-dataset and create-eval skills updated to recommend the new HF wrapper.

- Makefile: \`make test\` referenced a \`test\` extra and
  \`test_py311_or_lower\` / \`test_py312_or_higher\` uv groups that don't
  exist in the template's pyproject.toml — \`make test\` failed
  immediately. Drop the upstream-shaped variable apparatus (single-eval
  forks don't need the multi-Python-version dep matrix, and CI already
  uses plain \`uv sync --frozen\`); recipe is now just
  \`uv run pytest \$(TEST_ARGS)\` with TEST_ARGS preserved as a
  pass-through. Verified: \`make test\` now runs 81 tests cleanly.

- .claude/skills/investigate-dataset/SKILL.md: the rate-limit
  troubleshooting bullet pointed at \`inspect_evals.utils.huggingface\`
  for "built-in retry with backoff", but that module is an inspect_evals
  package internal that isn't installed for template users. The
  template's own src/utils/huggingface.py is a schema-assertion helper,
  not a retry wrapper. Replaced with actionable advice (wait/retry, set
  HF_TOKEN to raise the per-IP rate limit).

- tools/run_autolint/checks/dependencies.py: had a hard-coded
  \`if imp != "inspect_evals":\` self-import filter from upstream. In
  template context the package name is contributor-chosen, so the
  hard-coded value is dead code. Removed; the existing \`local_modules\`
  check below it handles the legitimate self-import case for any project
  name. Comment notes the divergence so future syncs don't re-introduce
  the upstream version silently.
Two follow-on additions to the template-fit-residuals work:

1. Bring in the retry-aware HuggingFace wrappers from
   inspect_evals.utils.huggingface so the investigate-dataset and
   create-eval skills can recommend using them. They wrap inspect_ai's
   hf_dataset, datasets.load_dataset, huggingface_hub.snapshot_download
   and friends with backoff.on_exception, retrying 429s / 502s / cache
   misses caused by HF rate limiting. Also enforce a `revision=` kwarg
   on every call for reproducibility.

   Adapted in-place: telemetry hooks (record_backoff, record_hf_call)
   are stubbed out as no-ops since the template doesn't need
   registry-scale call tracking. The schema-assertion helpers that
   already lived in src/utils/huggingface.py are kept; the file is now
   sectioned into "schema assertions" + "retry-aware wrappers".

   Adds backoff, datasets, huggingface_hub to runtime deps with a
   comment explaining how to drop them if an eval doesn't use HF.
   Adds types-requests to dev. Adds a mypy override for transformers /
   sentence_transformers (lazy-imported by the wrapper, only required
   if an eval actually opts into them).

   investigate-dataset/SKILL.md and create-eval/SKILL.md updated to
   recommend `from utils.huggingface import hf_dataset` over importing
   from inspect_ai.dataset directly.

2. Wire pytest into the CI pipeline.

   - New `tests` job in checks.yml. Matrix across 3.11/3.12/3.13,
     fail-fast: false, runs `uv run pytest -q` (slow tests excluded by
     the conftest's --runslow gate).
   - ENFORCE_TESTS toggle added to tools/enforcement.config (default
     enforced — like ENFORCE_RUFF/MYPY, it's a correctness check).
   - Pytest step added to tools/run_checks.sh, so `make check` now
     covers tests too. Local run: 11/11 checks pass.
   - README's "Checks and enforcement" toggle list updated to include
     ENFORCE_TESTS with a note about the slow-test marker.

   The matrix doesn't need a per-Python-version dep group split: the
   template's dep surface is uniform across 3.11-3.13 (no conflicting
   version constraints), so `uv sync --frozen` works cleanly per
   matrix leg from the single `dev` group.
@Jay-Bailey Jay-Bailey changed the title Fix three template-fit residuals from inspect_evals upstream Template-fit residuals + HF retry wrapper + pytest in CI Apr 27, 2026
Comment thread src/utils/huggingface.py
f"Config {config_name!r} feature {feature!r} expected dtype "
f"matching {expected_dtype!r}, got {actual_dtype!r}"
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is it worth including this? This is mostly an issue for large codebases.

Argument against: Adds complexity for largely no benefit for most users.

Argument for: Not including it means that larger users will need to eventually solve this problem. Why not solve it for them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant