marionette / IMPROVEMENT_PLAN.md
RemiFabre
Add improvement plan and overhaul analysis (V2)
173459a
|
Raw
History Blame Contribute Delete
27.2 kB

Marionette App - Comprehensive Improvement Plan

Purpose: This document is a structured brief for coding agents. It captures every bug, UX issue, feature request, and architectural concern raised during review. Each item includes technical context with file references so agents can work autonomously.

Guiding principle: The app must do the basics flawlessly before adding features. Stability and cross-platform/cross-browser compatibility always win over polish. If there's a choice between a shinier solution that might break on some OS/browser and a simpler one that works everywhere, choose the simpler one.

Tech stack: Vanilla JS + HTML5 + CSS3 frontend, Python/FastAPI backend. No framework. The app runs as a ReachyMiniApp served on port 8042.


Priority Levels

Level Meaning
P0 Critical bugs β€” breaks core functionality, blocks users
P1 UX overhaul β€” the interface is confusing, scares new users
P2 Performance & startup β€” slow loading, blocking startup
P3 Hugging Face integration β€” login, upload workflow
P4 Dataset management β€” recording vs downloaded, selective upload
P5 Future features β€” nice-to-have, implement after stability
P6 Experimental β€” deprioritize, keep or park cleanly

P0 β€” Critical Bugs

P0-1: Progress bar (blue meter) is completely broken during recording

Symptoms: The blue countdown meter that shows remaining recording time behaves erratically. Instead of smoothly counting down from the recording duration to zero, it goes down, then jumps back up, then down again. After the visual reaches zero, recording continues for much longer than expected. The final saved move shows as 0 seconds or 1 second when played back. This was observed on Mac; may also affect other platforms.

Root cause analysis: The timing synchronization between frontend and backend is fragile. The system relies on:

  1. Backend sets countdown_ends_at (Unix timestamp) and recording_started_at + recording_duration β€” see main.py:622-644
  2. Frontend polls /api/state every 1500ms and interpolates locally between polls β€” see main.js:729-821
  3. The transition from countdown→recording is handled client-side when countdown reaches 0 via startLocalRecordingCountdown() at main.js:814-821
  4. But the backend also sends a mode change to "recording" which arrives via the next poll, causing updateTimingState() to reset timingState with potentially different values

The likely bug: When the poll arrives with mode="recording", modeChanged is true, so remainingStart is set to Math.max(total, 0.01) (line 756) β€” this resets the bar to full, causing the "jump back up" effect. Then on subsequent polls, modeChanged is false, so serverRemaining is used, which may desync with the local animation. The localRecordingActive flag (line 791) attempts to handle this but doesn't fully prevent the race condition.

Files to fix:

  • marionette/static/main.js lines 729-821 (timing state management)
  • marionette/main.py lines 614-700 (state transitions during recording)

Requirements for the fix:

  • The countdown bar must smoothly go from 100% to 0% during the preparation period (3s)
  • The recording bar must smoothly go from 100% to 0% during the recording duration
  • These must be visually distinct (different color, different label, different style)
  • No jumps, no resets, no going backwards
  • Must work with 1500ms polling interval and possible network jitter
  • Must work identically across Chrome, Firefox, Safari on Linux, Mac, Windows
  • When recording ends, it must actually stop β€” no ghost recording continuing

Suggested approach: Consider having the backend send absolute timestamps for both start and end of recording, and having the frontend compute progress purely from (now - start) / (end - start). Eliminate the dual local/server timing state. One source of truth.


P0-2: Audio upload fails when duration is not a multiple of 0.5 seconds

Symptoms: When uploading an audio file whose duration is not a clean multiple of 0.5 seconds, the upload reportedly fails and the file is not marked as successfully uploaded. Tested with a 201.7s file (failed) vs a 12.5s file (succeeded). Note: the tester interpretation may be partially wrong β€” it could also be a file-size issue with very long files, or a separate rounding bug.

Root cause analysis:

  • The HTML <input type="number" step="0.5"> at index.html:30 constrains the duration spinner to 0.5s increments
  • After upload, the JS sets duration: durationInput.value = audioDuration.toFixed(1) β€” but if the HTML validity check rejects a value that's not a multiple of 0.5 (like 5.2), the form may fail validation
  • The backend accepts any float gt=0.5, le=300.0 via Pydantic (main.py:139), so the constraint is purely frontend
  • For very long files (>300s), the backend's le=300.0 constraint would reject the duration

Files to fix:

  • marionette/static/index.html line 30: change step="0.5" to step="0.1" or step="any"
  • marionette/static/main.js: ensure uploaded audio duration is set correctly without rounding issues
  • marionette/main.py line 139: consider raising le=300.0 limit or auto-truncating to max

Requirements:

  • Audio upload must work for any file duration (up to a reasonable max, e.g., 600s)
  • Duration field must accept arbitrary precision (at least 0.1s granularity)
  • The auto-filled duration after upload must exactly match the audio file duration
  • Remove the up/down spinner arrows on the duration field (see P1-5)

P0-3: Recording continues past the specified duration

Symptoms: User starts a 5-second recording. The stop button remains active and audio keeps being captured well past 10 seconds. When the move is finally saved, playing it back shows it as 0 or 1 second. This was observed on Mac.

Root cause analysis: This is likely related to P0-1 (the timing state bug). The backend recording loop at main.py:901+ runs _capture_motion() which loops until time.perf_counter() - start >= duration. If the frontend thinks recording hasn't finished (due to the timing desync), the "Stop Recording" button stays visible, but the actual recording may have already stopped server-side. The disconnect creates confusion.

Alternatively, there could be a race condition where the recording thread doesn't properly terminate β€” check the _recording_cancel_event and the stop_event.wait() logic in the capture loop.

Files to investigate:

  • marionette/main.py β€” _capture_motion() method (line 901+), check the loop termination condition
  • marionette/static/main.js β€” how the frontend handles the transition from recordingβ†’idle

Requirements:

  • Recording must stop precisely at the specified duration (within ~100ms tolerance)
  • Frontend must immediately reflect that recording has ended
  • The saved move duration must match what was recorded
  • Cross-platform, cross-browser

P0-4: Movement replay amplitude much smaller than recorded

Symptoms: One user recorded movements by physically moving the robot head. On playback, the robot moved in the correct directions and for the correct duration, but the amplitude of movements was dramatically smaller than what was recorded.

Root cause analysis: Multiple possible causes:

  1. PID controller lag: The robot's internal P-controller cannot track fast movements, so target positions are reached with significant undershoot. This is the most likely explanation.
  2. Modified PID parameters: The user may have changed motor PID gains to lower values, causing weaker tracking.
  3. Interpolation issue: The RecordedMove.evaluate(t) method interpolates between recorded poses. If the playback sampling doesn't align well with the recorded timestamps, peaks could be smoothed out.
  4. Motion model applied accidentally: If the moving_average motion model was enabled, it would smooth/reduce movement amplitude β€” check motion_models.py.

Files to investigate:

  • marionette/main.py lines 706+ (_perform_playback, _stream_playback)
  • marionette/motion_models.py β€” all models, especially moving_average
  • Check if the experimental motion models feature is on by default somewhere

Action items:

  • Verify that no_model is the default and that no model is accidentally applied
  • Add a diagnostic mode or log that compares recorded target positions vs. actual positions during playback
  • Consider adding a warning in the UI about PID tracking limitations
  • Contact the affected user and ask if they changed PID parameters

P1 β€” UX Overhaul

P1-1: Redesign the recording section for clarity

