fix(gym): separate policy and controller action traces

[yuecideng]

Description

This PR makes EmbodiChain action semantics explicit across RL rollouts, expert demonstrations, and LeRobot recording.

Previously, one rollout action field could represent either a raw policy request or an executed controller command. That ambiguity caused stale qpos snapshots, incompatible action widths when policy and joint recorders were combined, and loss of velocity data for position-velocity demonstrations.

The implementation now provides:

• ActionTrace with separate requested, processed, and executed action stages.
• Explicit rollout_action_semantics values for policy-request and expert-controller buffers.
• Expert controller qpos history with compatibility aliases for existing callers.
• Correct preservation of complete [qpos, qvel] position-velocity expert actions.
• Safe inactive-row masking for direct controller demos without an ActionManager.
• Tests for direct controller snapshots, mixed-width recorders, position-velocity encoding, and ActionManager traces.

The existing action contracts and rollout buffer layout remain compatible; the new trace and semantics metadata clarify how each recorded action should be interpreted.

Related: #680

Type of change

• Bug fix (non-breaking change)
• Enhancement (non-breaking change)
• New feature (non-breaking change)
• Breaking change (existing functionality will not work without modification)
• Documentation update

Validation

• Focused EmbodiChain tests: 145 passed under Python 3.11.
• black --check --diff --color ./ passed.
• API documentation coverage: 2293/2293 exports.
• git diff --check passed.

Checklist

• I have run the black . command to format the code base.
• I reviewed affected documentation and agent context; existing guidance remains accurate.
• I have added tests that prove my fix is effective.
• Dependencies have been updated, if applicable.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds action tracing to capture policy requests and controller commands.

The PR does not appear safe to merge because mixed position-velocity recording can save policy requests under an expert-action schema.

Findings

1.  Legacy recorder saves policy requests ▶
2.  Mixed recording remains untested ▶

Fix with agent prompt

### Issue 1
embodichain/lab/gym/envs/managers/datasets.py:515-522
When a policy-contract recorder runs alongside a legacy recorder in a position-velocity expert environment, the shared rollout buffer holds policy requests rather than `[qpos, qvel]` commands. This branch returns those requests for the legacy recorder, so it saves actions with the wrong meaning and potentially the wrong width for its declared expert schema.

### Issue 2
tests/gym/envs/test_demo.py:224-232
These tests stop after preprocessing and masking; they never check what the recorder saves from executed-qpos history. Add a recording test with simultaneous policy and legacy recorders whose action widths differ. Without it, the mixed-contract behavior this change targets can regress unnoticed.

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!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR refreshes direct-controller qpos snapshots, retains controller-qpos history for recorders, adds action traces, and preserves position-velocity rollout actions for legacy recording.

• Mixed policy and legacy recording still needs a distinction between policy requests and expert commands when the expert spec uses position-velocity actions.
• The earlier mixed-recording test concern remains: the added tests check action-list selection, not persistence with both recorder types active.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  P[Policy action] --> B[Shared rollout buffer: policy request]
  P --> M[ActionManager]
  M --> H[Controller qpos history]
  B --> R[Policy-contract recorder]
  B --> L[Legacy recorder with position-velocity spec]
  H -. not selected by new branch .-> L

Loading

_{Reviews (2) · Last reviewed commit: "refactor(gym): unify action traces and c..."}

[greptile-apps]

Mixed recording remains untested These tests stop after preprocessing and masking; they never check what the recorder saves from executed-qpos history. Add a recording test with simultaneous policy and legacy recorders whose action widths differ. Without it, the mixed-contract behavior this change targets can regress unnoticed.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/gym/envs/test_demo.py
Line: 224-232

Comment:
**Mixed recording remains untested** These tests stop after preprocessing and masking; they never check what the recorder saves from executed-qpos history. Add a recording test with simultaneous policy and legacy recorders whose action widths differ. Without it, the mixed-contract behavior this change targets can regress unnoticed.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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!

[greptile-apps]

Legacy recorder saves policy requests When a policy-contract recorder runs alongside a legacy recorder in a position-velocity expert environment, the shared rollout buffer holds policy requests rather than [qpos, qvel] commands. This branch returns those requests for the legacy recorder, so it saves actions with the wrong meaning and potentially the wrong width for its declared expert schema.

Prompt To Fix With AI

This is a comment left during a code review.
Path: embodichain/lab/gym/envs/managers/datasets.py
Line: 515-522

Comment:
**Legacy recorder saves policy requests** When a policy-contract recorder runs alongside a legacy recorder in a position-velocity expert environment, the shared rollout buffer holds policy requests rather than `[qpos, qvel]` commands. This branch returns those requests for the legacy recorder, so it saves actions with the wrong meaning and potentially the wrong width for its declared expert schema.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.