mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
2e29c7223f
## Description
Fixes intermittent (`~25-test`) failures in `test` shards caused by a
GitHub Actions cache race between the `prefetch-model` job and the four
parallel `test` shards.
Closes #<!-- no upstream issue number yet -->
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- Added `id: restore-hfcache` to the "Restore HuggingFace model cache"
step in the `test` job so its cache-hit outcome is observable.
- Added a conditional "Fallback model download if cache missed" step
immediately after the restore, gated on
`steps.restore-hfcache.outputs.cache-hit != 'true'`. When the cache
misses it runs the same authenticated `snapshot_download` retry loop
that `prefetch-model` already uses (same
`snapshot_download('sentence-transformers/all-MiniLM-L6-v2')`, same
default `~/.cache/huggingface` cache root, same unpinned
`huggingface_hub` — byte-for-byte the warm path's mechanism), with
`HF_HUB_OFFLINE=0` / `TRANSFORMERS_OFFLINE=0` scoped to that step only,
so the model lands where pytest looks before pytest starts.
- `TRANSFORMERS_OFFLINE: "1"` on the actual `pytest` step is unchanged.
- The `prefetch-model` job and shared cache key remain the warm-path
optimisation.
- **Added `.github/workflows/**` to the `code` paths-filter group** (the
gate `test` / `prefetch-model` / `build-wheel` / `lint` read via
`needs.changes.outputs.code == 'true'`). Rationale: a change to *how the
tests run* must be validated by the test suite it governs. Without this,
a PR that only touches `ci.yml` matches only the separate `workflows`
filter, so `code=false` and every test job is **skipped** — a CI change
would merge on a hollow green having never executed the pipeline it
modifies. With this line **this PR is self-validating**: the four `test`
shards and `prefetch-model` actually run and exercise the new cache-miss
fallback path. The separate `workflows` filter is left unchanged.
- Polish: the fallback retry loop no longer sleeps after its final (6th)
attempt — it only backs off when another attempt will follow, saving up
to 30s of wasted runner time on a hard failure.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [ ] New tests added for new functionality
- [x] Manual testing performed
### Test Output
```text
YAML validation: python -c "import yaml,sys; yaml.safe_load(open('.github/workflows/ci.yml')); print('YAML valid')"
→ YAML valid
pre-commit hooks (Sync plugin versions, ruff, ruff-format, mypy): all Passed/Skipped
```
## Real Behavior Proof
- **Root cause**: GitHub Actions cache is eventually-consistent. The
`prefetch-model` job saves the model under `Linux-models-allMiniLM-v2`.
The four `test` shards are independent runner VMs that restore from that
key concurrently. If a shard reaches the restore step before the cache
entry has propagated to the storage layer it gets a cache miss. With
`TRANSFORMERS_OFFLINE=1` on the runner and no model on disk, any test
that instantiates `LocalEmbedder` (≈25 tests) crashes with
`OSError`/`LocalEntryNotFoundError`. Since only some shards miss per run
the failure appears random.
- **Fix rationale**: The inline fallback approach (adding an `id` to the
restore step + a conditional download step) is the smallest possible
diff — two logical additions inside the existing `test` job, no new
jobs, no new artifacts, no changes to any other job. The alternative
(artifact-based sharing via `upload-artifact` / `download-artifact`)
would have been more reliable but required restructuring
`prefetch-model` and the `test` job more significantly. Given the
existing retry loop in `prefetch-model` already handles transient
HuggingFace failures, reusing it as a fallback is the right call.
- **Validated on this PR**: by adding `.github/workflows/**` to the
`code` filter, the `test` shards (×4) and `prefetch-model` execute on
this very PR and pass — so the modified pipeline is proven, not skipped.
- **Not tested**: a live cache miss is not deterministically
reproducible on-demand (it depends on Actions cache propagation timing);
the fallback is byte-for-byte the prefetch-model job's proven download
path, so its correctness rests on that parity.
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works (N/A — CI-only change, no Python source modified)
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable (N/A — CI
infrastructure fix)
## Additional Notes
The `prefetch-model` job is preserved as the warm-path optimisation: on
a typical run the cache hits and the fallback step is skipped entirely
(no extra cost). The fallback only fires on the rare cache-consistency
miss that was previously causing flakiness.
|
||
|---|---|---|
| .. | ||
| ci.yml | ||
| devcontainers.yml | ||
| docker.yml | ||
| docs.yml | ||
| eval.yml | ||
| init-e2e.yml | ||
| init-native-e2e.yml | ||
| install-native-e2e.yml | ||
| network-diff-capture.yml | ||
| pr-health.yml | ||
| publish.yml | ||
| release-please.yml | ||
| release.yml | ||
| rust.yml | ||
| stale.yml | ||
| wrap-e2e.yml | ||
| wrap-native-e2e.yml | ||