Conversation
meta-cla
bot
added
the
CLA Signed
Greptile SummaryThis PR adds a Findings: The canonical template has 3 mechanical issues that should be fixed before merge:
Additionally, there is 1 alignment point for human review: the template sets Confidence Score: 1/5
Sequence DiagramsequenceDiagram
participant User
participant Skill as /generate-openenv-env (SKILL.md)
participant Template as assets/openenv_env_template/
participant Refs as references/
participant Repo as envs/<name>_env/
User->>Skill: "generate an env for textarena"
Skill->>Refs: Read openenv-tutorial-01-environments.md (Part 10)
Skill->>Refs: Read openenv-docs-environment-builder.md
Skill->>Refs: Read env-generation-checklist.md
Skill->>Skill: Research target library API
Skill->>Skill: Select archetype (typed / MCP / specialized)
Skill->>Skill: openenv init <name>_env --output-dir envs
note over Skill: Falls back to manual scaffold if CLI unavailable
Skill->>Template: Copy files as reference
Template-->>Repo: models.py, client.py, server/, openenv.yaml
Skill->>Repo: Implement reset(), step(), state()
Skill->>Repo: Implement _step_payload(), _parse_result()
Skill->>Repo: openenv build && openenv validate --verbose
Repo-->>User: Scaffold + validation report
Last reviewed commit: 3e2fddf |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def reset(self) -> __ENV_CLASS_NAME__Observation: | ||
| """ | ||
| Reset the environment. | ||
|
|
||
| Returns: | ||
| __ENV_CLASS_NAME__Observation with a ready message | ||
| """ | ||
| self._state = State(episode_id=str(uuid4()), step_count=0) | ||
| self._reset_count += 1 | ||
|
|
||
| return __ENV_CLASS_NAME__Observation( | ||
| echoed_message="__ENV_TITLE_NAME__ environment ready!", | ||
| message_length=0, | ||
| done=False, | ||
| reward=0.0, | ||
| ) |
There was a problem hiding this comment.
Template reset() violates Gymnasium API invariant
INVARIANTS.md specifies the required signature as reset(seed?, episode_id?) -> Observation. The template's reset(self) has no parameters, so every environment scaffolded from this template will be incompatible with training orchestration code that passes seed= or episode_id= (e.g., the EnvPool helpers). This is the canonical scaffold, so the problem will propagate to all generated environments.
SKILL.md (§7) also explicitly states: "Prefer reset/step signatures compatible with Environment: reset(seed=None, episode_id=None, **kwargs)".
| def reset(self) -> __ENV_CLASS_NAME__Observation: | |
| """ | |
| Reset the environment. | |
| Returns: | |
| __ENV_CLASS_NAME__Observation with a ready message | |
| """ | |
| self._state = State(episode_id=str(uuid4()), step_count=0) | |
| self._reset_count += 1 | |
| return __ENV_CLASS_NAME__Observation( | |
| echoed_message="__ENV_TITLE_NAME__ environment ready!", | |
| message_length=0, | |
| done=False, | |
| reward=0.0, | |
| ) | |
| def reset(self, seed=None, episode_id=None, **kwargs) -> __ENV_CLASS_NAME__Observation: |
Rule Used: .claude/docs/INVARIANTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/generate-openenv-env/assets/openenv_env_template/server/__ENV_NAME___environment.py
Line: 53-68
Comment:
**Template reset() violates Gymnasium API invariant**
`INVARIANTS.md` specifies the required signature as `reset(seed?, episode_id?) -> Observation`. The template's `reset(self)` has no parameters, so every environment scaffolded from this template will be incompatible with training orchestration code that passes `seed=` or `episode_id=` (e.g., the `EnvPool` helpers). This is the canonical scaffold, so the problem will propagate to all generated environments.
`SKILL.md` (§7) also explicitly states: "Prefer reset/step signatures compatible with `Environment`: `reset(seed=None, episode_id=None, **kwargs)`".
```suggestion
def reset(self, seed=None, episode_id=None, **kwargs) -> __ENV_CLASS_NAME__Observation:
```
**Rule Used:** .claude/docs/INVARIANTS.md ([source](https://app.greptile.com/review/custom-context?memory=dbd1ab5e-bd4d-4701-9de0-9817404155a9))
How can I resolve this? If you propose a fix, please make it concise.| # Enable concurrent WebSocket sessions. | ||
| # Set to True if your environment isolates state between instances. | ||
| # When True, multiple WebSocket clients can connect simultaneously, each | ||
| # getting their own environment instance (when using factory mode in app.py). | ||
| SUPPORTS_CONCURRENT_SESSIONS: bool = True |
There was a problem hiding this comment.
ALIGNMENT FLAG: SUPPORTS_CONCURRENT_SESSIONS inconsistent with max_concurrent_envs
The template sets SUPPORTS_CONCURRENT_SESSIONS: bool = True, but server/app.py uses max_concurrent_envs=1. SKILL.md (line 119) explicitly states: "Set SUPPORTS_CONCURRENT_SESSIONS=True only when isolation is real. Set max_concurrent_envs in create_app accordingly (1 when False, >1 when True)". Having both set in contradictory directions signals opposing concurrency intent.
- Principle at stake: "One canonical way to build environments" (PRINCIPLES.md) / be hands-on
- The concern: Agents copying this template will carry forward a contradiction—they'll see
SUPPORTS_CONCURRENT_SESSIONS=Truebut be unable to run multiple sessions - Suggested reviewer: @Darktex
Align by choosing one:
Option A: Set SUPPORTS_CONCURRENT_SESSIONS = False (matches max_concurrent_envs=1)
Option B: Change app.py to use max_concurrent_envs=4 (matches True)
For a generic echo-style template, Option A (False/1) is the safer default.
| # Enable concurrent WebSocket sessions. | |
| # Set to True if your environment isolates state between instances. | |
| # When True, multiple WebSocket clients can connect simultaneously, each | |
| # getting their own environment instance (when using factory mode in app.py). | |
| SUPPORTS_CONCURRENT_SESSIONS: bool = True | |
| # Set to True only when your environment isolates state between instances | |
| # and you set max_concurrent_envs > 1 in server/app.py. | |
| SUPPORTS_CONCURRENT_SESSIONS: bool = False |
Rule Used: .claude/docs/PRINCIPLES.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/generate-openenv-env/assets/openenv_env_template/server/__ENV_NAME___environment.py
Line: 42-46
Comment:
**ALIGNMENT FLAG: SUPPORTS_CONCURRENT_SESSIONS inconsistent with max_concurrent_envs**
The template sets `SUPPORTS_CONCURRENT_SESSIONS: bool = True`, but `server/app.py` uses `max_concurrent_envs=1`. `SKILL.md` (line 119) explicitly states: "Set `SUPPORTS_CONCURRENT_SESSIONS=True` only when isolation is real. Set `max_concurrent_envs` in `create_app` accordingly (1 when `False`, >1 when `True`)". Having both set in contradictory directions signals opposing concurrency intent.
- **Principle at stake**: "One canonical way to build environments" (PRINCIPLES.md) / be hands-on
- **The concern**: Agents copying this template will carry forward a contradiction—they'll see `SUPPORTS_CONCURRENT_SESSIONS=True` but be unable to run multiple sessions
- **Suggested reviewer**: @darktex
Align by choosing one:
Option A: Set `SUPPORTS_CONCURRENT_SESSIONS = False` (matches `max_concurrent_envs=1`)
Option B: Change `app.py` to use `max_concurrent_envs=4` (matches `True`)
For a generic echo-style template, Option A (False/1) is the safer default.
```suggestion
# Set to True only when your environment isolates state between instances
# and you set max_concurrent_envs > 1 in server/app.py.
SUPPORTS_CONCURRENT_SESSIONS: bool = False
```
**Rule Used:** .claude/docs/PRINCIPLES.md ([source](https://app.greptile.com/review/custom-context?memory=67b96369-b31e-4918-84c9-a0a4e3c8aa97))
How can I resolve this? If you propose a fix, please make it concise.| try: | ||
| from openenv.core.env_server.http_server import create_app | ||
| except Exception as e: # pragma: no cover | ||
| raise ImportError( | ||
| "openenv is required for the web interface. Install dependencies with '\n uv sync\n'" | ||
| ) from e |
There was a problem hiding this comment.
Overly broad exception catch
except Exception as e will swallow errors that are not import failures (e.g., AttributeError, SyntaxError in the openenv module itself), masking the real error and making debugging much harder. Since the intent is only to catch missing-dependency scenarios, this should be except ImportError.
| try: | |
| from openenv.core.env_server.http_server import create_app | |
| except Exception as e: # pragma: no cover | |
| raise ImportError( | |
| "openenv is required for the web interface. Install dependencies with '\n uv sync\n'" | |
| ) from e | |
| try: | |
| from openenv.core.env_server.http_server import create_app | |
| except ImportError as e: # pragma: no cover | |
| raise ImportError( | |
| "openenv is required for the web interface. Install dependencies with '\n uv sync\n'" | |
| ) from e |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/generate-openenv-env/assets/openenv_env_template/server/app.py
Line: 31-36
Comment:
**Overly broad exception catch**
`except Exception as e` will swallow errors that are not import failures (e.g., `AttributeError`, `SyntaxError` in the openenv module itself), masking the real error and making debugging much harder. Since the intent is only to catch missing-dependency scenarios, this should be `except ImportError`.
```suggestion
try:
from openenv.core.env_server.http_server import create_app
except ImportError as e: # pragma: no cover
raise ImportError(
"openenv is required for the web interface. Install dependencies with '\n uv sync\n'"
) from e
```
How can I resolve this? If you propose a fix, please make it concise.| openenv[core]>=0.2.0 | ||
| fastapi>=0.115.0 | ||
| uvicorn>=0.24.0 |
There was a problem hiding this comment.
Package name and version mismatch with pyproject.toml
requirements.txt specifies openenv[core]>=0.2.0, but pyproject.toml specifies openenv-core[core]>=0.2.1. These are two different package names with different minimum versions. Since the Dockerfile uses uv sync (which reads pyproject.toml), the requirements.txt is most likely to be used by developers or pip install -r — creating silent environment differences and confusion.
The package name should be aligned. Based on pyproject.toml and the SKILL.md docs (which reference openenv-core[core]), the correct name is openenv-core:
| openenv[core]>=0.2.0 | |
| fastapi>=0.115.0 | |
| uvicorn>=0.24.0 | |
| openenv-core[core]>=0.2.1 | |
| fastapi>=0.115.0 | |
| uvicorn>=0.24.0 |
Note: pyproject.toml also lacks a newline at end of file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/generate-openenv-env/assets/openenv_env_template/server/requirements.txt
Line: 1-3
Comment:
**Package name and version mismatch with pyproject.toml**
`requirements.txt` specifies `openenv[core]>=0.2.0`, but `pyproject.toml` specifies `openenv-core[core]>=0.2.1`. These are two different package names with different minimum versions. Since the Dockerfile uses `uv sync` (which reads `pyproject.toml`), the `requirements.txt` is most likely to be used by developers or `pip install -r` — creating silent environment differences and confusion.
The package name should be aligned. Based on `pyproject.toml` and the SKILL.md docs (which reference `openenv-core[core]`), the correct name is `openenv-core`:
```suggestion
openenv-core[core]>=0.2.1
fastapi>=0.115.0
uvicorn>=0.24.0
```
**Note**: `pyproject.toml` also lacks a newline at end of file.
How can I resolve this? If you propose a fix, please make it concise.1 participant
This PR adds a skill to help agents generate openenv compatible envs.