Keep continuous batching config separate from GenerationConfig
Author: VectorPeak
Pull request: yes

﻿<!-- ci-dashboard-badge:start -->
[![CI](https://transformers-ci.lor-e.huggingface.cool/badge/pr?pr=47038)](https://transformers-ci.lor-e.huggingface.cool/d/pytest-observability-by-pr/pytest-observability-branch?var-pr=47038)
<!-- ci-dashboard-badge:end -->

# What does this PR do?

Addresses #47039

This PR keeps `ContinuousBatchingConfig` separate from `GenerationConfig` serialization. Continuous batching configuration is runtime/hardware-specific, while `GenerationConfig` is model-generation-specific, so the two configs should live side by side rather than nested inside `generation_config.json`.

## What Problem This Solves

`GenerationConfig` previously accepted a `continuous_batching_config` kwarg and stored it as a generation config attribute. When that nested object went through `GenerationConfig.save_pretrained()` / `GenerationConfig.to_json_string()`, dataclass values without a custom `to_dict()` could be serialized as JSON `null`.

The earlier framing was a nested serialization-loss bug. After reviewer clarification, the intended boundary is different: continuous batching config should not be owned by `GenerationConfig` at all. This PR updates the behavior accordingly so `continuous_batching_config` is consumed during `GenerationConfig` initialization instead of being retained as an extra serializable generation config field.

The ownership boundary is now explicit:

- `GenerationConfig`: model generation behavior.
- `ContinuousBatchingConfig`: runtime/hardware batching behavior that should live beside generation config.

## Change

- Remove `GenerationConfig`-side `continuous_batching_config` attribute handling and dict rehydration.
- Drop `continuous_batching_config` during `GenerationConfig.__init__()` so it is not preserved through the generic extra-kwargs path.
- Revert the broad generic dataclass serialization fallback, keeping the existing preference for dataclasses that define their own `to_dict()`.
- Keep `ContinuousBatchingConfig.__post_init__()` responsible for rehydrating nested `varlen_compile_config` and `decode_compile_config` dictionaries into `CompileConfig` objects.
- Update tests to cover the side-by-side behavior: `ContinuousBatchingConfig` round-trips nested `CompileConfig` fields from dict data, while `GenerationConfig` does not serialize `continuous_batching_config`.

This narrows the PR to the reviewer-requested config ownership boundary. It does not change continuous batching runtime scheduling, generation execution, compile defaults, or unrelated generation config fields.

## Evidence

Local validation:

```text
python -m pytest tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_continuous_batching_config_from_dict tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_generation_config_does_not_serialize_continuous_batching_config tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_serialize_generation_watermarking_config -q
python -m ruff format --check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
python -m ruff check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
git diff --check
```

A focused sanity check confirms the side-by-side behavior:

```text
has_attr False
in_dict False
in_json False
```

This verifies that passing `continuous_batching_config` to `GenerationConfig(...)` no longer keeps it as a generation config attribute and no longer writes it into `GenerationConfig.to_dict()` or `GenerationConfig.to_json_string()`.

## Possible call chain / impact

```text
User / service constructs generation config
  -> GenerationConfig(continuous_batching_config=...)
  -> GenerationConfig.__init__(...)
  -> continuous_batching_config is consumed instead of retained as a generation config field
  -> GenerationConfig.to_dict() / to_json_string() do not emit nested continuous batching config

Runtime / serving code keeps batching config separately
  -> ContinuousBatchingConfig(...)
  -> nested compile config dicts are restored to CompileConfig in __post_init__
```

This PR makes the config boundary explicit: generation settings remain in `GenerationConfig`, while continuous batching settings should live beside it as runtime/hardware configuration.

## Code Agent Policy

- [x] I confirm that this is not a pure code agent PR.

AI assistance was used for diagnosis, patch drafting, validation planning, and PR text preparation. The human submitter should review all changed lines and understand the diff before checking this box.

## Before submitting

- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the contributor guideline and the Pull Request checks?
- [x] Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  - Coordination issue: #47039.
- [x] Did you make sure to update the documentation with your changes according to the guidelines?
  - No docs update needed; this clarifies config ownership and adds focused regression coverage.
- [x] Did you write any new necessary tests?

Validation run locally:

```text
python -m pytest tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_continuous_batching_config_from_dict tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_generation_config_does_not_serialize_continuous_batching_config tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_serialize_generation_watermarking_config -q
python -m ruff format --check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
python -m ruff check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
git diff --check
```

Limitations:

```text
make fix-repo
```

was not run because `make` is unavailable in the current Windows PowerShell environment.

## Who can review?

Continuous batching / generation reviewers from the template are likely relevant after tests pass and the ownership direction is confirmed: @remi-or, @ArthurZucker, @McPatate.


Comments:
CI note: the remaining red check appears to be a self-hosted runner/container infrastructure failure rather than a test failure from this PR.

The CI recap reports `68,570` tests with `0` failures. The failing job is `tests_non_model [shard 6/8]`, and it failed during `Initialize containers` before checkout, dependency installation, or test execution. The relevant log lines are:

```text
Pod ... is unhealthy with phase status Failed
TypeError: Cannot read properties of null (reading 'jobPod')
Executing the custom container implementation failed. Please contact your self hosted runner administrator.
```

I do not have permission to rerun the failed upstream job from the fork side.

@remi-or Thanks for clarifying. That makes sense.

So the PR is basically shifting from “we found a bug where saving loses the config” to “this config probably shouldn’t be saved inside GenerationConfig in the first place.” Right?

@remi-or Thanks, that helps clarify the boundary.

I wanted to better understand the intended user-facing workflow here. If `ContinuousBatchingConfig` is runtime/hardware-specific and should live beside `GenerationConfig`, should it be saved/loaded through a separate config artifact instead of `GenerationConfig.save_pretrained()`?

The behavior that motivated this PR is that `GenerationConfig` currently accepts `continuous_batching_config`, but `save_pretrained()` silently serializes it as `null`, so the values are lost after a save/load round trip. If the preferred design is to keep the configs separate, would the right fix be to remove/deprecate that nested `GenerationConfig` path and make the side-by-side path explicit?

@remi-or I updated the PR in the side-by-side direction from the review:

- removed the `GenerationConfig`-side `continuous_batching_config` handling
- dropped the nested config during `GenerationConfig.__init__()` so it is not kept through the extra-kwargs path
- reverted the broad generic dataclass fallback
- updated the test coverage to check `ContinuousBatchingConfig` dict round-trip behavior separately, plus that `GenerationConfig` does not serialize `continuous_batching_config`

Local validation:

```text
python -m pytest tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_continuous_batching_config_from_dict tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_generation_config_does_not_serialize_continuous_batching_config tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_serialize_generation_watermarking_config -q
python -m ruff format --check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
python -m ruff check src/transformers/generation/configuration_utils.py tests/generation/test_configuration_utils.py
git diff --check
```

Checked the current red CI. The focused validation for this PR is passing locally:

- `python -m pytest tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_continuous_batching_config_from_dict tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_generation_config_does_not_serialize_continuous_batching_config tests/generation/test_configuration_utils.py::GenerationConfigSerializationTest::test_serialize_generation_watermarking_config -q` -> 3 passed
- `ruff format --check`, `ruff check`, and `git diff --check` -> passed

The remaining failed checks look unrelated to the continuous batching config ownership change:

- `tests_non_model [shard 0/8]`: CLI tests fail with `AttributeError: 'HFCliTyperGroup' object has no attribute '_add_completion'`
- `tests_torch [shard 0/8]`: `BigBirdPegasusModelTest::test_training_gradient_checkpointing_use_reentrant_false` fails with missing `qa_outputs.bias`

I tried rerunning the failed jobs, but GitHub returned `Must have admin rights to Repository`. Could someone with permissions rerun the failed checks?

@zucchini-nlp @remi-or Thanks for the latest clarification here. Before changing the code again, I wanted to confirm the preferred compatibility direction.

**Option A:** keep the config ownership side-by-side, but restore a small legacy runtime fallback in `ContinuousMixin.init_continuous_batching()`:

- explicit `continuous_batching_config` argument wins
- if absent, and `gen_config.continuous_batching_config` already exists as a legacy/custom attribute, use it
- otherwise create a default `ContinuousBatchingConfig()`

This would not reintroduce `ContinuousBatchingConfig` as something owned or serialized by `GenerationConfig`; it would only keep a compatibility bridge for callers that may already attach it there manually.

**Option B:** keep the separation strict and do not read `gen_config.continuous_batching_config` at runtime anymore. Users would need to pass `continuous_batching_config` side by side through the runtime API.

Which direction would you prefer for this PR? Happy to implement either one.

@zucchini-nlp @remi-or Thanks again for the guidance here.

I rechecked the current PR state, and the implementation is currently following **Option B**: `ContinuousBatchingConfig` is kept fully separate from `GenerationConfig` ownership. In particular:

- `GenerationConfig` consumes but does not retain `continuous_batching_config`, so it is not serialized through `to_dict()` / `to_json_string()`.
- `ContinuousMixin.init_continuous_batching()` no longer reads `gen_config.continuous_batching_config` as a runtime fallback.
- callers should pass `continuous_batching_config` side by side when they need a non-default continuous batching config; otherwise the runtime creates a default `ContinuousBatchingConfig()`.

Happy for another review on this direction. If you think we should keep a small compatibility bridge instead — i.e. **Option A**, where runtime still falls back to an existing `gen_config.continuous_batching_config` without making it owned/serialized by `GenerationConfig` — please just @ me and I can adjust accordingly.

<!-- ci-dashboard-recap:start -->

---

### CI recap

**Dashboard:** [View test results in Grafana](https://transformers-ci.lor-e.huggingface.cool/d/pytest-observability-by-pr/pytest-observability-branch?var-pr=47038)
**Latest run:** [28856029452:2](https://github.com/huggingface/transformers/actions/runs/28856029452)
**Result:** `failure` | **Jobs:** 15 | **Tests:** 172,556 | **Failures:** 1 | **Duration:** 20h 0m

<!-- ci-dashboard-recap:end -->

Since this is obviously a code agent PR, that it adds no depreciation warning and a test we don't really need, I am closing this in favor of https://github.com/huggingface/transformers/pull/47291 (cc. @zucchini-nlp)

@remi-or  That's rude... I did review the agent's code.

@remi-or but you’re right. I’m still  a noob......honestly, I didn’t fully understand what the requirement was either.

Seeing your profile, I don't think you are a noob 😅 
it's just that it looked fully agentic, plus it was a small change and the agent did not manage to solve the issue. If you want to contribute again, it's highly welcomed of course. Please take note of CONTRIBUTING.md .
As a personal note, I think it's a more pleasant experience for everyone if the agent does not write the PR messages; and regarding your second message, you get a better understanding of the code by writing it yourself.

@remi-or 
Sorry about that haha. I took another look just now, and honestly I think you were right.

The main issue was that I didn’t really get what you were trying to say at first. Also, I may have been relying on the agent a bit too much. Maybe I’ve been doing too much vibe coding at work...

Thanks again. I actually learned a lot from this.

@remi-or 
:}I think you might be overestimating me a bit. You may not believe it, but those PRs were actually all done within about half a month.

I built my own agent pipeline and used Codex to roughly filter possible PR candidates. The constraints were: keep the code change as small as possible, ideally under 20 lines, so reviewers can quickly understand and review it; and try to find issues with real user evidence or a real call chain.

The hardest part is the human filtering. I probably need to go through 10 to 20 candidate PRs to pick one that is actually worth submitting. That still requires a bit of taste in reading code. Bug fixes or small text/doc fixes are usually easier to get accepted.

But you’re also right: good projects do have a higher bar. MinerU has around 70k stars, but only about 20 open PRs, probably partly to keep the repo from looking too messy.

Of course, there are also some projects that basically die on this because they just don’t accept agent-assisted PRs at all. Zig is an example: when people are willing to contribute, they get cursed out as “AI slop” and AI-generated code is forbidden from being merged.