fix: alias fallback when manifest arms lack side field (silent bimanual cal sync no-op) - #64
Open
Shiki42 wants to merge 1 commit into
Open
Conversation
refresh_bimanual_cal_dirs() called _pair_arms_by_side -> _validate_bimanual_arm_sides,
which strictly required every arm in a 2-arm role to carry side ∈ {"left","right"}.
On real machines whose manifest.json was written by older code paths the side field
is missing entirely; the validator raised, the outer try/except Exception in
refresh_bimanual_cal_dirs swallowed it, and bimanual_{left,right}.json under
calibration/bimanual_{followers,leaders}/ silently stopped refreshing after every
recalibration. Single-arm mode (which reads <serial>.json directly) kept working,
so bimanual mode silently loaded stale calibration — observed symptom on host b
was the right-side shoulder_pan visibly offset in bimanual mode while single-arm
mode was accurate.
Fix:
- _pair_arms_by_side: if all sides are empty/missing, fall back to alias keyword
inference (the same heuristic command/helpers.py:group_arms already uses).
When any side IS set, keep validating strictly via _validate_bimanual_arm_sides
so malformed manifests (mixed case, duplicated sides) still surface as errors
instead of being masked by the alias path.
- _pair_arms_by_alias: defensively coerce alias to "" so {"alias": None} produces
a controlled ValueError, not a TypeError.
- refresh_bimanual_cal_dirs: drop the try/except Exception wrapper. With the
alias fallback in place, remaining errors are real bugs and must propagate.
- _validate_bimanual_arm_sides is unchanged — _validate_arms still uses it for
strict schema enforcement on new manifests.
Tests in tests/test_manifest_bimanual_sync.py cover: explicit-side pairing,
alias fallback (the exact reversed-array swap scenario from
docs/bimanual-left-right-swap-bug.md), malformed-side rejection, alias=None
handled as ValueError, and end-to-end refresh that overwrites a pre-seeded
stale alias file under a no-side manifest.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
refresh_bimanual_cal_dirssilently no-op'd when manifest arms lacked thesidefield —_pair_arms_by_sideraised, the outertry/except Exceptionswallowed it, andcalibration/bimanual_{followers,leaders}/bimanual_{left,right}.jsonstopped refreshing after every recalibration. Bimanual mode then loaded stale per-arm calibration while single-arm mode (which reads<serial>.jsondirectly) stayed correct._pair_arms_by_sidenow falls back to alias-keyword inference when all sides are empty/missing (the exact heuristiccommand/helpers.py:group_armsalready uses). Malformed sides (mixed case, duplicated values) still surface via_validate_bimanual_arm_sidesinstead of being masked.refresh_bimanual_cal_dirsno longer wraps work intry/except Exception: logger.warning(...). With the fallback in place, remaining errors are real bugs and must propagate.Observed symptom
On host b:
manifest.jsonhad nosidefield on any of its 4 arms (legacy state). After re-calibrating the left follower,bimanual_followers/bimanual_left.jsonandbimanual_followers/bimanual_right.jsonended up with identical content (md5821d96…), both pointing at the left follower's per-serial source — the right follower's5B61037326.jsonnever reached the alias. Right-side shoulder_pan was grossly offset in bimanual mode while single-arm mode on either side alone was accurate. Root cause matches the description indocs/bimanual-left-right-swap-bug.md, but specifically the no-side branch that PR #32's strict-side fix didn't handle.Test plan
pytest tests/test_manifest_bimanual_sync.py -xvs(6 passed)[right_follower, left_follower]with noside(the regression scenario)ValueErrorLeft, duplicatedleft) raiseValueErrorinstead of silently falling backalias=NoneraisesValueErrorinstead ofTypeErrorrefresh_bimanual_cal_dirswith a pre-seeded stale alias file proves overwrite, not just creationbimanual_*md5s match the per-serial sources of both arms.🤖 Generated with Claude Code