44 lines
4.2 KiB
Markdown
44 lines
4.2 KiB
Markdown
# Review Report
|
|
|
|
- Task: 20260831-105824-plan-code-ui-audit
|
|
- Agent: reviewer
|
|
- Model: openai-codex/gpt-5.6-sol
|
|
- Effort: high
|
|
- Scope: retrospective verdict on iteration 02 before iteration 03
|
|
- Verdict: changes-requested
|
|
|
|
## Findings
|
|
|
|
1. **High — transcript dedupe remained asymmetric and unbounded.** `Sources/LiveKit/TranscriptHandler.swift:36-65` handles primary→data only when the matching final is last, while data→primary duplicates; `TranscriptHistory.swift:25-36` permanently suppresses adjacent legitimate repeats. Iteration 03 item 1 is required.
|
|
2. **High — microphone interruption ownership was incorrect.** The iteration-02 baseline resumed based on `isMuted`, which could unmute a user-muted mic. The current partial flag still lacks disconnect/new-interruption cleanup (`VoiceSession.swift:55-57`, `:378-420`). Iteration 03 item 2 is required but not yet complete.
|
|
3. **High — media interruption was unsafe.** `MediaPlayerController.swift:63-76` ignores `.shouldResume`, and polling/reconcile (`:166-178`, `:215-228`) can immediately restart media during an interruption. Iteration 03 item 3 is required.
|
|
4. **Medium — 206 content length used the returned range length, not total resource size.** Iteration 03 item 4 correctly identifies the defect. The current attempted correction at `PinnedMediaResourceLoader.swift:94` does not compile and therefore is not accepted.
|
|
5. **Medium — `MediaAPI.streamURL` incorrectly fetched progressive bytes as text in the iteration-02 baseline.** Iteration 03 item 5 is correct; the current `MediaAPI.swift:185-190` construction-only version resolves it, though the helper has no callers.
|
|
6. **High — failed/retry lifecycle remained incomplete.** A final agent timeout sets failed without room/wake-lock teardown (`VoiceSession.swift:182-193`), and retry can reuse that room (`:109-131`). This was missing from iteration 03.
|
|
7. **High — text mode could locally re-enable the mic.** The ready-state mic toggle remains active in text mode (`ConversationView.swift:97-108`). This was missing from iteration 03.
|
|
8. **Medium — transcript persistence/clear claims remained false in UI.** Persisted entries are never rendered and Clear only clears storage (`ConversationView.swift:33-37`; `OrbView.swift:64-107`). This was missing from iteration 03.
|
|
9. **Medium — state inference contradicted the current web reference.** speaking→thinking can stick (`ConversationState.swift:54-66`) whereas `web/app.js:414-426` explicitly fixed it to speaking→listening. This was missing from iteration 03.
|
|
|
|
## Acceptance Criteria
|
|
|
|
- The iteration-02 changes correctly fixed the no-op text binding, open-partial replacement, desired-state assignment/gate, deterministic reconcile seam, independent join sync, and stop track-ID capture.
|
|
- They did **not** establish complete correctness for transcript dedupe, interruptions, retry lifecycle, reporting/stale callbacks, persistence UI, or full PLAN/UPDATE_PATH parity.
|
|
- Historical coder evidence reports 12 tests passed and a successful simulator build for iteration 02. That evidence is accepted as historical only; it does not approve behavior and cannot be rerun against the current partially modified tree.
|
|
|
|
## Verification Evidence
|
|
|
|
- Reviewed `iterations/01-request.md`, `01-implementation.md`, `02-request.md`, `02-implementation.md`, current complete diff, tests, product sources, and reference server/web code.
|
|
- Current XcodeBuildMCP `test_sim` and `build_sim` both fail compiling `PinnedMediaResourceLoader.swift:94`; this failure belongs to unreported partial iteration-03 work, not the documented iteration-02 snapshot.
|
|
- Exact current evidence and completion matrix are in `../review.md`.
|
|
|
|
## Residual Risks
|
|
|
|
- No real-device qualification.
|
|
- Historical iteration-02 tests omit VoiceSession lifecycle/interruption, ControlChannel protocol, persistence restoration, TLS/config, accessibility, and exact media reporting behavior.
|
|
- The current worktree no longer represents a clean iteration-02 state because iteration-03 edits are already present without an implementation report.
|
|
|
|
## Required Actions
|
|
|
|
- Treat iteration 02 as **changes-requested**, not approved.
|
|
- Complete iteration 03 plus the missing high/medium findings listed above, restore a passing current build/test suite, and request another independent review.
|