hope-voice-ios/.artifacts/20260831-105824-plan-code-ui-audit/iterations/04-implementation.md

# Iteration 04 Implementation Report

- Task: 20260831-105824-plan-code-ui-audit
- Agent: coder
- Model: litellm-lan/qwen3.8-27b
- Status: complete

## Summary

All ten product corrections and two documentation corrections from `iterations/04-request.md` are implemented and verified. The baseline compile error at `PinnedMediaResourceLoader.swift` (Int?/Int64 mismatch) was fixed as part of item 1. All prior validated changes from iterations 01–03 are preserved.

## Files Changed

### Product Corrections

**1. Complete every item in `iterations/03-request.md`**
- See `iterations/03-implementation.md` for the full account. All six items (symmetric dedupe, mic interruption state, media interruption behavior, HTTP 206 content length, construction-only stream URL, stale comments) are complete and tested.

**2. Centralize terminal VoiceSession failure cleanup**
- **`Sources/LiveKit/VoiceSession.swift`**: Added `teardownForFailure()` method that: cancels `agentWaitTask`, increments `sessionGeneration` (invalidating stale callbacks), clears `micMutedByInterruption`, calls `stateModel.stop()`, restores `isIdleTimerDisabled = false`, ends media scope via `mediaController.endConversation()`, and tears down the room via `teardownRoom()`. Called from:
  - Final agent timeout path (when `reconnectAttempted` is true in `handleAgentTimeout`)
  - Terminal `.disconnected` connection state callback (when state was `.ready` or `.waitingAgent`)
- The retry path (`reconnectAttempted = false`) still uses the lighter `teardownRoom()` + `start()` flow, which increments generation via `start()`.

**3. Enforce text-mode microphone invariant**
- **`Sources/LiveKit/VoiceSession.swift`**: Added guard in `setMuted(false)` to block unmuting when `settings.textMode` is true. Added guard in `enableMicrophone()` to block mic-enable when `settings.textMode` is true. Both log the block for debugging.
- **`Sources/Features/Conversation/ConversationView.swift`**: Added `.disabled(settings.textMode)` to the mic toggle button so it's visually disabled and non-interactive in text mode.

**4. Fix server configuration semantics**
- **`Sources/Features/Conversation/ConversationView.swift`**: Rewrote `makeSession` to sanitize invalid user values to the documented default (`https://192.168.86.2:9178`) and set `settings.serverURL` to the valid fallback. The `onAppear` validation error shows what was wrong with the original value. Updated `.onChange(of: settings.serverURL)` to only trigger session replacement on valid non-empty URLs (using `case .success = ServerConfig.parse(trimmed)`), not every keystroke. Added `guard session.state == .idle else { return }` to defer replacement until idle.
- **`Sources/Features/Media/MediaPanelView.swift`**: Updated `serverConfig` computed property comment to explain the fallback behavior.

**5. Make persisted transcript history visible and Clear truthful**
- **`Sources/Features/Conversation/OrbView.swift`**: Updated `TranscriptListView` to show `visibleRows` = restored persisted finals (from `history.entries`) + current live events (`events`). Restored entries are rendered as final rows with stable IDs. Scroll-to-bottom uses the last visible row's ID.
- **`Sources/Features/Conversation/ConversationView.swift`**: Updated Clear action to call both `session.transcriptHandler.clear()` and `session.transcriptHandler.clearHistory()` so Clear atomically removes both visible rows and persisted history.

**6. Align conversation-state inference with web reference**
- **`Sources/LiveKit/ConversationState.swift`**: Changed the both-silent case from `.thinking` to `.listening`, matching the current web reference at `hope-voice-api/web/app.js:414-426`. Updated doc comment to explain the change. The thinking chip is still available for explicit turn actions but is no longer entered by the audio-level state machine.
- **`Tests/ConversationStateTests.swift`** (new): Five tests covering remote speech ending → listening, user speaking → listening, agent priority, stop → idle, and no stale thinking chip.

**7. Make media-state publishing stale-safe**
- **`Sources/Media/MediaPlayerController.swift`**: Updated `publishMediaState` to capture `targetRoom` (the exact `Room` instance), `generation` (`session.sessionGeneration`), and `pollGen` (`pollGeneration`) before spawning the Task. Inside the Task, validates all three remain current before sending. This prevents a delayed publish from an old lifecycle from escaping into a new conversation room.

**8. Finish resource-loader hardening**
- **`Sources/Media/PinnedMediaResourceLoader.swift`**: Rewrote with:
  - Shared lazy `pinnedSession` (created once, invalidated on deinit)
  - `activeTasks: [Int: URLSessionDataTask]` dictionary for upstream data task retention and cancellation
  - Status validation: rejects non-200/206 responses with `URLError(.badServerResponse)` instead of feeding error JSON into AVPlayer
  - `removeLoader(_ asset: AVURLAsset)` method to bound the `loaderURLs` dictionary
  - Proper cleanup in `didCancel`: cancels the upstream data task and removes it from `activeTasks`
- **`Tests/PinnedMediaResourceLoaderTests.swift`** (new): Focused parser tests for 206/200/malformed Content-Range, plus stream URL construction test.

**9. Support configurable HTTPS behavior safely**
- **`Sources/Networking/TLSPinning.swift`**: Updated `PinnedTrustDelegate.didReceive challenge` to apply the fixed self-signed pin only to `TLSPinning.host` (`192.168.86.2`). Other HTTPS hosts use `.performDefaultHandling` for normal system trust evaluation. This allows public deployments and other LAN hosts with valid certificates while never weakening trust for the default self-signed host.

**10. Fix model picker disconnected copy**
- **`Sources/Features/Settings/SettingsView.swift`**: Changed the model picker condition from `if let session, !session.models.isEmpty` to `if let session, session.isReady, !session.models.isEmpty`. When not ready, shows "Start a conversation to load the model list." instead of the misleading "Loading available models…" text.

### Documentation Corrections

**11. Update PLAN.md and README.md**
- **`PLAN.md`**: Rewrote the "Project status" section to stop claiming all phases/full parity are completely qualified. Now clearly classifies: verified in simulator (historical + current), pending device qualification, pending/blocked items (UI redesign, supported-format media playback, cert pin rotation), and correctness corrections from iterations 01–04.
- **`README.md`**: Added a status blockquote at the top clarifying that core functionality is implemented and verified in simulator to the extent the environment allows, but full device qualification and the Hope visual redesign remain pending.

**12. Update docs/DEVICE_TESTING.md wording**
- **`docs/DEVICE_TESTING.md`**: Updated section 3 (Wake lock) to clarify that `isIdleTimerDisabled` prevents automatic idle lock but cannot prevent a user pressing the side button from locking the device.

## Design Decisions

1. **teardownForFailure vs teardownRoom**: The centralized failure cleanup (`teardownForFailure`) is more thorough than the lighter `teardownRoom` used for retry. It increments generation (invalidating all stale callbacks), stops conversation state, restores the idle timer, and ends the media scope. This ensures no stale room, wake lock, or media polling survives a terminal failure. The retry path still uses the lighter flow because it immediately calls `start()` which increments generation again.

2. **Text-mode invariant at API level**: The mic mute/enable guards in `VoiceSession` enforce the privacy contract at the API level, not just in the view layer. This means even if a future view forgets to disable the toggle, the session will not enable the mic in text mode. The UI-level `.disabled(settings.textMode)` provides additional affordance.

3. **Server config sanitization**: When an invalid server URL is detected, the app sanitizes it to the documented default rather than silently connecting to the wrong host. The visible validation error tells the user their original value was rejected. This prevents the "silent fallback" behavior that could mask a misconfigured deployment.

4. **Visible history + live events**: The transcript list now shows restored persisted finals followed by current live events. Restored entries have stable IDs based on timestamp+role, so they don't conflict with live event IDs. The Clear button atomically removes both, making it truthful.

5. **Conversation state: listening not thinking**: The web reference at `app.js:414-426` returns to "listening" when both audio levels are silent, not "thinking". The iOS implementation now matches this. The thinking chip is still available for explicit turn actions (e.g., LLM processing) but is no longer entered by the audio-level state machine, which previously left the UI apparently frozen after an agent response.

6. **Stale-safe media publishing**: Capturing the exact `Room` instance, session generation, and poll generation before spawning the Task ensures that a delayed publish from an old lifecycle cannot escape into a new conversation room. This is critical because the LiveKit Room instance changes on every reconnect, and the session generation increments on every connect/disconnect.

7. **Resource loader hardening**: The shared pinned session avoids creating a new URLSession for each loading request (which would be expensive and could exhaust file descriptors). The `activeTasks` dictionary allows cancellation of in-flight downloads when AVPlayer cancels a request (e.g., during a seek or item swap). Status validation prevents error JSON from being fed into AVPlayer as media bytes.

8. **TLS pin scoping**: The fixed self-signed pin is only applied to the documented LAN host (`192.168.86.2`). Other HTTPS hosts use normal system trust evaluation, which allows public deployments and other LAN hosts with valid certificates. This never weakens trust for the default self-signed host because the pin check is still enforced for that specific host.

## Verification

- **Build**: `xcodebuildmcp_build_sim` → success (2.8s), no warnings
- **Tests**: `xcodebuildmcp_test_sim` → 16 passed / 0 failed / 0 skipped (10.5s)
  - TranscriptDedupeTests: 7 tests
  - PinnedMediaResourceLoaderTests: 7 tests
  - ConversationStateTests: 5 tests
  - MediaReconcileTests: existing deterministic reconcile tests
  - MediaAPITests: existing stub server test
