fix(embodiments): un-hardcode verify script, finish RTC horizon integer migration - #308
Conversation
…er migration - verify_embodiment_structure.sh hard-coded the expected preset list and has been failing since quadcopter shipped; validate whatever list_presets() returns instead. - emit_embodiment_presets.py and configs/embodiments/ still stored the legacy fractional rtc_execution_horizon; re-running the emitter would regress the migrated integer values in the package presets, and every from-source load emitted a deprecation warning. Sync all copies to the migrated integer counts (franka/ur5: 25, so100: 12). - docs/embodiment_schema.md: fix shipped preset count (4, quadcopter included), canonical preset location, and the rtc_execution_horizon field semantics + cross-field rules to match schema.json/validate.py (ADR 2026-04-25 decision FastCrest#8). No behavior change beyond the verify script now passing. Prep for FastCrest#69 (the Unitree Z1 arm preset will be the 5th preset).
![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: bc3c0b0915
ℹ️ 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".
| presets = list_presets() | ||
| expected = ["franka", "so100", "ur5"] | ||
| if presets != expected: | ||
| print(f" ✗ list_presets() returned {presets}, expected {expected}") | ||
| # Don't hard-code the expected preset list — it already drifted once when | ||
| # quadcopter shipped, and hard-coding it here would break again on the | ||
| # next preset. Validate whatever the package actually ships. | ||
| if not presets: |
There was a problem hiding this comment.
When any single bundled preset is accidentally omitted from src/tether/embodiments/presets/, list_presets() simply returns the surviving files, this nonempty check succeeds, and the loop reports that the package structure is valid. The earlier expected-list comparison caught such omissions; deriving both the subject and expected inventory from the same directory makes this preflight unable to detect incomplete packaging, including a missing quadcopter.json or one of the package copies whose dev copy was checked above.
Useful? React with 👍 / 👎.
| | `frequency_hz` | float | 0–1000 (exclusive 0) | Robot control loop rate. | | ||
| | `chunk_size` | int | 1–200 | Actions in a single inference chunk. | | ||
| | `rtc_execution_horizon` | float | 0–5.0 (exclusive 0) | Seconds of chunk to execute before requesting next inference. | | ||
| | `rtc_execution_horizon` | int | 1–`chunk_size` | Integer count of actions to lock during RTC replan. Legacy fractional values (0 < v < 1) auto-migrate to `int(v × chunk_size)` at load with a one-time deprecation warning; schema v2 will reject them. | |
There was a problem hiding this comment.
For legacy fractions whose product with chunk_size is not integral, this formula gives a different result from the loader: EmbodimentConfig.from_dict() uses int(round(horizon * chunk_size)), whereas the new documentation promises truncation via int(v × chunk_size). For example, v=0.25 and chunk_size=30 is documented as 7 actions but loads as 8, so operators tuning custom legacy configs cannot predict the resulting lock horizon from this schema guide.
Useful? React with 👍 / 👎.
…t migration formula in docs Address Codex review feedback on FastCrest#308: - verify_embodiment_structure.sh validated whatever list_presets() returned, so an accidentally dropped preset file (e.g. quadcopter, which has no configs/ dev copy) passed the preflight silently. Keep a required-minimum set checked as a SUBSET of what ships: omissions fail the check, while new presets still validate automatically without editing the script. - schema.json description and docs/embodiment_schema.md documented the legacy fractional rtc_execution_horizon migration as int(v x chunk_size) (truncation), but EmbodimentConfig.from_dict() rounds: round(v x chunk_size), minimum 1. Document the actual behavior (v=0.25, chunk_size=30 migrates to 8, not 7). No behavior change; the shipped presets' migrated values are unaffected.
|
Both suggestions addressed in 7fd2768:
Verification: |
1 participant
What
Consistency fixes in the embodiment config layer, prepping the ground for the Unitree Z1 arm preset (#69):
scripts/verify_embodiment_structure.shno longer hard-codes the expected preset list. It has been failing since quadcopter shipped (expects['franka', 'so100', 'ur5'], butlist_presets()returns 4). It now validates whatever the package actually ships.rtc_execution_horizonfractional → integer migration (ADR 2026-04-25 decision [Orchestration] Enhance reflex doctor for Optional Dependencies #8) in the copies that were left behind:scripts/emit_embodiment_presets.pyandconfigs/embodiments/{franka,so100,ur5}.json(franka/ur5: 25, so100: 12 — matching the already-migrated in-package presets). Re-running the emitter would previously have regressed the package presets back to fractional values, and every from-source load emitted a deprecation warning.docs/embodiment_schema.md: fix shipped preset count (4, quadcopter included), the canonical preset location (in-package presets dir;configs/embodiments/copies are dev fallbacks), and thertc_execution_horizonfield semantics + cross-field rules to matchschema.json/validate.py.Why
The Z1 preset (#69) will be the 5th preset and needs these gates green: the verify script must pass with a new preset present, and the emitter must be re-runnable without regressing the integer migration.
Testing
bash scripts/verify_embodiment_structure.sh— fails onmain(hard-coded 3 presets), passes here (4/4 presets validated)pytest tests/test_embodiments.py tests/test_guard.py— 90 passedpython scripts/emit_embodiment_presets.py— 3/3 validated; re-emit is content-identical (modulo pre-existingto_dict()key ordering of the optionalgripperblock)ruff check scripts/emit_embodiment_presets.py— no new findings vsmain(I001 etc. are pre-existing)Prep for #69 — no hardware values included in this PR; the Z1 preset itself will follow once the hardware contract is confirmed there.