Skip to content

Conversation

@surfiniaburger
Copy link
Contributor

No description provided.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 26, 2025
surfiniaburger and others added 8 commits October 27, 2025 07:02
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Refactored client parser to be more robust and include 'done' flag.
- Made unit tests deterministic and self-contained using mocking.
- Updated README with correct paths and reliable server polling logic.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
  This commit introduces a number of improvements to the dipg_safety_env, focusing on improving test coverage, fixing bugs, and clarifying
  documentation.

  The key changes are:
   - A new test file with unit tests for all reward functions.
   - A new end-to-end test for the step() function.
   - The environment tests have been moved to the tests/ directory.
   - The tests now use a mock dataset, removing the need to download external files for testing.
   - A bug in the match_format_exactly reward function's regex has been fixed.
   - A corrupted file that was causing a SyntaxError has been repaired.
   - The README.md has been updated to reflect these changes and provide clear instructions on how to run the tests.
Copy link
Contributor

@pankit-eng pankit-eng left a comment

Choose a reason for hiding this comment

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

I'd like for @Darktex to take a look since this environment brings in a couple of relevant abstractions - dataloading & reward pipelines that we have mentioned in RFCs as well.

@@ -0,0 +1,38 @@
# scripts/download_dataset.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this script getting used?

&& rm -rf /var/lib/apt/lists/*

# Install all necessary Python packages for the server, including gunicorn
RUN pip install --no-cache-dir \
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to rebase on top of @Mortimerp9 's recent change to move deps into requirements.txt

@Darktex Darktex self-requested a review October 30, 2025 23:40
@Darktex
Copy link
Contributor

Darktex commented Oct 31, 2025

Hm, so this one is a clone of #97 -- let me just close this and review in the same place in 97...

@Darktex Darktex closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. New Environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants