fix: EnvGroup nested routing silently misroutes to envs[0]#1058
Open
dieutx wants to merge 4 commits intoPrimeIntellect-ai:mainfrom
Open
fix: EnvGroup nested routing silently misroutes to envs[0]#1058dieutx wants to merge 4 commits intoPrimeIntellect-ai:mainfrom
dieutx wants to merge 4 commits intoPrimeIntellect-ai:mainfrom
Conversation
… fallback When an EnvGroup is nested inside another EnvGroup, the outer group previously overwrote inner task names, causing all rollouts to be silently misrouted to envs[0] and scoring to return reward=0.0. Changes: - get_env_for_task now raises ValueError for unknown tasks instead of silently falling back to envs[0] - Nested EnvGroup task names are preserved in the outer group's dataset and env_map, enabling correct routing through the nesting chain Closes PrimeIntellect-ai#1008
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Use dataset["task"] unique values instead of env.env_names to handle arbitrary nesting depth (3+ levels) - Register inner task names from eval datasets too, fixing eval-only nested EnvGroups where build_dataset() returns None - Update docs/environments.md and docs/reference.md to document nested EnvGroup support and ValueError behavior - Add tests for eval-only nested EnvGroup and 3-level deep nesting
When a nested EnvGroup is detected, the initial outer name entry (e.g., "my_envs") was left in env_map as a phantom key. This could cause get_env_for_task to succeed at the outer level but fail inside the inner group, and would list misleading task names in error messages.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
…task When the outer env_name matches an inner task name (e.g., outer name "math" with an inner group containing a "math" task), the unconditional pop would remove the just-registered valid task. Now only removes the stale entry when it doesn't collide with an inner task name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1008 — when an
EnvGroupis wrapped inside anotherEnvGroup(as prime-rl does), the outer group overwrites inner task names, causing:envs[0]viaget_env_for_taskfallbackreward=0.0for all unrecognized task namesChanges
get_env_for_task: RaisesValueErrorwith available task names instead of silently falling back toenvs[0]__init__dataset loop: When a sub-env is anEnvGroup, preserves its inner task names in the dataset and registers them inenv_mapfor correct routingEnvGrouptask preservation and mixed nested/flat routingType of Change
Test plan
test_get_env_for_taskupdated: unknown task now raisesValueErrortest_nested_env_group_preserves_inner_tasks: inner task names preserved in outer dataset and env_maptest_nested_env_group_with_flat_env: mixed nested EnvGroup + flat env routing works correctlyruff checkandruff formatpassNote
Medium Risk
Changes
EnvGrouprouting semantics by removing the silent fallback toenvs[0]and raising aValueErrorfor unknown tasks, which could break callers relying on the previous behavior. Nested task registration touches dataset construction andenv_mapwiring, so mis-registration could affect rollout routing/scoring across tasks.Overview
Fixes nested
EnvGrouptask routing so innertasklabels are preserved through dataset concatenation and registered in the outerenv_map, preventing misrouting when groups are wrapped inside other groups.get_env_for_tasknow raises aValueErrorlisting available tasks instead of silently falling back toenvs[0]; tests and docs are updated and expanded to cover unknown-task errors, nested groups (including eval-only and deep nesting), and mixed nested/flat configurations.Written by Cursor Bugbot for commit 851174a. This will update automatically on new commits. Configure here.