Repository navigation
feat: play a text's narration, and bring the old library across (M7) - #34
Conversation
M7. Per-text audio, with the transport the prototype had and the behaviours it got wrong. The scrubber commits once. The prototype bound its slider straight to `player.seek` with zero tolerance, so every drag delta was a frame-accurate seek; they cancelled each other and fought the 10 Hz observer writing the old value back under the finger. Here a drag moves `scrubTime` and nothing else, engine reports are ignored while it is non-nil, and the finger coming up issues exactly one seek. That requirement is about calls rather than about sound, which is why `AudioEngine` is a protocol: against `AVPlayer` it can only be checked by ear, against a spy it is an assertion. The playhead is published at most once a second. The display shows `m:ss`, and the prototype woke the actor laying out CJK text ten times a second to render a string that had not changed. The engine observes at 4 Hz and the model forwards only when the second changes; the exact playhead is kept unpublished for persistence, so stopping mid-sentence resumes mid-sentence. Now-playing info is pushed on transitions only. The system extrapolates the position from the last elapsed time and rate it was given, so telling it less often is also telling it better. The audio session is activated on first play, never at launch — activating one you are not using interrupts whatever the reader was listening to, for an app that might never make a sound. Interruptions pause and resume; audio the reader paused themselves is never resumed for them; unplugging headphones pauses and does not resume, because plugging them back in must not start their audio in the open. Narration is copied in and recorded by a path **relative** to the audio directory. The prototype stored an absolute path containing the container UUID, which changes across an iOS reinstall or restore, so every attachment broke at once long after the mistake. The playhead is written while the reader is using the app rather than only on teardown: macOS delivers neither a scene-phase change nor `onDisappear` reliably on quit, so saving only at the end saved nothing. It is written when playback stops, once a minute while playing, when another text takes the transport, and on scene-phase change. Also here, because it is the other half of the same milestone: the prototype's `library.db` is migrated on first launch. Texts, revealed words split out of the newline-joined string they were packed into, and `last_opened_at`. `scroll_offset` is discarded deliberately — a pixel offset means nothing after a font change and the prototype never restored it either. An `audio_path` that still resolves is copied inside; one that does not is reported rather than failing the migration. The file is renamed rather than deleted, so a migration that went wrong has not cost anything. And a defect found on the way: `ReaderMetrics.minimumHitTarget` had gone missing and left its documentation attached to `tokenCornerRadius`, which is why the transport's first buttons were 9x11 targets. Verified on the running app: attach, play, scrub, pause, quit, relaunch — the transport comes back parked at 0:04 where it was left. The migration was run against the real prototype database: 3 texts and 152 revealed words recovered from the blob, counts matching the source exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoAdd per-text narration playback and migrate prototype libraries
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
All fifteen were real. They cluster into five. **Who may resume narration.** One flag was doing two jobs, and the two cases contradict each other. A system interruption that never invites us back left its claim standing, so the next word the reader tapped ended by resuming audio the system had taken away; and losing the output while already paused by an interruption left that claim set, so the interruption ending played into a room with no headphones in it. Now `pausedForSpeech` and `pausedByInterruption` are separate, each undone only by its own counterpart, and `.outputGone` clears both before its guard rather than after it. **Speech that never happens.** `SpeechModel.speak` declines in three ordinary situations — VoiceOver on, an empty word, no Chinese voice — and says so by never starting. The narration ducked for an utterance that never came and stayed paused until the reader pressed play, which on a Mac with no Chinese voice is every single tap. **Process-wide singletons used per window.** `MPRemoteCommandCenter` and `AVAudioSession` are one per process, so registering per window meant a lock-screen play reaching every window's transport, and one window finishing a track deactivating the session another was still playing through. Commands now route to the controller that most recently claimed the lock screen, and the session is reference counted. **Shared transport, unguarded `await`.** `loadAudio` and `attachAudio` wrote to the shared transport after asynchronous work without checking the text was still open, so changing text mid-lookup loaded one text's narration under another's name. Save tasks read the playhead when they ran rather than when they were scheduled, which could write a new track's position under the old text's id. Both now check, and the playhead is read at schedule time. **Destructive ordering.** Attaching deleted the existing narration before copying the replacement, so a failed copy left a row pointing at a file that was gone; it now stages the copy and moves it into place. Replacing an `.m4a` with an `.mp3` left the old file with nothing pointing at it. A replaced narration kept the previous one's stored playhead, so it resumed partway through a recording it had never played. A parked position was clamped only to zero, so a shorter replacement showed a slider past its own end until the duration arrived. **Migration overreach.** A reader who has used both versions has a library that is partly theirs already. Carrying prototype audio over a text that was matched by content replaced a narration they had chosen; stamping `last_opened_at` on it moved it backwards in the sidebar. Both now apply only to texts this migration created. `imported_at` is carried, so a migrated library keeps the order it was built in. And an audio file the old container will not give up is a note rather than a thrown error — it used to abort the migration before the rename that marks it done, so it would fail again on every launch for ever. Nine mutants planted against the fixes; all nine killed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All fifteen findings addressed in Who may resume narration (1, 13) — one flag was doing two jobs, and the two cases contradict each other. A system interruption that never invites us back left its claim standing, so the next word the reader tapped ended by resuming audio the system had taken away. And losing the output while already paused by an interruption left that claim set, so the interruption ending played into a room with no headphones in it. Speech that never happens (7) — Process-wide singletons used per window (2, 8) — Shared transport, unguarded Destructive ordering (6, 9, 12, 15) — attaching deleted the existing narration before copying the replacement, so a failed copy left a row pointing at nothing; it now stages the copy and moves it into place. Replacing Migration overreach (5, 10, 11, 14) — a reader who has used both versions has a library that is partly theirs already. Carrying prototype audio over a content-matched text replaced a narration they chose; stamping Nine mutants planted against the fixes, all nine killed. 384 + 97 + 40 + 30 tests pass; two suites were split as they crossed the length limit, along the seams the new tests created ( Re-verified on the running app after the changes: resumes from the stored 🤖 Generated with Claude Code |
Three things CI caught that this machine could not. `SystemPlaybackSession.deinit` reached main-actor state from a nonisolated `deinit`. The same problem as `AVPlayerAudioEngine`'s, and the same fix — a teardown closure — but the property is inside `#if os(iOS)`, so the macOS build compiled happily and only the iOS one failed. Both app targets are now built locally before pushing, which is what should have happened. The reference-counted audio session was never applied. The edit that was supposed to add it silently matched nothing and reported success, and because it changed no code the build and the tests agreed. It is here now: `AVAudioSession` is one per process, so the last transport out is the one that puts it down, and an `isActive` flag keeps the count balanced against a repeated activate or a deactivate without one. `Migration`'s synthesised memberwise initializer was private, because a `private` stored property drags it down. Xcode 26.6, which CI pins, enforces that; the newer toolchain here does not, so it built locally and failed there. Spelled out explicitly, which is correct on both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Milestone M7 — audio playback, plus the prototype
library.dbmigration the plan files under the same milestone.The transport
Attach a narration to any text and it plays in a bar below the reader: play/pause, ten-second skips, scrubber, five speeds, elapsed and total time, and lock screen / Control Centre controls. Fidelity invariant 6.
Four behaviours are the point, each one a prototype defect:
The scrubber commits once. The prototype bound its slider straight to
player.seekwithtoleranceBefore/After: .zero, so every drag delta issued a frame-accurate seek; they cancelled each other and fought the 10 Hz observer writing the old value back under the finger. Here a drag movesscrubTimeand nothing else, engine reports are ignored while it is non-nil, and the finger coming up issues exactly one seek.That requirement is about calls, not about sound, which is why
AudioEngineis a protocol — againstAVPlayerit can only be checked by ear; against a spy it is an assertion:The playhead publishes at most once a second. The display shows
m:ss, and the prototype woke the actor laying out CJK text ten times a second to re-render a string that had not changed. The engine observes at 4 Hz, the model forwards only when the whole second changes, and the exact playhead is kept unpublished for persistence — so stopping mid-sentence resumes mid-sentence.Now-playing info is pushed on transitions only. The system extrapolates position from the last elapsed time and rate it was given, so telling it less often is telling it better; each update restarts the extrapolation.
The audio session is activated on first play, never at launch. Activating a session you are not using interrupts whatever the reader was listening to, for an app that might never make a sound. Interruptions pause and resume; audio the reader paused themselves is never resumed for them; unplugging headphones pauses and does not resume, because plugging them back in must not start their audio out loud in the room.
Speaking a tapped word ducks the narration and resumes it after — the attached audio is usually the same narration, so hearing both is confusing rather than merely noisy — and only if it was the speech that paused it.
Relative paths
Narration is copied into the container and recorded by a path relative to the audio directory. The prototype stored an absolute path containing the app container's UUID, which changes across an iOS reinstall or restore, so every attachment broke at once, silently, long after the mistake.
LibraryRepository.attachAudioalready rejected an absolute path; this is the code that gives it a relative one.Saving the playhead
Written while the reader is using the app, not only on teardown. macOS delivers neither a scene-phase change nor
onDisappearreliably on quit, so a transport that saved only at the end saved nothing at all — I found this by quitting and querying the database. It is now written when playback stops, once a minute while playing, when another text takes the transport, and on scene-phase change. Changing text in the sidebar was the commonest of those and was losing it outright, because by the time.task(id:)runstextIDis already the new one.Prototype library migration
library.dbis migrated on first launch: texts, revealed words split out of the newline-joined string they were packed into, andlast_opened_at.scroll_offsetis discarded deliberately — a pixel offset is meaningless after a font change, a resize or a move between platforms, and the prototype never restored it either. Anaudio_paththat still resolves is copied inside; one that does not is reported rather than failing the migration. The file is renamed rather than deleted, so a migration that went wrong has cost nothing — and renaming rather than setting a flag keeps the state with the thing it describes, unlike the prototype'sUserDefaults.bool(forKey: "dictionaryParsed_bkrs_v1"), which still said "parsed" after the database was deleted.Run against the real prototype database: 3 texts, 152 revealed words, counts matching the source exactly.
Also
ReaderMetrics.minimumHitTargethad gone missing at some point and left its documentation attached totokenCornerRadius— the third instance in two PRs of a declaration being inserted between a comment and what it documented. That is why the transport's first buttons measured 9×11 points; they are now 44×24.Verification
On the running app: attach a narration, play, scrub, pause, quit, relaunch — the transport returns parked at
0:04where it was left, and does not start playing by itself. All five controls are in the accessibility tree with labels (checked with an AX walker, since System Events' traversal truncates on the ~250 token buttons in the reading surface).384 + 93 + 35 tests pass. Thirteen mutants planted against the new code and all thirteen killed — three after a first attempt exposed weak tests, including one where a hard link passed a "the file is copied" assertion until the test mutated the original in place through a file handle.
Not in this milestone
iOS is built by CI but still unverified on a simulator — no runtime is installed on this machine, so
make run-ioshas never launched anything. The transport's 44×24 hit targets are a Mac compromise; touch wants the full 44 vertically, which M8 will take.🤖 Generated with Claude Code