tarekziade's picture
tarekziade HF Staff
Publish TRF rules proposal page
954020d verified
Raw History Blame Contribute Delete
49.2 kB
<!doctype html><html><head><meta charset=utf8><meta name=viewport content="width=device-width,initial-scale=1"><style>:root{color-scheme:light}body{margin:0;padding:0;font:14px -apple-system,BlinkMacSystemFont,sans-serif;background:#faf9f5;color:#141413}img{max-width:100%}[hidden]:not([hidden=until-found]){display:none!important}</style></head><body>
<title>TRF Rules for tests/models</title>
<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=IBM+Plex+Sans:ital,wght@0,400;0,500;0,600;1,400&family=IBM+Plex+Serif:wght@500;600&family=IBM+Plex+Mono:wght@400;500;600&display=swap">
<style>
:root {
--paper: #f3f6f3;
--surface: #fdfefd;
--ink: #161d18;
--ink-soft: #3b4841;
--muted: #62716a;
--rule: #d6ded8;
--rule-soft: #e6ebe6;
--accent: #1c6b4a;
--accent-soft: #e2efe7;
--del: #9b2f35;
--del-soft: #f6e6e6;
--warn: #8a5f14;
--warn-soft: #f4ecdb;
--code-bg: #f7f9f7;
--shadow: 0 1px 2px rgba(22, 29, 24, .05), 0 8px 24px -18px rgba(22, 29, 24, .35);
--maxw: 74rem;
color-scheme: light;
}
@media (prefers-color-scheme: dark) {
:root:not([data-theme="light"]) {
--paper: #0e1310;
--surface: #151b17;
--ink: #e7ede9;
--ink-soft: #c2cec7;
--muted: #8d9d94;
--rule: #263029;
--rule-soft: #1e2621;
--accent: #5cba8c;
--accent-soft: #16281f;
--del: #e2898d;
--del-soft: #2a1a1c;
--warn: #d6a64e;
--warn-soft: #29210f;
--code-bg: #111713;
--shadow: 0 1px 2px rgba(0, 0, 0, .4), 0 10px 30px -20px rgba(0, 0, 0, .8);
color-scheme: dark;
}
}
:root[data-theme="dark"] {
--paper: #0e1310;
--surface: #151b17;
--ink: #e7ede9;
--ink-soft: #c2cec7;
--muted: #8d9d94;
--rule: #263029;
--rule-soft: #1e2621;
--accent: #5cba8c;
--accent-soft: #16281f;
--del: #e2898d;
--del-soft: #2a1a1c;
--warn: #d6a64e;
--warn-soft: #29210f;
--code-bg: #111713;
--shadow: 0 1px 2px rgba(0, 0, 0, .4), 0 10px 30px -20px rgba(0, 0, 0, .8);
color-scheme: dark;
}
body {
background: var(--paper);
color: var(--ink);
font-family: "IBM Plex Sans", ui-sans-serif, system-ui, sans-serif;
font-size: 16px;
line-height: 1.6;
-webkit-font-smoothing: antialiased;
}
.wrap { max-width: var(--maxw); margin: 0 auto; padding: 0 1.5rem 6rem; }
.prose { max-width: 42rem; }
h1, h2, h3 { font-family: "IBM Plex Serif", ui-serif, Georgia, serif; font-weight: 600; text-wrap: balance; margin: 0; }
h1 { font-size: clamp(2rem, 4.4vw, 3rem); line-height: 1.1; letter-spacing: -.015em; }
h2 { font-size: 1.45rem; letter-spacing: -.01em; }
h3 { font-size: 1.1rem; }
p { margin: 0; }
code, pre, .mono { font-family: "IBM Plex Mono", ui-monospace, SFMono-Regular, monospace; }
.eyebrow {
font-family: "IBM Plex Mono", monospace; font-size: .7rem; font-weight: 500;
letter-spacing: .16em; text-transform: uppercase; color: var(--muted);
}
/* ---- masthead ---- */
header.mast { border-bottom: 1px solid var(--rule); background: var(--surface); }
.mast-inner { max-width: var(--maxw); margin: 0 auto; padding: 3.5rem 1.5rem 2.25rem; display: flex; flex-direction: column; gap: 1.25rem; }
.mast h1 span { color: var(--accent); }
.lede { font-size: 1.075rem; color: var(--ink-soft); max-width: 44rem; }
.provenance {
display: flex; flex-wrap: wrap; gap: .4rem 1.5rem; padding-top: .5rem;
border-top: 1px solid var(--rule-soft); font-size: .8rem; color: var(--muted);
}
.provenance b { font-family: "IBM Plex Mono", monospace; font-weight: 500; color: var(--ink-soft); font-variant-numeric: tabular-nums; }
.revnote { display: flex; flex-direction: column; gap: .5rem; padding: 1rem 1.15rem; border: 1px solid var(--rule); border-left: 2px solid var(--accent); border-radius: 2px; background: var(--accent-soft); }
.revnote ul { margin: 0; padding-left: 1.1rem; display: flex; flex-direction: column; gap: .4rem; font-size: .9rem; color: var(--ink-soft); }
.revnote ul b { color: var(--ink); }
.revnote ul li { max-width: 52rem; }
section { margin-top: 3.5rem; display: flex; flex-direction: column; gap: 1.25rem; }
.section-head { display: flex; flex-direction: column; gap: .45rem; }
/* ---- summary table ---- */
.tablewrap { overflow-x: auto; border: 1px solid var(--rule); border-radius: 3px; background: var(--surface); box-shadow: var(--shadow); }
table { border-collapse: collapse; width: 100%; font-size: .875rem; }
thead th {
text-align: left; padding: .7rem .9rem; border-bottom: 1px solid var(--rule);
font-family: "IBM Plex Mono", monospace; font-size: .68rem; font-weight: 500;
letter-spacing: .12em; text-transform: uppercase; color: var(--muted); white-space: nowrap;
}
tbody td { padding: .7rem .9rem; border-bottom: 1px solid var(--rule-soft); vertical-align: baseline; }
tbody tr:last-child td { border-bottom: 0; }
td.num, th.num { text-align: right; font-family: "IBM Plex Mono", monospace; font-variant-numeric: tabular-nums; white-space: nowrap; }
td.id a { font-family: "IBM Plex Mono", monospace; font-weight: 500; color: var(--accent); text-decoration: none; border-bottom: 1px solid transparent; }
td.id a:hover, td.id a:focus-visible { border-bottom-color: var(--accent); }
.hit { color: var(--del); font-weight: 500; }
.chip {
display: inline-block; padding: .1rem .45rem; border-radius: 2px;
font-family: "IBM Plex Mono", monospace; font-size: .7rem; font-weight: 500;
letter-spacing: .04em; white-space: nowrap;
}
.chip-now { background: var(--accent-soft); color: var(--accent); }
.chip-cut { background: var(--warn-soft); color: var(--warn); }
.chip-off { background: var(--rule-soft); color: var(--muted); }
.chip-rev { background: transparent; color: var(--accent); border: 1px solid var(--accent); }
/* ---- rule cards ---- */
.cards { display: flex; flex-direction: column; gap: 1.5rem; }
article.rule {
background: var(--surface); border: 1px solid var(--rule); border-radius: 3px;
box-shadow: var(--shadow); scroll-margin-top: 1.5rem; overflow: hidden;
}
.rule-head { padding: 1.25rem 1.4rem 1.1rem; display: flex; flex-direction: column; gap: .7rem; }
.rule-id { display: flex; align-items: center; gap: .6rem; flex-wrap: wrap; }
.rule-id .n { font-family: "IBM Plex Mono", monospace; font-size: .82rem; font-weight: 600; letter-spacing: .06em; color: var(--accent); }
.rule-head h3 { max-width: 44rem; }
.metrics { display: flex; flex-wrap: wrap; gap: .35rem 1.4rem; font-size: .8rem; color: var(--muted); }
.metrics span b { font-family: "IBM Plex Mono", monospace; font-variant-numeric: tabular-nums; font-weight: 600; color: var(--ink-soft); }
.metrics span.bad b { color: var(--del); }
.note { font-size: .9rem; color: var(--ink-soft); max-width: 44rem; }
.codeblock { border-top: 1px solid var(--rule); background: var(--code-bg); position: relative; }
.codebar {
display: flex; align-items: center; justify-content: space-between; gap: 1rem;
padding: .45rem .7rem .45rem 1.4rem; border-bottom: 1px solid var(--rule-soft);
}
.codebar .eyebrow { font-size: .65rem; }
pre { margin: 0; padding: 1rem 1.4rem 1.35rem; overflow-x: auto; font-size: .8rem; line-height: 1.65; color: var(--ink-soft); }
pre b { font-weight: 500; color: var(--ink); }
pre .k { color: var(--accent); }
pre .add { color: var(--accent); }
pre .del { color: var(--del); }
button.copy {
font-family: "IBM Plex Mono", monospace; font-size: .7rem; letter-spacing: .06em;
text-transform: uppercase; color: var(--muted); background: transparent;
border: 1px solid var(--rule); border-radius: 2px; padding: .25rem .55rem; cursor: pointer;
}
button.copy:hover { color: var(--accent); border-color: var(--accent); }
button.copy:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; }
button.copy[data-done="1"] { color: var(--accent); border-color: var(--accent); }
ul.reject { list-style: none; padding: 0; margin: 0; display: flex; flex-direction: column; gap: .9rem; }
ul.reject li { padding-left: 1.1rem; border-left: 2px solid var(--rule); font-size: .92rem; color: var(--ink-soft); max-width: 44rem; }
ul.reject b { font-weight: 600; color: var(--ink); }
.ship { display: flex; flex-direction: column; gap: .5rem; font-size: .92rem; color: var(--ink-soft); }
.ship p { max-width: 44rem; }
footer { margin-top: 4rem; padding-top: 1.25rem; border-top: 1px solid var(--rule); font-size: .8rem; color: var(--muted); }
a { color: var(--accent); }
@media (prefers-reduced-motion: reduce) { * { transition: none !important; animation: none !important; } }
</style>
<header class="mast">
<div class="mast-inner">
<p class="eyebrow">mlinter rule proposal Β· revision 6, after review</p>
<h1>Nine rules for <span>tests/models</span></h1>
<p class="lede">
Every rule below enforces a convention transformers already documents in the testing guide or
CONTRIBUTING β€” not one we are inventing. Each was measured against the repository first, so the
burn-down is known before the number is claimed.
</p>
<div class="provenance">
<span>measured at <b>d6a47b6ada</b></span>
<span><b>892</b> test files</span>
<span><b>340</b> test_modeling_*.py</span>
<span>ids <b>TRF060–TRF070</b>, two withdrawn</span>
</div>
<div class="revnote">
<p class="eyebrow">Changed in revision 6</p>
<ul>
<li><b>TRF063 rewritten, its premise conceded.</b> "A skip must have a reason" was theatre β€” 2590 of 2617 already do, 93% over 30 characters. The objection that the explanations are poor is the finding: the rule now targets deferrals with nothing tracking them (<b>100</b> of those, plus the 27 with no reason), and only 34 reasons in the whole suite reference an issue.</li>
<li><b>TRF064 dropped.</b> Its count was my error (505 β†’ <b>118</b>, once <code>**kwargs</code> forwarding and <code>num_beams</code> / <code>temperature</code> / <code>generation_config=</code> are honored), and its last false-positive source lives in a hub file no static check can read. Shipping it off by default would have made it a docs page, not a check β€” transformers' own <code>utils/rules.toml</code> has two such rules, TRF003 and TRF037, and nothing there passes <code>--enable-rules</code>, so neither has ever fired. <b>Every rule left in this set is enabled by default.</b></li>
<li><b>TRF066 moved behind a cutoff.</b> A personal-account fixture is sometimes deliberate, so the 60 existing references are grandfathered; only new models start in the org, and a new intentional case still has <code>trf-ignore</code>.</li>
<li><b>TRF069 withdrawn.</b> A mixin can require the reason as data instead of a comment, which is stronger than a lint rule and cheaper to satisfy β€” folded into the mixin sketch above.</li>
</ul>
<p class="note" style="font-size:.82rem">Earlier rounds: TRF066 rewritten from "no generated weights" to fixture hosting, TRF070 added for third-party URLs, TRF066 extended to dataset repos, TRF061 rebuilt around the mixin.</p>
</div>
</div>
</header>
<div class="wrap">
<section>
<div class="section-head">
<p class="eyebrow">The set at a glance</p>
<h2>What each rule costs to turn on</h2>
</div>
<div class="tablewrap">
<table>
<thead>
<tr>
<th>Rule</th><th>Catches</th><th class="num">Violations</th><th class="num">Compliant</th><th>Ship as</th>
</tr>
</thead>
<tbody>
<tr><td class="id"><a href="#TRF060">TRF060</a></td><td>Real weights loaded without <code>@slow</code></td><td class="num hit">26</td><td class="num">879</td><td><span class="chip chip-now">fix now</span></td></tr>
<tr><td class="id"><a href="#TRF061">TRF061</a></td><td>Integration class that never cleans the device</td><td class="num hit">193</td><td class="num">35</td><td><span class="chip chip-cut">cutoff</span></td></tr>
<tr><td class="id"><a href="#TRF062">TRF062</a></td><td>Hardcoded <code>"cuda"</code> instead of <code>torch_device</code></td><td class="num hit">24</td><td class="num">β€”</td><td><span class="chip chip-now">fix now</span></td></tr>
<tr><td class="id"><a href="#TRF063">TRF063</a></td><td>Skip that defers a problem with nothing tracking it</td><td class="num hit">127</td><td class="num">2490</td><td><span class="chip chip-now">fix now</span></td></tr>
<tr><td class="id"><a href="#TRF065">TRF065</a></td><td>Model test class without <code>@require_torch</code></td><td class="num hit">9</td><td class="num">579</td><td><span class="chip chip-now">fix now</span></td></tr>
<tr><td class="id"><a href="#TRF066">TRF066</a></td><td>Fixture checkpoint or dataset on a personal account</td><td class="num hit">60</td><td class="num">192</td><td><span class="chip chip-cut">cutoff</span></td></tr>
<tr><td class="id"><a href="#TRF067">TRF067</a></td><td>Causal-LM test not on <code>CausalLMModelTest</code></td><td class="num hit">106</td><td class="num">84</td><td><span class="chip chip-cut">cutoff</span></td></tr>
<tr><td class="id"><a href="#TRF068">TRF068</a></td><td>Test file with no integration test at all</td><td class="num hit">18</td><td class="num">320</td><td><span class="chip chip-now">fix now</span></td></tr>
<tr><td class="id"><a href="#TRF070">TRF070</a></td><td>Fixture fetched from a third-party URL</td><td class="num hit">202</td><td class="num">325</td><td><span class="chip chip-cut">cutoff</span></td></tr>
</tbody>
</table>
</div>
<div class="ship prose">
<p>
<b>Discovery.</b> <code>mlinter/mlinter.py:61</code> already anticipates this: <code>TEST_PATTERNS</code>
needs <code>test_modeling_*.py</code> added, and every rule gates on the file-name prefix exactly as
TRF042 does. Cutoff dates below are placeholders β€” <code>2026-09-01</code> grandfathers everything
already in the repository, so the rule only bites new models and burns down.
</p>
</div>
</section>
<section>
<div class="section-head">
<p class="eyebrow">Upstream prerequisite</p>
<h2>A mixin, not 370 copies of <code>setUp</code></h2>
<p class="note">The strongest piece of review feedback, with the numbers behind it: <code>tests/models</code> holds <b>492 integration classes</b> that hand-roll <b>370 lifecycle methods</b> β€” 152 <code>setUp</code>, 174 <code>tearDown</code>, 31 <code>setUpClass</code>, 17 <code>tearDownClass</code> β€” nearly all doing the same two calls. The naming converged on its own with nobody enforcing it: <code>test_small_model_integration_test</code> in 44 classes, <code>…_batch</code> in 36, <code>test_inference_no_head</code> in 49, <code>model_id</code> as the checkpoint attribute in 37.</p>
</div>
<div class="codeblock" style="border: 1px solid var(--rule); border-radius: 3px; background: var(--surface); box-shadow: var(--shadow);">
<div class="codebar"><span class="eyebrow">sketch Β· tests/test_integration_mixin.py</span><button class="copy">Copy</button></div>
<pre>@slow
@require_torch
class IntegrationTesterMixin:
"""Lifecycle and standard checks for a model's real-weights tests."""
model_id = None # the checkpoint under test
prompts = ["Hello, world"] # inputs the standard tests feed it
expected_logits = None # tensor or Expectations(...); None skips the logits test
expected_texts = None # None skips the generation test
# what this model can be held to
test_logits = True
test_greedy_generation = True
test_batched_generation = False # opt in where padding matters
has_public_checkpoint = True # False records "checkpoint not released yet"
def setUp(self):
cleanup(torch_device, gc_collect=False)
def tearDown(self):
cleanup(torch_device, gc_collect=False)</pre>
</div>
<div class="ship prose">
<p>Attribute names are a sketch for discussion, not a proposal to merge as written. What matters for the rules:</p>
<ul class="reject" style="margin-top:.75rem">
<li><b>TRF061 does not block on it.</b> The rule asks for cleanup at both ends and resolves base classes, so a hand-written pair passes today and inheriting the mixin passes the day it lands β€” no rule change, no re-run of the burn-down.</li>
<li><b>TRF068 gets a better record.</b> <code>has_public_checkpoint = False</code> is a structured way to say "no integration test yet, checkpoint is not out", which a <code># trf-ignore</code> comment only approximates. The comment is the mechanism until the mixin exists; after that, the flag is.</li>
<li><b>It absorbs the former TRF064.</b> The mixin's own generation test passes <code>do_sample=False</code>, so every model that adopts it gets deterministic generation by construction β€” no rule, no 118-call burn-down, and nothing for a reviewer to remember.</li>
<li><b>It replaces TRF069 outright.</b> Rather than a lint rule demanding a comment beside <code>test_resize_embeddings = False</code>, the flag can carry its own reason β€” <code>unsupported = {"resize_embeddings": "Acme ties encoder and decoder embeddings"}</code>, or a string value in place of <code>False</code> β€” and the mixin asserts it is non-empty. Enforced by construction, no rule number, and the reason ends up somewhere a test report can print it. That is why TRF069 is withdrawn from this set.</li>
</ul>
<h3 style="margin-top:1.75rem">Conversion cost, and the one decision it needs</h3>
<p>The mixin makes TRF061's burn-down <em>larger</em> in lines touched β€” 290 class declarations rewritten rather than two methods added to each β€” but entirely mechanical: a base-class insertion plus a <code>model_id</code> attribute the class usually already has under that name, so a script can do the bulk and leave the odd <code>setUpClass</code> case for review.</p>
<p>Which leaves one call to make: does the conversion land as <b>its own PR</b> ahead of the rules, or does it <b>ride along with the cutoff</b> β€” new models comply on arrival, existing ones convert whenever their file is next touched? The first gives TRF061 clean numbers immediately and one large diff to review; the second costs nothing up front and leaves the 193 leaking classes leaking a while longer. The rule text is identical either way.</p>
</div>
</section>
<section>
<div class="section-head">
<p class="eyebrow">Tier 1 β€” five rules</p>
<h2>Settled convention, small burn-down</h2>
<p class="note">Compliance is already 96–99% on each of these. They can land enabled, with the existing violations fixed in the same PR.</p>
</div>
<div class="cards">
<article class="rule" id="TRF060">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF060</span><span class="chip chip-now">fix now</span></div>
<h3>A test that loads real pretrained weights must be marked <code>@slow</code></h3>
<div class="metrics">
<span class="bad"><b>26</b> in test_modeling_*</span>
<span><b>879</b> already correct</span>
<span><b>122</b> more in tokenizer/processor tests, for a later widening</span>
</div>
<p class="note">The testing guide states it outright: mark a test <code>@slow</code> if it downloads weights. This is the checkable proxy for β€œheavy” β€” <em>not</em> a GPU requirement, which 60 fast tests carry deliberately.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF060]
description = "A test that loads real pretrained weights must be marked @slow."
default_enabled = true
allowlist_models = ["auto"]
[rules.TRF060.explanation]
what_it_does = "In tests/models/*/test_modeling_*.py, flags a test method that calls from_pretrained() on a hub id -- an `org/name` string literal, or a class-level constant holding one -- unless the method or its class carries @slow. Ids that name random weights (anything matching `tiny-`, `-tiny`, `tiny-random` or `random-`) are not real weights and are left to the fast suite. `auto` is allowlisted: test_modeling_auto.py resolves remote-code repositories by design, and those fixtures are the subject under test rather than model weights."
why_bad = "The pull-request CI runs every test that is not marked @slow, on every pull request in the repository. A test that pulls real weights charges its download and its inference to contributors touching unrelated files, and turns a hub hiccup into a red CI on their branch. @slow is what moves it to the nightly run that is provisioned for it."
diff = '''
class AcmeIntegrationTest(unittest.TestCase):
<span class="add">+ @slow</span>
def test_inference_logits(self):
model = AcmeForCausalLM.from_pretrained("acme-ai/acme-7b").to(torch_device)
'''</pre>
</div>
</article>
<article class="rule" id="TRF062">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF062</span><span class="chip chip-now">fix now</span></div>
<h3>Tests must take the device from <code>torch_device</code>, never a hardcoded <code>"cuda"</code> string</h3>
<div class="metrics">
<span class="bad"><b>9</b> device literals</span>
<span class="bad"><b>15</b> direct torch.cuda.* calls</span>
<span>zero burn-down risk</span>
</div>
<p class="note">The slow suite runs on XPU, NPU and ROCm too. Comparisons like <code>torch_device == "cuda"</code> and <code>Expectations</code> SM keys stay untouched β€” naming the hardware is the point in both.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF062]
description = "Tests must take the device from torch_device, never a hardcoded \"cuda\" string."
default_enabled = true
allowlist_models = []
[rules.TRF062.explanation]
what_it_does = "Flags a hardcoded accelerator string in a test file: `.to(\"cuda\")`, a `device=` or `device_map=` keyword whose value is a `cuda` literal, and direct `torch.cuda.*` calls that have a device-agnostic helper in testing_utils (`empty_cache` -&gt; `backend_empty_cache`, `device_count` -&gt; `backend_device_count`, `manual_seed_all` -&gt; `backend_manual_seed_all`). Comparisons such as `torch_device == \"cuda\"` and Expectations SM keys such as `(\"cuda\", (8, 6))` are not flagged: naming the hardware is the point in both."
why_bad = "The slow suite runs on XPU, NPU and ROCm as well as CUDA. A hardcoded device string either errors on those runners or, worse, quietly leaves the tensor somewhere the model is not, so the test measures nothing and reports green. torch_device and the backend_* helpers are already resolved for the runner in hand."
diff = '''
<span class="del">-inputs = tokenizer("Hello", return_tensors="pt").to("cuda")</span>
<span class="del">-torch.cuda.empty_cache()</span>
<span class="add">+inputs = tokenizer("Hello", return_tensors="pt").to(torch_device)</span>
<span class="add">+backend_empty_cache(torch_device)</span>
'''</pre>
</div>
</article>
<article class="rule" id="TRF063">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF063</span><span class="chip chip-now">fix now</span><span class="chip chip-rev">rewritten</span></div>
<h3>A skip that defers a problem must link the issue tracking it</h3>
<div class="metrics">
<span class="bad"><b>100</b> untracked deferrals</span>
<span class="bad"><b>27</b> with no reason at all</span>
<span><b>34</b> of 2590 reasons carry a link</span>
</div>
<p class="note">The original rule β€” "a skip must have a reason" β€” was not worth a number: 2590 of 2617 skips already have one, 93% over 30 characters. The complaint that the explanations are poor is the real finding, and it is checkable: <code>"This is broken now, no idea why"</code>, <code>"TODO Arthur I have to skip for now because I don't understand it"</code>, <code>"Some undefined behavior encountered with test versions of this model. Skip for now."</code> (23 uses). A deferral without a tracking link never comes back.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF063]
description = "A skip that defers a problem must link the issue tracking it."
default_enabled = true
allowlist_models = []
[rules.TRF063.explanation]
what_it_does = "In any tests/models test file, flags `@unittest.skip` whose reason is missing, is a bare verdict (`TODO`, `Not implemented`, `Very flaky`), or defers the problem without naming where it is tracked. Deferral is read from the wording -- `for now`, `temporarily`, `to investigate`, `needs a fix`, `undefined behaviour`, `no idea why` -- and a reason carrying an issue reference (`#12345`) or a URL satisfies the rule however it is phrased. A reason that states a permanent architectural fact (`Acme has no inputs_embeds`, `this module does not support standalone training`) is not a deferral and needs no link."
why_bad = "Requiring any reason at all would be theatre: 2590 of 2617 skips already carry one. What is missing is the 4% that say a test was turned off and nothing else -- 23 copies of `Skip for now`, `broken now, no idea why`, `keep hitting edge cases`. Those are the ones that never get switched back on, because there is nothing to close and nobody owns them; only 34 reasons in the whole suite reference an issue. A link turns a silent hole in coverage into tracked work."
diff = '''
<span class="del">-@unittest.skip(reason="Some undefined behavior with test versions of this model. Skip for now.")</span>
<span class="add">+@unittest.skip(reason="Tiny checkpoint emits NaN logits past 2 layers, see #43210")</span>
def test_batched_generation(self):
'''</pre>
</div>
</article>
<article class="rule" id="TRF065">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF065</span><span class="chip chip-now">fix now</span></div>
<h3>A collected model test class must be decorated with <code>@require_torch</code></h3>
<div class="metrics">
<span class="bad"><b>9</b> undecorated classes</span>
<span><b>579</b> already correct</span>
</div>
<p class="note">Without it the class still gets collected in a torch-free environment, and every test errors on a <code>NameError</code> instead of skipping. Mostly encoder-only classes: <code>T5EncoderOnlyModelTest</code>, <code>UdopEncoderOnlyModelTest</code>.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF065]
description = "A collected model test class must be decorated with @require_torch."
default_enabled = true
allowlist_models = []
[rules.TRF065.explanation]
what_it_does = "In test_modeling_*.py, flags a collected test class that mixes in the shared suite (ModelTesterMixin, or one of CausalLMModelTest / VLMModelTest / ALMModelTest) and carries no `require_torch` decorator. The narrower gates count: require_torch_gpu, require_torch_accelerator, require_torch_multi_gpu and friends all imply torch."
why_bad = "Model test files import their torch symbols under `if is_torch_available():`, so without the decorator the class is still collected in an environment without torch and every one of its tests errors on a NameError instead of skipping. The decorator is what turns that into an honest skip."
diff = '''
<span class="add">+@require_torch</span>
class AcmeModelTest(ModelTesterMixin, unittest.TestCase):
all_model_classes = (AcmeModel, AcmeForCausalLM)
'''</pre>
</div>
</article>
<article class="rule" id="TRF068">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF068</span><span class="chip chip-now">fix now</span></div>
<h3>A model test file must contain at least one <code>@slow</code> integration test</h3>
<div class="metrics">
<span class="bad"><b>18</b> files with none</span>
<span><b>320</b> have one</span>
<span class="chip chip-rev">escape hatch added</span>
</div>
<p class="note">CONTRIBUTING asks contributors to prioritize exactly this test. A model whose checkpoint is not public yet writes <code># trf-ignore: TRF068</code> naming what it is waiting on, so the owed test is greppable when the release lands; permanent exemptions (backbone files) go in <code>allowlist_models</code>.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF068]
description = "A model test file must contain at least one @slow integration test."
default_enabled = true
allowlist_models = []
[rules.TRF068.explanation]
what_it_does = "Flags a tests/models/*/test_modeling_*.py file in which no test method or test class carries @slow. Reported once per file, on the first collected test class. A model whose checkpoint is not published yet records that on the class -- `has_public_checkpoint = False` once the integration mixin exists, or `# trf-ignore: TRF068` above the class until it does, naming the checkpoint it is waiting on -- the directive is the record that the integration test is still owed, and grep finds every one of them when a release lands. Files covering a backbone or sub-component that the parent model's file already exercises end-to-end belong in allowlist_models instead, since that exemption is permanent."
why_bad = "The mixin suite runs tiny random configs and checks shapes, gradients and save/load round-trips. It passes on a model whose attention scaling, rope layout or mask is wrong, because wrong numbers have the right shape. A file with no integration test has never once compared this architecture against a checkpoint, which is the only check that would have caught that -- and CONTRIBUTING asks contributors to prioritize exactly that test."
diff = '''
<span class="add">+@slow</span>
<span class="add">+@require_torch</span>
<span class="add">+class AcmeIntegrationTest(unittest.TestCase):</span>
<span class="add">+ def test_inference_logits(self):</span>
<span class="add">+ model = AcmeForCausalLM.from_pretrained("acme-ai/acme-7b").to(torch_device)</span>
<span class="add">+ expected = torch.tensor([[-1.2345, 0.6789, 2.3456]])</span>
<span class="add">+ torch.testing.assert_close(out.logits[0, :1, :3], expected, atol=1e-4, rtol=1e-4)</span>
'''</pre>
</div>
</article>
</div>
</section>
<section>
<div class="section-head">
<p class="eyebrow">Tier 2 β€” four rules</p>
<h2>Documented or agreed, widely unfollowed β€” land behind a cutoff date</h2>
<p class="note">Each of these is written down, or agreed in review, and followed by a minority of models. The cutoff mechanism is what makes them shippable: new models comply, existing ones burn down, and an intentional exception still has <code>trf-ignore</code>.</p>
</div>
<div class="cards">
<article class="rule" id="TRF061">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF061</span><span class="chip chip-cut">cutoff 2026-09-01</span><span class="chip chip-rev">rewritten</span></div>
<h3>An integration test class must clean the accelerator up in <code>setUp</code> and <code>tearDown</code></h3>
<div class="metrics">
<span class="bad"><b>193</b> classes with neither</span>
<span><b>62</b> with one of the two</span>
<span><b>35</b> fully correct</span>
</div>
<p class="note">The leak lands on someone else's PR: the slow suite runs model after model in one process, so weights left resident OOM the <em>next</em> model's tests while the class that leaked reports green. An <em>inherited</em> lifecycle counts β€” that is the point of the mixin above, and why the rule needs no revision when it lands.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF061]
description = "An integration test class must clean the accelerator up in setUp and tearDown."
default_enabled = true
allowlist_models = []
cutoff_date = "2026-09-01"
[rules.TRF061.explanation]
what_it_does = "In test_modeling_*.py, for a collected test class whose @slow tests load pretrained weights, checks that the class cleans the device up at both ends of every test. Inheriting a base that does it -- the proposed IntegrationTesterMixin -- satisfies the rule outright; so does calling `cleanup(torch_device, ...)`, or backend_empty_cache plus gc.collect, in the class's own setUp and tearDown. Bases are resolved in the same file and one hop into another model's test class, as TRF042 does, so a class that inherits its lifecycle is never reported for not repeating it. Reported once per class, on the class definition."
why_bad = "The slow suite runs model after model in one process on one accelerator. A class that returns without freeing its weights leaves them resident, and the next model in the run hits an out-of-memory error -- so the failure surfaces in someone else's file, and the model that actually leaked looks green. Cleanup at both ends is what makes a class survive a predecessor that skipped its own. The rule accepts an inherited lifecycle deliberately: 492 integration classes currently hand-roll 370 setUp/tearDown methods to do the same two calls, and one shared mixin is both less to write and the only way the next change to what cleanup means -- another cache to drop, another reset to call -- lands in one place instead of 370."
diff = '''
<span class="del">-class AcmeIntegrationTest(unittest.TestCase):</span>
<span class="add">+class AcmeIntegrationTest(IntegrationTesterMixin, unittest.TestCase):</span>
<span class="add">+ model_id = "acme-ai/acme-7b"</span>
<span class="add">+</span>
@slow
def test_inference_logits(self):
'''</pre>
</div>
</article>
<article class="rule" id="TRF066">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF066</span><span class="chip chip-cut">cutoff 2026-09-01</span><span class="chip chip-rev">grandfathered</span></div>
<h3>A test fixture β€” generated checkpoint or fixture dataset β€” must live under <code>hf-internal-testing</code></h3>
<div class="metrics">
<span class="bad"><b>17</b> checkpoint refs on personal accounts</span>
<span class="bad"><b>43</b> dataset refs, 16 repos</span>
<span><b>192</b> already team-owned</span>
</div>
<p class="note">Hosting a fixture on a personal account is sometimes a deliberate call, so the rule ships behind a cutoff: the 60 existing references are grandfathered and only new models have to start in the org, with <code># trf-ignore: TRF066</code> for a new case that is genuinely intentional. Published corpora β€” <code>facebook/xnli</code>, <code>mozilla-foundation/common_voice_11_0</code> β€” are data the test consumes, not fixtures, and are never flagged.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF066]
description = "A test fixture -- generated checkpoint or fixture dataset -- should live under hf-internal-testing."
default_enabled = true
allowlist_models = []
cutoff_date = "2026-09-01"
[rules.TRF066.explanation]
what_it_does = "Flags two kinds of fixture reference in a tests/models test file whose owner is not hf-internal-testing. First, a from_pretrained() hub id that names generated weights (`tiny-random-*`, `*-random-tiny`, `random-*`, `dummy-*`); a published checkpoint that merely has `tiny` or `small` in its name -- facebook/convnext-tiny-224, openai/whisper-tiny, asapp/sew-d-tiny-100k -- is real released weights and is never flagged. Second, a load_dataset(), hf_hub_download() or snapshot_download() repo id whose name reads as a test fixture: `dummy`, `fixtures`, `example`, `sample`, `test`, `testing`, `demo` or `ci` as a whole token. A published corpus keeps its own name and is left alone -- facebook/xnli, mozilla-foundation/common_voice_11_0 and google/code_x_glue_ct_code_to_text are data the test consumes, not fixtures the CI owns. Whether the test is @slow does not matter: a fixture the CI depends on has to be hosted where the team can restore it either way."
why_bad = "Generated checkpoints and hand-made fixture assets are both legitimate -- the smallest published checkpoint often does not fit the runner, and a deterministic logits check catches a numerical regression even when the output is garbage. What matters is who can delete the fixture. 17 checkpoint references and 43 dataset references point at personal accounts: patrickvonplaten/longformer-random-tiny, sgugger/funnel-random-tiny, nielsr/example-pdf, raushan-testing-hf/videos-test, anton-l/superb_dummy. Any of them takes a model's whole suite down the day that person tidies their profile, and nobody else can restore it. hf-internal-testing is the org the team owns, utils/create_dummy_models.py already uploads checkpoints there, and fixture assets belong beside them in an hf-internal-testing/fixtures_* dataset repo."
diff = '''
@slow
def test_inference_logits(self):
<span class="del">- model = AcmeForCausalLM.from_pretrained("jdoe/tiny-random-acme")</span>
<span class="add">+ model = AcmeForCausalLM.from_pretrained("hf-internal-testing/tiny-random-acme")</span>
<span class="del">- path = hf_hub_download(repo_id="jdoe/acme-test-images", filename="cats.jpg", repo_type="dataset")</span>
<span class="add">+ path = hf_hub_download(</span>
<span class="add">+ repo_id="hf-internal-testing/fixtures_acme", filename="cats.jpg", repo_type="dataset"</span>
<span class="add">+ )</span>
'''</pre>
</div>
</article>
<article class="rule" id="TRF067">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF067</span><span class="chip chip-cut">cutoff 2026-09-01</span></div>
<h3>A causal-LM model test should build on <code>CausalLMModelTest</code>, not hand-rolled mixins</h3>
<div class="metrics">
<span class="bad"><b>106</b> hand-assembled</span>
<span><b>84</b> on the shared base</span>
<span>cross-file: reads modeling_*.py</span>
</div>
<p class="note">This is TRF042's shape applied to modeling tests. A hand-assembled class silently ships without training and tensor-parallel coverage, and stops inheriting whatever the shared base gains next.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF067]
description = "A causal-LM model test should build on CausalLMModelTest, not hand-rolled mixins."
default_enabled = true
allowlist_models = []
cutoff_date = "2026-09-01"
[rules.TRF067.explanation]
what_it_does = "When the model directory defines a &lt;Model&gt;ForCausalLM, flags a collected test class in test_modeling_*.py that mixes ModelTesterMixin in directly instead of inheriting CausalLMModelTest. Reads the companion modeling_*.py to decide whether the model is a causal LM; a test class whose bases cannot be resolved is inconclusive and is skipped. Encoder-decoder and standalone-decoder test classes in a file that also has a causal-LM class are not the target -- the rule reports the class that stands in for the model's main suite."
why_bad = "CausalLMModelTest is the recommended base in the testing guide, and it carries TrainingTesterMixin and TensorParallelTesterMixin on top of the three shared mixins. A hand-assembled class silently ships without training and tensor-parallel coverage, and it stops inheriting whatever the shared base gains next -- which is how the same model ends up with a hundred fewer tests than the model merged beside it."
diff = '''
<span class="del">-class AcmeModelTest(ModelTesterMixin, GenerationTesterMixin, PipelineTesterMixin, unittest.TestCase):</span>
<span class="del">- all_model_classes = (AcmeModel, AcmeForCausalLM)</span>
<span class="del">-</span>
<span class="del">- def setUp(self):</span>
<span class="del">- self.model_tester = AcmeModelTester(self)</span>
<span class="add">+class AcmeModelTester(CausalLMModelTester):</span>
<span class="add">+ if is_torch_available():</span>
<span class="add">+ base_model_class = AcmeModel</span>
<span class="add">+</span>
<span class="add">+</span>
<span class="add">+class AcmeModelTest(CausalLMModelTest, unittest.TestCase):</span>
<span class="add">+ model_tester_class = AcmeModelTester</span>
'''</pre>
</div>
</article>
<article class="rule" id="TRF070">
<div class="rule-head">
<div class="rule-id"><span class="n">TRF070</span><span class="chip chip-cut">cutoff 2026-09-01</span><span class="chip chip-rev">new</span></div>
<h3>A remote test fixture must be fetched from the Hub, not a third-party URL</h3>
<div class="metrics">
<span class="bad"><b>202</b> third-party URLs</span>
<span><b>90</b> test files affected</span>
<span><b>325</b> already hub-hosted</span>
</div>
<p class="note">16 hosts nobody here controls, including <code>thumbs.dreamstime.com</code>, <code>cdn.britannica.com</code>, a personal GitHub raw file β€” and <code>picsum.photos</code>, used 12 times, which returns a <em>different random image on every request</em>.</p>
</div>
<div class="codeblock">
<div class="codebar"><span class="eyebrow">rules.toml</span><button class="copy">Copy</button></div>
<pre>[rules.TRF070]
description = "A remote test fixture must be fetched from the Hub, not a third-party URL."
default_enabled = true
allowlist_models = []
cutoff_date = "2026-09-01"
[rules.TRF070.explanation]
what_it_does = "Flags an `http(s)://` string literal in a tests/models test file whose host is not huggingface.co or hf.co -- the images, videos, audio clips and PDFs that tests download as inputs. Hub repo ids passed to from_pretrained(), load_dataset() or hf_hub_download() are not URLs and are unaffected, so the fix is always to upload the asset to a dataset repo (`hf-internal-testing/fixtures_*`, as test_modeling_pix2struct.py already does with fixtures_docvqa) and reference it by id."
why_bad = "202 fixture URLs across 90 test files point at hosts nobody here controls: images.cocodataset.org, cdn.britannica.com, thumbs.dreamstime.com, a personal file in a GitHub repo, and picsum.photos, which returns a different random image on every request. Each one is a way for a model's integration test to break, or to quietly start testing a different input, because of a change made by a stranger -- and the expected logits in the test were recorded against the old asset. A fixture on the Hub is as permanent and as reviewable as the test that uses it."
diff = '''
<span class="del">-url = "http://images.cocodataset.org/val2017/000000039769.jpg"</span>
<span class="del">-image = Image.open(requests.get(url, stream=True).raw)</span>
<span class="add">+path = hf_hub_download(</span>
<span class="add">+ repo_id="hf-internal-testing/fixtures_acme", filename="cats.jpg", repo_type="dataset"</span>
<span class="add">+)</span>
<span class="add">+image = Image.open(path)</span>
'''</pre>
</div>
</article>
</div>
</section>
<section>
<div class="section-head">
<p class="eyebrow">Due diligence</p>
<h2>Considered and rejected</h2>
</div>
<ul class="reject">
<li><b>A <code>@slow</code> generation test must pin its sampling (the former TRF064).</b> Dropped, not demoted. The corrected count is 118 of 989 slow <code>generate()</code> calls β€” after honoring the 290 that forward <code>**kwargs</code> and the 87 that pin <code>num_beams</code> / <code>temperature</code> / <code>generation_config=</code> β€” but a checkpoint whose own <code>generation_config.json</code> sets <code>do_sample: false</code> is deterministic without the keyword, and no static check can read that file. Shipping it off by default would have made it a docs page rather than a check: transformers' <code>utils/rules.toml</code> already carries two off-by-default rules, TRF003 and TRF037, and nothing in that repo passes <code>--enable-rules</code>, so neither has ever fired. The determinism belongs in the integration mixin's standard generation test, and the airtight version is a hub-side assertion in the nightly job.</li>
<li><b>A skip must carry any reason at all (the original TRF063).</b> Already true 99% of the time, and blind to the reasons that are useless. Replaced by the tracking-link version, which targets the 4% that actually hide work.</li>
<li><b>Justification comments on disabled mixin flags (the former TRF069).</b> Withdrawn on review β€” a mixin can require the reason as structured data, which no comment-presence check can match. See the prerequisite section.</li>
<li><b>Integration tests must load real weights, not generated ones.</b> The original TRF066, dropped on review: a 1B checkpoint may not fit the runner, and a deterministic logits assertion catches a numerical regression even when the generated model's output is garbage. TRF066 now polices where the fixture is hosted, which is the part that actually breaks CI.</li>
<li><b><code>require_torch_gpu</code> implies <code>@slow</code>.</b> It doesn't. 664 tests carry a GPU-ish gate and 60 are deliberately fast β€” <code>test_multi_gpu_data_parallel_forward</code>, <code>test_can_use_device_map</code> β€” because the PR CI has GPU runners for tiny models. "Downloads real weights" is the checkable proxy for heavy, which is why TRF060 is written that way.</li>
<li><b><code>torch.testing.assert_close</code> over <code>assertTrue(torch.allclose(...))</code>.</b> 486 occurrences and better failure diagnostics, but no correctness story β€” style, with a large burn-down attached.</li>
<li><b><code>pipeline_model_mapping</code> required alongside <code>PipelineTesterMixin</code>.</b> Zero violations; <code>utils/add_pipeline_model_mapping_to_test.py</code> already keeps it honest.</li>
<li><b>MoE integration tests calling <code>set_experts_implementation("eager")</code>.</b> The guide suggests it, commented out in its own example, and 2 of 124 tests do it. Too new to enforce β€” worth revisiting once the convention settles.</li>
<li><b><code>tempfile.mkdtemp()</code> without cleanup.</b> Two occurrences in modeling tests. Not worth a rule number.</li>
</ul>
</section>
<section>
<div class="section-head">
<p class="eyebrow">Known gap</p>
<h2>What the extended TRF066 still misses</h2>
</div>
<p class="note">TRF066 keys off fixture naming, so a <em>real</em> checkpoint or asset pulled from a personal account under a plain name β€” <code>nielsr/audio-spectogram-transformer-checkpoint</code> in <code>test_modeling_audio_spectrogram_transformer.py:227</code> β€” reads exactly like a published repo to a static check. Nothing in the id separates a personal account from an org, so catching those needs a hub lookup (out of scope: mlinter is static analysis only) or a maintained org allowlist. Noted so nobody assumes the rule covers it.</p>
</section>
<footer>
Counts produced by static AST probes matching each rule's described check, run over
<code>tests/models</code> at <code>d6a47b6ada</code>. Sources: <code>docs/source/en/testing.md</code>,
<code>CONTRIBUTING.md</code>, and the existing <code>mlinter/rules.toml</code> conventions.
</footer>
</div>
<script>
document.querySelectorAll("button.copy").forEach(function (btn) {
btn.addEventListener("click", function () {
var pre = btn.closest(".codeblock").querySelector("pre");
var text = pre.innerText;
var done = function () {
btn.textContent = "Copied";
btn.dataset.done = "1";
setTimeout(function () { btn.textContent = "Copy"; btn.dataset.done = "0"; }, 1600);
};
if (navigator.clipboard && navigator.clipboard.writeText) {
navigator.clipboard.writeText(text).then(done, done);
} else {
var ta = document.createElement("textarea");
ta.value = text;
document.body.appendChild(ta);
ta.select();
try { document.execCommand("copy"); } catch (e) {}
document.body.removeChild(ta);
done();
}
});
});
</script>
</body></html>