Skip to content

fix(textarena): honor reset(seed) so episodes are reproducible - #1078

Open
adithya-s-k wants to merge 2 commits into
huggingface:mainfrom
adithya-s-k:fix/textarena-reset-seed
Open

fix(textarena): honor reset(seed) so episodes are reproducible#1078
adithya-s-k wants to merge 2 commits into
huggingface:mainfrom
adithya-s-k:fix/textarena-reset-seed

Conversation

@adithya-s-k

@adithya-s-k adithya-s-k commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

What

TextArenaEnvironment.reset() accepts a seed argument but never uses it. It calls self._ta_env.reset(num_players=...) without seeding, so the episode (for Wordle, the secret word, chosen via the global random module) is non-deterministic on every reset. The client -> HTTP -> server plumbing already forwards seed into environment.reset(seed=...); only the wrapper was dropping it.

This makes the environment non-reproducible, and it specifically breaks GRPO-style training: all rollouts in a group must share the same episode (same word) for the group-relative advantage baseline to be meaningful. Today each rollout in a group draws a different word.

Repro (before)

from textarena_env.server.environment import TextArenaEnvironment
env = TextArenaEnvironment(env_id="Wordle-v0", num_players=1)
env.reset(seed=123); w1 = env._ta_env.state.game_state["secret_word"]
env.reset(seed=123); w2 = env._ta_env.state.game_state["secret_word"]
assert w1 == w2   # fails on main

Fix

TextArena's own reset(seed=...) does not apply the seed to word selection (Wordle uses random.choice(word_list)), so the wrapper seeds the process-global random (and numpy if present) immediately before the underlying reset:

  • atomic under a process-wide lock so seed + selection can't interleave across concurrent sessions,
  • prior RNG state restored afterwards so unseeded sessions sharing the process are undisturbed,
  • seed also forwarded to the underlying reset for games/versions that consume it,
  • seed=None keeps the existing random behaviour.

Tests

tests/envs/test_textarena_seed.py (skipped if textarena isn't installed):

  • same seed -> same word, across repeated resets and across separate instances
  • seed actually drives word selection
  • unseeded reset still works
  • a seeded reset restores the global RNG stream

Validated locally with textarena 0.7.4: 5 passed. ruff format/ruff check clean.


Note

Medium Risk
Seeded resets mutate global RNG state under a lock; incorrect restoration could affect concurrent unseeded sessions, though the implementation explicitly saves and restores state in a finally block.

Overview
TextArenaEnvironment.reset(seed=...) is now honored so Wordle (and similar) episodes are reproducible—required for GRPO-style rollouts that must share the same episode within a group.

reset() routes through new _seeded_reset: when a seed is set, the wrapper temporarily seeds process-global random and numpy (if installed), calls the underlying reset (forwarding seed when supported via _ta_reset), then restores prior RNG state. A process-wide _SEED_LOCK keeps seed+selection atomic for concurrent sessions; seed=None keeps prior unseeded behavior.

Adds tests/envs/test_textarena_seed.py (skipped without textarena) covering same-seed determinism across resets/instances, seed-driven word variety, unseeded resets, and no leakage into the global RNG stream.

Reviewed by Cursor Bugbot for commit 2131c0f. Bugbot is set up for automated code reviews on this repo. Configure here.

TextArenaEnvironment.reset() accepted a `seed` argument but ignored it: it
called `self._ta_env.reset(num_players=...)` without seeding, so the Wordle
secret word (chosen via the global `random` module) was non-deterministic on
every reset. This made the environment non-reproducible and, in particular,
broke GRPO-style training where all rollouts in a group must share the same
episode (same word) for the group-relative advantage baseline to be valid.

TextArena's own reset(seed=...) does not apply the seed to word selection, so
we seed the process-global `random` (and `numpy`, if present) immediately
before the underlying reset. A lock keeps seed+selection atomic across
concurrent sessions, and the prior RNG state is restored afterwards so unseeded
sessions sharing the process are undisturbed. The seed is also forwarded to the
underlying reset for games/versions that consume it.

Adds tests: same seed -> same word (across resets and instances), seed drives
selection, unseeded reset still works, and a seeded reset restores global RNG.
@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

"""
if seed is None:
self._ta_reset(seed)
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unseeded reset skips seed lock

Medium Severity

_seeded_reset only acquires _SEED_LOCK when seed is set, so an unseeded reset (and __init__'s direct _ta_env.reset) can run while another session has temporarily reseeded the process-global RNGs. With SUPPORTS_CONCURRENT_SESSIONS and a thread-pool server, that interleaving can steal draws from a seeded episode or break same-seed reproducibility.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 58723a9. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2131c0f. Configure here.

"""
if seed is None:
self._ta_reset(seed)
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unseeded reset skips seed lock

Medium Severity

Unseeded _seeded_reset calls _ta_reset without _SEED_LOCK, so another session can consume the process-global random stream while a seeded reset holds the hijacked RNG. That breaks the atomic seed-plus-selection guarantee and can assign the wrong episode under concurrent sessions.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2131c0f. Configure here.

try:
random.seed(seed)
if _np is not None:
_np.random.seed(seed)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large seeds crash with numpy

Low Severity

When numpy is importable, _seeded_reset always calls numpy.random.seed, which rejects integers outside 0..2**32-1. A valid OpenEnv seed that random.seed accepts then raises ValueError and aborts reset, even for games that only use the Python random module.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2131c0f. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preservation approach is preferable to a bare seed forward, but this is not concurrency-safe yet. OpenEnv declares SUPPORTS_CONCURRENT_SESSIONS = True, so both seeded and unseeded resets must participate in the same lock. Please add a deterministic interleaving test with the actual TextArena dependency (the repository lock resolves 0.7.4), not an optional test that can silently skip.

Open in Web View Automation 

Sent by Cursor Automation: Release

and we restore the prior RNG state afterwards so that unseeded sessions
sharing the process are left undisturbed.
"""
if seed is None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seed=None bypasses _SEED_LOCK, so an unseeded reset can run after another thread calls random.seed(seed) but before it selects/restores. That consumes the seeded stream, perturbs the unseeded session, and can change the seeded episode. Hold the same lock around every underlying reset; for seeded calls, save/seed/reset/restore while holding it. Also, TextArena 0.7.4 already forwards seed through SinglePlayerState.__init__ to State.__init__, which calls random.seed(seed), so update the docstring’s contrary claim.

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.

2 participants