fix(tests): un-quarantine encoder acceptance suites, sync quarantine inventory - #1606
Merged
jlarson4 merged 1 commit intoAug 5, 2026
Conversation
…inventory Two changes that both make tests/QUARANTINES.md true again. 1. Un-quarantine the encoder acceptance suites (TransformerLensOrg#1605) test_hooked_encoder.py and test_hooked_encoder_decoder.py were skipped by 3efbd6e ("Cleanup (TransformerLensOrg#1129)", 2025-11-15) as "CI test pollution". There is no order-dependence in either: T5 is 22 passed / 1 skipped / 0 failed standalone and inside the full tier, and BERT had a single failure, test_bert_block. That failure was a stale API assumption, not pollution. BertLayer.forward returned tuple[torch.Tensor] in transformers 4.56 and returns a bare Tensor in 5.x, so huggingface_block(embed_out)[0] went from unpacking the tuple to indexing the batch dimension ([1,8,768] vs [8,768]). The module was quarantined while the pin was still transformers>=4.56 — the test was green at the time — and broke at the v5 floor bump in 8e8d9d4 (TransformerLensOrg#1315, 2026-05-19), unobserved because the module was dark. The sibling [0] unpacks at :58/:88/:108 are still correct; BertSelfAttention and BertEncoder kept their contracts. Restores 40 tests (36 runnable, 4 environment-gated). 2. Sync the inventory with the files TransformerLensOrg#1603 removed The reanchoring in TransformerLensOrg#1603 deleted 35 test files without touching tests/QUARANTINES.md, leaving 10 rows across 6 deleted files: test_hooked_transformer.py (x2), test_multi_gpu.py (x2), test_tensor_extraction_consistency.py, test_tokenization_methods.py, unit/components/test_attention.py (x3) and test_mlp_factory.py. Those rows are removed. With test_hooked_transformer.py gone, no whole-file quarantines remain, so the "⚠️ Technical debt — whole-file" section is dropped entirely. AGENTS.md and contributing.md both asserted the HookedTransformer acceptance suite was quarantined and named the two encoder files alongside it; both are corrected. Also fixes three drifted line numbers in the inventory and notes that the multi-GPU validation record refers to a since-removed file. Verified: tests/acceptance -m "not slow" -> 230 passed, 31 skipped, 0 failed. Every path referenced in QUARANTINES.md now exists, and every module-level skip marker in tests/ is accounted for. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
4 tasks
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
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.
Description
Two changes both making
tests/QUARANTINES.mdtrue again.1. Un-quarantine the encoder acceptance suites
test_hooked_encoder.pyandtest_hooked_encoder_decoder.pywere skipped in3efbd6e("Cleanup (#1129)", 2025-11-15) as "CI test pollution". Neither shows any order-dependence.T5 —
22 passed, 1 skipped, 0 failedstandalone and inside the full tier. No defect; the skip is simply removed.BERT — 13 of 16 passed; the one failure was
test_bert_block:BertLayer.forwardreturnedtuple[torch.Tensor]in transformers 4.56 and returns a baretorch.Tensorin 5.x, so the test's[0]went from unpacking the tuple to indexing the batch dimension. One line.Flagging for review: the module was quarantined 2025-11-15, while the pin was still
transformers>=4.56— so this test was passing when it was skipped. It broke at the v5 floor bump in8e8d9d4(#1315) on 2026-05-19 and went unseen for ~2.5 months because the module was dark. The three sibling[0]unpacks in the same file (:58,:88,:108) are still correct —BertSelfAttentionandBertEncoderkept their contracts.Restores 40 tests (36 runnable, 4 environment-gated).
2. Sync the inventory with the files #1603 removed
Per the discussion on #1603 and #1605. That PR deleted 35 test files without touching
tests/QUARANTINES.md, leaving 10 rows pointing at files that no longer exist:acceptance/test_hooked_transformer.pyacceptance/test_multi_gpu.pyintegration/test_tensor_extraction_consistency.pyintegration/test_tokenization_methods.pyunit/components/test_attention.pyunit/factories/test_mlp_factory.pyWith⚠️ Technical debt — whole-file" section is dropped entirely.
test_hooked_transformer.pygone, no whole-file quarantines remain, so the "AGENTS.mdanddocs/source/content/contributing.mdboth stated that the HookedTransformer acceptance suite was quarantined and named the two encoder files alongside it; both are corrected to reflect that the suite no longer exists and the encoder suites now run. Also fixes three drifted line numbers and notes that the multi-GPU validation record refers to a since-removed file.Verification
pytest tests/acceptance -m "not slow"→ 230 passed, 31 skipped, 11 deselected, 0 failedQUARANTINES.mdnow resolves to a file that existstests/is accounted for in the inventorypycln/isort/blackno-ops;mypycleanNotes for the maintainer
The two restored suites are cheap- BERT ~2.5 GB peak, T5 ~1.8 GB, ~35 s each. Having measured on the serial acceptance tier before #1603 landed, adding them changed peak RSS by less than run-to-run noise (7.53 GB without vs 6.26 GB with), so they should not affect the
coverage-testjob's memory headroom. Those figures are macOS-arm64 CPU and will differ on the Linux runners-compatibility-checksis the job that actually exercises the restored tests.Not included here but raised separately in #1605:
redwood_attn_2lis still listed intransformer_lens/supported_models.py:24but cannot load on the current dependency floor — its tokenizer repo (ArthurConmy/redwood_tokenizer) ships atokenizer.jsonwith 31,856 of 51,743 merges referencing tokens absent from its own vocab, whichtokenizers0.22.1 rejects. The only test that exercised it was deleted by #1603, so this is now purely a model-support question.Type of change
Checklist:
test_bert_blockis red-before / green-after)