Skip to content

Conversation

@init27
Copy link
Contributor

@init27 init27 commented Nov 7, 2025

Add NetHack Learning Environment (NLE) Integration

Summary

Adds a complete integration of the NetHack Learning Environment (NLE) as an OpenEnv environment.

What's Added

Core Implementation (8 files, ~600 LOC)

Models (models.py):

  • NLEAction - Discrete action space (113 actions: movement, commands, magic, inventory)
  • NLEObservation - Rich observation space with 14+ types (~140KB JSON per step)
  • NLEState - Episode state tracking (step count, game status, character info)

Client (client.py):

  • NLEEnv - HTTP client implementing HTTPEnvClient[NLEAction, NLEObservation]
  • Handles both old gym (0.25) and new gym (0.26+) API compatibility
  • Supports from_docker_image() for automatic container deployment

Server (server/):

  • NLEEnvironment - Environment wrapper implementing OpenEnv Environment ABC
  • Wraps NLE's gym interface with reset(), step(), state property
  • app.py - FastAPI application using create_app()
  • Dockerfile - Python 3.11 with NLE compiled from source

Environment Characteristics

Action Space:

  • Type: Discrete (113 actions, 0-112)
  • Categories: Movement (8), Diagonal (8), Commands (97)
  • Examples: Move north (0), Eat (37), Search (50), Inventory (104)

Observation Space (~140KB/step):

  • glyphs (21×79): Symbolic dungeon map
  • blstats (26): HP, XP, gold, dungeon level, etc.
  • message (256): Game message buffer
  • chars, colors, specials (21×79 each): Visual display
  • inv_glyphs, inv_strs, inv_letters, inv_oclasses: Inventory
  • tty_chars, tty_colors, tty_cursor: Terminal display

Reward:

  • Default: Score delta (current_score - previous_score)
  • Range: Unbounded (positive for progress)

Episode Termination:

  • Death (player dies)
  • Ascension (player wins - very rare!)
  • Aborted (max steps reached, default 5000)

Design Decisions & Assumptions

1. Assumption: Beefy Compute Available

Decision: Use simple JSON serialization, include all observations by default

2. Gym API Compatibility Layer

Decision: Support both gym 0.25 and 0.26+ APIs

3. Python 3.11 Requirement

4. Single Task Implementation

Decision: Implement only NetHackScore task (maximize score)

What Was Skipped / Deferred

Phase 2 Features (Low Priority)

These can be added later if needed:

  1. Task Variants

    • NetHackStaircase (reach stairs)
    • NetHackOracle (find oracle)
    • NetHackGold (collect gold)
    • NetHackEat (maximize hunger)
    • NetHackScout (maximize exploration)
  2. Seeding API

    • NLE supports deterministic seeding for reproducibility
    • env.seed(core=42, disp=123, reseed=False)
  3. TTYrec Recording/Replay

    • NLE can save game recordings (ttyrec files)
    • Currently disabled by default (save_ttyrec_every=0)
  4. Wizard Mode

    • Debug mode for testing (spawn items, teleport, etc.)
  5. Performance Optimizations

    • Binary serialization (msgpack)
    • Compression (gzip)
    • Selective observations

References

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 7, 2025
@init27 init27 requested review from Darktex and zkwentz November 7, 2025 03:55
@github-actions
Copy link

github-actions bot commented Nov 7, 2025

✅ Deployment succeeded for nle_env

Nice work! Wait for a code review and we're ready to go.

You can iterate locally or validate fixes by running scripts/deploy_to_hf.sh --env "nle_env".

@Darktex Darktex requested a review from Copilot November 7, 2025 18:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a NetHack Learning Environment (NLE) integration to OpenEnv, providing HTTP-based access to the challenging roguelike game NetHack 3.6.6 for reinforcement learning research.

