Make postprocessed_action a genuinely optional step-output field

[eeshsaxena]

BaseTextEnvStepOutput declares postprocessed_action as Optional[str] = None, but a default value on a TypedDict field is a no-op, so the key stays required:

>>> BaseTextEnvStepOutput.__required_keys__
frozenset({'observations', 'reward', 'done', 'metadata', 'postprocessed_action'})
>>> BaseTextEnvStepOutput.__optional_keys__
frozenset()

Every step() implementation omits it, e.g. the gsm8k env returns BaseTextEnvStepOutput(observations=[], reward=reward, done=done, metadata={}), so the declared type doesn't match how the output is actually constructed and a type checker flags each of those returns for a missing key.

I moved the always-present fields into a required base and marked postprocessed_action optional via a total=False subclass. This keeps the public class name and uses only stdlib typing (skyrl-gym targets Python 3.10, before typing.NotRequired). After the change the four core fields stay required and postprocessed_action is optional.

Added tests/test_base_text_env.py asserting the required/optional split; it fails on main and passes here.

uv run --extra dev pytest skyrl-gym/tests/test_base_text_env.py
# 1 passed

Summary by CodeRabbit

• Bug Fixes

  • Updated text environment step outputs so postprocessed_action is optional when not provided.
  • Improved compatibility for consumers handling step output data.

• Tests

  • Added coverage confirming outputs can be created without postprocessed_action.
  • Verified required and optional fields are classified correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b5241cea-05d4-47d7-ad44-6a61167dfca3

📥 Commits

Reviewing files that changed from the base of the PR and between a202ebe and 37c951e.

📒 Files selected for processing (2)

• skyrl-gym/skyrl_gym/envs/base_text_env.py
• skyrl-gym/tests/test_base_text_env.py

---

📝 Walkthrough

Walkthrough

The step output TypedDict now separates required fields from the optional postprocessed_action field. Tests verify the key sets and construction without the optional field.

Changes

Text environment output contract

Layer / File(s)	Summary
Output contract and validation skyrl-gym/skyrl_gym/envs/base_text_env.py, skyrl-gym/tests/test_base_text_env.py	BaseTextEnvStepOutput now uses required and optional TypedDict fields. Tests verify the key sets and valid construction without postprocessed_action.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: making postprocessed_action genuinely optional in the step output.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[eeshsaxena]

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (Make postprocessed_action a genuinely optional step-output field), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining SkyRL!