huggingface / huggingface/serge

Treat a verify run that never exercised the patch as unjudged, not as a rejection

Open Beginner friendly
#121 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
51
Forks
10
Avg merge
11h 32m
Merged PRs (30d)
36

Description

Companion to huggingface/transformers-ci#117, which proposes a new verify verdict `not_exercised` for a run in which **both trees failed before the test body ran** (model load, collection, weight conversion). Today that situation is reported as `not_fixed`, which is a claim about the patch made from a run that never touched it.

## What to change

`gate_did_not_run()` in `reviewbot/verify.py` lists the verdicts that say nothing about the candidate — `timeout`, `no_result`, `dispatch_failed`, `no_targets`. `not_exercised` belongs in that set: the branch is kept, no PR is opened, and the outcome travels in `TaskResult.to_json()` so the reconciler can say verification could not run. Exactly the treatment #110 introduced.

`error` should **stay out** of the set. #110's reasoning still holds — a workflow that ran and reported something unverifiable is a real reason to refuse — with one exception, handled on the transformers-ci side: a `skipped` that is the *point* of the patch (an `import_or_config` group fixed with `@require_flash_attn`) becomes `fixed` in the verdict computer, so serge never sees it as `error` at all.

## Why it matters

huggingface/transformers#48582 is a correct patch — `@require_flash_attn` + `@mark.flash_attn_test` on a test that asks for `attn_implementation="flash_attention_2"`, matching the convention every other flash-attention test in transformers follows. It exists **only because the gate was off** that night. With the gate on, its branch would have been deleted and the group re-dispatched the following night, indefinitely, because every correct fix for it produces a skip.

Two more (huggingface/transformers#48551, #48538) are retained-memory OOM fixes whose verdicts came from runs that OOMed at load or died in weight conversion — `not_exercised` cases.

## Ordering

Land transformers-ci#117 first: until the verdict computer emits `not_exercised`, there is nothing here to accept. The change on this side is a one-line addition to the tuple plus a test that the branch survives and the outcome is reported.

Context: all three PRs come from the 2026-09-04..06 window when `VERIFY_ON_GPU` was unset by a bad values file (fixed in #120); the 11 PRs opened in that window were re-verified by hand, and 4 of the 11 verdicts turned out not to mean what they said.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in reviewbot/verify.py at gate_did_not_run(), then trace TaskResult.to_json() to see how the outcome is reported. Add not_exercised to the existing verdict tuple, and add a regression test confirming the branch is retained and the outcome is serialized; transformers-ci#117 must land first.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
ci-cd
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.