fix(skills): address code-review findings on the pick/skills-API branch

[theo-michel]

Summary

Fixes for the findings from this review of #542, targeting theo/pick-wrist-servo so they fold into that PR.

• pick_any_object._wrist_descend: explicit check_cancelled() at the top of the descend loop — the fast path (fresh wrist frame already buffered, the common case after each 0.5 s blocking move) never reached self.sleep(), so a Stop rode the descent for up to 60 s.
• pick_any_object._close_twist_lift: the joint-space lift's move_to_joint_positions return is now checked (it returns False instead of raising); a failed/skipped lift falls back to the FK-verified move_checked cartesian lift with the grip kept, instead of letting _grasp_verified back the base up 0.15 m with the EE still at floor_z.
• pick_any_object: loop deadlines moved from time.time() to time.monotonic() (matching brain_client_node's NTP-step rationale); _sweet_box() hoisted out of the 30 ms follow loop.
• Skill.say(wait=True): _wait_for_speech_end now aborts on cancel instead of ignoring a Stop for up to 45 s. Best-effort early return, no raise — say() also runs in post-cancel teardown paths.
• catalog.py: __init__ and reload_all now load physical skills and write the physical_skills/ refs before importing code skills. Previously, on a fresh workspace every from physical_skills import X skill was rostered broken for the whole first pass, self-healing only via the optional watchdog-based watcher.
• initializer.py: _regenerate_physical_refs now includes every non-code roster entry instead of a 4-type allowlist. The allowlist omitted "poses" (a real roster type per SkillInfo.msg), so with a poses skill present the two writers of physical_skills/__init__.py never agreed — each rewrite retriggered the other's full reload forever; the content-compare could never converge.
• arm_control.cpp: throttled warning when the idle gain decay is blocked by a short load read, instead of silently holding stiff gains (behavior unchanged — staying stiff on unknown load is the safe call).

Deliberately not addressed (author's call): the emptied CLAUDE.md (confirmed intentional), the removed test suites / test_backwards_compat.py (deliberate per the PR body), composed children's interface-halt at call end (design question — halting shared interfaces mid-parent-run could stop parent-commanded motion), per-skill storage namespace collisions (changing the path orphans existing stored data — needs a migration decision), the double reload per recorded episode, and the head-camera constants scaling wrist pixels (latent only — both cameras run 640×480).

Validation

• I ran the relevant local validation, or explained why it was skipped: ruff check + ruff format --check clean on the touched Python files, clang-format --dry-run --Werror clean on arm_control.cpp, pyright error count identical to the branch baseline (57, all missing-ROS-deps environment noise). No robot here — the descend-cancel and lift-fallback paths need a hardware pass.
• If I changed simulator behavior, config, or assets, I checked that the simulator starts with ./innate-sim up. — N/A, no simulator changes.
• If I changed simulator asset files or asset references, I published the asset pack and committed the updated sim/assets.lock.json. — N/A.

https://claude.ai/code/session_01QePrpqbeHd4TN8KGB5RTNY

---

Generated by Claude Code

[greptile-apps]

Greptile Summary

Improves cancellation responsiveness and physical-skill loading reliability while adding safer manipulation fallback handling and arm-load diagnostics.

• Regenerates physical-skill references before importing code skills and aligns roster filtering across both reference writers.
• Makes speech and object-picking loops cancellation-aware and uses monotonic deadlines.
• Falls back to a verified Cartesian lift when the joint-space lift reports failure.
• Logs throttled warnings when incomplete load readings prevent idle gain decay.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defects identified.

The revised loading order, cancellation checks, monotonic deadlines, Boolean lift-result handling, and diagnostic logging remain consistent with their surrounding runtime contracts.

Important Files Changed

Filename	Overview
ros2_ws/src/brain/brain_client/brain_client/agents/initializer.py	Aligns physical-reference filtering with the catalog by including every registered non-code skill.
ros2_ws/src/brain/brain_client/brain_client/skills/catalog.py	Generates physical references before code-skill imports during initialization and full reloads.
ros2_ws/src/brain/brain_client/brain_client/skills/types.py	Makes blocking speech waits return promptly after cancellation and uses monotonic deadlines.
ros2_ws/src/mars_bot/mars_arm/mars_arm/arm_control.cpp	Adds a throttled diagnostic when missing load readings prevent safe idle gain decay.
workspace/innate_skills/pick_any_object.py	Improves cancellation and deadline handling and adds a verified fallback when joint-space lifting fails.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Discover physical skills] --> B[Generate physical_skills refs]
  B --> C[Import code skills]
  C --> D[Publish combined skill roster]
  E[Pick object] --> F[Joint-space lift]
  F -->|success| G[Continue grasp verification]
  F -->|failure| H[FK-verified Cartesian lift]
  H --> G

Loading

_{Reviews (1): Last reviewed commit: "fix(skills): address code-review finding..." | Re-trigger Greptile}