Key changes:

  • Implements NLE environment wrapper with full observation space support (14+ observation types)
  • Provides HTTP server/client architecture for remote environment access
  • Includes Docker containerization with NLE compilation from source

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/envs/nle_env/server/nle_environment.py Core environment implementation wrapping NLE's gym interface with OpenEnv's Environment ABC
src/envs/nle_env/server/app.py FastAPI application setup with environment variable configuration
src/envs/nle_env/server/init.py Server module initialization
src/envs/nle_env/server/Dockerfile Docker build configuration with NLE compilation and dependencies
src/envs/nle_env/models.py Data models for actions, observations, and state
src/envs/nle_env/client.py HTTP client for interacting with NLE server
src/envs/nle_env/init.py Package initialization and exports
src/envs/nle_env/README.md Comprehensive documentation for the NLE environment
examples/nle_random_agent.py Example script demonstrating random agent gameplay

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +22 to +23
@dataclass
class NLEAction(Action):
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The base class Action uses @dataclass(kw_only=True), so child classes should also explicitly use @dataclass(kw_only=True) for consistency with other environments in the codebase (e.g., echo_env, browsergym_env, finrl_env). Change to @dataclass(kw_only=True).

Copilot uses AI. Check for mistakes.
action_id: int # Index into nethack.USEFUL_ACTIONS (0-112)


@dataclass
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The base class Observation uses @dataclass(kw_only=True), so child classes should also explicitly use @dataclass(kw_only=True) for consistency with other environments in the codebase (e.g., echo_env, browsergym_env, finrl_env). Change to @dataclass(kw_only=True).

Suggested change
@dataclass
@dataclass(kw_only=True)

Copilot uses AI. Check for mistakes.

```bash
# Build from repository root (not from server directory)
cd /Users/sanyambhutani/GH/OpenEnv
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

This hardcoded absolute path should be replaced with a generic placeholder path. Consider using a relative path or a generic path like cd /path/to/OpenEnv or just refer to the 'repository root'.

Suggested change
cd /Users/sanyambhutani/GH/OpenEnv
cd /path/to/OpenEnv # Replace with the path to your repository root

Copilot uses AI. Check for mistakes.
from __future__ import annotations

from dataclasses import dataclass, field
from typing import Any, Dict, List, Optional
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Import of 'Any' is not used.
Import of 'Dict' is not used.

Suggested change
from typing import Any, Dict, List, Optional
from typing import List, Optional

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Darktex Darktex left a comment

Choose a reason for hiding this comment

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

Excellent implementation with strong documentation. 3 critical issues must be fixed before merge.

See inline comments for specific fixes needed.

- action_id=50: Search (s)
"""

action_id: int # Index into nethack.USEFUL_ACTIONS (0-112)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 CRITICAL: Missing Action Validation

No validation that action_id is within valid range (0-112). Invalid actions could cause runtime errors.

Suggested Fix:

@dataclass
class NLEAction(Action):
    action_id: int  # Index into nethack.USEFUL_ACTIONS (0-112)

    def __post_init__(self):
        if not 0 <= self.action_id <= 112:
            raise ValueError(f"action_id must be in range 0-112, got {self.action_id}")


```bash
# Build from repository root (not from server directory)
cd /Users/sanyambhutani/GH/OpenEnv
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 CRITICAL: Hardcoded User Path

This is a user-specific absolute path that won't work for others.

Fix:

# Build from repository root
cd /path/to/OpenEnv
# or
cd $(git rev-parse --show-toplevel)

over HTTP.
"""

from typing import Dict
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 CRITICAL: Incomplete Type Annotations

Dict is used without full type specification throughout this file (lines 14, 57, 71, 129).

Fix:

from typing import Any, Dict

Then update all method signatures:

  • _step_payload() -> Dict[str, Any]
  • _parse_result(payload: Dict[str, Any]) -> ...
  • _parse_state(payload: Dict[str, Any]) -> ...

"""
super().__init__(transform=transform)

if NLE is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 IMPORTANT: Improve Error Message

Error message should clarify Docker vs local development context.

Suggested:

if NLE is None:
    raise ImportError(
        "NLE is not installed. This should not happen in Docker deployments.\n"
        "For local development, install with: pip install nle\n"
        "For Docker builds, verify the build completed successfully."
    )

# This source code is licensed under the BSD-style license found in the
# LICENSE file in the root directory of this source tree.

"""Server module for NLE environment."""
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 IMPORTANT: Missing __all__ Export

For consistency with the main package, consider adding:

from .nle_environment import NLEEnvironment

__all__ = ["NLEEnvironment"]

"""
Convert NLE gym observation to NLEObservation.

With beefy compute, we just convert numpy arrays to lists.
Copy link
Contributor

Choose a reason for hiding this comment

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

🟢 MINOR: Informal Comment

"beefy compute" is a bit informal for production code. Consider:

# Convert numpy arrays to nested lists for JSON serialization.
# This approach prioritizes simplicity over optimization, which is
# acceptable given the compute resources available.

if __name__ == "__main__":
import uvicorn

# NLE must run single-threaded (workers=1) due to C extension
Copy link
Contributor

Choose a reason for hiding this comment

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

🟢 MINOR: Clarify Threading Comment

Explain why single-threading is needed:

# NLE must run single-threaded (workers=1) because the NetHack C extension
# uses global state and is not thread-safe

@@ -0,0 +1,126 @@
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

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

✅ POSITIVE: Excellent Example

This example is clear, well-documented, and demonstrates proper usage. Great work!

@@ -0,0 +1,316 @@
# NetHack Learning Environment (NLE) for OpenEnv
Copy link
Contributor

Choose a reason for hiding this comment

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

✅ POSITIVE: Outstanding Documentation

This README is comprehensive, well-structured, and includes multiple examples. Excellent work on documentation!

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Nov 7, 2025

✅ Deployment succeeded for nle_env

Nice work! Wait for a code review and we're ready to go.

You can iterate locally or validate fixes by running scripts/deploy_to_hf.sh --env "nle_env".

Copy link
Contributor

@Darktex Darktex left a comment

Choose a reason for hiding this comment

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

Note: This is an automated review by Claude Code (alignment-reviewer agent), not a human review. The account posting this is shared with the human maintainer.


This is a GitHub PR, not a Phabricator diff. Let me review based on the diff provided in the original message. I'll analyze the code directly from the diff.

PR #161 Review: Add NLE env to OpenEnv

Previous Review Summary

Key points from @Darktex and @copilot-pull-request-reviewer:

  • 3 critical issues requiring fixes (per @Darktex - specifics not provided in inline comments visible here)
  • Generally excellent implementation with strong documentation
  • Deployment succeeded to HuggingFace test space
  • Need to verify critical issues are addressed

Tier 1: Bugs, Lint Issues, Security, Debug Code

✅ Automated Checks

Lint Check: ❌ Could not run (uv not installed in environment)

  • Recommendation: Run uv run ruff format src/ tests/ --check locally to verify formatting

Debug Code Check: ✅ PASS

  • No debug code found in the new NLE environment files
  • Existing print statements are in CLI/test files (not part of this PR)

🔴 CRITICAL: HTTP-Only Implementation Violates Current Architecture

Location: src/envs/nle_env/client.py:21, src/envs/nle_env/server/app.py

Issue: This PR implements an HTTP-only environment using HTTPEnvClient, but according to INVARIANTS.md line 73:

Note: We are in the process of deprecating HTTP (see PR #252) in favor of WebSocket-only, but we are still transitioning and both protocols are currently available.

The Problem:

  1. The client extends HTTPEnvClient (line 21 in client.py)
  2. The server uses create_app() which provides HTTP endpoints (app.py line 46)
  3. WebSocket support for the Gym-like API is the future direction
  4. New environments should ideally use WebSocket or at minimum be prepared for migration

Recommendation:

  • Short-term (acceptable): Document in the PR that this is HTTP-based and will need migration when PR #252 lands
  • Better: Check if WebSocket support is available and use that instead
  • Flag for human review: Is it acceptable to add new HTTP-only environments during the WebSocket transition?

🟡 Bug: Gym API Version Compatibility Issue

Location: src/envs/nle_env/server/nle_environment.py:116-128, 147-162

Issue: The code handles both old and new Gym API versions:

# Handle both old gym API (returns obs dict) and new API (returns tuple)
if isinstance(reset_result, tuple):
    gym_obs, _ = reset_result  # Unpack (observation, info)
else:
    gym_obs = reset_result  # Old API

The Problem: This defensive coding is good, BUT:

  1. The Dockerfile (line 36) installs gym without version pinning
  2. No documentation of which gym version is tested/supported
  3. Future gym updates could break compatibility silently

Recommendation:

# Pin gym version for reproducibility
RUN pip install --no-cache-dir nle gym==0.26.2  # or whichever version tested

🟡 Bug: Generic Type Mismatch in Environment Class

Location: src/envs/nle_env/server/nle_environment.py:27

Issue: The class signature is:

class NLEEnvironment(Environment):

But according to PATTERNS.md line 65, it should be:

class MyEnvironment(Environment[MyAction, MyObservation, MyState]):

The Problem:

  • Missing generic type parameters violates the type safety pattern
  • INVARIANTS.md line 14: "All environments must use Environment[ActT, ObsT, StateT] generics"

Fix Required:

class NLEEnvironment(Environment[NLEAction, NLEObservation, NLEState]):

This is a Type Safety Invariant Violation (INVARIANTS.md lines 13-16).


🟡 Security: Docker Build Context

Location: src/envs/nle_env/README.md:49-50

Issue: The build command is:

docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .

The Problem: Build context is repo root (.), which means:

  • Entire repo copied to Docker build context
  • Potentially includes .git/, venv, cache files
  • Slower builds, larger context
  • Could accidentally include secrets if they exist in repo

Recommendation: Add .dockerignore file or document why full context is needed (likely for copying src/core/ and src/envs/nle_env/)


Tier 2: Alignment with Principles and Invariants

🟢 PASS: Dual API Boundary (INVARIANTS.md lines 41-58)

The environment correctly:

  • Exposes only Gym-like API (reset, step, state)
  • Does NOT expose these to agents via MCP tools
  • Agent would access NetHack through the observation space, not simulation controls

✅ No violations detected


🟢 PASS: Client-Server Separation (INVARIANTS.md lines 59-62)

  • Client (client.py) imports only from models.py and core/
  • Server imports only from models.py and core/
  • No cross-boundary imports

✅ Separation maintained correctly


🟢 PASS: Rewards in Environment (INVARIANTS.md lines 64-67)

Reward computation happens inside the NLE gym environment and is passed through:

reward = float(reward)  # Line 168 in nle_environment.py

The NLE library computes rewards (score delta by default), and the wrapper correctly keeps this internal.

✅ Principle maintained


🟡 ALIGNMENT FLAG: "Beefy Compute" Assumption in Documentation

Location: Throughout README.md and code comments

Examples:

  • README.md line 241: "With beefy compute (64+ cores, 256GB+ RAM, 10Gbps network)"
  • client.py line 30: "With beefy compute, we use simple JSON serialization"
  • nle_environment.py line 36: "With beefy compute, we use simple JSON serialization"

Concern: This violates PRINCIPLES.md line 8:

Minimize human-agent divergence: Tools that work for humans should work for agents

The Issue:

  • The "beefy compute" assumption is presented as a design decision
  • But it's not in PRINCIPLES.md or any RFC
  • Creates an unstated assumption about deployment infrastructure
  • What happens when users deploy to modest hardware?

Questions for Human Review:

  1. Is "beefy compute" an official OpenEnv principle that should be in PRINCIPLES.md?
  2. Should environments gracefully degrade on modest hardware?
  3. Should this be configurable (e.g., observation subsetting)?

Recommendation: Either:

  • Add "beefy compute" to PRINCIPLES.md as an explicit assumption, OR
  • Remove these comments and just implement simply without the justification, OR
  • Make observation subset configurable for resource-constrained deployments

🟡 Pattern Deviation: Observation Structure

Location: src/envs/nle_env/models.py:42-92

Issue: The NLEObservation class has 17 optional fields (all Optional[...]), which differs from the pattern in PATTERNS.md where observations typically have required fields.

Comparison:

# Pattern (PATTERNS.md line 35)
class MyObservation(BaseModel):
    result: str       # Required
    reward: float     # Required
    done: bool        # Required

# NLE implementation
class NLEObservation(Observation):
    glyphs: Optional[List[List[int]]] = None  # All optional
    blstats: Optional[List[int]] = None
    # ... 15 more optional fields

Analysis: This is actually reasonable for NLE because:

  • NLE supports 14+ observation types that can be selectively enabled
  • The env is configured with observation_keys to choose which ones to include
  • Making them all optional allows flexibility

Recommendation: ✅ Accept this deviation BUT add a comment in models.py explaining why all fields are optional (relates to NLE's configurable observation space).


🔴 CRITICAL: Missing Episode ID Initialization

Location: src/envs/nle_env/server/nle_environment.py:103

Issue:

self._episode_id: Optional[str] = None

Then in state property (line 194):

return NLEState(
    episode_id=self._episode_id,  # Could be None!
    ...
)

The Problem:

  • Episode ID is None until first reset
  • If someone calls state before reset(), they get episode_id=None
  • This could break logging/tracking systems expecting string IDs

Fix:

def __init__(self, ...):
    # ... 
    self._episode_id: str = f"nle_init_{int(time.time() * 1000000)}"

🟡 Inconsistency: Docker Build Instructions

Location: README.md lines 49-50 vs Dockerfile comments

README says:

# Build from repository root (not from server directory)
cd /Users/sanyambhutani/GH/OpenEnv
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .

Issues:

  1. Hardcoded absolute path /Users/sanyambhutani/GH/OpenEnv (should be relative)
  2. Should follow the pattern from other environments
  3. Comment says "from repository root" but command cd's to specific path

Fix:

# Build from repository root
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .

Summary

Tier 1 Issues (MUST FIX):

  1. CRITICAL: Missing generic type parameters on NLEEnvironment class - violates type safety invariant
  2. CRITICAL: Missing episode ID initialization - can return None before reset
  3. CRITICAL: Flag HTTP-only implementation during WebSocket migration period (needs human decision)
  4. BUG: Pin gym version in Dockerfile for reproducibility
  5. SECURITY: Add .dockerignore or document why full repo context needed for Docker build

Tier 2 Issues (SHOULD ADDRESS):

  1. ALIGNMENT FLAG: "Beefy compute" assumption not documented in PRINCIPLES.md - needs human review
  2. PATTERN: All-optional observation fields deviate from pattern (acceptable but should document why)
  3. INCONSISTENCY: Fix hardcoded path in README Docker build command

Positive Observations:

✅ Excellent documentation in README.md
✅ Proper client-server separation maintained
✅ Dual API boundary respected (no MCP exposure of simulation controls)
✅ Rewards correctly kept inside environment boundary
✅ Good handling of both old/new Gym API versions (but needs version pinning)
✅ Comprehensive observation space implementation


Recommendation

Changes Requested - Fix Tier 1 critical issues before merge:

  1. Add generic type parameters to NLEEnvironment
  2. Initialize episode_id in __init__
  3. Pin gym version in Dockerfile
  4. Get human approval on HTTP vs WebSocket question
  5. Fix hardcoded path in README

After these fixes, this will be an excellent addition to OpenEnv! 🎮


Automated review by Claude Code | Learn more about OpenEnv's agentic workflow

Copy link

Copilot AI commented Jan 13, 2026

@Darktex I've opened a new pull request, #289, to work on those changes. Once the pull request is ready, I'll request review from you.

@zkwentz
Copy link
Contributor

zkwentz commented Jan 21, 2026

@greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 21, 2026

Greptile Summary

Adds complete NetHack Learning Environment (NLE) integration as a new OpenEnv environment, enabling RL agents to play the challenging roguelike game NetHack through a standardized HTTP interface.

Key Changes:

  • Implemented NLEEnvironment server wrapping the NLE gym interface with reset/step/state methods
  • Created NLEEnv HTTP client extending HTTPEnvClient for remote environment access
  • Defined NLEAction (discrete, 113 actions), NLEObservation (~140KB with 14+ observation types), and NLEState data models
  • Added FastAPI server application with Docker deployment support
  • Included comprehensive README documentation with examples and configuration guide
  • Provided nle_random_agent.py example demonstrating usage patterns

Architecture:

  • Handles both old gym (0.25) and new gym (0.26+) API compatibility in reset/step methods
  • Converts numpy arrays to nested lists for JSON serialization
  • Supports from_docker_image() pattern for automatic container deployment
  • Single-threaded server (workers=1) due to NLE C extension global state requirements

Style Improvements Needed:

  • Dockerfile should use openenv-base image pattern instead of building from scratch (reduces duplication and build time)
  • README contains hardcoded path that should be generic
  • Dependencies like fastapi/uvicorn should be removed from Dockerfile since base image provides them

Confidence Score: 4/5

  • Safe to merge with minor style improvements recommended
  • The implementation is solid and follows OpenEnv patterns correctly. The code handles gym API compatibility well, properly extends base classes, and includes comprehensive documentation. The main issues are stylistic: the Dockerfile doesn't follow the standard base image pattern used by other environments, which increases duplication and build complexity but doesn't affect functionality. All core logic is sound.
  • Pay attention to src/envs/nle_env/server/Dockerfile - consider refactoring to use the openenv-base image pattern for consistency with other environments

Important Files Changed

Filename Overview
src/envs/nle_env/models.py Defines NLEAction, NLEObservation, and NLEState dataclasses - clean implementation with proper type hints and documentation
src/envs/nle_env/client.py HTTP client implementation correctly extends HTTPEnvClient with proper parsing methods
src/envs/nle_env/server/nle_environment.py Environment wrapper correctly handles both old and new gym APIs with proper observation conversion
src/envs/nle_env/server/Dockerfile Dockerfile builds from scratch instead of using openenv-base image pattern, installs all dependencies manually
src/envs/nle_env/README.md Comprehensive documentation with examples, but contains hardcoded path in build instructions

Sequence Diagram

sequenceDiagram
    participant User as User/Agent
    participant Client as NLEEnv<br/>(HTTPEnvClient)
    participant Docker as Docker Container
    participant Server as FastAPI App<br/>(app.py)
    participant Env as NLEEnvironment<br/>(Environment)
    participant NLE as NLE Gym<br/>(NetHack)

    Note over User,NLE: Initialization Phase
    User->>Client: NLEEnv.from_docker_image("nle-env:latest")
    Client->>Docker: Start container (port 8000)
    Docker->>Server: Launch uvicorn server
    Server->>Env: Create NLEEnvironment instance
    Env->>NLE: Initialize NLE gym env
    NLE-->>Env: Ready
    Env-->>Server: Ready
    Server-->>Docker: Server running
    Docker-->>Client: Return base_url
    Client-->>User: NLEEnv instance

    Note over User,NLE: Episode Reset
    User->>Client: reset()
    Client->>Server: POST /reset
    Server->>Env: reset()
    Env->>NLE: nle_env.reset()
    NLE-->>Env: gym_obs (numpy arrays)
    Env->>Env: Convert to NLEObservation<br/>(arrays to lists)
    Env-->>Server: NLEObservation
    Server-->>Client: JSON response
    Client->>Client: Parse to StepResult[NLEObservation]
    Client-->>User: StepResult

    Note over User,NLE: Action Execution
    User->>Client: step(NLEAction(action_id=0))
    Client->>Client: Convert to JSON payload
    Client->>Server: POST /step {action_id: 0}
    Server->>Env: step(NLEAction)
    Env->>NLE: nle_env.step(action_id)
    NLE->>NLE: Execute action in NetHack
    NLE-->>Env: (obs, reward, done, info)
    Env->>Env: Convert observation<br/>(numpy to lists)
    Env->>Env: Update episode state
    Env-->>Server: NLEObservation
    Server-->>Client: JSON {observation, reward, done}
    Client->>Client: Parse to StepResult
    Client-->>User: StepResult

    Note over User,NLE: State Query
    User->>Client: state()
    Client->>Server: GET /state
    Server->>Env: state property
    Env-->>Server: NLEState
    Server-->>Client: JSON state
    Client->>Client: Parse to NLEState
    Client-->>User: NLEState

    Note over User,NLE: Cleanup
    User->>Client: close()
    Client->>Docker: Stop container
    Docker->>Server: Shutdown
    Server->>Env: close()
    Env->>NLE: nle_env.close()
    NLE-->>Env: Closed
    Docker-->>Client: Container stopped
    Client-->>User: Done
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile


# NLE requires build dependencies, so we start from a base image with build tools
# Using Python 3.11 for kw_only dataclass support (required by OpenEnv core)
FROM python:3.11-slim
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Use the standard openenv-base image pattern like other environments (atari, browsergym) instead of building from scratch. This reduces duplication, build time, and ensures consistency.

Suggested change
FROM python:3.11-slim
FROM openenv-base:latest

Then add a comment block above explaining the build pattern:

# Configurable base image - defaults to local build, can be overridden for CI/CD
# Base image provides: fastapi, uvicorn, requests, curl, PYTHONPATH=/app/src
#
# Local build: docker build -t openenv-base:latest -f src/core/containers/images/Dockerfile .
#              docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
#
# CI/CD build: docker build --build-arg BASE_IMAGE=ghcr.io/meta-pytorch/openenv-base:latest \
#              -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
ARG BASE_IMAGE=openenv-base:latest
FROM ${BASE_IMAGE}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/envs/nle_env/server/Dockerfile
Line: 9:9

Comment:
**style:** Use the standard `openenv-base` image pattern like other environments (atari, browsergym) instead of building from scratch. This reduces duplication, build time, and ensures consistency.

```suggestion
FROM openenv-base:latest
```

Then add a comment block above explaining the build pattern:
```dockerfile
# Configurable base image - defaults to local build, can be overridden for CI/CD
# Base image provides: fastapi, uvicorn, requests, curl, PYTHONPATH=/app/src
#
# Local build: docker build -t openenv-base:latest -f src/core/containers/images/Dockerfile .
#              docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
#
# CI/CD build: docker build --build-arg BASE_IMAGE=ghcr.io/meta-pytorch/openenv-base:latest \
#              -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
ARG BASE_IMAGE=openenv-base:latest
FROM ${BASE_IMAGE}
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +27 to +33
# NLE requires cmake to be available during installation
RUN pip install --no-cache-dir \
fastapi \
uvicorn \
requests \
pydantic \
numpy
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Since the base image already includes fastapi, uvicorn, and requests, these can be removed. Only install NLE-specific dependencies here.

Suggested change
# NLE requires cmake to be available during installation
RUN pip install --no-cache-dir \
fastapi \
uvicorn \
requests \
pydantic \
numpy
# Install NLE (this will compile NetHack from source)
# Using the stable version from PyPI
RUN pip install --no-cache-dir nle gym numpy

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/envs/nle_env/server/Dockerfile
Line: 27:33

Comment:
**style:** Since the base image already includes `fastapi`, `uvicorn`, and `requests`, these can be removed. Only install NLE-specific dependencies here.

```suggestion
# Install NLE (this will compile NetHack from source)
# Using the stable version from PyPI
RUN pip install --no-cache-dir nle gym numpy
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +48 to +50
# Build from repository root (not from server directory)
cd /Users/sanyambhutani/GH/OpenEnv
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Hardcoded absolute path should be replaced with generic instruction

Suggested change
# Build from repository root (not from server directory)
cd /Users/sanyambhutani/GH/OpenEnv
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
# Build from repository root (not from server directory)
cd <repository-root>
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/envs/nle_env/README.md
Line: 48:50

Comment:
**style:** Hardcoded absolute path should be replaced with generic instruction

```suggestion
# Build from repository root (not from server directory)
cd <repository-root>
docker build -f src/envs/nle_env/server/Dockerfile -t nle-env:latest .
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@burtenshaw
Copy link
Collaborator

@init27 It looks like this PR is no longer compatible with the code base. I'd suggest closing and opening a new PR with the env in envs/, just for simplicity.

@burtenshaw
Copy link
Collaborator

@init27 @Darktex as discussed, I'll close because they are out of sync with the current project. Hope to see them reopened.

@burtenshaw burtenshaw closed this Feb 6, 2026
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.

6 participants