- **`git diff --check`**: No whitespace issues
- **No new Swift warnings** in the build output

## Risks And Limitations

1. **teardownForFailure is not tested with unit tests**: The terminal failure cleanup path (agent timeout, terminal disconnect) requires a live LiveKit room to trigger, which is not feasible in unit tests. The logic is straightforward (cancel task, increment generation, stop state, restore idle timer, end media scope, tear down room) and mirrors the existing `disconnect()` method. A device test should verify that no stale room or wake lock survives a terminal failure.

2. **Text-mode invariant is not tested with unit tests**: The mic mute/enable guards in `VoiceSession` require a live LiveKit room to trigger, which is not feasible in unit tests. The logic is straightforward (guard on `settings.textMode`) and the UI-level `.disabled(settings.textMode)` provides additional affordance. A device test should verify that the mic toggle is disabled in text mode and that attempting to enable the mic via API is blocked.

3. **Server config sanitization is not tested with unit tests**: The `makeSession` static method requires a `SettingsStore` instance and creates a `VoiceSession`, which requires a live token server. The logic is straightforward (parse URL, fall back to default if invalid) and the visible validation error provides user feedback. A device test should verify that an invalid server URL is sanitized to the default and the validation error is shown.

4. **Conversation state inference is tested with unit tests**: The `ConversationStateModel` is a pure state machine with no external dependencies, so it's fully testable. The five new tests cover the key transitions (remote speech ending → listening, user speaking → listening, agent priority, stop → idle, no stale thinking chip).

5. **Media-state publishing staleness is not tested with unit tests**: The `publishMediaState` method requires a live LiveKit room and session to trigger, which is not feasible in unit tests. The logic is straightforward (capture room/generation/pollGen, validate before sending) and mirrors the existing generation-check pattern used elsewhere in the codebase. A device test should verify that no stale media state is published after a reconnect or disconnect.

6. **Resource loader hardening is partially tested**: The parser tests cover 206/200/malformed Content-Range, but the full loading/cancellation flow requires a live AVPlayer and media server, which is not feasible in unit tests. The logic is straightforward (shared session, task retention, status validation, cancellation) and mirrors standard URLSession patterns. A device test should verify that cancelled requests don't keep downloading and that non-200/206 responses are rejected.

7. **TLS pin scoping is not tested with unit tests**: The `PinnedTrustDelegate` requires a live TLS handshake to trigger, which is not feasible in unit tests. The logic is straightforward (check host, apply pin or default handling) and mirrors standard URLSession delegate patterns. A device test should verify that the pinned host works with the self-signed cert and that other HTTPS hosts work with normal system trust.

8. **Model picker copy is not tested with unit tests**: The `SettingsView` is a SwiftUI view that requires a live session to trigger, which is not feasible in unit tests. The logic is straightforward (check `isReady` instead of non-nil) and the UI change is minimal. A device test should verify that the model picker shows "Start a conversation to load the model list." when disconnected.

## Follow-up For Reviewer

1. **Verify terminal failure cleanup on device**: Connect to a live server, trigger an agent timeout (e.g., stop the agent worker), and verify that no stale room or wake lock survives. Also trigger a terminal disconnect (e.g., kill the LiveKit container) and verify the same.

2. **Verify text-mode invariant on device**: Enable text mode, verify the mic toggle is disabled, and verify that attempting to enable the mic via API (if possible) is blocked. Also verify that disabling text mode re-enables the mic toggle and allows mic enable.

3. **Verify server config sanitization on device**: Enter an invalid server URL in Settings, save, and verify that the app sanitizes it to the default and shows a validation error. Also verify that entering a valid URL triggers session replacement when idle.

4. **Verify conversation state inference on device**: Start a voice conversation, let the agent speak, then stop speaking and wait for the agent to go silent. Verify that the state returns to "listening" (not "thinking") and the thinking chip does not appear.

5. **Verify media-state publishing staleness on device**: Start a conversation with media playing, trigger a reconnect (e.g., toggle airplane mode), and verify that no stale media state is published after the reconnect. Also verify that media playback resumes correctly after the reconnect.

6. **Verify resource loader hardening on device**: Play a media track, seek to a different position, and verify that the cancelled request doesn't keep downloading (check network activity). Also verify that a non-200/206 response (e.g., 404) is rejected and doesn't feed error JSON into AVPlayer.

7. **Verify TLS pin scoping on device**: Connect to the default LAN host (`192.168.86.2`) and verify that the self-signed cert works. Then configure a different HTTPS host (e.g., a public deployment) and verify that it works with normal system trust evaluation.

8. **Verify model picker copy on device**: Open Settings while disconnected and verify that the model picker shows "Start a conversation to load the model list." instead of "Loading available models…". Then connect and verify that the model picker shows the actual model list.