GitForCausalLM double-shifts the training loss and leaks targets across batch rows (regression from #35875)
Author: Yigtwxx
Pull request: no

### System Info

- `transformers`: main (`7ea2320c76`), also present in every release since v4.49
- Code-level bug, weight-independent — reproducible on CPU, no GPU or pretrained weights needed

### Who can help?

@zucchini-nlp

### Description

`GitForCausalLM.forward` shifts the labels manually and then hands the result to `self.loss_function`, which shifts them a **second** time. The model is trained to predict token `t + 2` from position `t` instead of `t + 1`.

`src/transformers/models/git/modeling_git.py:1081-1092`:

```python
if labels is not None:
    # we are doing next-token prediction; shift prediction scores and input ids by one
    num_image_tokens = self.git.encoder.layer[0].attention.self.image_patch_tokens
    shifted_logits = logits[:, num_image_tokens:-1, :].contiguous()
    labels = labels[:, 1:].contiguous()              # shift #1
    loss = self.loss_function(
        shifted_logits.view(-1, self.config.vocab_size),
        labels.view(-1),                             # binds positionally to `labels`
        vocab_size=self.config.vocab_size,
        **kwargs,
    )
```

`GitForCausalLM` resolves `loss_type` to `"ForCausalLM"`, so `self.loss_function` is `ForCausalLMLoss`. Its signature is `(logits, labels, vocab_size, num_items_in_batch, ignore_index, shift_labels)`, so the already-shifted tensor binds to `labels` and `shift_labels` stays `None`. `src/transformers/loss/loss_utils.py:61-64` then does:

```python
if shift_labels is None:
    labels = nn.functional.pad(labels, (0, 1), value=ignore_index)
    shift_labels = labels[..., 1:].contiguous()      # shift #2
```

Two things follow from this. First, the targets are off by one extra position. Second — and this is the part that does not show up in the loss value alone — the manual shift flattens the labels to 1-D *before* the call, so the internal pad-and-slice operates on the flattened tensor and the shape stays consistent (`N → N+1 → N`). No error is raised, and each batch row's final target is silently pulled in from the **next row** of the batch.

### Regression

This was correct until #35875 ("Fix model kwargs", `28f73bc307`), which mechanically swapped the loss implementation but left the manual shift in place:

```diff
-            loss_fct = CrossEntropyLoss()
-            loss = loss_fct(shifted_logits.view(-1, self.config.vocab_size), labels.view(-1))
+            loss = self.loss_function(
+                shifted_logits.view(-1, self.config.vocab_size),
+                labels.view(-1),
```

`CrossEntropyLoss` does not shift, `ForCausalLMLoss` does. This is the same regression mechanism that was fixed for Moonshine in #46784.

I checked the rest of what #35875 touched. On current main only three call sites still combine a manual shift with `self.loss_function`: `csm` is correct (it passes `labels=None, shift_labels=shift_labels`), `moshi` has the same bug but is already covered by the open #36928, and `git` is the remaining unfixed one.

### Reproduction

CPU, no pretrained weights:

```python
import torch
from torch.nn import CrossEntropyLoss

from transformers import GitConfig, GitForCausalLM
from transformers.models.git.configuration_git import GitVisionConfig

torch.manual_seed(0)
vision_config = GitVisionConfig(
    hidden_size=32, num_hidden_layers=2, num_attention_heads=4,
    intermediate_size=37, image_size=32, patch_size=16,
)
config = GitConfig(
    vision_config=vision_config.to_dict(), vocab_size=99, hidden_size=32,
    num_hidden_layers=2, num_attention_heads=4, intermediate_size=37,
)
model = GitForCausalLM(config).eval()

input_ids = torch.randint(3, config.vocab_size, (2, 7))
pixel_values = torch.randn(2, 3, 32, 32)
labels = input_ids.clone()

with torch.no_grad():
    out = model(input_ids=input_ids, pixel_values=pixel_values, labels=labels)

num_image_tokens = model.git.encoder.layer[0].attention.self.image_patch_tokens
shifted_logits = out.logits[:, num_image_tokens:-1, :]
V = config.vocab_size

aligned = CrossEntropyLoss()(shifted_logits.reshape(-1, V), labels[:, 1:].reshape(-1))

# what the code actually computes: a second shift, applied to the flattened targets
flat = labels[:, 1:].reshape(-1)
leaked = torch.nn.functional.pad(flat, (0, 1), value=-100)[1:]
double_shifted = CrossEntropyLoss()(shifted_logits.reshape(-1, V), leaked)

print("model loss            :", out.loss.item())
print("aligned CE (expected) :", aligned.item())
print("double-shifted CE     :", double_shifted.item())
print("matches aligned       :", torch.allclose(out.loss, aligned))
print("matches double-shift  :", torch.allclose(out.loss, double_shifted))
print("row 0 correct targets :", labels[0, 1:].tolist())
print("row 0 actual targets  :", leaked[:6].tolist())
print("row 1 labels          :", labels[1, 1:].tolist())
```

Output on main:

```
model loss            : 4.584834575653076
aligned CE (expected) : 4.646134853363037
double-shifted CE     : 4.584834575653076
matches aligned       : False
matches double-shift  : True
row 0 correct targets : [28, 71, 8, 58, 61, 15]
row 0 actual targets  : [71, 8, 58, 61, 15, 19]
row 1 labels          : [19, 42, 30, 58, 94, 67]
```

The loss matches the double-shifted computation exactly, and row 0's last target (`19`) is row 1's first label.

### Expected behavior

The target for position `t` should be `labels[t + 1]`, and targets must not cross batch-row boundaries.

### Suggested fix

Unlike Moonshine, GIT cannot simply drop the manual shift: `logits` contains `num_image_tokens` leading image positions that have to be sliced off regardless. Passing `shift_labels` explicitly suppresses the internal shift, which is the convention already used by `csm` (`modeling_csm.py:619-621`):

```diff
             shifted_logits = logits[:, num_image_tokens:-1, :].contiguous()
-            labels = labels[:, 1:].contiguous()
+            shift_labels = labels[:, 1:].contiguous()
             loss = self.loss_function(
-                shifted_logits.view(-1, self.config.vocab_size),
-                labels.view(-1),
+                logits=shifted_logits,
+                labels=None,
                 vocab_size=self.config.vocab_size,
+                shift_labels=shift_labels,
                 **kwargs,
             )
```

Keeping the tensors 2-D also removes the cross-row leak, since `ForCausalLMLoss` flattens them itself. With this applied, the repro above prints `4.646135` for both the model loss and the aligned CE. I have the fix and a regression test ready (the full `tests/models/git` suite passes: 190 passed, 297 skipped) and am happy to open a PR if you agree with the direction.

> Disclosure: this investigation and the proposed patch were prepared with AI assistance, and reviewed and verified by me.


Comments: