New Environment: Classic Grid World (with Community How-To Guide) - #221
New Environment: Classic Grid World (with Community How-To Guide)#221yuvrajpant56 wants to merge 4 commits into
Conversation
meta-cla
Bot
added
the
CLA Signed
|
Validation reported issues. Review the log and re-run locally with
You can deploy the environment to Hugging Face Spaces by running |
|
Hi @init27 @burtenshaw @jspisak as discussed in issue #200, here is the PR for the "Grid World" environment. I have built the environment and written a detailed README.md inside the grid_world folder to serve as a "how-to" guide for the community, as you suggested. Please let me know what you think! |
|
Thanks for this @yuvrajpant56. This looks good. Could you push the env to the hf hub and update the |
|
@burtenshaw Done! I have pushed the environment to the Hugging Face Hub and updated docs/environments.md with the link. I also added the grid_world.md documentation page. Ready for review. |
Darktex
left a comment
Darktex
left a comment
There was a problem hiding this comment.
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.
Now I have sufficient context. Let me produce the comprehensive review:
PR #221 Review: Grid World Environment
Previous Review Summary
The existing reviews highlight:
- Validation failure (github-actions) - The environment validation failed, though details weren't shown
- Request to deploy (burtenshaw) - Asked to push to Hugging Face Hub and update docs
- Deployment completed (yuvrajpant56) - Author confirmed deployment and docs update
Tier 1: Critical Issues (Bugs, Security, Lint)
🔴 CRITICAL: Wrong Directory Structure
Location: All files in src/envs/grid_world_env/
Issue: The PR places the environment in src/envs/ but the established pattern (per echo_env and all other envs) is envs/ (without the src/ prefix). Looking at the repo structure:
- Reference:
envs/echo_env/,envs/atari_env/, etc. - This PR:
src/envs/grid_world_env/❌
Impact:
- Environment won't be discoverable by OpenEnv tooling
- Import paths will be incorrect
- Breaks consistency with all other environments
Fix Required: Move all files from src/envs/grid_world_env/ to envs/grid_world_env/
🔴 CRITICAL: Incorrect Import Paths
Location: Multiple files (models.py, client.py, server/grid_world_environment.py, server/app.py)
Issue: The imports use incorrect module paths:
# grid_world_env/models.py (line 7)
from core.env_server import Action, Observation, State
# Should be (per echo_env pattern):
from openenv.core.env_server.types import Action, Observation, StateImpact: Code will fail at runtime with ImportError
Files affected:
models.py:7-8- Missingopenenv.prefix and wrong submoduleclient.py:1- Wrong import path forHTTPEnvClientserver/app.py:14- Wrong import forcreate_fastapi_appserver/grid_world_environment.py:2- Wrong import forEnvironment
Fix Required: Update all imports to match echo_env pattern with proper openenv.core.* paths and try/except for standalone support.
🔴 CRITICAL: Using Deprecated HTTP Pattern
Location: client.py:3 - Inherits from HTTPEnvClient
Issue: Per INVARIANTS.md line 69-73:
"We are in the process of deprecating HTTP (see PR #252) in favor of WebSocket-only, but we are still transitioning"
The echo_env reference implementation uses EnvClient (WebSocket-based), not HTTPEnvClient.
Evidence from echo_env/client.py:31:
class EchoEnv(EnvClient[EchoAction, EchoObservation, State]):Impact:
- Using deprecated API that's being phased out
- Won't work with WebSocket-based infrastructure
- Inconsistent with current best practices
Fix Required: Change from HTTPEnvClient to EnvClient and update implementation accordingly.
🔴 CRITICAL: Wrong App Creation Function
Location: server/app.py:25
Issue:
# grid_world uses (WRONG):
app = create_fastapi_app(env, GridWorldAction, GridWorldObservation)
# echo_env uses (CORRECT):
app = create_app(EchoEnvironment, EchoAction, EchoObservation, env_name="echo_env")Problems:
- Function name is
create_app, notcreate_fastapi_app - Passes instance (
env) instead of class (GridWorldEnvironment) - Missing
env_nameparameter - Doesn't support WebSocket sessions (needs class factory pattern)
Impact: Server won't start, wrong API exposed
Fix Required: Match echo_env pattern exactly.
🟡 MODERATE: Incorrect State Model
Location: models.py:30-37, server/grid_world_environment.py:21
Issue: GridWorldState incorrectly inherits from State and duplicates base fields:
@dataclass
class GridWorldState(State): # ❌ State is not meant to be inherited
agent_x: int = 0
agent_y: int = 0
goal_x: int = 0
goal_y: int = 0
grid_size: int = 0
episode_steps: int = 0 # ❌ Duplicates State.step_countCorrect pattern (from echo_env:48, 95-102):
# Don't inherit from State, use State directly
class EchoEnvironment(Environment):
def __init__(self):
self._state = State(episode_id=str(uuid4()), step_count=0)
@property
def state(self) -> State:
return self._stateImpact:
- Type confusion between base State and extended version
- Duplicate step counting (both
episode_stepsandstep_count) - Violates the established pattern
Fix Required:
- Don't inherit GridWorldState from State
- Use plain State in the environment
- Store grid-specific data as environment private fields, not in State
- Remove
episode_steps(usestep_countfrom State)
🟡 MODERATE: Missing Pydantic Models
Location: models.py - Uses @dataclass instead of Pydantic
Issue: Per PATTERNS.md:78-82 and echo_env reference:
# Should use Pydantic BaseModel, not @dataclass
from pydantic import Field
class EchoAction(Action): # Action is a Pydantic model
message: str = Field(..., min_length=1, description="...")Current code (grid_world_env):
from dataclasses import dataclass # ❌
@dataclass
class GridWorldAction(Action): # Mixing dataclass with Pydantic parentImpact:
- Won't serialize correctly for FastAPI
- Missing validation features
- Inconsistent with project patterns
Fix Required: Remove @dataclass, use proper Pydantic model definition with Field() validators.
🟡 MODERATE: Missing Print Statement
Location: server/grid_world_environment.py:27
Issue:
def reset(self) -> GridWorldObservation:
print("Resetting Grid World environment...") # ❌ Debug printEvidence: The debug check found similar prints in test files, but production code should not have print statements.
Impact: Pollutes production logs
Fix Required: Remove the print statement.
🟡 MODERATE: Spurious Test File
Location: test_my_env.py at repo root
Issue: Test file added to repository root instead of tests/ directory. This violates project structure.
Impact:
- Clutters repo root
- Won't be run by pytest
- Not following test conventions
Fix Required: Either remove or move to tests/envs/test_grid_world.py and adapt to pytest format.
🟡 MODERATE: Missing Copyright Headers
Location: All grid_world_env files
Issue: All files lack Meta copyright headers that are present in echo_env:
# Copyright (c) Meta Platforms, Inc. and affiliates.
# All rights reserved.
#
# This source code is licensed under the BSD-style license found in the
# LICENSE file in the root directory of this source tree.Impact: License compliance issue
Fix Required: Add copyright headers to all Python files.
🟢 MINOR: Incorrect Reward on Reset
Location: server/grid_world_environment.py:44
Issue:
def reset(self) -> GridWorldObservation:
return GridWorldObservation(
...
reward=None, # ❌ Should be numeric, not None
done=False
)Per echo_env:60-66:
def reset(self) -> EchoObservation:
return EchoObservation(
...
reward=0.0, # ✓ Explicit float
done=False,
)Impact: Type inconsistency, potential None propagation bugs
Fix Required: Change reward=None to reward=0.0
Tier 2: Alignment Issues
⚠️ ALIGNMENT FLAG: Missing Try/Except Import Pattern
Principle at stake: "Be hands-on" - Environments should work both in-repo and standalone
The concern: All grid_world imports are in-repo only. The echo_env reference (models.py:15-21, client.py:16-28) uses try/except to support both:
try:
# In-repo imports
from openenv.core.env_server.types import Action, Observation
except ImportError:
# Standalone imports (when installed via pip)
from openenv.core.env_server.types import Action, ObservationGrid world has none of this, making it unusable as a standalone environment.
Suggested reviewer: @burtenshaw
⚠️ ALIGNMENT FLAG: Missing Main Entry Point
Principle at stake: "Be hands-on" - ready-to-use implementations
The concern: echo_env/server/app.py includes a main() function (lines 41-53) that enables:
def main():
import uvicorn
uvicorn.run(app, host="0.0.0.0", port=8000)
if __name__ == "__main__":
main()This allows users to run uv run --project . server or python -m envs.grid_world_env.server.app without Docker. Grid world lacks this convenience.
Suggested reviewer: @burtenshaw
⚠️ ALIGNMENT FLAG: Shell Script vs Pytest
Principle at stake: Consistency with project test infrastructure
The concern: Grid world provides test_grid_world.sh (a shell script) instead of pytest-based tests like other environments. While the shell script is thorough, it doesn't integrate with the project's test infrastructure.
Per CLAUDE.md build commands:
PYTHONPATH=src:envs uv run pytest tests/ -v --tb=shortSuggested action: Add proper pytest tests in tests/envs/test_grid_world.py (shell script can remain as additional integration test).
Suggested reviewer: @init27
Documentation Quality Assessment
✅ Strengths
The README.md in grid_world_env is excellent as a learning resource:
- Clear overview of environment mechanics
- Detailed "How-To" guide for building OpenEnv environments
- Good explanations of each component's purpose
- Helpful example gameplay walkthrough
⚠️ Issues
-
Outdated scaffolding command (README.md:56):
Open init grid_world_env # ❌ Command doesn't existShould be:
openenv init grid_world_env -
Directory structure in README (line 62-73) shows correct structure but the actual PR has wrong paths (src/envs vs envs)
-
Missing docstrings: Unlike echo_env which has comprehensive docstrings, grid_world files have minimal documentation in the code itself.
Summary & Recommended Actions
Must Fix Before Merge (Tier 1 Critical)
- ✅ Move entire environment from
src/envs/grid_world_env/→envs/grid_world_env/ - ✅ Fix all import paths to use
openenv.core.*(match echo_env pattern) - ✅ Switch from
HTTPEnvClienttoEnvClient(WebSocket-based) - ✅ Fix
server/app.pyto usecreate_app()with correct signature - ✅ Redesign State model - don't inherit, use
Statedirectly - ✅ Convert from
@dataclassto Pydantic models with Field validators - ✅ Remove debug print statement from reset()
- ✅ Remove or relocate
test_my_env.pyfrom repo root - ✅ Add Meta copyright headers to all files
- ✅ Fix
reward=None→reward=0.0in reset()
Should Fix (Tier 2 Alignment)
- ✅ Add try/except import pattern for standalone support
- ✅ Add
main()entry point to server/app.py - ✅ Add pytest tests to
tests/envs/test_grid_world.py - ✅ Fix README command (
Open init→openenv init) - ✅ Add comprehensive docstrings matching echo_env quality
Validation Failure Investigation
The github-actions bot reported validation failure but provided no details. After merge prep, recommend running:
openenv validate --verbose envs/grid_world_envPositive Notes
- Excellent educational value - The README serves as a great "How-To" guide
- Complete implementation - All core components are present
- Good test coverage - Shell script tests are thorough
- Deployed to HF - Author followed through on deployment request
- Simple, clear environment - Grid world is perfect for learning/testing
The core idea and effort are solid. The issues are primarily about matching established patterns from echo_env and fixing the directory structure. Once aligned with the reference implementation, this will be a valuable addition to the project.
Automated review by Claude Code | Learn more about OpenEnv's agentic workflow
|
@yuvrajpant56 Could you respond to the suggestions in the Claude code review and resolve conflicts with main please. Looking forward to merging gridworld. |
|
@burtenshaw After doing correction in my code i found that my code fails in openenv validation. I am seeing that openenv validate fails on echo_env as well: "Dependency on openenv-core is deprecated" (venv) yuvrajpant@macbookpro OpenEnv % openenv validate --verbose envs/grid_world_env Issues found:
Supported deployment modes: Issues found:
Supported deployment modes: Could you please help me resolve this issue. |
Greptile SummaryThis PR adds a new Grid World environment to the OpenEnv ecosystem - a classic 5x5 grid navigation environment where an agent moves from (0,0) to reach a goal at (4,4). The implementation follows OpenEnv architectural patterns and includes an excellent how-to guide for community contributors. Major Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as GridWorldEnv Client
participant API as FastAPI Server
participant Env as GridWorldEnvironment
participant State as GridWorldState
Note over Client,State: Environment Initialization
Client->>API: POST /reset
API->>Env: reset()
Env->>State: Reset agent position to (0,0)
Env->>State: Set episode_id = uuid()
Env->>State: Reset step counters
Env-->>API: GridWorldObservation(x=0, y=0, done=False)
API-->>Client: JSON response
Note over Client,State: Agent Takes Actions
Client->>API: POST /step {action: "DOWN"}
API->>Env: step(GridWorldAction)
Env->>State: Increment step_count
Env->>Env: Process movement logic
Env->>Env: Apply boundary constraints
Env->>State: Update agent_x, agent_y
Env->>Env: Calculate reward & done
Env-->>API: GridWorldObservation(reward=-0.1, done=False)
API-->>Client: JSON response
Client->>API: POST /step {action: "RIGHT"}
API->>Env: step(GridWorldAction)
Env->>State: Increment step_count
Env->>Env: Process movement logic
Env->>State: Update agent position
Env->>Env: Check if goal reached
Note over Env: If at goal (4,4): reward=1.0, done=True
Env-->>API: GridWorldObservation(reward, done)
API-->>Client: JSON response
Note over Client,State: State Inspection
Client->>API: GET /state
API->>Env: state property
Env->>State: Read full state
State-->>API: GridWorldState(agent_x, agent_y, goal_x, goal_y, etc)
API-->>Client: JSON response
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| gymnasium>=0.29.0 | ||
| ale-py>=0.8.0 | ||
| numpy>=1.24.0 | ||
| pandas>=2.0.0 |
There was a problem hiding this comment.
style: Grid World doesn't use gymnasium, ale-py, numpy, or pandas anywhere in the code - these are unnecessary dependencies copied from another environment.
| gymnasium>=0.29.0 | |
| ale-py>=0.8.0 | |
| numpy>=1.24.0 | |
| pandas>=2.0.0 | |
| # No additional dependencies needed beyond fastapi/uvicorn |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/envs/grid_world_env/server/requirements.txt
Line: 3:6
Comment:
**style:** Grid World doesn't use `gymnasium`, `ale-py`, `numpy`, or `pandas` anywhere in the code - these are unnecessary dependencies copied from another environment.
```suggestion
# No additional dependencies needed beyond fastapi/uvicorn
```
How can I resolve this? If you propose a fix, please make it concise.| import sys | ||
| import os | ||
|
|
||
| # This adds the current directory (OpenEnv) to Python's search path, | ||
| # so it can find the 'grid_world' folder. | ||
| current_dir = os.path.abspath(os.path.dirname(__file__)) | ||
| sys.path.insert(0, current_dir) | ||
|
|
||
| # Now we can import from the grid_world package. | ||
| from grid_world.client import GridWorldEnv | ||
|
|
||
| from grid_world.models import MoveAction | ||
|
|
||
| print("Attempting to start GridWorld environment...") | ||
|
|
||
| try: | ||
| # This will start the "grid_world:latest" Docker image | ||
| client = GridWorldEnv.from_docker_image("grid_world:latest") | ||
|
|
||
| print("--- Starting Game ---") | ||
| result = client.reset() | ||
| print(f"Initial Observation: {result.observation}") | ||
|
|
||
| # Try a few moves | ||
| print("Taking action: DOWN") | ||
| result = client.step(action=MoveAction.DOWN) | ||
| print(f" -> Obs: {result.observation}") | ||
| print(f" -> Reward: {result.reward}") | ||
|
|
||
| print("Taking action: RIGHT") | ||
| result = client.step(action=MoveAction.RIGHT) | ||
| print(f" -> Obs: {result.observation}") | ||
| print(f" -> Reward: {result.reward}") | ||
|
|
||
| print("Taking action: UP (trying to hit wall)") | ||
| result = client.step(action=MoveAction.UP) # Should go back to [0, 1] | ||
| result = client.step(action=MoveAction.UP) # Should hit wall | ||
| print(f" -> Obs: {result.observation}") | ||
| print(f" -> Reward: {result.reward}") # Should be -0.5 | ||
|
|
||
| except Exception as e: | ||
| print(f"An error occurred: {e}") | ||
|
|
||
| finally: | ||
| # This stops and removes the container | ||
| print("--- Cleaning up ---") | ||
| if 'client' in locals() and client: | ||
| client.close() | ||
| print("Test complete.") No newline at end of file |
There was a problem hiding this comment.
logic: This test file shouldn't be in the repo root - it contains incorrect imports (grid_world instead of src.envs.grid_world_env) and references a non-existent Docker image name.
| import sys | |
| import os | |
| # This adds the current directory (OpenEnv) to Python's search path, | |
| # so it can find the 'grid_world' folder. | |
| current_dir = os.path.abspath(os.path.dirname(__file__)) | |
| sys.path.insert(0, current_dir) | |
| # Now we can import from the grid_world package. | |
| from grid_world.client import GridWorldEnv | |
| from grid_world.models import MoveAction | |
| print("Attempting to start GridWorld environment...") | |
| try: | |
| # This will start the "grid_world:latest" Docker image | |
| client = GridWorldEnv.from_docker_image("grid_world:latest") | |
| print("--- Starting Game ---") | |
| result = client.reset() | |
| print(f"Initial Observation: {result.observation}") | |
| # Try a few moves | |
| print("Taking action: DOWN") | |
| result = client.step(action=MoveAction.DOWN) | |
| print(f" -> Obs: {result.observation}") | |
| print(f" -> Reward: {result.reward}") | |
| print("Taking action: RIGHT") | |
| result = client.step(action=MoveAction.RIGHT) | |
| print(f" -> Obs: {result.observation}") | |
| print(f" -> Reward: {result.reward}") | |
| print("Taking action: UP (trying to hit wall)") | |
| result = client.step(action=MoveAction.UP) # Should go back to [0, 1] | |
| result = client.step(action=MoveAction.UP) # Should hit wall | |
| print(f" -> Obs: {result.observation}") | |
| print(f" -> Reward: {result.reward}") # Should be -0.5 | |
| except Exception as e: | |
| print(f"An error occurred: {e}") | |
| finally: | |
| # This stops and removes the container | |
| print("--- Cleaning up ---") | |
| if 'client' in locals() and client: | |
| client.close() | |
| print("Test complete.") | |
| # This file should be removed - use test_grid_world.sh instead |
Prompt To Fix With AI
This is a comment left during a code review.
Path: test_my_env.py
Line: 1:49
Comment:
**logic:** This test file shouldn't be in the repo root - it contains incorrect imports (`grid_world` instead of `src.envs.grid_world_env`) and references a non-existent Docker image name.
```suggestion
# This file should be removed - use test_grid_world.sh instead
```
How can I resolve this? If you propose a fix, please make it concise.| </svg> | ||
| </a> | ||
| <a class="environment-card__icon environment-card__icon--hf" href="https://huggingface.co/spaces/yuvrajpant56/grid_world_env" target="_blank" rel="noreferrer noopener" aria-label="Grid World on Hugging Face"> | ||
| <img src="https://huggingface.co/front/assets/huggingface_logo_noborder.svg" alt="" aria-hidden="true" /> |
There was a problem hiding this comment.
syntax: Typo - huggingface_logo_noborder uses underscore instead of hyphen like all other environments.
| <img src="https://huggingface.co/front/assets/huggingface_logo_noborder.svg" alt="" aria-hidden="true" /> | |
| <img src="https://huggingface.co/front/assets/huggingface_logo-noborder.svg" alt="" aria-hidden="true" /> |
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: docs/environments.md
Line: 258:258
Comment:
**syntax:** Typo - `huggingface_logo_noborder` uses underscore instead of hyphen like all other environments.
```suggestion
<img src="https://huggingface.co/front/assets/huggingface_logo-noborder.svg" alt="" aria-hidden="true" />
```
<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.
|
I encountered significant merge conflicts due to the branch being outdated. I have re-based the changes onto a fresh branch and submitted a clean Pull Request in #318. Please review that one instead. Closing this now. |
4 participants
Hi @init27, as discussed in issue #200, here is the PR for the "Grid World" environment. I have built the environment and written a detailed README.md inside the grid_world folder to serve as a "how-to" guide for the community, as you suggested. Please let me know what you think!