IMPROVEMENT_PLAN.md

# 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:
```python
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) |