Skip to content

[codex] fix stale echo docs imports#908

Merged
burtenshaw merged 2 commits into
mainfrom
codex/fix-echo-doc-imports
Jul 3, 2026
Merged

[codex] fix stale echo docs imports#908
burtenshaw merged 2 commits into
mainfrom
codex/fix-echo-doc-imports

Conversation

@burtenshaw

Copy link
Copy Markdown
Collaborator

Summary

  • replace stale EchoAction imports in Echo docs with the actual MCP action surface
  • update Echo examples to use CallToolAction and the current observation shape
  • refresh the getting-started Echo snippets to match the exported client API

Why

The lingering docs still showed from echo_env import EchoAction, EchoEnv, but echo_env exports EchoEnv, CallToolAction, and ListToolsAction instead. That keeps issue #379 partially alive even though the original root README and KernRL import paths have already moved forward.

Partially addresses #379.

Validation

  • searched repo docs for stale from echo_env import EchoAction imports after the patch

@burtenshaw burtenshaw marked this pull request as ready for review July 3, 2026 07:02
@burtenshaw burtenshaw merged commit ef05f96 into main Jul 3, 2026
2 checks passed
@burtenshaw burtenshaw deleted the codex/fix-echo-doc-imports branch July 3, 2026 07:03
@burtenshaw burtenshaw added documentation Improvements or additions to documentation size: small Small pull request labels Jul 3, 2026 — with Cursor

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Alignment Review Report

Scope: docs-only PR (docs/source/environments/echo.md, docs/source/getting-started.md, envs/echo_env/README.md).

Overall this is a correct and welcome fix. The old snippets imported EchoAction/EchoObservation, which echo_env no longer exports (it exports only EchoEnv, CallToolAction, ListToolsAction), so they raised ImportError. Migrating to CallToolAction(tool_name=..., arguments=...) matches the real MCP API — I verified the new imports, tool names (echo_message, echo_with_length), and the CallToolObservation type against a live echo_env server. However, the rewritten examples introduce a few factual inaccuracies that I confirmed by running the env end-to-end.

Automated Checks

  • Lint (.claude/hooks/lint.sh): FAIL, but only in ~20 pre-existing envs/ files (e.g. envs/opencode_env/*, envs/chat_env/*) that this PR does not touch. This PR contains no .py changes and introduces zero lint findings — the failure is not caused by this PR.
  • Debug code (.claude/hooks/check-debug.sh): FOUND, but only pre-existing console.print(...) in src/openenv/cli/commands/serve.py and pre-existing TODOs in src/. None are in PR-touched files — not caused by this PR.

Open RFCs Context

  • RFC 003 – MCP Support (In Review, @Darktex, @pankit-eng): directly relevant; the move to CallToolAction + reading observation.result aligns with its direction. (Minor: RFC 003's illustrative code uses parameters= while the implementation and these docs correctly use arguments= — docs match the code, no action needed.)
  • RFC 010 – "ECHO" env-token world modeling (Draft, @thegovind): despite the name, unrelated to echo_env (it is a training-loss technique). No conflict.
  • Other In-Review RFCs (000/001/002/004/005) do not touch this doc surface.

Tier 1: Fixes Required

All verified at runtime against a live echo_env server (default WebSocket client).

  • docs/source/environments/echo.md:83 & envs/echo_env/README.md:95 — "The echo environment returns 0.0 reward for tool calls" is incorrect. CallToolAction steps return reward=None; only reset() returns 0.0. The observation bullet labeling reward as (float) (echo.md:78, README.md:90) is likewise None for tool calls.
  • docs/source/environments/echo.md:32,51, envs/echo_env/README.md:44,63, docs/source/getting-started.md:93result.observation.result is not the echoed string. It is the full FastMCP tool-result envelope: {'content': [{'type': 'text', 'text': 'Hello!', ...}], 'structured_content': {'result': 'Hello!'}, 'data': 'Hello!', 'is_error': False}. So print(result.observation.result) prints that whole dict, not Hello!. Use result.observation.result["data"] (verified → 'Hello!') or the convenience method client.call_tool("echo_message", message="Hello!") (verified → 'Hello!').
  • docs/source/getting-started.md:74 — after the echo_message step, print(result.reward) prints None (per above), which is not an illustrative "getting started" output. Consider printing the echoed value instead.

Tier 2: Alignment Discussion

Principle Conflicts

None identified. Docs-only change: no external reward computation, no client↔server import-boundary crossing, no Gym-API divergence.

RFC Conflicts

None identified. Consistent with RFC 003 (MCP Support).

Optional design note (not a blocker): the reward mismatch can be fixed either in the docs (state None) or, if 0.0 is the intended semantics for no-op tool steps, in the environment (MCPEnvironment._async_handle_call_tool could set reward=0.0). That is a small reward-convention decision under the "Rewards in environment" principle — suggested reviewer: @Darktex (authored the reward principle/invariant), with PR author @burtenshaw.

Summary

  • 3 doc-accuracy issues to fix (reward value, observation.result shape, print(result.reward)None) — all verified at runtime.
  • 0 alignment points for human review (1 optional reward-convention note).
  • 0 RFC conflicts.
  • Both automated-check failures are pre-existing and unrelated to this PR.
Open in Web View Automation 

Sent by Cursor Automation: Pre-review

- "Hi" → reward: 0.2
- "Hello, World!" → reward: 1.3
- Empty message → reward: 0.0
The echo environment returns `0.0` reward for tool calls. It is intended as a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at runtime: CallToolAction steps return reward=None, not 0.0 — only reset() returns 0.0. Please correct this sentence, and the reward (float) bullet on line 78 (it is None for tool calls). If 0.0 is actually the intended semantics, the fix belongs in the environment (MCPEnvironment._async_handle_call_tool), not the docs.

arguments={"message": "Hello!"},
)
)
print(result.observation.result)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

result.observation.result is not the echoed string — it is the full FastMCP tool-result envelope:

{'content': [{'type': 'text', 'text': 'Hello!', ...}], 'structured_content': {'result': 'Hello!'}, 'data': 'Hello!', 'is_error': False}

So this prints the whole dict, not Hello!. Use result.observation.result["data"] (verified → 'Hello!') or client.call_tool("echo_message", message="Hello!") (verified → 'Hello!'). Same applies to line 32's → Echoed: '{result.observation.result}'.

arguments={"message": "Hello, World!"},
)
)
print(result.reward)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After a CallToolAction step, result.reward is None (tool calls do not set a reward; only reset() returns 0.0), so this prints None. Consider printing the tool result instead, e.g. print(result.observation.result["data"]), or switch to client.call_tool(...).

Comment thread envs/echo_env/README.md
- "Hi" → reward: 0.2
- "Hello, World!" → reward: 1.3
- Empty message → reward: 0.0
The echo environment returns `0.0` reward for tool calls. It is intended as a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified at runtime: tool-call steps return reward=None, not 0.0 (only reset() returns 0.0). Please fix this sentence and the reward (float) bullet on line 90. Mirrors the same issue in docs/source/environments/echo.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size: small Small pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant