Repository navigation
Consolidate Open Duck Rust policy runtime - #12
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25d8feb469
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pending = CommandInput::Rejected { | ||
| sequence: rejection.sequence, | ||
| reason: RejectionReason::Invalid, | ||
| }; |
There was a problem hiding this comment.
When a valid target and then a malformed public request arrive within the same 20 ms control period, the bounded drain first stores the target but this assignment replaces it with Rejected; consequently, tick_ingress never admits the valid target. Keep rejection evidence separately from the latest valid target so a noisy or malformed publisher cannot discard another publisher's command and distort requested/admitted/applied evidence.
AGENTS.md reference: AGENTS.md:L35-L36
Useful? React with 👍 / 👎.
| target[i] = (OPEN_DUCK_DEFAULT_POSE[i] + action[i] * OPEN_DUCK_ACTION_SCALE) | ||
| .clamp(self.previous[i] - max_delta, self.previous[i] + max_delta); |
There was a problem hiding this comment.
For repeated finite but very large model actions, this slew clamp advances each actuator by about 0.105 rad per policy tick and can exceed the Python oracle's ±10 rad limit after roughly two seconds. OpenDuckSimPlant accepts every finite value, so the Rust default can continue applying targets that the oracle rejects; apply the same absolute bound before returning the target to preserve the claimed safety and parity behavior.
Useful? React with 👍 / 👎.
| } else { | ||
| rejection_evidence.record_decode_failure(); | ||
| rejection_latched = true; |
There was a problem hiding this comment.
This new decode-failure accounting is not reached for malformed targets received through the supported Zenoh path: open-duck-runtime.rs lines 26-37 drops any payload for which decode_target fails and forwards only valid bytes to this socket. Thus a malformed or non-finite public target still leaves rejection_counts.decode at zero and reports a clean run; the runtime must forward an attributable rejection event or account for it before filtering.
Useful? React with 👍 / 👎.
| if drained == MAX_INGRESS_PER_TICK { | ||
| timing.ingress_drops = timing.ingress_drops.saturating_add(1); |
There was a problem hiding this comment.
Whenever exactly eight datagrams are available, this increments ingress_drops even if the queue is now empty; when more than eight are available, the remainder stays queued for the next tick rather than being dropped. Workloads that reach the drain cap therefore publish fabricated drop counts, making the newly added timing/backpressure evidence unsuitable for experiment comparisons.
Useful? React with 👍 / 👎.
Summary
Verification
scripts/cargo-mujoco test --workspacecargo fmt --all -- --checkThis PR supersedes the prior 22-commit branch history with one five-commit reviewable stack.