Skip to content

fix(rollout): fix nested group sorting in fully async rollout - #2345

Open
looput wants to merge 1 commit into
THUDM:mainfrom
looput:fix-nested-group
Open

fix(rollout): fix nested group sorting in fully async rollout#2345
looput wants to merge 1 commit into
THUDM:mainfrom
looput:fix-nested-group

Conversation

@looput

@looput looput commented Sep 2, 2026

Copy link
Copy Markdown

Summary

  • Support nested list[list[Sample]] groups in fully async rollout sorting.
  • Recursively find the leaf Sample.index value.
  • Avoid treating Python's list.index method as a sample index.

Bug Trigger Cause

The fully-async rollout path assumes that each rollout group is a flat:

list[Sample]

However, the multi-agent(examples/multi_agent) custom generation function may return multiple samples for a single input sample. After generate_and_rm_group gathers the results, the
actual structure becomes:

list[list[Sample]]

The original sorting logic directly reads .index from the first-level item:

index = getattr(sample, "index", None)

In the nested case, sample is a Python list. Since Python lists already have a built-in index() method, getattr(sample, "index") returns a
builtin_function_or_method instead of a numeric Sample.index.

The code then executes:
int(index)
which raises:

TypeError: int() argument must be a string, a bytes-like object or a real number, not 'builtin_function_or_method'

In short, the sorting logic did not support nested sample groups and mistakenly treated Python's list.index method as the sample index.

Validation

  • Reproduced the original nested-group TypeError.
  • Verified that the updated traversal correctly resolves the leaf sample index.
  • Passed python -m py_compile slime/rollout/fully_async_rollout.py.

#2344

@looput looput changed the title fix: fix nested group sorting in fully async rollout fix(rollout): fix nested group sorting in fully async rollout Sep 2, 2026
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