Current problems:

  • The subtitle "Manually guide Reachy Mini, capture the motion, build a dataset" uses jargon ("dataset") that non-technical users don't understand
  • All options (duration, label, description, audio source) are shown upfront, overwhelming new users
  • The first user who tried it was confused by the countdown bar β€” they thought the 3-second preparation countdown was the actual recording and tried to move super fast
  • The "Idle" pill in the top right is meaningless to users

Requirements:

  • Landing state: show a big, clear explanation of what the app does in simple terms. Something like: "Move Reachy Mini's head with your hands to create animated movements β€” with sound!"
  • Show only essential controls by default: a name field a duration field and a big "Start Recording" button
  • Audio source, and description should be in a collapsible "Options" or "Settings" section
  • Default duration should be sensible (5s is fine)
  • Default audio source should be robot microphone (already is)
  • Add a clear message near the button: "You'll have 3 seconds to prepare before recording starts"

Countdown/recording visual redesign:

  • The preparation countdown (3s) must be dramatically different from the recording progress
  • Preparation: show big countdown numbers on screen: 3... 2... 1... with large, animated text (or any other robust way)
  • Maybe use the full screen or a modal overlay for the countdown (ony if this is robust)
  • Recording: show a clearly labeled progress bar with "Recording... 4.2s remaining"
  • The two phases must be impossible to confuse

Files to modify:

  • marionette/static/index.html β€” restructure the recording panel
  • marionette/static/style.css β€” new styles for countdown, simplified layout
  • marionette/static/main.js β€” countdown display logic, show/hide advanced options

P1-2: Remove the description field from the recording form

Rationale: No one uses it in practice. The description is still saved in the dataset JSON if present, but we don't need to expose it in the UI. This simplifies the form.

Files: marionette/static/index.html lines 40-48 (remove the textarea), main.js (remove references to descriptionInput in form handling)


P1-3: Rename "Move label (optional)" to just "Name"

Current: <label for="label">Move label (optional)</label> with placeholder "e.g. gentle-nod"

Change to: <label for="label">Name</label> with placeholder "e.g. happy-dance"

Files: marionette/static/index.html line 38


P1-4: Move audio source selection to settings/options

Current state: Audio source (mic / upload / none) is shown in the main recording form, which confuses first-time users who don't understand what it means.

Desired behavior: By default, audio records from the robot's mic. The audio source option should be in the collapsible settings section. Users discover it naturally after their first recording.

Files: marionette/static/index.html lines 51-73


P1-5: Remove up/down spinner arrows from duration field

Current: <input type="number" step="0.5"> shows browser-default increment/decrement arrows.

Options:

  • Use type="text" inputmode="decimal" with manual validation
  • Or use CSS to hide the spinners: input[type=number]::-webkit-inner-spin-button { display: none; }
  • Just let users type the duration they want

Files: marionette/static/index.html line 26-34, marionette/static/style.css


P1-6: Add a settings/gear icon for advanced options

Concept: A gear icon (βš™) somewhere accessible (header? near the record button?) that opens a settings panel containing:

  • Duration setting
  • Audio source selection (mic / upload / none)
  • Startup voice message selection (see P2-3)
  • Dataset root folder
  • Hugging Face login status
  • Experimental features

This replaces the current "Configuration" collapsible section and absorbs some options currently in the main form.

Files: New CSS/HTML structure needed. The current <details id="config-details"> section at index.html:164-242 should be absorbed into this.


P1-7: Simplify the community datasets display

Current problems: When clicking "Fetch community datasets", the list shows a wall of text per dataset: tags, descriptions, all metadata. It's overwhelming and ugly.

Desired display per dataset:

  • Username / dataset name
  • Number of likes, number of downloads
  • Date
  • Number of moves and total duration (if available from metadata)
  • Optional: expandable toggle to see the full list of files

Files: marionette/static/main.js β€” the renderCommunityDatasets() function (or equivalent), style.css


P1-8: Rename recording to just "Name" behavior

