Clarify static apple demo reset timing

[xyao-nv]

Summary

Doc only. Clarify static apple demo reset timing.

Detailed description

• Tell operators to wait for success auto-freeze before resetting to avoid incomplete recordings.
• Correct policy eval cmds

[isaaclab-review-bot]

📋 Documentation Review

This PR improves the static apple demo documentation by clarifying the reset timing during teleoperation and updating the evaluation commands to use the correct policy type.

✅ Changes Look Good

Teleoperation Clarifications (step_2_teleoperation.rst):

• The added guidance about waiting for the simulation to freeze before pressing Reset is helpful. This should prevent users from inadvertently saving incomplete demonstrations.
• The new "Wait for success freeze" bullet point reinforces this important workflow step.

Evaluation Updates (step_4_evaluation.rst):

• The policy type change from ActionChunkingClientSidePolicy to Gr00tRemoteClosedloopPolicy with the added YAML config path appears to be a necessary correction.
• Removing the outdated notes about ActionChunkingClientSidePolicy vs Gr00tRemoteClosedloopPolicy reduces confusion.
• The timeout update (30s → 6s) and episode count adjustment (4 → 5) align the documentation with current behavior.

💭 Minor Observations

1. Timeout Change (30s → 6s): This is a significant reduction. If this reflects actual system behavior, great. If users might still need the longer timeout in certain scenarios, consider mentioning this is configurable or noting when longer timeouts might be needed.

2. Removed Troubleshooting Content: The removed ModuleNotFoundError troubleshooting and action shape mismatch guidance was quite detailed. If these issues can still occur with the new policy type, users might benefit from equivalent guidance. If these issues are now obsolete with the updated approach, the removal is appropriate.

3. Empty Line at L129: There appears to be a double blank line after the policy runner command block where content was removed. This is a minor RST style nit.

✔️ Summary

This is a focused documentation improvement that should help users avoid a common pitfall (resetting too early during recording) and provides accurate policy evaluation commands. The changes are clear and well-scoped.

---

Automated review by Isaac Lab Review Bot

[greptile-apps]

Greptile Summary

This is a documentation-only PR that clarifies the static apple demo workflow in two areas: reset timing during teleoperation data collection, and corrected policy evaluation commands.

• step_2_teleoperation.rst: Replaces the ambiguous "reset between demos" instruction with an explicit directive to wait for auto-termination after a successful placement, and adds a dedicated "Wait for reset automatically after success" best-practice bullet to prevent incomplete recordings.
• step_4_evaluation.rst: Switches both single-env and parallel eval commands to Gr00tRemoteClosedloopPolicy (previously the parallel command used ActionChunkingClientSidePolicy), adds --policy_config_yaml_path, corrects the episode-timeout description from 30 s to 6 s, and updates the expected num_episodes from 4 to 5. The notes explaining the architectural distinction between the two client classes and the action-horizon mismatch troubleshooting tip are removed.

Confidence Score: 4/5

Safe to merge once the parallel-eval client-class change is confirmed against the implementation.

The teleoperation clarifications are straightforward and correct. The evaluation file removes the explicit note that Gr00tRemoteClosedloopPolicy is single-env only and now uses it for --num_envs 5; if the implementation was not updated in tandem, operators following the parallel eval command will get incorrect or broken behavior with no in-doc warning.

docs/pages/example_workflows/static_apple/step_4_evaluation.rst — the parallel eval client-class change warrants a quick cross-check against the current Gr00tRemoteClosedloopPolicy implementation to confirm it handles vectorized environments.

Important Files Changed

Filename	Overview
docs/pages/example_workflows/static_apple/step_2_teleoperation.rst	Clarifies reset timing: replaces "reset between demos" with explicit instruction to wait for auto-termination; adds "Wait for reset automatically after success" best-practice bullet.
docs/pages/example_workflows/static_apple/step_4_evaluation.rst	Updates parallel eval command to use Gr00tRemoteClosedloopPolicy + YAML config (previously ActionChunkingClientSidePolicy); corrects timeout from 30s to 6s and num_episodes from 4 to 5; removes notes distinguishing the two client classes and the action-horizon troubleshooting tip.

Sequence Diagram

sequenceDiagram
    participant Op as Operator
    participant Sim as Simulation
    participant HDF5 as HDF5 File

    Op->>Sim: Complete task (place apple on plate)
    Note over Sim: Success condition triggered
    Sim-->>Sim: Auto-freeze / num_success_steps recording
    Sim->>HDF5: Save demonstration
    Sim-->>Sim: Auto-reset simulation
    Sim-->>Op: Signal: ready for next demo
    Note over Op: Wait here (new guidance) before starting next demo

Loading

_{Reviews (3): Last reviewed commit: "fix" | Re-trigger Greptile}

[isaaclab-review-bot]

📋 Documentation Review

This PR improves the static apple demo documentation by clarifying reset timing during teleoperation and updating the evaluation commands to use the correct policy type.

✅ Changes Look Good

Teleoperation Clarifications (step_2_teleoperation.rst):

• The added guidance about waiting for the simulation to freeze before pressing Reset is clear and actionable. This should prevent users from inadvertently saving incomplete demonstrations.
• The new "Wait for success freeze" bullet point in the best practices section reinforces this important workflow step.
• The wording clarification ("may record an incomplete or failed demonstration to the dataset file") is precise.

Evaluation Updates (step_4_evaluation.rst):

• The policy type change from ActionChunkingClientSidePolicy to Gr00tRemoteClosedloopPolicy with the added --policy_config_yaml_path flag appears to be a necessary correction for the current workflow.
• Removing the outdated notes about parallel vs single-env policy types reduces confusion now that the same policy works for both.
• The timeout update (30s → 6s) and episode count adjustment (4 → 5) align the documentation with current expected behavior.

💭 Minor Observations

1. Removed Troubleshooting Content: The removed troubleshooting guidance (ModuleNotFoundError, action shape mismatch) was quite detailed. If these issues can still occur, users might benefit from equivalent guidance for the new policy type. If these scenarios are now obsolete with the updated approach, the removal is appropriate.

2. Double Blank Line (L129-130): After the policy runner command block, there is an extra blank line where the removed notes used to be. Minor RST style nit.

✔️ Summary

This is a well-scoped documentation improvement. The reset timing guidance addresses a real user pain point, and the evaluation command updates bring the docs in line with the current workflow.

---

Automated review by Isaac Lab Review Bot