Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a safety-gated rollout harness for ReBot policies, centering around a strict checkpoint handoff contract, fail-closed safety/workspace validation, and operator-controlled live episodes, with offline evaluation and deterministic reporting to support Person 4鈥檚 rollout workflow.
Changes:
- Adds
rebot_operator_kit.rolloutcontract types plus checkpoint/profile validation and a seven-jointSafetyGovernor. - Adds the Person 4 rollout harness in
p3_vlm_orchestrator.policy_rollout(runner, LeRobot checkpoint adapter, workspace guard, keyboard/signal stop, offline evaluator, held-out reporting). - Adds extensive unit tests plus an operator runbook and a convenience launcher script.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| rebot_operator_kit/tests/test_rollout_safety.py | Unit tests for action freshness/limits/step-cap behavior and governor fault latching. |
| rebot_operator_kit/tests/test_rollout_contract.py | Tests for observation immutability and checkpoint bundle validation contract. |
| rebot_operator_kit/rollout/safety.py | Fail-closed seven-joint action validation with shadow clamp vs live reject behavior. |
| rebot_operator_kit/rollout/contracts.py | Shared immutable observation and adapter Protocol boundaries. |
| rebot_operator_kit/rollout/checkpoint.py | Dependency-free checkpoint/profile sidecar validation and contract extraction. |
| rebot_operator_kit/rollout/init.py | Package docstring for rollout interfaces. |
| p3_vlm_orchestrator/tests/test_workspace_guard.py | Tests for calibrated polygon + Z-prism workspace validation and SDK FK laziness. |
| p3_vlm_orchestrator/tests/test_rebot_policy_robot.py | Tests for authenticated hardware binding, calibration/driver fingerprinting, and safe send/observe behavior. |
| p3_vlm_orchestrator/tests/test_policy_rollout_runner.py | End-to-end tests for runner sequencing, durability/audit, stop handling, and fail-closed faulting. |
| p3_vlm_orchestrator/tests/test_policy_evaluation.py | Tests for held-out manifest/audit validation, deterministic reporting, and comparison ranking. |
| p3_vlm_orchestrator/tests/test_lerobot_policy.py | Tests for LeRobot checkpoint loading, processor restoration, prediction validation, and lazy import boundaries. |
| p3_vlm_orchestrator/tests/test_keyboard_stop.py | Tests for keyboard/signal stop handling and idempotent cleanup. |
| p3_vlm_orchestrator/tests/test_episode_control.py | Tests for bounded operator-confirmed episodes (30s/300 action caps, retry semantics, stop precedence). |
| p3_vlm_orchestrator/tests/init.py | Package docstring for Person 4 rollout tests. |
| p3_vlm_orchestrator/policy_rollout/workspace_guard.py | Workspace guard loading/authentication and FK-based prism validation. |
| p3_vlm_orchestrator/policy_rollout/runner.py | Core rollout loop with audit logging, safety checks, optional workspace guard, and episode controls. |
| p3_vlm_orchestrator/policy_rollout/offline.py | Offline evaluation over finalized datasets with strict sample validation. |
| p3_vlm_orchestrator/policy_rollout/lerobot_policy.py | Lazy LeRobot adapter loading saved processors + strict postprocessing validation. |
| p3_vlm_orchestrator/policy_rollout/keyboard_stop.py | Terminal-reader + SIGINT/SIGTERM stop source with verdict publishing. |
| p3_vlm_orchestrator/policy_rollout/evaluation.py | Held-out manifest + JSONL audit validation, report writing, and checkpoint comparison. |
| p3_vlm_orchestrator/policy_rollout/dummy_policy.py | Deterministic dummy policies for exercising the runner. |
| p3_vlm_orchestrator/policy_rollout/init.py | Public exports for the rollout harness. |
| p3_vlm_orchestrator/PERSON4_RUNBOOK.md | Operator runbook describing gates, episode workflow, and reporting rules. |
| p3_vlm_orchestrator/08_policy_rollout.command | Launcher wiring runtime roots/PYTHONPATH and running the CLI module. |
馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+356
to
+366
| expected_keys = ( | ||
| "observation.images.front", | ||
| "observation.images.side", | ||
| "observation.state", | ||
| "task", | ||
| ) | ||
| if tuple(prepared) != expected_keys: | ||
| raise ValueError( | ||
| "LeRobot prepared observation keys must be exactly " | ||
| f"{expected_keys}; received {tuple(prepared)}" | ||
| ) |
Comment on lines
+324
to
+334
| sdk_root = str(sdk_repo) | ||
| if sdk_root not in sys.path: | ||
| sys.path.insert(0, sdk_root) | ||
|
|
||
| def fk_deg_to_xyz_mm(action_deg: np.ndarray) -> np.ndarray: | ||
| # Pinocchio and the SDK remain lazy until a live validation is requested. | ||
| from reBotArm_control_py.kinematics import joint_to_pose | ||
|
|
||
| physical_six_rad = np.radians(np.asarray(action_deg[:6], dtype=float)) | ||
| position_m, _ = joint_to_pose(physical_six_rad) | ||
| return np.asarray(position_m, dtype=float) * 1000.0 |
Comment on lines
+31
to
+49
| with _PLUGIN_REGISTRATION_LOCK: | ||
| discover = importlib.metadata.distributions | ||
| policy_distributions = tuple( | ||
| distribution | ||
| for distribution in discover() | ||
| if isinstance(distribution.metadata.get("Name"), str) | ||
| and distribution.metadata["Name"].replace("-", "_").startswith( | ||
| "lerobot_policy_" | ||
| ) | ||
| ) | ||
|
|
||
| def discover_policies(*args: Any, **kwargs: Any) -> tuple[Any, ...]: | ||
| return policy_distributions | ||
|
|
||
| importlib.metadata.distributions = discover_policies | ||
| try: | ||
| registrar() | ||
| finally: | ||
| importlib.metadata.distributions = discover |
Comment on lines
+557
to
+565
| finally: | ||
| try: | ||
| self.robot.disconnect() | ||
| except Exception as exc: | ||
| cleanup_fault_reason = ( | ||
| "disconnect failed: " + self._exception_text(exc) | ||
| ) | ||
| terminal_reason = "fault" | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
front/sidecamera orderWhy
Person 4 needs a tested run-loop that can accept Person 3's checkpoint, evaluate it without hardware first, and only permit bounded physical actions after every safety and operator gate passes.
Validation
compileall, launcher syntax/help, lazy-import checks, andgit diff --checkpassKnown hardware/runtime blockers
plane_to_armworkspace calibration is intentionally null, so hardware rollout fails closed