Skip to content

Close three ITF guidance gaps the 2026-09-04..06 serge PRs fell into - #114

Merged
tarekziade merged 1 commit into
mainfrom
tarek/itf-guidance-gaps
Sep 7, 2026
Merged

tarekziade merged 1 commit into
mainfrom
tarek/itf-guidance-gaps

Conversation

@tarekziade

Copy link
Copy Markdown
Collaborator

Three published task PRs on transformers were wrong in ways the instruction set permitted. Each gap is one block in integration_failure_triage.py.

1. output_mismatch only knows about the expected side — #48553

_MISMATCH_GUIDANCE is ~60 lines on device keys, tolerance precedents and value plausibility, ending with "do not delete, weaken, or comment out the assertion". The patch added .flatten() to output.sequences.squeeze().tolist() — the actual side, which the block never frames. It cannot pass (2N tokens against an 8-token expectation) and left the next two assertions nested.

The real cause was a shape mismatch: #48425 had changed the input_audio fixture from one 2-channel sample to two mono samples. Two new bullets: the actual side is off limits, and a shape/length mismatch is never expectation drift — read the fixture and check it against what the expectation assumes (a flat list of N tokens, one waveform, a .squeeze() expected to collapse everything all say batch size 1).

2. The crash block steers away from a test-side placement bug — #48536

_CRASH_GUIDANCE says treat a crash as a library bug and do NOT edit the test. But the failing test did from_pretrained(..., device_map="auto") and then model.to(torch_device): the accelerate hooks survive .to(), so tensors keep crossing devices however the model is written. The patch dutifully added a .to(aligner.device) hop inside get_image_features — papering over a placement bug for every user.

