fix(grpo): skip padding when trainset length is divisible by step size

[detail-app]

Warning

GitHub issue creation failed

Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as Unknown issue.

You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.

Detail bug report: View on Detail

📝 Changes Description

This MR/PR contains the following changes:

GRPO.update_shuffled_trainset (dspy/teleprompt/grpo.py) pads the shuffled training-example ids so the total length is a multiple of num_dspy_examples_per_grpo_step (S). The padding-count formula S - (N % S) returns S instead of 0 when the trainset length N is already divisible by S, so the if num_to_pad > 0 guard never suppresses padding in the aligned case. This appends a full extra step worth of duplicate ids to the tail of shuffled_trainset_ids.

• Bug: When N % S == 0, each epoch is inflated from N to N + S examples. Once a training step's slice reaches the padded tail (gated on num_train_steps * S > N, i.e. small trainsets or multi-epoch runs), one extra GRPO step per epoch trains on ids already seen earlier in that epoch, and the epoch/reshuffle boundary fires one step late. The bug is dormant for N >= num_train_steps * S (the default num_train_steps=100, S=1 config with N >= 100).
• Fix: Replace S - (N % S) with the standard modulo formula (S - (N % S)) % S. This yields 0 in the aligned case and is identical to the old formula for every N, S with N % S != 0, preserving the divisibility invariant the shuffler relies on.
• Tests: Added two regression tests in tests/teleprompt/test_grpo.py that the existing tests could not catch (they pass coincidentally under the bug because the padded block happens to be a full permutation of the dataset):
  • test_grpo_dataset_shuffler_no_padding_when_divisible — asserts no padding is added when N % S == 0.
  • test_grpo_dataset_shuffler_across_epoch_boundary_divisible — drives the shuffler across an epoch boundary with a divisible (N=6, S=3) pair and asserts the epoch counter advances at the unpadded length, with each id appearing exactly once per epoch.

Closes Unknown issue

✅ Contributor Checklist

• Pre-Commit checks are passing (locally and remotely) — uv run ruff check clean on changed files; the only remaining ruff error in grpo.py is a pre-existing RUF017 at line 576, unrelated to this change (verified pre-existing by stashing the fix).
• Title of your PR / MR corresponds to the required format
• Commit message follows required format {label}(dspy): {message} — fix(grpo): skip padding when trainset length is divisible by step size

⚠️ Warnings

AI disclosure: This change was authored by Detail: Automatic Fixes. The bug was identified via analysis of the padding arithmetic and confirmed by reproduction against the real select_training_sample_and_update_shuffled_trainset code path.

Testing notes:

• Unit tests, lint (ruff), and the new regression tests all pass via uv run pytest tests/teleprompt/test_grpo.py -v (5 passed). A negative control (temporarily reverting the one-line fix) confirms both new tests fail on the buggy code while the three pre-existing tests still pass.
• A dormant-regime equivalence check confirmed that with the default config (N=100, S=1, num_train_steps=100) the consumed batches and RNG stream are byte-identical between the buggy and fixed versions — only the harmless internal shuffled_trainset_ids length (101 vs 100) and id_freqs total differ, never read because no step reaches the padded tail.
• End-to-end GRPO.compile() verification could not be run: the in-tree OpenAI provider raises LMUnsupportedFeatureError (no in-tree provider implements the reinforce interface), and the published arbor-ai==0.2.2 does not export the ArborGRPO/ArborProvider API the RL tutorial notebooks import. This is an upstream/infra gap, not a limitation of the fix; the unit tests exercise the same select_training_sample_and_update_shuffled_trainset code path (including the epoch-boundary/reshuffle trigger) that compile() calls.

---

Automatic Fixes PRs can be configured here.

[greptile-apps]

Greptile Summary

Corrects GRPO trainset padding so datasets already divisible by the examples-per-step count do not receive an unnecessary full block of duplicate IDs.

• Preserves padding for non-divisible trainset lengths.
• Adds regression coverage for aligned trainsets and epoch-boundary reshuffling.

Confidence Score: 5/5

The PR appears safe to merge; the fix is narrowly scoped and preserves existing behavior for non-divisible trainset lengths.

No actionable failures remain: aligned datasets now contain exactly one permutation per epoch, while indexing and batch-size invariants remain valid and are covered by regression tests.

Important Files Changed

Filename	Overview
dspy/teleprompt/grpo.py	Uses the standard modulo formula to avoid padding already aligned trainsets while preserving the required divisibility invariant.
tests/teleprompt/test_grpo.py	Adds focused tests verifying zero padding for divisible lengths and correct sample uniqueness and epoch advancement across boundaries.

_{Reviews (1): Last reviewed commit: "fix(grpo): skip padding when trainset le..." | Re-trigger Greptile}