Current behavior: Users must enter a "Move label" which gets slugified. If empty, the system generates take-YYYYMMDD-HHMMSS.

Feature request: Allow renaming moves after recording. Add a small edit/rename button next to each move in the list.

Files: Would need a new API endpoint PATCH /api/moves/{move_id} to rename files, and frontend UI in main.js renderMoves().

Priority note: This is nice-to-have within P1.


P2 β€” Performance & Startup

P2-1: Startup audio is blocking β€” web UI loads very late

Symptoms: When launching Marionette from the dashboard, the robot takes ~10+ seconds before the web page becomes interactive (especially on wireless Reachy Mini with CM4 compute). The user sees nothing happening for a long time.

Root cause: In main.py:526-543, the startup sequence is:

reachy_mini.media.play_sound(str(intro_path))  # sends play command
time.sleep(intro_duration)                       # BLOCKS for full duration

Then it does _goto_sleep_and_release(), then plays a second sound (click_the_gear.wav) with another time.sleep(). The web server (FastAPI) is already running by this point (it's started before run() is called), but the run() method blocks the main event loop, so _mode stays uninitialized and the app can't process requests during this time.

Desired behavior:

  • The startup voice message should play immediately when the app starts β€” give an instant audible cue that the app is launching
  • While the voice plays, the web UI should load and show a "Loading..." or "Starting up..." state ASAP
  • The voice should NOT block the web server from serving the page
  • By the time the voice message ends, the UI should be fully interactive

Suggested approach:

  • Make play_sound() non-blocking (it may already be β€” it sends a command to the robot). Remove the time.sleep(intro_duration) or replace it with a non-blocking wait that still allows the FastAPI server to handle requests
  • Or: run the startup animation sequence in a background thread so the web server can serve pages immediately
  • Add a frontend state for "starting_up" that shows a nice loading indicator

Files:

  • marionette/main.py lines 514-611 (the run() method)

P2-2: Make startup voice non-blocking or parallel with page load

This is a refinement of P2-1. The key insight: play_sound() sends a command to the robot's speakers. The time.sleep() is just to wait before changing motor states. We could:

  1. Start playing the sound
  2. Immediately allow the web UI to load (set mode to "starting_up")
  3. In the background, wait for the sound to finish, then do the motor state transitions
  4. Set mode to "idle" when ready

P2-3: Support multiple startup voice messages with selection

Current state: One hardcoded intro file at marionette/assets/intro_marionette.wav

Desired: 3-4 voice message options:

  1. Full explanation: Describes what the app does (good for first-time users)
  2. Fun/joke version: The current one (inner joke that many people get)
  3. Short greeting: A quick "Marionette is ready!" for experienced users
  4. Silent: No voice at all

The selection should be available in the settings panel (see P1-6) and persisted in dataset_registry.json.

Files:

  • New audio files in marionette/assets/
  • marionette/main.py β€” run() method, registry persistence
  • Frontend settings UI

P3 β€” Hugging Face Integration

P3-1: Auto-detect Hugging Face login status

Current problem: Users must manually run huggingface-cli login in a terminal and then type their username in the app. This is a complete blocker for non-technical users.

Desired behavior:

  • On app load, check if the user is logged into Hugging Face (check ~/.huggingface/token or use huggingface_hub.whoami())
  • Display login status clearly in the UI: "Logged in as username" or "Not logged in"
  • If not logged in, show a button to initiate login

Technical approach: The reachy_mini repo has a login flow implemented in the web dashboard. Investigate how it works and replicate it in Marionette. The huggingface_hub library provides whoami() which returns the username if a valid token exists.

Files:

  • marionette/main.py β€” add a /api/hf/status endpoint
  • marionette/static/main.js β€” display login state, remove manual username input
  • marionette/static/index.html β€” replace the username text field with login status display

P3-2: Remove manual Hugging Face username field

Current: <input id="hf-username" type="text" placeholder="username" /> at index.html:132

Replace with: Auto-detected username from whoami(). If not logged in, show a "Log in to Hugging Face" button instead.

Files: index.html, main.js, main.py

P3-3: In-app Hugging Face login flow

Desired: A button that opens a browser-based OAuth flow or provides a simple token-paste mechanism, similar to what the Reachy Mini dashboard does.

Reference implementation: Look at how the Reachy Mini dashboard handles HF login. The dashboard likely uses the huggingface_hub device flow or token-based login.

Requirements:

  • Needs maximum authorization scope (create repos, push datasets)
  • Should work on all OSes
  • Should be integrated directly in the Marionette app, not require a separate terminal

P4 β€” Dataset Management

P4-1: Separate recording folder from downloaded datasets

Current problem: If you download someone else's dataset and it becomes the "active dataset", then recording a new move writes into that downloaded folder. You get a mix of your moves and someone else's moves. This is very confusing.

Desired behavior:

  • Downloaded datasets should be read-only by default (or at least, new recordings should never accidentally go into a downloaded folder)
  • There should be a clear separation between "my recordings" and "downloaded community moves"
  • The UI should make it obvious which dataset is local vs. downloaded

Suggested approaches (let an agent think deeply about this):

  1. Tag datasets as "local" vs "downloaded" in the registry
  2. Prevent recording into downloaded datasets (show a warning, redirect to a local dataset)
  3. Separate the "play" dataset from the "record" dataset in the UI
  4. Use visual indicators (icons, colors) to distinguish local vs. community datasets

Files:

  • marionette/main.py β€” DatasetEntry dataclass, _load_dataset_registry(), _save_dataset_registry()
  • marionette/static/main.js β€” dataset selector UI
  • marionette/static/index.html β€” dataset display

P4-2: Selective move upload to Hugging Face

Current state: Users select moves via checkboxes and upload selected ones. This works.

Desired improvements:

  • Show which moves have been uploaded (already partially done with "uploaded" badge at main.js:308-313)
  • Make the uploaded/not-uploaded status more visually prominent
  • Consider a "select all" / "select none" toggle
  • After uploading, the synced state should be clearly shown

Files: main.js renderMoves(), style.css


P4-3: Cross-platform default dataset root path

Current defaults (from main.py):

  • Windows: ~/Documents/ReachyMini/datasets
  • macOS: ~/Library/Application Support/ReachyMini/datasets
  • Linux: ~/reachy_mini_datasets

Suggestion: Consider using a hidden folder like ~/.reachy_mini/datasets on Linux (consistent with the convention of dotfiles for app data). Verify that these paths work well on each OS. The current paths seem reasonable but should be tested.

Note: The root folder is configurable in settings, so this is mostly about the default being sensible.


P5 β€” Future Features (implement after stability)

P5-1: YouTube audio extraction

Concept: User pastes a YouTube URL, the app extracts the audio, and it becomes available as an audio source for recording (same as the upload flow, but from a URL).

Requirements:

  • Don't re-extract every time β€” cache the extracted audio
  • Allow multiple recordings with the same extracted audio
  • Legal considerations: ensure this is for personal/educational use

Technical approach: Use yt-dlp library to extract audio. Store in temp directory with a hash of the URL as filename.

Priority: Low β€” implement only after P0-P3 are solid.


P5-2: Move renaming

Concept: Allow renaming a move after recording. Currently the name is set at recording time.

Technical approach: Rename the JSON and WAV files, update the registry.

Priority: Nice-to-have within the P1 UX work.


P6 β€” Experimental Features (Park or Keep)

P6-1: Remove denoise feature

Current state: Works by recording motor noise during a silent playback, then subtracting it from the original audio. Gated behind an experimental toggle. But this doesn't work well enough, let's remove it completely.

Note: In my tests, if the movements recorded are too fast, the motors do a lot of noise and the microphone seems to "stop recording". This is due to a setting on the mic board that tries to automatically scale the mic sensitivity to avoid saturating it. In some cases this config is good, but imo for the Marionette use case, it's best to disable this parameter. To do this one can do: python src/reachy_mini/media/audio_control_utils.py PP_AGCONOFF --values 0 But this is a very manual step, and doing it requires being in the correct virtual env and the procedures is different between reachy mini wireless and lite. Could we find a way to make it possible from Marionette? Otherwise, at minima document how to do this for technical people, for example:

On a wireless unit, ssh to it and:
git clone https://github.com/pollen-robotics/reachy_mini.git
cd reachy_mini
source /venvs/mini_daemon/bin/activate
python src/reachy_mini/media/audio_control_utils.py PP_AGCONOFF --values 0
(on the lite it's the same thing, but with your usual virtual environment)

P6-2: Assessment of motion models

Current state: 4 models (no_model, lead_compensation, moving_average, static_offset) applied during playback. Gated behind experimental toggle.

Assessment needed: Same as denoise β€” evaluate if this adds value. The lead_compensation model (shift 2 frames ahead) could help with the P0-4 amplitude issue. But if it's not well-tested, it's a source of bugs.

Action: Either:

  • Keep it hidden behind experimental toggle and ensure no_model is always the default
  • Or remove from UI entirely and document it as a future feature

Files: marionette/motion_models.py, main.py motion model application in playback


Testing Strategy

Recommended test matrix

Test Linux Mac Windows Firefox Chrome Safari
Page loads and shows UI
Start recording (default settings)
Countdown displays correctly
Recording progress bar is smooth
Recording stops at specified duration
Move appears in list after recording
Play move back
Upload audio file (various durations)
Record with uploaded audio
Upload to Hugging Face
Download community dataset
Settings persist across page reload

Automated testing ideas

  1. Backend unit tests: Test the recording/playback state machine, duration validation, audio upload handling, file naming, registry persistence
  2. Frontend tests: Use a headless browser (Playwright or Puppeteer) to test the countdown timer accuracy, form validation, progress bar behavior
  3. Integration tests: Mock the robot connection and test the full flow: record β†’ save β†’ play β†’ upload
  4. Cross-browser CI: Run Playwright tests against Chrome, Firefox, and WebKit (Safari engine)

Hardware testing checklist

Available hardware for testing:

  • Ubuntu Linux (desktop)
  • Firefox and Chrome browsers
  • Reachy Mini Wireless (CM4 compute)
  • Reachy Mini Lite (laptop compute)

Tests requiring hardware:

  • Startup voice message timing
  • Actual motion recording and playback accuracy
  • Audio recording from robot microphone
  • Motor noise denoise quality

Agent Workflow Recommendations

For bug fixes (P0):

Use Ralph Loop with clear success criteria:

  • P0-1/P0-3: Success = progress bar decreases monotonically during recording, recording stops within 200ms of target duration (testable with Playwright)
  • P0-2: Success = audio upload works for files of duration 5.2s, 12.5s, 201.7s, 0.7s (testable with unit tests)

For UX overhaul (P1):

Use feature-dev skill for the redesign. The agent should:

  1. Analyze the current UI
  2. Propose a new layout
  3. Implement iteratively

For cross-platform stability:

Consider setting up Playwright tests that can run in CI. This gives Ralph Loop measurable exit criteria.


File Reference

File Lines Purpose
marionette/main.py ~1853 Backend: all API routes, recording/playback logic, HF integration
marionette/static/index.html 252 Frontend: HTML structure
marionette/static/main.js ~1350 Frontend: all JS logic, polling, timing, UI updates
marionette/static/style.css ~??? Frontend: all styling
marionette/motion_models.py ~??? Experimental motion compensation models
marionette/assets/ β€” Audio files (intro, click_the_gear)
dataset_registry.json β€” Persistent app state (datasets, preferences, features)