A carve-out now names that exact shape, labels dropping the device map a real fix rather than test-editing, and still refuses the manual device hop (pointing at _no_split_modules when the test's placement is coherent).

3. Bad-commit clusters got an empty addendum — #48535

instruction_addendum returned "" for clusters, on the grounds that the attributed commit is a stronger signal — but nothing said what to do with it. That commit is usually itself a fix. The patch re-guarded #47988's update_post_processor() call with a predicate the base class already applies, making it dead code, and allenai/OLMo-7B-hf started appending <|endoftext|> to every prompt again (verified locally).

_CULPRIT_GUIDANCE covers it: establish what the culprit protected from evidence in the tree, say in body how the patch keeps it protected, check whether the predicate you are adding already exists upstream, and call a revert a revert. It is prepended to the mode block when a cluster also qualifies for one (all-OOM, conversion), so the muse_glimmer fix from #113 is preserved.

Note on tools

Every new bullet points only at read_file / grep. The task agent has no git tool and fetch_url is restricted to huggingface.co, so "read the culprit commit" or "run git log -L" would not be instructions it can follow — there is a test asserting the culprit block contains neither.

326 passed. Two pre-existing cluster tests were updated (they asserted the empty addendum) and the redundant all-OOM one folded in.

Three published task PRs were wrong in ways the instruction set permitted.

`output_mismatch` (transformers#48553): the block is ~60 lines about the
expected side of the assertion, so rewriting the ACTUAL side read as a code
fix. `.flatten()` was added to `output.sequences.squeeze().tolist()` to make a
nested list compare against a flat expectation — which cannot pass (it yields
2N tokens for an 8-token expectation) and left the next two assertions nested.
The mismatch was a SHAPE mismatch, i.e. the input had changed: a fixture that
went from one 2-channel sample to two mono samples. Two new bullets: the actual
side is off limits, and a shape/length mismatch is never expectation drift —
read the fixture and check it against what the expectation assumes.

Crash on a device mismatch (transformers#48536): the crash block says treat it
as a library bug and do NOT edit the test, but the test had asked for
`device_map="auto"` and then called `model.to(torch_device)`. The accelerate
hooks survive `.to()`, so tensors keep crossing devices however the model is
written. A carve-out now names that shape, labels dropping the device map a
real fix, and keeps refusing a manual `.to(other.device)` inside `forward`.

Bad-commit clusters (transformers#48535): `instruction_addendum` returned ""
for them, on the grounds that the attributed commit is a stronger signal — but
nothing said what to do with it. That commit is usually itself a fix, and the
patch re-guarded #47988's `update_post_processor()` call with a predicate the
base class already applies, making it dead code and restoring OLMo's EOS bug.
`_CULPRIT_GUIDANCE` now covers it: establish what the culprit protected from
evidence in the tree, say in `body` how the patch keeps it protected, and call
a revert a revert. It is prepended to the mode block when a cluster has one.

The blocks point only at `read_file`/`grep`: the task agent has no git tool and
`fetch_url` is restricted to huggingface.co, so "read the culprit commit" is not
an instruction it can follow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tarekziade
tarekziade merged commit f082d85 into main Sep 7, 2026
@tarekziade
tarekziade deleted the tarek/itf-guidance-gaps branch September 7, 2026 07:47
tarekziade added a commit to huggingface/serge that referenced this pull request Sep 7, 2026
…118)

`build_task_user_prompt` head-truncates the instruction at
`MAX_INSTRUCTION_CHARS`, so an over-long one loses its **end** — and
transformers-ci appends its newest guidance last.

Its `output_mismatch` instruction (shared trunk + per-category block) is
**8,821 chars** after
[transformers-ci#114](huggingface/transformers-ci#114),
against a cap of **8,000**. The 821 chars that fell off were exactly the
bullets that change existed to add:

```
[... truncated, 821 chars omitted ...]
```
> cessor/tokenizer call) and check it against what the expectation
assumes: a flat list of N tokens, one waveform, one `.squeeze()` that
the test expects to collapse everything — all of those say batch size 1…

So the guidance was written, reviewed, merged, and never reached the
model. Nothing on either side noticed: the truncation is silent apart
from a marker in the prompt.

12,000 leaves room above today's largest block (sizes: output_mismatch
8,821 · cluster 4,753 · crash 4,688 · load_error 3,217 ·
import_or_config 3,121 · trunk alone 2,641). The other half is
[transformers-ci#115](huggingface/transformers-ci#115),
a test that fails when a category outgrows this number — that repo is
where the growth happens.

`24 passed` in `tests/test_prompts.py`.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
tarekziade added a commit that referenced this pull request Sep 7, 2026
…tion (#115)

* Guard the per-category instruction against serge's truncation cap

#114 pushed the `output_mismatch` instruction to 8,821 chars. serge's
`build_task_user_prompt` head-truncates the instruction at
MAX_INSTRUCTION_CHARS (8,000 at the time), so the 821 chars that fell off the
end were the two bullets that PR existed to add — the guidance was written,
merged, and never reached the model.

serge raises the cap to 12,000; this is the half that belongs here, where the
blocks are written and where the next one will grow. Every category's
`build_instruction` output is checked against that number, with the failure
message saying what truncation would cost.

Sizes today: output_mismatch 8,821 · cluster 4,753 · crash 4,688 ·
load_error 3,217 · import_or_config 3,121 · trunk alone 2,641.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Compress every prompt block: 21,141 -> 18,886 chars of delivered text

Every one of these strings is sent to the model; the `#` comments around them
are not, so the rationale and the dated precedents stay where they are and only
the delivered text shrinks. Same rules, same thresholds, same PR numbers and
examples — the 112 phrases the test module pins are all still there.

  block                      before   after
  _INSTRUCTION                 2641    2192
  _MISMATCH_GUIDANCE           6178    5031
  _CRASH_GUIDANCE              2058    1969
  _CULPRIT_GUIDANCE            2110    2041
  _OOM_LOAD_GUIDANCE           4624    4333
  _OOM_RETENTION_GUIDANCE      1881    1734
  the four small blocks        1649    1586

The largest single win is not prose: the trunk told the agent to go and read the
root `AGENTS.md` / `CLAUDE.md` / `.ai/AGENTS.md`, which serge already injects
verbatim into the system prompt as REPO CONVENTIONS. That paragraph paid for the
conventions twice and bought three tool calls; it now says they are already in
the prompt and points only at what the agent does need to open — the failing
model's test file and its docs page.

Delivered instruction, worst case: output_mismatch 8,821 -> 7,225 chars, which
fits inside even the old 8,000-char cap with room to spare.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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