From b28c69ac442027db7488b4b69996bbfd08478318 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:40:21 +0300 Subject: [PATCH 1/2] feat: place phrase boundaries by hand, by dragging or by tapping along (M11, 5/4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M11, a fifth part the plan had folded into the fourth. The practice screen shipped without it, so M11 stayed unticked; this is what ticks it. Forced alignment is right most of the time and wrong in a way the reader can hear: a boundary lands half a syllable late and the next phrase opens with the tail of the last word. Correcting that is a drag, so the editor draws the recording's shape around the phrase — the only waveform in the app, and drawn only here, because everywhere else the question is "how far through am I" and here it is "does that syllable belong inside". Two operations, deliberately different. A **drag** corrects one phrase and stops at its neighbour, leaving any silence alignment excluded where it is; where two phrases already meet it moves both, because the line between them is one thing and not two. A **tap** is a statement about both phrases made while the recording plays, so it closes whatever gap was there. A reader tapping sentences in has no way to say "leave 300 milliseconds between them", and no reason to want to. Tapping along is also the whole answer for a system that cannot listen: a Mac older than macOS 26, a system with no Chinese speech model, or a reader who will not download several hundred megabytes. The text is split into sentences with no times worth having, the recording plays once, and ⌘↩ places each boundary. A few dozen lines, and always exactly right — which is why it is here rather than a third alignment path guessing from silence. That option is now offered from the invitation screen as well as after a failure, since a reader who already knows the recogniser cannot follow their recording should not have to watch it fail first. What a reader places is marked `manual` and survives re-alignment. What was merely shuffled out of their way is not, so a later listen is free to replace it — which is why `movePhrase` now takes an origin instead of assuming one. Three things found on the running app rather than in the tests: - Splitting without listening opened the editor on a flat line, because it set the flag instead of going through `startEditing`, which is what reads the shape. - Stopping part-way through tapping left every phrase ahead collapsed to the minimum length, stacked up behind the playhead. They are now spread over the recording that is left — not right, but evenly wrong, which is what a placeholder split is. - Mark was on ⌘M, which is the system's Minimize. A view's shortcut competing with a menu command is a fight to lose, so it is ⌘↩: free while the writing field is off screen, and already "commit this" elsewhere on the screen. Verified against 听力64 and its narration: a drag moved both sides of a shared boundary and wrote them down as `manual`; splitting without listening gave 27 placeholders over the full 133 seconds at zero confidence; tapping placed boundaries at the playhead and advanced; and stopping after one tap left the remaining twenty-five sharing 4.75 seconds each rather than a quarter of a second each, with the last ending exactly at the end of the recording. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 + README.md | 2 +- .../Listening/BoundaryEditing.swift | 237 +++++++++++++ .../Listening/ForcedAlignment.swift | 2 +- .../LibraryRepository+Listening.swift | 8 +- Sources/HanReaderPlayback/WaveformPeaks.swift | 217 ++++++++++++ Sources/HanReaderUI/Design/DesignTokens.swift | 23 ++ .../Models/PracticeModel+Alignment.swift | 105 ++++++ .../Models/PracticeModel+Boundaries.swift | 242 ++++++++++++++ .../HanReaderUI/Models/PracticeModel.swift | 139 ++------ .../HanReaderUI/Screens/BoundaryEditor.swift | 240 ++++++++++++++ .../Screens/PracticeComponents.swift | 57 ++++ .../HanReaderUI/Screens/PracticeScreen.swift | 59 +++- .../BoundaryEditingTests.swift | 312 ++++++++++++++++++ .../WaveformPeaksTests.swift | 177 ++++++++++ .../PracticeBoundaryTests.swift | 263 +++++++++++++++ 16 files changed, 1979 insertions(+), 115 deletions(-) create mode 100644 Sources/HanReaderCore/Listening/BoundaryEditing.swift create mode 100644 Sources/HanReaderPlayback/WaveformPeaks.swift create mode 100644 Sources/HanReaderUI/Models/PracticeModel+Alignment.swift create mode 100644 Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift create mode 100644 Sources/HanReaderUI/Screens/BoundaryEditor.swift create mode 100644 Tests/HanReaderCoreTests/BoundaryEditingTests.swift create mode 100644 Tests/HanReaderPlaybackTests/WaveformPeaksTests.swift create mode 100644 Tests/HanReaderUITests/PracticeBoundaryTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b75001..1e4e256 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,17 @@ it needs to read like release notes rather than a commit log. - Sentences can be graded easy, middling or hard, and a filter drills only the ones still worth drilling: graded hard, never attempted, or last attempted below the pass mark. +- Phrase boundaries can be corrected by hand. The practice screen draws the + recording's shape around the sentence you are on — the only waveform in the + app — and its start and end are draggable. A boundary you place is kept when + the recording is listened to again, so a correction is made once rather than + every time. +- A recording can be divided without any speech recognition at all: the text is + split into sentences, the recording plays once, and you press Mark as each + one begins. That is the path for a Mac older than macOS 26, for a system with + no Chinese speech model, and for anyone who would rather not download several + hundred megabytes. It is also the quickest repair when a recogniser has + simply failed to follow a recording. - Per-text audio. Attach a narration to any text and it plays in a transport below the reader, with a scrubber, ten-second skips, five speeds and lock screen and Control Centre controls. The playhead is remembered, so the next diff --git a/README.md b/README.md index dc0dbab..dde40ee 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ framework imports. Details in [`Docs/ARCHITECTURE.md`](Docs/ARCHITECTURE.md). - [x] **M5** — **the macOS reader: first usable version** - [x] **M6** — **BKRS DSL import** - [x] **M7** — **audio playback** -- [ ] **M11** — **listening and dictation practice (听力 / 听写)** — practice screen done, phrase-boundary editor to come +- [x] **M11** — **listening and dictation practice (听力 / 听写)** - [ ] **M8** — iOS and iPadOS - [ ] **M9** — accessibility and Russian localization - [ ] **M10** — 1.0 diff --git a/Sources/HanReaderCore/Listening/BoundaryEditing.swift b/Sources/HanReaderCore/Listening/BoundaryEditing.swift new file mode 100644 index 0000000..ff3da6d --- /dev/null +++ b/Sources/HanReaderCore/Listening/BoundaryEditing.swift @@ -0,0 +1,237 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation + +/// Which end of a phrase is being dragged. +public enum PhraseBoundary: Sendable, Hashable { + case start + case end +} + +/// Moving a phrase's boundaries by hand. +/// +/// Forced alignment is right most of the time and wrong in a way the reader +/// can hear: a boundary lands half a syllable late and the next phrase opens +/// with the tail of the last word. Correcting that is a drag, and this is the +/// arithmetic behind it — kept here, away from the view, because the rules +/// are about phrases rather than about pixels. +/// +/// Nothing here decides *which words* belong to a phrase. Only the times +/// move, which is why `LibraryRepository.movePhrase` writes times alone and +/// marks the phrase `manual` so a later re-alignment leaves it be. +public enum BoundaryEditing { + /// The shortest a phrase may be dragged down to. + /// + /// Not zero: a phrase of no length plays nothing, and a reader who + /// collapses one by accident has no handle left to pull it back out + /// with — the two would sit on the same pixel. + public static let minimumDuration = 0.25 + + /// Moves one boundary, keeping an abutting neighbour attached to it. + /// + /// Where two phrases touch, the boundary between them is one thing and + /// not two: alignment emits them that way, and a reader dragging the + /// line between two sentences means "the first ends here and the second + /// starts here". Moving only one side would open a silent gap, or an + /// overlap, that they never asked for and would then have to repair from + /// the other phrase. + /// + /// Where there is already a gap — silence the alignment deliberately + /// left out — the handles are independent, and a drag is clamped at the + /// neighbour rather than pushing it. + /// + /// - Returns: every phrase whose times changed, in index order. Empty if + /// the drag asked for nothing the rules allow. + public static func move( + _ boundary: PhraseBoundary, + of index: Int, + to time: Double, + in phrases: [Phrase], + duration: Double, + ) + -> [Phrase] + { + guard let position = phrases.firstIndex(where: { $0.index == index }) else { return [] } + let phrase = phrases[position] + let previous = position > 0 ? phrases[position - 1] : nil + let next = position < phrases.count - 1 ? phrases[position + 1] : nil + + switch boundary { + case .start: + let floor = previous + .map { abuts($0.end, phrase.start) ? $0.start + minimumDuration : $0.end } ?? 0 + let ceiling = phrase.end - minimumDuration + guard floor <= ceiling else { return [] } + let start = time.clamped(to: floor ... ceiling) + guard start != phrase.start else { return [] } + var moved = [phrase.moved(start: start, end: phrase.end)] + if let previous, abuts(previous.end, phrase.start) { + moved.insert(previous.moved(start: previous.start, end: start), at: 0) + } + return moved + + case .end: + let floor = phrase.start + minimumDuration + let ceiling = next + .map { abuts(phrase.end, $0.start) ? $0.end - minimumDuration : $0.start } ?? + duration + guard floor <= ceiling else { return [] } + let end = time.clamped(to: floor ... ceiling) + guard end != phrase.end else { return [] } + var moved = [phrase.moved(start: phrase.start, end: end)] + if let next, abuts(phrase.end, next.start) { + moved.append(next.moved(start: end, end: next.end)) + } + return moved + } + } + + /// Places the boundary between two phrases at one time. + /// + /// Different from dragging, and deliberately so. A drag is a correction + /// to one phrase and stops at its neighbour, leaving any silence the + /// alignment excluded where it is. A tap is a statement about *both* + /// phrases — "the last one ended here and this one starts here" — made + /// while the recording plays, so it closes whatever gap was there. A + /// reader tapping sentences in has no way to express "and leave 300 + /// milliseconds of silence between them", and no reason to want to. + /// + /// - Parameter index: the phrase the boundary opens. Phrase zero opens + /// at the start of the recording and has no boundary before it. + /// - Returns: both phrases, or nothing if the tap asked for something + /// the rules do not allow. + public static func place( + boundaryBefore index: Int, + at time: Double, + in phrases: [Phrase], + ) + -> [Phrase] + { + guard let position = phrases.firstIndex(where: { $0.index == index }), position > 0 else { + return [] + } + let previous = phrases[position - 1] + let phrase = phrases[position] + let floor = previous.start + minimumDuration + let ceiling = phrase.end - minimumDuration + guard floor <= ceiling else { return [] } + let boundary = time.clamped(to: floor ... ceiling) + guard boundary != previous.end || boundary != phrase.start else { return [] } + return [ + previous.moved(start: previous.start, end: boundary), + phrase.moved(start: boundary, end: phrase.end), + ] + } + + /// Spreads phrases nobody reached over the recording that is left. + /// + /// Tapping along places the boundary in *front* of each phrase, so the + /// ones still ahead keep the times a placeholder split gave them — times + /// that are now behind the playhead. Left alone, a reader who tapped + /// three sentences of thirty and stopped would find the other + /// twenty-seven collapsed to the minimum length, stacked up behind where + /// they stopped. Spreading them over the time that remains puts them + /// back where they started: not right, but evenly wrong, and the same + /// kind of wrong a placeholder split is. + /// + /// Their origin is deliberately **not** changed. Nobody placed these, so + /// a later re-alignment should be free to replace them — which is + /// exactly what `manual` would prevent. + /// + /// - Parameter index: the first phrase that was never reached. + public static func respread( + from index: Int, + after time: Double, + in phrases: [Phrase], + duration: Double, + ) + -> [Phrase] + { + guard let position = phrases.firstIndex(where: { $0.index == index }) else { return [] } + let remaining = phrases[position...] + guard !remaining.isEmpty else { return [] } + // Equal slices, with a floor so that a reader who stopped at the + // very end of a recording is not left with phrases of no length. The + // last phrase therefore ends exactly at `duration` when there was + // room, and past it when there was not — which is the same answer + // `tidy` gives, and the transport simply plays to the end. + let slice = max(minimumDuration, (duration - time) / Double(remaining.count)) + return remaining.enumerated().map { offset, phrase in + Phrase( + index: phrase.index, + start: time + Double(offset) * slice, + end: time + Double(offset + 1) * slice, + textRange: phrase.textRange, + confidence: phrase.confidence, + origin: phrase.origin, + grade: phrase.grade, + ) + } + } + + /// Whether two phrases meet rather than merely come close. + /// + /// A tolerance rather than equality, because these times have been + /// through a recogniser, a re-cut and a round trip through SQLite as + /// doubles. A millisecond is far below anything a reader can hear or + /// aim at, and treating a one-microsecond gap as a gap would make the + /// shared-boundary rule fire or not fire unpredictably. + static func abuts(_ end: Double, _ start: Double) -> Bool { + abs(start - end) < 0.001 + } + + /// The span of recording to draw around a phrase. + /// + /// Context either side, because a boundary cannot be judged from the + /// phrase alone: the question is always whether the syllable just + /// outside belongs inside, and a strip cropped to the phrase hides + /// exactly that. Clamped to the recording, and widened on the other side + /// when one end runs out, so the window keeps its width at the start and + /// end of a file instead of shrinking where there is least to see. + public static func window( + around phrase: Phrase, + duration: Double, + context: Double = 1.5, + ) + -> Range + { + let width = min(duration, phrase.duration + context * 2) + guard width > 0 else { return 0 ..< max(duration, minimumDuration) } + var start = phrase.start - context + if start < 0 { + start = 0 + } + if start + width > duration { + start = max(0, duration - width) + } + return start ..< start + width + } +} + +extension Phrase { + /// The same phrase at a different place in the recording. + /// + /// Marked `manual`, because every caller here is a reader correcting + /// what the recogniser did, and that is precisely what a later + /// re-alignment must not overwrite. + func moved(start: Double, end: Double) -> Phrase { + Phrase( + index: index, + start: start, + end: end, + textRange: textRange, + // A boundary somebody placed is not a guess, whatever the + // recogniser thought of it beforehand — so the warning triangle + // goes away once they have fixed it. + confidence: 1, + origin: .manual, + grade: grade, + ) + } +} + +extension Double { + fileprivate func clamped(to range: ClosedRange) -> Double { + Swift.min(Swift.max(self, range.lowerBound), range.upperBound) + } +} diff --git a/Sources/HanReaderCore/Listening/ForcedAlignment.swift b/Sources/HanReaderCore/Listening/ForcedAlignment.swift index 413d6e7..9e4452a 100644 --- a/Sources/HanReaderCore/Listening/ForcedAlignment.swift +++ b/Sources/HanReaderCore/Listening/ForcedAlignment.swift @@ -253,7 +253,7 @@ public enum ForcedAlignment { /// /// Confidence zero throughout, which is what tells the UI to offer the /// boundary editor rather than present these as answers. - static func spread(_ phrases: [Range], across duration: Double) -> [Phrase] { + public static func spread(_ phrases: [Range], across duration: Double) -> [Phrase] { guard !phrases.isEmpty else { return [] } let slice = (duration > 0 ? duration : 0) / Double(phrases.count) return phrases.enumerated().map { index, range in diff --git a/Sources/HanReaderPersistence/LibraryRepository+Listening.swift b/Sources/HanReaderPersistence/LibraryRepository+Listening.swift index 48c230e..98439fb 100644 --- a/Sources/HanReaderPersistence/LibraryRepository+Listening.swift +++ b/Sources/HanReaderPersistence/LibraryRepository+Listening.swift @@ -126,17 +126,23 @@ extension LibraryRepository { /// Marked `manual` so a later re-alignment can leave it alone. Only the /// times change: dragging a boundary in the waveform says nothing about /// which words belong to the phrase. + /// - Parameter origin: `manual` for a boundary somebody placed, which is + /// every drag and every tap. Pass `recognised` only for times that + /// were shuffled around on a reader's behalf rather than chosen by + /// them — the phrases nobody reached while tapping along — or a later + /// re-alignment will preserve a guess as though it were a decision. public func movePhrase( _ index: Int, in id: TextID, start: Double, end: Double, + origin: PhraseOrigin = .manual, ) async throws { try await dbQueue.write { db in try db.execute(sql: """ UPDATE phrase SET startTime = ?, endTime = ?, origin = ? WHERE textId = ? AND idx = ? - """, arguments: [start, end, PhraseOrigin.manual.rawValue, id.rawValue, index]) + """, arguments: [start, end, origin.rawValue, id.rawValue, index]) } } diff --git a/Sources/HanReaderPlayback/WaveformPeaks.swift b/Sources/HanReaderPlayback/WaveformPeaks.swift new file mode 100644 index 0000000..4d1cac0 --- /dev/null +++ b/Sources/HanReaderPlayback/WaveformPeaks.swift @@ -0,0 +1,217 @@ +// HanReader — MIT licensed. See LICENSE. + +import AVFoundation +public import Foundation + +/// The shape of a recording, at the resolution a strip can draw. +/// +/// One peak per bucket of equal duration. Not samples: a two-minute file at +/// 44.1 kHz is five million of them and a strip is a few hundred points +/// wide, so the only honest thing to keep is the loudest value in each +/// bucket — which is what makes a syllable visible as a bump and a pause as +/// a flat. +public nonisolated struct WaveformPeaks: Sendable, Hashable { + /// Zero to one, one per bucket, in time order. + public let values: [Float] + /// Seconds the whole run covers, so a bucket's time can be worked out + /// without knowing how many there are. + public let duration: Double + + public init(values: [Float], duration: Double) { + self.values = values + self.duration = duration + } + + public static let empty = Self(values: [], duration: 0) + + /// A scaled time as an index into `values`. + /// + /// Clamped at both ends, because a span can sit wholly outside the + /// recording: the editor asks for exactly that the moment a phrase's + /// times outlive the file they were measured against, as when a + /// narration is replaced by a shorter one. + private static func index(_ value: Double, upTo count: Int) -> Int { + guard value > 0 else { return 0 } + guard value < Double(count) else { return count } + return Int(value) + } + + public var isEmpty: Bool { + values.isEmpty || duration <= 0 + } + + /// The loudest sample in the bucket covering `time`, or zero outside the + /// recording. + /// + /// The guard is what keeps the index in range: a time below `duration` + /// cannot scale to a bucket at or past the end. There is deliberately no + /// second clamp behind it — a clamp that can never fire reads as though + /// the bound were in doubt, and the next person to touch this would have + /// to work out which of the two was load-bearing. + public func value(at time: Double) -> Float { + guard !isEmpty, time >= 0, time < duration else { return 0 } + return values[Int(time / duration * Double(values.count))] + } + + /// The peaks covering a span, resampled to `count` buckets. + /// + /// Resampled rather than sliced because a strip has a fixed width and + /// the span it shows changes with the phrase: taking the maximum across + /// each output bucket keeps a short transient visible when zoomed out, + /// which averaging would smooth away into a flat line. + public func resampled(over span: Range, count: Int) -> [Float] { + guard !isEmpty, count > 0 else { + return Array(repeating: 0, count: max(0, count)) + } + let width = (span.upperBound - span.lowerBound) / Double(count) + return (0 ..< count).map { bucket in + let from = span.lowerBound + Double(bucket) * width + let scale = Double(values.count) / duration + // Both ends clamped into the array, and the upper end clamped + // against the lower one as well. A span that starts past the end + // of the recording would otherwise produce a lower bound above + // its upper bound, and `Range` traps on that rather than coming + // back empty. + // Floor and ceiling, not truncation at both ends: an output + // bucket narrower than a stored one would otherwise land on the + // same index twice and draw nothing where there is signal. + // + // The ceiling can never fall below the floor — the span has + // width and both are clamped the same way — so the range cannot + // invert. It can be empty, which is what a span outside the + // recording comes to, and that is the case the guard is for. + let lower = Self.index((from * scale).rounded(.down), upTo: values.count) + let upper = Self.index(((from + width) * scale).rounded(.up), upTo: values.count) + let range = lower ..< upper + guard !range.isEmpty else { return 0 } + return values[range].max() ?? 0 + } + } +} + +/// Reads a recording's shape, once, so it can be drawn. +/// +/// Separate from `AudioEngine`: the transport plays a file and this measures +/// one, and the only screen that draws a waveform is the boundary editor. +/// Nothing else in the app has ever needed the samples. +public enum WaveformReader { + /// How many buckets a whole file is reduced to. + /// + /// Fixed rather than derived from the view's width, because the strip + /// zooms: it shows a few seconds around one phrase, and a file-wide + /// resolution matched to the window would be far too coarse once + /// zoomed in. 12,000 over a two-minute file is about a hundred a + /// second — fine enough to see a syllable, and a 48 KB array. + public static let bucketCount = 12000 + + /// Measures `url` and returns one peak per bucket. + /// + /// Runs off the main actor and checks for cancellation between blocks, + /// because a reader who moves to another text while this is running + /// should not be waiting on it. + public static func peaks( + of url: URL, + buckets: Int = bucketCount, + ) async throws + -> WaveformPeaks + { + let asset = AVURLAsset(url: url) + guard let track = try await asset.loadTracks(withMediaType: .audio).first else { + return .empty + } + let duration = try await CMTimeGetSeconds(asset.load(.duration)) + guard duration.isFinite, duration > 0, buckets > 0 else { return .empty } + + let reader = try AVAssetReader(asset: asset) + let output = AVAssetReaderTrackOutput(track: track, outputSettings: settings) + guard reader.canAdd(output) else { return .empty } + reader.add(output) + guard reader.startReading() else { + throw reader.error ?? CocoaError(.fileReadCorruptFile) + } + defer { reader.cancelReading() } + + let values = try fill(buckets, from: output, over: duration) + if reader.status == .failed { + throw reader.error ?? CocoaError(.fileReadCorruptFile) + } + return WaveformPeaks(values: normalised(values), duration: duration) + } + + /// Mono float, so one sample is one number and there is no channel + /// interleaving to unpick. The strip draws one line, so mixing the + /// channels down is what it wants anyway. + private static var settings: [String: Any] { + [ + AVFormatIDKey: kAudioFormatLinearPCM, + AVLinearPCMBitDepthKey: 32, + AVLinearPCMIsFloatKey: true, + AVLinearPCMIsBigEndianKey: false, + AVLinearPCMIsNonInterleaved: false, + AVNumberOfChannelsKey: 1, + ] + } + + /// Reads every sample, keeping the loudest in each bucket. + private static func fill( + _ buckets: Int, + from output: AVAssetReaderTrackOutput, + over duration: Double, + ) throws + -> [Float] + { + var values = [Float](repeating: 0, count: buckets) + var samplesSeen = 0 + // Worked out from the first buffer's sample rate rather than + // assumed: an mp3 at 24 kHz and an m4a at 44.1 kHz would otherwise + // fill the array at different speeds and one of them would end + // half empty. + var samplesPerBucket = 0 + + while let buffer = output.copyNextSampleBuffer() { + try Task.checkCancellation() + if samplesPerBucket == 0 { + guard let rate = sampleRate(of: buffer), rate > 0 else { continue } + samplesPerBucket = max(1, Int(rate * duration) / buckets) + } + for sample in samples(of: buffer) { + let bucket = samplesSeen / samplesPerBucket + samplesSeen += 1 + guard bucket < buckets else { break } + values[bucket] = max(values[bucket], abs(sample)) + } + } + return values + } + + private static func sampleRate(of buffer: CMSampleBuffer) -> Double? { + CMSampleBufferGetFormatDescription(buffer) + .flatMap { + CMAudioFormatDescriptionGetStreamBasicDescription($0)?.pointee.mSampleRate + } + } + + private static func samples(of buffer: CMSampleBuffer) -> [Float] { + guard let block = CMSampleBufferGetDataBuffer(buffer) else { return [] } + let length = CMBlockBufferGetDataLength(block) + var data = [Float](repeating: 0, count: length / MemoryLayout.size) + guard CMBlockBufferCopyDataBytes( + block, + atOffset: 0, + dataLength: length, + destination: &data, + ) == kCMBlockBufferNoErr else { return [] } + return data + } + + /// Scales the loudest point to one. + /// + /// A quiet recording is otherwise a flat line in the strip, which tells + /// the reader nothing about where the words are — and where the words + /// are is the only reason the strip is drawn. Absolute level is not the + /// question here; relative shape is. + static func normalised(_ values: [Float]) -> [Float] { + guard let loudest = values.max(), loudest > 0 else { return values } + return values.map { $0 / loudest } + } +} diff --git a/Sources/HanReaderUI/Design/DesignTokens.swift b/Sources/HanReaderUI/Design/DesignTokens.swift index df255c1..bb7f590 100644 --- a/Sources/HanReaderUI/Design/DesignTokens.swift +++ b/Sources/HanReaderUI/Design/DesignTokens.swift @@ -55,6 +55,20 @@ nonisolated enum ReaderColor { outcome == .correct ? .clear : dictation(outcome).opacity(0.14) } + /// The waveform in the boundary editor, and the part of it belonging to + /// the phrase being edited. + /// + /// The phrase is drawn in the accent and its surroundings are not, so + /// the question the editor exists to answer — does that syllable belong + /// inside? — is one the reader can see rather than measure. + static let waveform = Color.secondary.opacity(0.45) + static let waveformInPhrase = Color.accentColor.opacity(0.85) + + /// A faint accent fill marking a region rather than an object: the span + /// of recording a phrase covers, and the bar that takes over the screen + /// while boundaries are being tapped in. + static let highlightWash = Color.accentColor.opacity(0.1) + /// The fill behind a highlighted token. /// /// Takes the resolved `TokenHighlight` rather than an opacity, so that the @@ -126,6 +140,15 @@ nonisolated enum ReaderMetrics { static let minimumHitTarget = 44.0 /// Corner radius on a token's highlight. static let tokenCornerRadius = 4.0 + + /// How tall the boundary editor's waveform strip is, and how wide its + /// draggable edges are. + /// + /// The grab area is deliberately far wider than the line it draws: a + /// boundary is a one-point line and nobody can hit one with a mouse, let + /// alone a finger. + static let waveformHeight = 96.0 + static let waveformHandleWidth = 22.0 } /// Animations, with reduce-motion handled once. diff --git a/Sources/HanReaderUI/Models/PracticeModel+Alignment.swift b/Sources/HanReaderUI/Models/PracticeModel+Alignment.swift new file mode 100644 index 0000000..ecf1caa --- /dev/null +++ b/Sources/HanReaderUI/Models/PracticeModel+Alignment.swift @@ -0,0 +1,105 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPlayback + +/// Working out where each phrase is said, which is what makes the rest of +/// the screen possible. +/// +/// Split from the model's own file because it is a different concern with +/// its own failure modes — a model to download, a system too old, a +/// recording that cannot be read — and because the model was over the size +/// a class is allowed to be once the boundary editor arrived. +extension PracticeModel { + /// Works out where each phrase is said, which is what makes any of the + /// rest possible. + func align(installingModel: Bool = false) { + guard alignTask == nil else { return } + alignGeneration += 1 + let generation = alignGeneration + alignTask = Task { [weak self] in + await self?.runAlignment(installingModel: installingModel, generation: generation) + guard let self, generation == alignGeneration else { return } + alignTask = nil + } + } + + func cancelAlignment() { + alignTask?.cancel() + alignTask = nil + // Bumped so that whatever the stopped run does on its way out + // belongs to a generation nobody is listening to any more. + alignGeneration += 1 + phase = phrases.isEmpty ? .needsAlignment : .ready + } + + private func runAlignment(installingModel: Bool, generation: Int) async { + guard #available(macOS 26.0, iOS 26.0, *) else { + phase = .unavailable(.systemTooOld) + return + } + guard let track = try? await services.library.audioTrack(for: textID) else { + phase = .noAudio + return + } + let file = AudioAttachment.file(for: track, in: services.locations) + let aligner = SpeechAligner() + guard generation == alignGeneration else { return } + phase = .aligning(SpeechAlignmentProgress(fraction: nil)) + + do { + let heard = try await aligner.listen( + to: file, + installIfNeeded: installingModel, + progress: { [weak self] progress in + Task { @MainActor in self?.report(progress, from: generation) } + }, + ) + try Task.checkCancellation() + + let splitting = PhraseSplitting.forAudio( + characters: content.count, + duration: heard.duration, + ) + let placed = ForcedAlignment.align( + phrases: PhraseSplitter.phrases(in: content, splitting: splitting), + in: content, + to: heard, + ) + try await services.library.savePhrases(placed, for: textID) + // Checked again after the write, not only after the listening. + // Stop pressed while the database was busy would otherwise let + // the run finish and announce itself ready. + try Task.checkCancellation() + guard generation == alignGeneration else { return } + phrases = try await services.library.phrases(for: textID) + attempts = try await services.library.attempts(for: textID) + current = firstUnfinished + phase = .ready + } catch is CancellationError { + guard generation == alignGeneration else { return } + phase = phrases.isEmpty ? .needsAlignment : .ready + } catch let problem as SpeechAlignmentProblem { + guard generation == alignGeneration else { return } + phase = .unavailable(obstacle(for: problem)) + } catch { + guard generation == alignGeneration else { return } + phase = .unavailable(.failed(error.localizedDescription)) + } + } + + private func report(_ progress: SpeechAlignmentProgress, from generation: Int) { + guard generation == alignGeneration, case .aligning = phase else { return } + phase = .aligning(progress) + } + + private func obstacle(for problem: SpeechAlignmentProblem) -> AlignmentObstacle { + switch problem { + case .systemTooOld: .systemTooOld + case .languageUnsupported: .languageUnsupported + case .modelUnavailable: .modelMissing + case let .unreadable(reason): .failed(reason) + } + } +} diff --git a/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift b/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift new file mode 100644 index 0000000..98e10dd --- /dev/null +++ b/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift @@ -0,0 +1,242 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPlayback + +/// Placing phrase boundaries by hand. +/// +/// Two ways in, for two different situations: +/// +/// - **Correcting one boundary**, which is what forced alignment leaves +/// behind: it is right most of the time and wrong in a way the reader can +/// hear, a syllable late, so the next phrase opens with the tail of the +/// last word. That is a drag on a waveform. +/// - **Placing all of them**, for a Mac too old for `SpeechTranscriber` or a +/// system with no Chinese speech at all. The text is split into phrases +/// with no times worth having, the recording plays once, and the reader +/// presses a key as each phrase begins. A few dozen lines, always exactly +/// right, and the same editor repairs it afterwards. +extension PracticeModel { + /// Where the reader is up to while tapping boundaries in. + struct TapAlong: Equatable { + /// The phrase whose start the next tap places. Counting from one, + /// because the first phrase starts where the recording does and + /// needs no tap. + var next: Int + + /// Whether every boundary has been placed. + func isFinished(of count: Int) -> Bool { + next >= count + } + } + + // MARK: - The waveform + + /// Opens the editor, reading the recording's shape the first time. + /// + /// Read on demand rather than at load: it is tens of millions of samples + /// for a file most readers never edit, and the practice screen is + /// useful long before the waveform is. + func startEditing() { + isEditing = true + // Opening the editor shows the phrase, because a boundary cannot be + // placed without knowing which words it is meant to contain. That + // counts as having looked, exactly as "Show the answer" does, and + // saying so is better than letting a reader peek through a side + // door. + reveal() + guard peaks.isEmpty, waveformTask == nil else { return } + guard let file = audioFile else { return } + isReadingWaveform = true + waveformTask = Task { [weak self] in + let read = try? await WaveformReader.peaks(of: file) + guard let self, !Task.isCancelled else { return } + peaks = read ?? .empty + isReadingWaveform = false + waveformTask = nil + } + } + + func stopEditing() async { + isEditing = false + await cancelTapAlong() + } + + /// The span of recording the editor draws, around the phrase being + /// edited. + var editorWindow: Range { + guard let phrase else { return 0 ..< max(playback.duration, 1) } + return BoundaryEditing.window( + around: phrase, + duration: max(playback.duration, phrase.end), + ) + } + + // MARK: - Dragging a boundary + + /// Moves one end of the current phrase, and the neighbour that shares + /// it. + /// + /// Written straight through to the library rather than held and saved + /// on close: a drag is the whole edit, there is nothing to confirm, and + /// a correction lost because a sheet was dismissed is a correction the + /// reader makes twice. + func move(_ boundary: PhraseBoundary, to time: Double) async { + guard let phrase else { return } + let moved = BoundaryEditing.move( + boundary, + of: phrase.index, + to: time, + in: phrases, + duration: max(playback.duration, phrases.last?.end ?? 0), + ) + await save(moved) + } + + func apply(_ moved: [Phrase]) { + var byIndex = Dictionary(uniqueKeysWithValues: moved.map { ($0.index, $0) }) + phrases = phrases.map { byIndex.removeValue(forKey: $0.index) ?? $0 } + } + + /// Plays just the phrase being edited, so a boundary can be judged by + /// ear rather than by eye. + func playSpan() { + guard let phrase else { return } + playback.play(segment: phrase.audioRange, drill: .once) + } + + // MARK: - Tapping the boundaries in + + /// Splits the text into phrases with no times worth having, so there is + /// something to tap against. + /// + /// Equal slices, confidence zero — which is exactly what tells the + /// screen these are placeholders rather than answers. + func splitWithoutListening() async { + guard !content.isEmpty else { return } + let duration = max(playback.duration, 1) + let splitting = PhraseSplitting.forAudio( + characters: content.count, + duration: duration, + ) + let ranges = PhraseSplitter.phrases(in: content, splitting: splitting) + do { + try await services.library.savePhrases( + ForcedAlignment.spread(ranges, across: duration), + for: textID, + ) + phrases = try await services.library.phrases(for: textID) + attempts = try await services.library.attempts(for: textID) + current = phrases.first?.index ?? 0 + phase = phrases.isEmpty ? .needsAlignment : .ready + // Through `startEditing` rather than setting the flag, or the + // editor opens on a flat line: reading the recording's shape is + // part of opening it, and placeholders are the one case where + // there is nothing to do until the shape is on screen. + startEditing() + } catch { + Log.error("practice", "could not split without listening: \(error)") + phase = .unavailable(.failed(error.localizedDescription)) + } + } + + /// Starts the recording from the top, ready to be tapped through. + func beginTapAlong() { + guard phrases.count > 1 else { return } + tapping = TapAlong(next: 1) + select(phrases[0].index) + playback.clearSegment() + playback.seek(to: 0) + playback.play() + } + + /// Marks the start of the next phrase at wherever the recording is now. + /// + /// The tap also closes the phrase before it, because that is what the + /// reader means: a boundary is the line between two phrases, not two + /// separate facts. The phrases after it are left where they are and + /// pushed out of the way only as far as the rules require, so a reader + /// who taps a few and stops still has a usable text. + func tapBoundary() async { + // Through `upcoming` rather than indexing again: the bound belongs + // in one place, and that place is already the one the screen shows + // the reader. + guard var tapping, let next = upcoming else { return } + await save(BoundaryEditing.place( + boundaryBefore: next.index, + at: playback.playhead, + in: phrases, + )) + tapping.next += 1 + if tapping.isFinished(of: phrases.count) { + self.tapping = nil + } else { + self.tapping = tapping + } + select(phrases[min(tapping.next, phrases.count) - 1].index) + } + + /// Spreads whatever was never tapped over the recording that is left. + /// + /// Each tap places the boundary in *front* of a phrase, so the ones + /// still ahead keep placeholder times that are now behind the playhead. + /// Stopping three sentences into thirty would otherwise leave + /// twenty-seven stubs piled up where the reader stopped. Spread from the + /// last tap, which is where the phrase they are hearing begins and the + /// last thing about this recording anybody actually decided. + func settleUntapped() async { + guard let tapping, tapping.next <= phrases.count, tapping.next > 0 else { return } + let opened = phrases[tapping.next - 1] + await save(BoundaryEditing.respread( + from: opened.index, + after: opened.start, + in: phrases, + duration: max(playback.duration, phrases.last?.end ?? 0), + )) + } + + /// Applies moved phrases and writes them down. + func save(_ moved: [Phrase]) async { + guard !moved.isEmpty else { return } + // Applied locally first, so the strip follows the drag rather than + // the database round trip. + apply(moved) + do { + for phrase in moved { + try await services.library.movePhrase( + phrase.index, + in: textID, + start: phrase.start, + end: phrase.end, + // Carried through rather than assumed: everything a + // reader places is `manual`, and the phrases shuffled + // along behind them are not. + origin: phrase.origin, + ) + } + } catch { + Log.error("practice", "could not place a boundary: \(error)") + } + } + + func cancelTapAlong() async { + guard tapping != nil else { return } + playback.pause() + await settleUntapped() + tapping = nil + } + + /// The phrase whose start the next tap places. + var upcoming: Phrase? { + guard let tapping, tapping.next < phrases.count else { return nil } + return phrases[tapping.next] + } + + // MARK: - Where the recording is + + var audioFile: URL? { + guard let track = audioTrack else { return nil } + return AudioAttachment.file(for: track, in: services.locations) + } +} diff --git a/Sources/HanReaderUI/Models/PracticeModel.swift b/Sources/HanReaderUI/Models/PracticeModel.swift index fadbd96..354705b 100644 --- a/Sources/HanReaderUI/Models/PracticeModel.swift +++ b/Sources/HanReaderUI/Models/PracticeModel.swift @@ -51,10 +51,14 @@ final class PracticeModel { case needsWork } - private(set) var phase: Phase = .noAudio - private(set) var phrases: [Phrase] = [] - private(set) var attempts: [Int: DictationAttempt] = [:] - private(set) var current = 0 + // Settable from this module rather than `private(set)`, because this + // type's implementation spans files — `private` in Swift reaches + // extensions in the *same* file only — and the boundary editor lives in + // one of them. Nothing outside the module can see any of it. + var phase: Phase = .noAudio + var phrases: [Phrase] = [] + var attempts: [Int: DictationAttempt] = [:] + var current = 0 /// The last check, or nil if this phrase has not been answered yet. private(set) var result: DictationResult? /// Whether the transcript is on screen. Off until checked, because the @@ -65,13 +69,25 @@ final class PracticeModel { var filter: Filter = .all var drill = PlaybackModel.Drill.once + /// Whether the boundary editor is open. + var isEditing = false + /// The recording's shape, read once the editor is first opened. + var peaks = WaveformPeaks.empty + var isReadingWaveform = false + /// Where the reader is up to when tapping boundaries in by hand, or nil + /// when they are not. + var tapping: TapAlong? + let textID: TextID - private let services: AppServices - private let playback: PlaybackModel - private var content = "" - private var alignTask: Task? + let services: AppServices + let playback: PlaybackModel + private(set) var content = "" + /// The narration, kept so the boundary editor can find the file without + /// going back to the database for it. + var audioTrack: AudioTrack? + var alignTask: Task? /// Where the narration was before practice took the transport over. - private var resumeAt: Double? + var resumeAt: Double? /// Which alignment run is the current one. /// /// A cancelled task keeps running until its next suspension point, and @@ -79,7 +95,11 @@ final class PracticeModel { /// stopped run finishing late can clear the handle of the run that /// replaced it — leaving a second alignment going that nothing can /// stop. - private var alignGeneration = 0 + var alignGeneration = 0 + /// Internal rather than private because this type's implementation spans + /// files, and `private` in Swift reaches extensions in the *same* file + /// only. + var waveformTask: Task? init(textID: TextID, services: AppServices, playback: PlaybackModel) { self.textID = textID @@ -100,6 +120,7 @@ final class PracticeModel { phase = .noAudio return } + audioTrack = track let file = AudioAttachment.file(for: track, in: services.locations) guard FileManager.default.fileExists(atPath: file.path) else { phase = .noAudio @@ -125,6 +146,9 @@ final class PracticeModel { func close() async { alignTask?.cancel() alignTask = nil + waveformTask?.cancel() + waveformTask = nil + tapping = nil playback.pause() playback.clearSegment() if let resumeAt { @@ -134,7 +158,7 @@ final class PracticeModel { /// The first phrase not yet passed, so reopening a text resumes rather /// than starting the reader at the top of something half done. - private var firstUnfinished: Int { + var firstUnfinished: Int { phrases.first { attempts[$0.index]?.isPassing() != true }?.index ?? 0 } @@ -272,99 +296,6 @@ final class PracticeModel { Log.error("practice", "could not grade a phrase: \(error)") } } - - // MARK: - Alignment - - /// Works out where each phrase is said, which is what makes any of the - /// rest possible. - func align(installingModel: Bool = false) { - guard alignTask == nil else { return } - alignGeneration += 1 - let generation = alignGeneration - alignTask = Task { [weak self] in - await self?.runAlignment(installingModel: installingModel, generation: generation) - guard let self, generation == alignGeneration else { return } - alignTask = nil - } - } - - func cancelAlignment() { - alignTask?.cancel() - alignTask = nil - // Bumped so that whatever the stopped run does on its way out - // belongs to a generation nobody is listening to any more. - alignGeneration += 1 - phase = phrases.isEmpty ? .needsAlignment : .ready - } - - private func runAlignment(installingModel: Bool, generation: Int) async { - guard #available(macOS 26.0, iOS 26.0, *) else { - phase = .unavailable(.systemTooOld) - return - } - guard let track = try? await services.library.audioTrack(for: textID) else { - phase = .noAudio - return - } - let file = AudioAttachment.file(for: track, in: services.locations) - let aligner = SpeechAligner() - guard generation == alignGeneration else { return } - phase = .aligning(SpeechAlignmentProgress(fraction: nil)) - - do { - let heard = try await aligner.listen( - to: file, - installIfNeeded: installingModel, - progress: { [weak self] progress in - Task { @MainActor in self?.report(progress, from: generation) } - }, - ) - try Task.checkCancellation() - - let splitting = PhraseSplitting.forAudio( - characters: content.count, - duration: heard.duration, - ) - let placed = ForcedAlignment.align( - phrases: PhraseSplitter.phrases(in: content, splitting: splitting), - in: content, - to: heard, - ) - try await services.library.savePhrases(placed, for: textID) - // Checked again after the write, not only after the listening. - // Stop pressed while the database was busy would otherwise let - // the run finish and announce itself ready. - try Task.checkCancellation() - guard generation == alignGeneration else { return } - phrases = try await services.library.phrases(for: textID) - attempts = try await services.library.attempts(for: textID) - current = firstUnfinished - phase = .ready - } catch is CancellationError { - guard generation == alignGeneration else { return } - phase = phrases.isEmpty ? .needsAlignment : .ready - } catch let problem as SpeechAlignmentProblem { - guard generation == alignGeneration else { return } - phase = .unavailable(obstacle(for: problem)) - } catch { - guard generation == alignGeneration else { return } - phase = .unavailable(.failed(error.localizedDescription)) - } - } - - private func report(_ progress: SpeechAlignmentProgress, from generation: Int) { - guard generation == alignGeneration, case .aligning = phase else { return } - phase = .aligning(progress) - } - - private func obstacle(for problem: SpeechAlignmentProblem) -> AlignmentObstacle { - switch problem { - case .systemTooOld: .systemTooOld - case .languageUnsupported: .languageUnsupported - case .modelUnavailable: .modelMissing - case let .unreadable(reason): .failed(reason) - } - } } extension Int { diff --git a/Sources/HanReaderUI/Screens/BoundaryEditor.swift b/Sources/HanReaderUI/Screens/BoundaryEditor.swift new file mode 100644 index 0000000..9874b25 --- /dev/null +++ b/Sources/HanReaderUI/Screens/BoundaryEditor.swift @@ -0,0 +1,240 @@ +// HanReader — MIT licensed. See LICENSE. + +import HanReaderCore +import HanReaderPlayback +import SwiftUI + +/// Where a phrase starts and ends, as something to drag. +/// +/// The only place in the app that draws a waveform. Everywhere else the +/// recording is a line and a clock, because everywhere else the question is +/// "how far through am I" — here it is "does that syllable belong inside", +/// and that is a question about shape. +/// +/// ### The text is shown here +/// +/// Practice hides the transcript, and this reveals it. That is not a leak +/// but the point: a boundary cannot be placed without knowing which words it +/// is meant to contain. Opening the editor counts as giving up on the +/// phrase, the same as pressing "Show the answer", and the model records it +/// that way. +struct BoundaryEditor: View { + let model: PracticeModel + let style: ReaderStyle + + /// Where a handle has been dragged to but not yet committed. + /// + /// One write per gesture rather than one per delta, which is the lesson + /// the transport's scrubber already learnt: a frame-accurate write on + /// every drag event fights the thing reading it back. + @State private var dragging: (boundary: PhraseBoundary, time: Double)? + + var body: some View { + VStack(alignment: .leading, spacing: 12) { + header + if model.isReadingWaveform { + ProgressView("Reading the recording…") + .frame(height: ReaderMetrics.waveformHeight) + .frame(maxWidth: .infinity) + } else { + strip + } + controls + } + } + + // MARK: - What is being edited + + @ViewBuilder + private var header: some View { + if let phrase = model.phrase { + VStack(alignment: .leading, spacing: 4) { + Text(verbatim: phrase.text(in: model.content)) + .font(ReaderFont.base(style)) + .lineLimit(2) + HStack(spacing: 10) { + Text( + verbatim: "\(AudioBar.clock(phrase.start)) – \(AudioBar.clock(phrase.end))", + ) + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + if phrase.origin == .manual { + Label("Placed by you", systemImage: "hand.draw") + .font(.caption) + .foregroundStyle(.secondary) + } + } + } + } + } + + // MARK: - The strip + + private var strip: some View { + GeometryReader { geometry in + let window = model.editorWindow + let width = geometry.size.width + + ZStack(alignment: .leading) { + waveform(window: window, size: geometry.size) + if let phrase = model.phrase { + let from = x(of: start(of: phrase), in: window, width: width) + let to = x(of: end(of: phrase), in: window, width: width) + Rectangle() + .fill(ReaderColor.highlightWash) + .frame(width: max(0, to - from)) + .offset(x: from) + handle(.start, at: from, window: window, width: width) + handle(.end, at: to, window: window, width: width) + } + } + .contentShape(Rectangle()) + } + .frame(height: ReaderMetrics.waveformHeight) + .background { + RoundedRectangle(cornerRadius: ReaderMetrics.tokenCornerRadius) + .fill(.quaternary.opacity(0.4)) + } + } + + /// The recording's shape, bar by bar, with the phrase's own bars in the + /// accent colour. + private func waveform(window: Range, size: CGSize) -> some View { + Canvas { context, size in + let bars = max(1, Int(size.width / 2)) + let samples = model.peaks.resampled(over: window, count: bars) + let step = size.width / CGFloat(bars) + let middle = size.height / 2 + let span = model.phrase.map { start(of: $0) ... end(of: $0) } + + for (index, value) in samples.enumerated() { + let x = CGFloat(index) * step + // A floor, so silence is a visible line rather than a gap — + // a strip that disappears where the speaker paused reads as + // a drawing failure. + let height = max(1, CGFloat(value) * size.height * 0.9) + let time = window.lowerBound + + Double(index) / Double(bars) * (window.upperBound - window.lowerBound) + context.fill( + Path(CGRect(x: x, y: middle - height / 2, width: step * 0.8, height: height)), + with: .color( + span?.contains(time) == true + ? ReaderColor.waveformInPhrase + : ReaderColor.waveform, + ), + ) + } + } + .frame(width: size.width, height: size.height) + } + + /// One draggable edge. + private func handle( + _ boundary: PhraseBoundary, + at position: CGFloat, + window: Range, + width: CGFloat, + ) + -> some View + { + Rectangle() + .fill(ReaderColor.highlight) + .frame(width: 2) + .frame(maxHeight: .infinity) + .overlay { + // The grab area, far wider than the line and deliberately + // invisible: a one-point target cannot be hit with a mouse, + // let alone a finger. + Color.clear + .frame(width: ReaderMetrics.waveformHandleWidth) + .contentShape(Rectangle()) + .gesture( + DragGesture(minimumDistance: 1) + .onChanged { value in + dragging = ( + boundary, + time( + of: position + value.translation.width, + in: window, + width: width, + ), + ) + } + .onEnded { value in + let target = time( + of: position + value.translation.width, + in: window, + width: width, + ) + dragging = nil + Task { await model.move(boundary, to: target) } + }, + ) + } + .offset(x: position - 1) + .accessibilityLabel( + boundary == .start ? Text("Phrase start") : Text("Phrase end"), + ) + .accessibilityAdjustableAction { direction in + nudge(boundary, by: direction == .increment ? 0.1 : -0.1) + } + } + + // MARK: - Controls + + private var controls: some View { + HStack(spacing: 12) { + Button { + model.playSpan() + } label: { + Label("Play this phrase", systemImage: "play.fill") + } + .font(.caption) + + Button("Trim start") { nudge(.start, by: 0.1) } + .font(.caption) + Button("Widen start") { nudge(.start, by: -0.1) } + .font(.caption) + + Spacer() + Button("Tap them all in") { model.beginTapAlong() } + .font(.caption) + .disabled(model.phrases.count < 2) + Button("Done") { Task { await model.stopEditing() } } + .font(.caption) + } + } + + /// Moves a boundary by a fixed step, for a keyboard or VoiceOver. + /// + /// Dragging is not the only way in: a tenth of a second is about the + /// smallest difference that matters here and about the smallest anyone + /// can aim at, so the same step serves both. + private func nudge(_ boundary: PhraseBoundary, by seconds: Double) { + guard let phrase = model.phrase else { return } + let from = boundary == .start ? phrase.start : phrase.end + Task { await model.move(boundary, to: from + seconds) } + } + + // MARK: - Time and position + + private func start(of phrase: Phrase) -> Double { + dragging?.boundary == .start ? dragging?.time ?? phrase.start : phrase.start + } + + private func end(of phrase: Phrase) -> Double { + dragging?.boundary == .end ? dragging?.time ?? phrase.end : phrase.end + } + + private func x(of time: Double, in window: Range, width: CGFloat) -> CGFloat { + let span = window.upperBound - window.lowerBound + guard span > 0 else { return 0 } + return CGFloat((time - window.lowerBound) / span) * width + } + + private func time(of x: CGFloat, in window: Range, width: CGFloat) -> Double { + guard width > 0 else { return window.lowerBound } + let span = window.upperBound - window.lowerBound + return window.lowerBound + Double(x / width) * span + } +} diff --git a/Sources/HanReaderUI/Screens/PracticeComponents.swift b/Sources/HanReaderUI/Screens/PracticeComponents.swift index c9c0091..1769412 100644 --- a/Sources/HanReaderUI/Screens/PracticeComponents.swift +++ b/Sources/HanReaderUI/Screens/PracticeComponents.swift @@ -190,3 +190,60 @@ struct GradeRow: View { } } } + +/// Tapping the phrase boundaries in while the recording plays. +/// +/// The path for a system with no Chinese speech recognition, and the repair +/// for a recording the recogniser could not follow. It is a few dozen lines +/// and it is always exactly right, which is why it is here rather than a +/// second alignment algorithm guessing from silence. +struct TapAlongBar: View { + let model: PracticeModel + let style: ReaderStyle + + var body: some View { + VStack(alignment: .leading, spacing: 8) { + HStack(spacing: 8) { + Text("Press Mark as each phrase begins.") + .font(.callout.weight(.medium)) + Text("⌘↩") + .font(.caption) + .foregroundStyle(.tertiary) + } + if let next = model.upcoming { + Text(verbatim: next.text(in: model.content)) + .font(ReaderFont.base(style)) + .lineLimit(2) + .foregroundStyle(.secondary) + } + HStack(spacing: 12) { + // ⌘↩ rather than the obvious ⌘M: that is the system's + // Minimize, and a view's shortcut competing with a menu + // command is a fight to lose. ⌘↩ is free here — the writing + // field is not on screen while this is — and it already + // means "commit this" everywhere else on the screen. + Button("Mark") { + Task { await model.tapBoundary() } + } + .buttonStyle(.borderedProminent) + .keyboardShortcut(.return, modifiers: .command) + + if let tapping = model.tapping { + Text(verbatim: "\(tapping.next) / \(model.phrases.count - 1)") + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + } + Spacer() + Button("Stop", role: .cancel) { + Task { await model.cancelTapAlong() } + } + .font(.caption) + } + } + .padding(ReaderMetrics.readingColumnPadding / 2) + .background { + RoundedRectangle(cornerRadius: ReaderMetrics.tokenCornerRadius) + .fill(ReaderColor.highlightWash) + } + } +} diff --git a/Sources/HanReaderUI/Screens/PracticeScreen.swift b/Sources/HanReaderUI/Screens/PracticeScreen.swift index 67d01f9..1bb336e 100644 --- a/Sources/HanReaderUI/Screens/PracticeScreen.swift +++ b/Sources/HanReaderUI/Screens/PracticeScreen.swift @@ -59,7 +59,14 @@ struct PracticeScreen: View { ScrollView { VStack(alignment: .leading, spacing: 16) { PhraseTransport(model: model, playback: playback) - writing + if model.tapping != nil { + TapAlongBar(model: model, style: style) + } else { + if model.isEditing { + BoundaryEditor(model: model, style: style) + } + writing + } if let result = model.result { DictationMarkedView( result: result, @@ -131,6 +138,18 @@ struct PracticeScreen: View { ToolbarItem { DrillMenu(model: model) } + ToolbarItem { + Button { + if model.isEditing { + Task { await model.stopEditing() } + } else { + model.startEditing() + } + } label: { + Label("Edit boundaries", systemImage: "waveform.path") + } + .help("Drag where this phrase starts and ends.") + } } } @@ -152,8 +171,18 @@ private struct AlignmentInvitation: View { not sent anywhere. """) } actions: { - Button("Listen to the recording") { model.align() } - .buttonStyle(.borderedProminent) + VStack(spacing: 8) { + Button("Listen to the recording") { model.align() } + .buttonStyle(.borderedProminent) + // Offered here too, not only after something has gone + // wrong: a reader who already knows the recogniser cannot + // follow this recording should not have to wait for it to + // fail before they can do it themselves. + Button("Place the phrases myself") { + Task { await model.splitWithoutListening() } + } + .font(.caption) + } } } } @@ -200,9 +229,23 @@ private struct AlignmentObstacleView: View { } description: { Text(verbatim: explanation) } actions: { - if obstacle.canInstall { - Button("Download the model") { model.align(installingModel: true) } - .buttonStyle(.borderedProminent) + VStack(spacing: 8) { + if obstacle.canInstall { + Button("Download the model") { model.align(installingModel: true) } + .buttonStyle(.borderedProminent) + } + // Always offered, including when the model could be + // installed: a reader who will not download several hundred + // megabytes should still be able to practise, and this is + // the path that always works. + let myself = Button("Place the phrases myself") { + Task { await model.splitWithoutListening() } + } + if obstacle.canInstall { + myself.buttonStyle(.bordered) + } else { + myself.buttonStyle(.borderedProminent) + } } } } @@ -226,8 +269,8 @@ private struct AlignmentObstacleView: View { """ case .systemTooOld: """ - Finding phrases automatically needs macOS 26 or later. Placing \ - them by hand is not possible yet. + Finding phrases automatically needs macOS 26 or later. You can \ + still practise by placing the phrase boundaries yourself. """ case .languageUnsupported: "This system has no Chinese speech recognition to offer." diff --git a/Tests/HanReaderCoreTests/BoundaryEditingTests.swift b/Tests/HanReaderCoreTests/BoundaryEditingTests.swift new file mode 100644 index 0000000..f35ce53 --- /dev/null +++ b/Tests/HanReaderCoreTests/BoundaryEditingTests.swift @@ -0,0 +1,312 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import Testing + +@Suite("Moving a phrase boundary") +struct BoundaryEditingTests { + /// Three phrases that meet, which is what alignment usually emits. + private func abutting() -> [Phrase] { + [ + Phrase(index: 0, start: 0, end: 5, textRange: 0 ..< 5), + Phrase(index: 1, start: 5, end: 10, textRange: 5 ..< 10), + Phrase(index: 2, start: 10, end: 15, textRange: 10 ..< 15), + ] + } + + /// The same three with silence left out between them, which alignment + /// also emits wherever the speaker paused. + private func spaced() -> [Phrase] { + [ + Phrase(index: 0, start: 0, end: 4, textRange: 0 ..< 5), + Phrase(index: 1, start: 5, end: 9, textRange: 5 ..< 10), + Phrase(index: 2, start: 10, end: 14, textRange: 10 ..< 15), + ] + } + + private func move( + _ boundary: PhraseBoundary, + of index: Int, + to time: Double, + in phrases: [Phrase], + duration: Double = 15, + ) + -> [Phrase] + { + BoundaryEditing.move(boundary, of: index, to: time, in: phrases, duration: duration) + } + + // MARK: - A boundary two phrases share + + /// Where two phrases meet, the line between them is one thing. Moving + /// only one side would open a gap, or an overlap, that the reader never + /// asked for and would then have to repair from the other phrase. + @Test("Dragging a shared start moves the phrase before it too") + func sharedStartMovesBoth() { + let moved = move(.start, of: 1, to: 4, in: abutting()) + #expect(moved.map(\.index) == [0, 1]) + #expect(moved[0].end == 4) + #expect(moved[1].start == 4) + } + + @Test("Dragging a shared end moves the phrase after it too") + func sharedEndMovesBoth() { + let moved = move(.end, of: 1, to: 11, in: abutting()) + #expect(moved.map(\.index) == [1, 2]) + #expect(moved[0].end == 11) + #expect(moved[1].start == 11) + } + + // MARK: - A gap between two phrases + + /// Silence the alignment deliberately left out is not a boundary to + /// share. Here the handles are independent and a drag stops at the + /// neighbour rather than pushing it. + @Test("Across a gap only the dragged phrase moves") + func gapLeavesTheNeighbourAlone() { + let moved = move(.start, of: 1, to: 4.5, in: spaced()) + #expect(moved.map(\.index) == [1]) + #expect(moved[0].start == 4.5) + } + + @Test("A drag across a gap stops at the neighbour") + func gapClampsAtTheNeighbour() { + let moved = move(.start, of: 1, to: 1, in: spaced()) + #expect(moved.map(\.index) == [1]) + // Phrase 0 ends at 4, so phrase 1 cannot start before it. + #expect(moved[0].start == 4) + } + + // MARK: - What a drag may not do + + @Test("A phrase cannot be dragged shorter than the minimum") + func minimumDuration() { + let moved = move(.start, of: 1, to: 9.9, in: abutting()) + #expect(moved.last?.start == 10 - BoundaryEditing.minimumDuration) + #expect(moved.last?.duration == BoundaryEditing.minimumDuration) + } + + /// Dragging a shared boundary must not collapse the *neighbour* either, + /// which is the direction it is easy to forget: the clamp that protects + /// the phrase being dragged does nothing for the one coming with it. + @Test("Dragging a shared boundary cannot collapse the neighbour") + func neighbourKeepsItsMinimum() { + let moved = move(.start, of: 1, to: 0, in: abutting()) + #expect(moved.first?.index == 0) + #expect(moved.first?.duration == BoundaryEditing.minimumDuration) + #expect(moved.last?.start == BoundaryEditing.minimumDuration) + } + + @Test("A phrase cannot be dragged shorter than the minimum from its end") + func minimumDurationFromTheEnd() { + let moved = move(.end, of: 1, to: 5.1, in: abutting()) + #expect(moved.first?.index == 1) + #expect(moved.first?.end == 5 + BoundaryEditing.minimumDuration) + #expect(moved.first?.duration == BoundaryEditing.minimumDuration) + } + + /// The mirror of the start case, and just as easy to forget: the clamp + /// that protects the phrase being dragged does nothing for the one that + /// comes with it. + @Test("Dragging a shared end cannot collapse the next phrase") + func nextKeepsItsMinimum() { + let moved = move(.end, of: 1, to: 15, in: abutting()) + #expect(moved.map(\.index) == [1, 2]) + #expect(moved[0].end == 15 - BoundaryEditing.minimumDuration) + #expect(moved[1].duration == BoundaryEditing.minimumDuration) + } + + @Test("Across a gap only the dragged phrase's end moves") + func gapLeavesTheNextPhraseAlone() { + let moved = move(.end, of: 1, to: 9.5, in: spaced()) + #expect(moved.map(\.index) == [1]) + #expect(moved[0].end == 9.5) + } + + @Test("The last phrase cannot be dragged past the end of the recording") + func endOfRecording() { + let moved = move(.end, of: 2, to: 99, in: spaced(), duration: 15) + #expect(moved.last?.end == 15) + } + + @Test("The first phrase cannot be dragged before the start") + func startOfRecording() { + // Starting at 2 rather than 0, so there is somewhere to be dragged + // *from* and the clamp is what stops it rather than the drag being + // a no-op. + let phrases = [Phrase(index: 0, start: 2, end: 6, textRange: 0 ..< 5)] + let moved = move(.start, of: 0, to: -3, in: phrases) + #expect(moved.last?.start == 0) + } + + @Test("A drag that changes nothing reports nothing") + func noChange() { + #expect(move(.start, of: 1, to: 5, in: abutting()).isEmpty) + #expect(move(.end, of: 1, to: 10, in: abutting()).isEmpty) + } + + @Test("A phrase that is not there cannot be dragged") + func unknownPhrase() { + #expect(move(.start, of: 99, to: 3, in: abutting()).isEmpty) + } + + // MARK: - Tapping a boundary in + + /// A tap is a statement about both phrases, made while the recording + /// plays — so unlike a drag it closes whatever silence the alignment + /// left between them. + @Test("A tap closes the gap between two phrases") + func tapClosesTheGap() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced()) + #expect(placed.map(\.index) == [0, 1]) + #expect(placed[0].end == 4.5) + #expect(placed[1].start == 4.5) + } + + @Test("A tap before the previous phrase has begun is held off") + func tapCannotCollapseThePreviousPhrase() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 0, in: spaced()) + #expect(placed[0].duration == BoundaryEditing.minimumDuration) + #expect(placed[1].start == BoundaryEditing.minimumDuration) + } + + @Test("A tap after the next phrase has ended is held back") + func tapCannotCollapseTheNextPhrase() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 99, in: spaced()) + // Phrase 1 ends at 9, so the boundary stops short of it. + #expect(placed[1].start == 9 - BoundaryEditing.minimumDuration) + #expect(placed[1].duration == BoundaryEditing.minimumDuration) + } + + /// The first phrase opens where the recording does, so there is no + /// boundary in front of it to tap. + @Test("There is no boundary before the first phrase") + func noBoundaryBeforeTheFirst() { + #expect(BoundaryEditing.place(boundaryBefore: 0, at: 1, in: spaced()).isEmpty) + } + + @Test("A tap on a phrase that is not there places nothing") + func tapOnAnUnknownPhrase() { + #expect(BoundaryEditing.place(boundaryBefore: 99, at: 1, in: spaced()).isEmpty) + } + + @Test("A tap exactly where the boundary already is changes nothing") + func tapOnTheSamePlace() { + #expect(BoundaryEditing.place(boundaryBefore: 1, at: 5, in: abutting()).isEmpty) + } + + @Test("A tapped boundary is marked by hand on both sides") + func tapRecordsItsOrigin() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced()) + #expect(placed.allSatisfy { $0.origin == .manual }) + } + + // MARK: - Spreading what was never reached + + @Test("Untapped phrases share the recording that is left") + func respreadSharesWhatRemains() { + let spread = BoundaryEditing.respread(from: 1, after: 6, in: spaced(), duration: 16) + #expect(spread.map(\.index) == [1, 2]) + #expect(spread[0].start == 6) + #expect(spread[0].end == 11) + #expect(spread[1].start == 11) + #expect(spread[1].end == 16) + } + + /// Nobody placed these, so a later re-alignment must be free to replace + /// them — which is exactly what `manual` would prevent. + @Test("Spreading does not claim anybody placed them") + func respreadKeepsItsOrigin() { + let phrases = [ + Phrase(index: 0, start: 0, end: 4, textRange: 0 ..< 5, origin: .manual), + Phrase(index: 1, start: 5, end: 9, textRange: 5 ..< 10, origin: .recognised), + ] + let spread = BoundaryEditing.respread(from: 0, after: 0, in: phrases, duration: 16) + #expect(spread[0].origin == .manual) + #expect(spread[1].origin == .recognised) + } + + @Test("Spreading past the end of the recording still leaves room to play") + func respreadWithNoRoomLeft() { + let spread = BoundaryEditing.respread(from: 0, after: 15, in: spaced(), duration: 15) + #expect(spread.count == 3) + #expect(spread.allSatisfy { $0.duration >= BoundaryEditing.minimumDuration }) + } + + @Test("Spreading from a phrase that is not there spreads nothing") + func respreadFromAnUnknownPhrase() { + #expect(BoundaryEditing.respread(from: 99, after: 0, in: spaced(), duration: 15).isEmpty) + } + + // MARK: - What a move records + + /// A boundary somebody placed is not a guess, and re-aligning must not + /// overwrite it — which is what `origin` is for. + @Test("A moved phrase is marked manual and fully confident") + func moveRecordsItsOrigin() { + let phrases = [Phrase( + index: 0, + start: 0, + end: 5, + textRange: 0 ..< 5, + confidence: 0.2, + origin: .recognised, + grade: .hard, + )] + let moved = move(.end, of: 0, to: 6, in: phrases) + #expect(moved.last?.origin == .manual) + #expect(moved.last?.confidence == 1) + // And it is still the same phrase: same words, same grade. + #expect(moved.last?.textRange == 0 ..< 5) + #expect(moved.last?.grade == .hard) + } + + // MARK: - What to draw + + /// A boundary cannot be judged from the phrase alone — the question is + /// always whether the syllable just outside belongs inside. + @Test("The window shows context either side of the phrase") + func windowAddsContext() { + let window = BoundaryEditing.window( + around: Phrase(index: 1, start: 5, end: 10, textRange: 5 ..< 10), + duration: 60, + context: 1.5, + ) + #expect(window.lowerBound == 3.5) + #expect(window.upperBound == 11.5) + } + + /// At either end of a file the window keeps its width instead of + /// shrinking, so the strip does not get smaller exactly where a reader + /// is most likely to be fixing the first or last boundary. + @Test("The window keeps its width at the ends of the recording") + func windowKeepsItsWidthAtTheEnds() { + let first = BoundaryEditing.window( + around: Phrase(index: 0, start: 0, end: 5, textRange: 0 ..< 5), + duration: 60, + context: 1.5, + ) + #expect(first.lowerBound == 0) + #expect(first.upperBound - first.lowerBound == 8) + + let last = BoundaryEditing.window( + around: Phrase(index: 9, start: 55, end: 60, textRange: 0 ..< 5), + duration: 60, + context: 1.5, + ) + #expect(last.upperBound == 60) + #expect(last.upperBound - last.lowerBound == 8) + } + + @Test("A window wider than the recording is the whole recording") + func windowCannotExceedTheRecording() { + let window = BoundaryEditing.window( + around: Phrase(index: 0, start: 0, end: 4, textRange: 0 ..< 5), + duration: 4, + context: 10, + ) + #expect(window.lowerBound == 0) + #expect(window.upperBound == 4) + } +} diff --git a/Tests/HanReaderPlaybackTests/WaveformPeaksTests.swift b/Tests/HanReaderPlaybackTests/WaveformPeaksTests.swift new file mode 100644 index 0000000..973a079 --- /dev/null +++ b/Tests/HanReaderPlaybackTests/WaveformPeaksTests.swift @@ -0,0 +1,177 @@ +// HanReader — MIT licensed. See LICENSE. + +import AVFoundation +import Foundation +import Testing +@testable import HanReaderPlayback + +/// The shape of a recording, and the arithmetic that draws a span of it. +/// +/// The reading half is exercised against a file written here rather than a +/// committed fixture: an audio fixture would be a binary in the repository +/// for something a few lines of `AVAudioFile` can produce exactly, with the +/// signal known in advance so the assertions can be about values rather than +/// about "it returned something". +@Suite("Waveform peaks") +struct WaveformPeaksTests { + // MARK: - Reading a file + + /// Writes a mono 44.1 kHz file whose amplitude is `level(second)`. + private func write(seconds: Int, level: (Int) -> Float) throws -> URL { + let rate = 44100.0 + let format = try #require(AVAudioFormat(standardFormatWithSampleRate: rate, channels: 1)) + let url = URL.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).caf") + let file = try AVAudioFile(forWriting: url, settings: format.settings) + for second in 0 ..< seconds { + let buffer = try #require(AVAudioPCMBuffer( + pcmFormat: format, + frameCapacity: AVAudioFrameCount(rate), + )) + buffer.frameLength = AVAudioFrameCount(rate) + let amplitude = level(second) + let channel = try #require(buffer.floatChannelData) + for frame in 0 ..< Int(rate) { + // A sine rather than a constant, so the peak per bucket is a + // real maximum over a varying signal rather than the one + // value every sample happens to hold. + let phase = Double(frame) / rate * 440 * 2 * .pi + channel[0][frame] = amplitude * Float(sin(phase)) + } + try file.write(from: buffer) + } + return url + } + + @Test("A recording's loud and quiet seconds come back as loud and quiet buckets") + func readsTheShape() async throws { + // Loud, silent, half — so the test can tell order, silence and + // relative level apart. + let url = try write(seconds: 3) { second in + switch second { + case 0: 0.8 + case 1: 0 + default: 0.4 + } + } + defer { try? FileManager.default.removeItem(at: url) } + + let peaks = try await WaveformReader.peaks(of: url, buckets: 30) + #expect(peaks.values.count == 30) + #expect(abs(peaks.duration - 3) < 0.1) + + // Normalised, so the loudest second reaches one. + #expect(peaks.value(at: 0.5) > 0.9) + #expect(peaks.value(at: 1.5) < 0.05) + // Half the amplitude of the first second, give or take the bucket + // edges. + let quiet = peaks.value(at: 2.5) + #expect(quiet > 0.4 && quiet < 0.6) + } + + @Test("A file that is not audio comes back empty rather than throwing") + func notAudio() async throws { + let url = URL.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).m4a") + try Data("not audio".utf8).write(to: url) + defer { try? FileManager.default.removeItem(at: url) } + + // Either answer is acceptable — what must not happen is a crash or a + // hang. An unreadable narration is a file the reader chose, and the + // screen above shows the failure. + let peaks = try? await WaveformReader.peaks(of: url, buckets: 10) + #expect(peaks?.isEmpty ?? true) + } + + // MARK: - Normalising + + /// A quiet recording is otherwise a flat line, which tells the reader + /// nothing about where the words are. + @Test("The loudest point becomes one") + func normalising() { + #expect(WaveformReader.normalised([0.1, 0.2, 0.05]) == [0.5, 1.0, 0.25]) + } + + @Test("Silence stays silent rather than dividing by zero") + func normalisingSilence() { + #expect(WaveformReader.normalised([0, 0, 0]) == [0, 0, 0]) + #expect(WaveformReader.normalised([]).isEmpty) + } + + // MARK: - Drawing a span + + private let ramp = WaveformPeaks( + values: [0, 0.25, 0.5, 0.75, 1, 0.75, 0.5, 0.25], + duration: 8, + ) + + @Test("A span is resampled to the width it will be drawn at") + func resampling() { + #expect(ramp.resampled(over: 0 ..< 8, count: 4) == [0.25, 0.75, 1, 0.5]) + } + + /// Maximum rather than mean across an output bucket, so a short loud + /// moment stays visible when the strip is zoomed out — which is exactly + /// the moment a reader is looking for when they place a boundary. + @Test("Zooming out keeps a peak rather than averaging it away") + func resamplingKeepsPeaks() { + let spike = WaveformPeaks(values: [0, 0, 1, 0, 0, 0, 0, 0], duration: 8) + #expect(spike.resampled(over: 0 ..< 8, count: 2) == [1, 0]) + } + + @Test("A span narrower than the recording draws only that span") + func resamplingASpan() { + #expect(ramp.resampled(over: 4 ..< 8, count: 2) == [1, 0.5]) + } + + /// A span wholly outside the recording used to build a range whose + /// lower bound was above its upper one, and `Range` traps on that. The + /// editor asks for exactly this the moment a phrase's times outlive the + /// recording they were measured against — a narration replaced by a + /// shorter file. + @Test("A span outside the recording draws nothing rather than trapping") + func spanOutsideTheRecording() { + #expect(ramp.resampled(over: 20 ..< 24, count: 2) == [0, 0]) + #expect(ramp.resampled(over: -9 ..< -1, count: 2) == [0, 0]) + // Straddling the end: the part that exists is drawn, the rest is not. + let straddling = ramp.resampled(over: 6 ..< 14, count: 2) + #expect(straddling[0] > 0) + #expect(straddling[1] == 0) + + // A recording that opens loudly, so "nothing" cannot be mistaken for + // "the first bucket" — which is what a bucket outside the recording + // would read if it fell back to the start of the array. + let loudOpening = WaveformPeaks(values: [1, 0.5, 0.25], duration: 3) + #expect(loudOpening.resampled(over: 10 ..< 12, count: 2) == [0, 0]) + } + + /// Zoomed in far enough that an output bucket is narrower than an input + /// one, every bucket still reads the sample under it rather than coming + /// back empty. + @Test("Zooming past the stored resolution still draws the signal") + func resamplingBelowTheStoredResolution() { + let zoomed = ramp.resampled(over: 4 ..< 4.4, count: 8) + #expect(zoomed.allSatisfy { $0 == 1 }) + } + + @Test("A span of no width, or no buckets, draws nothing") + func degenerateSpans() { + #expect(ramp.resampled(over: 4 ..< 4, count: 3) == [0, 0, 0]) + #expect(ramp.resampled(over: 0 ..< 8, count: 0).isEmpty) + #expect(WaveformPeaks.empty.resampled(over: 0 ..< 8, count: 2) == [0, 0]) + } + + @Test("A time outside the recording has no value") + func outsideTheRecording() { + #expect(ramp.value(at: -1) == 0) + #expect(ramp.value(at: 8) == 0) + #expect(ramp.value(at: 100) == 0) + #expect(WaveformPeaks.empty.value(at: 0) == 0) + } + + @Test("A time inside the recording reads its own bucket") + func insideTheRecording() { + // Eight buckets over eight seconds, so one each. + #expect(ramp.value(at: 0.5) == 0) + #expect(ramp.value(at: 4.5) == 1) + #expect(ramp.value(at: 7.9) == 0.25) + } +} diff --git a/Tests/HanReaderUITests/PracticeBoundaryTests.swift b/Tests/HanReaderUITests/PracticeBoundaryTests.swift new file mode 100644 index 0000000..00867c8 --- /dev/null +++ b/Tests/HanReaderUITests/PracticeBoundaryTests.swift @@ -0,0 +1,263 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPersistence +import HanReaderPlayback +import Testing +@testable import HanReaderUI + +/// Placing phrase boundaries by hand: dragging one, and tapping them all in. +@MainActor +@Suite("Phrase boundaries") +struct PracticeBoundaryTests { + private typealias Support = PracticeTestSupport + + /// A model over three phrases that meet, with the transport reporting a + /// known length so the clamps have something to clamp against. + private func editing() async throws -> (model: PracticeModel, engine: SilentEngine) { + let (services, id) = try await Support.services(content: Support.content) + try await services.library.savePhrases( + (0 ..< 3).map { Support.phrase($0) }, + for: id, + ) + let engine = SilentEngine() + let playback = PlaybackModel(engine: engine, session: SilentSession(), wait: { _ in }) + let model = PracticeModel(textID: id, services: services, playback: playback) + await model.load() + // The transport learns a recording's length from the engine, and + // nothing here loads a real file. + engine.onDuration?(15) + return (model, engine) + } + + // MARK: - Dragging one boundary + + @Test("Dragging a boundary moves the phrase and the one it shares with") + func draggingMovesBoth() async throws { + let (model, _) = try await editing() + model.select(1) + await model.move(.start, to: 4) + + #expect(model.phrases[0].end == 4) + #expect(model.phrases[1].start == 4) + } + + /// Written straight through rather than held until the editor closes: a + /// correction lost because a sheet was dismissed is a correction the + /// reader makes twice. + @Test("A dragged boundary is saved at once") + func draggingPersists() async throws { + let (model, _) = try await editing() + model.select(1) + await model.move(.start, to: 4) + + let stored = try await model.services.library.phrases(for: model.textID) + #expect(stored[0].end == 4) + #expect(stored[1].start == 4) + // Marked by hand, which is what stops a later re-alignment + // overwriting it. + #expect(stored[1].origin == .manual) + } + + @Test("A drag the rules do not allow changes nothing") + func impossibleDragChangesNothing() async throws { + let (model, _) = try await editing() + model.select(1) + await model.move(.start, to: 5) + + let stored = try await model.services.library.phrases(for: model.textID) + #expect(stored.allSatisfy { $0.origin == .recognised }) + } + + /// The editor shows the transcript, because a boundary cannot be placed + /// without knowing which words it is meant to contain — so opening it + /// counts as having looked. + @Test("Opening the editor reveals the phrase") + func editingReveals() async throws { + let (model, _) = try await editing() + #expect(!model.isRevealed) + model.startEditing() + #expect(model.isEditing) + #expect(model.isRevealed) + + await model.stopEditing() + #expect(!model.isEditing) + } + + @Test("The editor draws context either side of the phrase") + func editorWindow() async throws { + let (model, _) = try await editing() + model.select(1) + // Phrase 1 runs 5 to 9, so the window is that plus context. + #expect(model.editorWindow.lowerBound < 5) + #expect(model.editorWindow.upperBound > 9) + } + + // MARK: - Without any recognition + + /// For a Mac too old for `SpeechTranscriber`, or a system with no + /// Chinese speech at all. + @Test("Splitting without listening gives phrases to tap against") + func splittingWithoutListening() async throws { + let (services, id) = try await Support.services(content: Support.content) + let engine = SilentEngine() + let model = PracticeModel( + textID: id, + services: services, + playback: PlaybackModel(engine: engine, session: SilentSession(), wait: { _ in }), + ) + await model.load() + engine.onDuration?(30) + #expect(model.phase == .needsAlignment) + + await model.splitWithoutListening() + #expect(model.phase == .ready) + #expect(model.phrases.count == 3) + // Confidence zero throughout, which is what tells the screen these + // are placeholders rather than answers. + #expect(model.phrases.allSatisfy { $0.confidence == 0 }) + #expect(model.phrases.last?.end == 30) + // And the editor is open, because placeholders are the one case + // where there is nothing to practise until they are fixed. + #expect(model.isEditing) + } + + // MARK: - Tapping them in + + @Test("Tapping along starts at the first phrase, from the top of the recording") + func tapAlongStarts() async throws { + let (model, engine) = try await editing() + model.select(2) + engine.onTime?(7) + + model.beginTapAlong() + #expect(model.tapping?.next == 1) + #expect(model.current == 0) + #expect(model.upcoming?.index == 1) + // Rewound, or the first tap would place a boundary against whatever + // the reader happened to be listening to a moment earlier. + #expect(model.playback.playhead == 0) + } + + /// A tap is one fact, not two: it closes the phrase that was playing and + /// opens the next. The fixture's phrases have silence between them, so + /// this also pins that a tap closes the gap — which is what a reader + /// tapping sentences in means, and what a drag deliberately does not do. + @Test("A tap places the boundary where the recording is") + func tapPlacesABoundary() async throws { + let (model, engine) = try await editing() + model.beginTapAlong() + engine.onTime?(4.5) + await model.tapBoundary() + + #expect(model.phrases[0].end == 4.5) + #expect(model.phrases[1].start == 4.5) + #expect(model.tapping?.next == 2) + // And the reader is now looking at the phrase they just opened. + #expect(model.current == 1) + } + + @Test("Tapping through the last boundary ends it") + func tapAlongFinishes() async throws { + let (model, engine) = try await editing() + model.beginTapAlong() + engine.onTime?(4.5) + await model.tapBoundary() + engine.onTime?(8) + await model.tapBoundary() + + #expect(model.tapping == nil) + #expect(model.upcoming == nil) + #expect(model.phrases[1].end == 8) + #expect(model.phrases[2].start == 8) + } + + @Test("A tap with nothing left to place does nothing") + func tapAfterTheEnd() async throws { + let (model, engine) = try await editing() + model.beginTapAlong() + engine.onTime?(4.5) + await model.tapBoundary() + engine.onTime?(8) + await model.tapBoundary() + + let before = model.phrases + engine.onTime?(12) + await model.tapBoundary() + #expect(model.phrases == before) + } + + @Test("Stopping leaves the boundaries already placed") + func cancellingKeepsWhatWasPlaced() async throws { + let (model, engine) = try await editing() + model.beginTapAlong() + engine.onTime?(4.5) + await model.tapBoundary() + await model.cancelTapAlong() + + #expect(model.tapping == nil) + let stored = try await model.services.library.phrases(for: model.textID) + #expect(stored[0].end == 4.5) + #expect(stored[1].start == 4.5) + // What was placed is marked by hand; what was merely shuffled out + // of the way is not, so a later re-alignment is free to replace it. + #expect(stored[0].origin == .manual) + #expect(stored[2].origin == .recognised) + } + + /// The bound lives here and `tapBoundary` goes through it, so this is + /// the one place it has to hold — including for a position past the end + /// that the normal run of taps never reaches. + @Test("A position past the last phrase has nothing upcoming") + func upcomingIsBounded() async throws { + let (model, _) = try await editing() + model.tapping = PracticeModel.TapAlong(next: model.phrases.count) + #expect(model.upcoming == nil) + + model.tapping = PracticeModel.TapAlong(next: model.phrases.count - 1) + #expect(model.upcoming?.index == model.phrases.count - 1) + } + + /// Each tap places the boundary in front of a phrase, so the ones still + /// ahead keep placeholder times that are now behind the playhead. + /// Stopping early would otherwise leave them collapsed to the minimum, + /// stacked up where the reader stopped. + @Test("Stopping early spreads what was never tapped over what is left") + func stoppingSpreadsTheRemainder() async throws { + let (model, engine) = try await editing() + model.beginTapAlong() + engine.onTime?(4.5) + await model.tapBoundary() + await model.cancelTapAlong() + + let stored = try await model.services.library.phrases(for: model.textID) + // Phrases 1 and 2 share the 10.5 seconds after the tap rather than + // sitting at the shortest length the rules allow. + #expect(stored[1].duration > BoundaryEditing.minimumDuration * 2) + #expect(stored[2].duration > BoundaryEditing.minimumDuration * 2) + #expect(stored[1].end == stored[2].start) + #expect(stored.last?.end == 15) + } + + @Test("A text with one phrase has no boundaries to tap") + func nothingToTap() async throws { + let (services, id) = try await Support.services(content: "一句话。") + try await services.library.savePhrases( + [Phrase(index: 0, start: 0, end: 4, textRange: 0 ..< 4)], + for: id, + ) + let model = Support.model(textID: id, services: services) + await model.load() + model.beginTapAlong() + #expect(model.tapping == nil) + } + + @Test("Closing the screen stops tapping along") + func closingStopsTapping() async throws { + let (model, _) = try await editing() + model.beginTapAlong() + await model.close() + #expect(model.tapping == nil) + } +} From 32b0eb658ad2ea58aca544427ab51693c8c64190 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:02:27 +0300 Subject: [PATCH 2/2] fix: answer the review on the boundary editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight findings, all real. Two of them matter. **A tap was pinned to a placeholder rather than to the playhead.** `place` clamped the boundary to the opened phrase's end minus the minimum, and during tap-along that end is wherever an equal-slice split happened to put it. A spoken sentence running past its placeholder is the ordinary case, not the exception, so the boundary was recorded somewhere the reader did not press — and the phrase they were opening collapsed to a quarter of a second. The tap now decides, held off only by the phrases either side needing a length each. **Stopping tap-along could replace a good alignment with equal slices.** `settleUntapped` spread from the last opened phrase without distinguishing a placeholder from a boundary somebody had placed, and with nothing tapped yet that was the first phrase — so opening tap-along and pressing Stop rewrote the lot. Each tap now lays out only the phrases ahead of it that nobody has placed, stopping at the first `manual` one, which leaves the text consistent at every moment and leaves nothing to settle. `settleUntapped` and `respread` are gone, and so is the hole. **Six more.** Two quick Marks either side of a database round trip both read the same upcoming phrase and placed the same boundary twice; the counter now advances before the write. `movePhrase` wrote times and origin but not confidence, so a corrected phrase came back from the database still flagged uncertain and the warning the reader had just answered reappeared. A failed write left the moved boundary on screen as though it had been kept; the screen is now taken back from the database. Dismissing the sheet part-way through tapping cleared the flag directly instead of going through the path Stop takes. `select` cleared `isRevealed` while the editor stayed open, so the editor showed each new phrase's text with the model believing nobody had looked. And `WaveformReader` bucketed by a fixed integer stride while `resampled` assumed `duration / count`, so the far end of a long recording was drawn out of step with what plays there. Co-Authored-By: Claude Opus 5 --- .../Listening/BoundaryEditing.swift | 102 +++++++++------- .../LibraryRepository+Listening.swift | 10 +- Sources/HanReaderPlayback/WaveformPeaks.swift | 14 ++- .../Models/PracticeModel+Boundaries.swift | 60 +++++----- .../HanReaderUI/Models/PracticeModel.swift | 13 ++- .../HanReaderUI/Screens/BoundaryEditor.swift | 2 +- .../Screens/PracticeComponents.swift | 6 +- .../HanReaderUI/Screens/PracticeScreen.swift | 6 +- .../BoundaryEditingTests.swift | 110 ++++++++++-------- .../PracticeBoundaryTests.swift | 59 ++++++++-- 10 files changed, 230 insertions(+), 152 deletions(-) diff --git a/Sources/HanReaderCore/Listening/BoundaryEditing.swift b/Sources/HanReaderCore/Listening/BoundaryEditing.swift index ff3da6d..7f44bca 100644 --- a/Sources/HanReaderCore/Listening/BoundaryEditing.swift +++ b/Sources/HanReaderCore/Listening/BoundaryEditing.swift @@ -104,6 +104,7 @@ public enum BoundaryEditing { boundaryBefore index: Int, at time: Double, in phrases: [Phrase], + duration: Double, ) -> [Phrase] { @@ -111,61 +112,57 @@ public enum BoundaryEditing { return [] } let previous = phrases[position - 1] - let phrase = phrases[position] + let tail = Array(phrases[position...]) + + // The tap decides where the boundary is. It is held off only by the + // phrase behind it needing a length, and by the phrases ahead of it + // needing one each — never by where a placeholder split happened to + // put the phrase being opened. A spoken sentence that runs past its + // placeholder is the ordinary case, not the exception, and pinning + // the tap to the placeholder would record the boundary somewhere + // the reader did not press. let floor = previous.start + minimumDuration - let ceiling = phrase.end - minimumDuration + let ceiling = duration - minimumDuration * Double(tail.count) guard floor <= ceiling else { return [] } let boundary = time.clamped(to: floor ... ceiling) - guard boundary != previous.end || boundary != phrase.start else { return [] } - return [ - previous.moved(start: previous.start, end: boundary), - phrase.moved(start: boundary, end: phrase.end), - ] + guard boundary != previous.end || boundary != tail[0].start else { return [] } + + var moved = [previous.moved(start: previous.start, end: boundary)] + moved += reflow(tail, from: boundary, to: duration) + return moved } - /// Spreads phrases nobody reached over the recording that is left. - /// - /// Tapping along places the boundary in *front* of each phrase, so the - /// ones still ahead keep the times a placeholder split gave them — times - /// that are now behind the playhead. Left alone, a reader who tapped - /// three sentences of thirty and stopped would find the other - /// twenty-seven collapsed to the minimum length, stacked up behind where - /// they stopped. Spreading them over the time that remains puts them - /// back where they started: not right, but evenly wrong, and the same - /// kind of wrong a placeholder split is. + /// Lays the phrases ahead of a tap out over the recording that is left. /// - /// Their origin is deliberately **not** changed. Nobody placed these, so - /// a later re-alignment should be free to replace them — which is - /// exactly what `manual` would prevent. + /// Only the ones nobody has placed. A `manual` phrase further down the + /// text is somebody's decision and the tap in front of it says nothing + /// about it, so the reflow stops there — which is also what keeps + /// stopping part-way from costing the reader the second half of a good + /// alignment. /// - /// - Parameter index: the first phrase that was never reached. - public static func respread( - from index: Int, - after time: Double, - in phrases: [Phrase], - duration: Double, + /// The first phrase is the one the tap opens, so its start is the + /// boundary and it is `manual`. The rest keep their origin: they are + /// still guesses, and a later re-alignment should be free to replace + /// them. + private static func reflow( + _ tail: [Phrase], + from boundary: Double, + to duration: Double, ) -> [Phrase] { - guard let position = phrases.firstIndex(where: { $0.index == index }) else { return [] } - let remaining = phrases[position...] - guard !remaining.isEmpty else { return [] } - // Equal slices, with a floor so that a reader who stopped at the - // very end of a recording is not left with phrases of no length. The - // last phrase therefore ends exactly at `duration` when there was - // room, and past it when there was not — which is the same answer - // `tidy` gives, and the transport simply plays to the end. - let slice = max(minimumDuration, (duration - time) / Double(remaining.count)) - return remaining.enumerated().map { offset, phrase in - Phrase( - index: phrase.index, - start: time + Double(offset) * slice, - end: time + Double(offset + 1) * slice, - textRange: phrase.textRange, - confidence: phrase.confidence, - origin: phrase.origin, - grade: phrase.grade, - ) + let reflowed = tail.prefix { $0.origin != .manual } + // At least the one being opened, even where the next phrase was + // placed by hand and is therefore left alone. + let count = max(1, reflowed.count) + let slice = max(minimumDuration, (duration - boundary) / Double(count)) + return (0 ..< count).map { offset in + let phrase = tail[offset] + let start = boundary + Double(offset) * slice + let end = start + slice + return offset == 0 + ? phrase.moved(start: start, end: end) + : phrase.at(start: start, end: end) } } @@ -214,6 +211,21 @@ extension Phrase { /// Marked `manual`, because every caller here is a reader correcting /// what the recogniser did, and that is precisely what a later /// re-alignment must not overwrite. + /// The same phrase at new times, with its origin and confidence left + /// alone — for times that were shuffled on a reader's behalf rather + /// than chosen by them. + func at(start: Double, end: Double) -> Phrase { + Phrase( + index: index, + start: start, + end: end, + textRange: textRange, + confidence: confidence, + origin: origin, + grade: grade, + ) + } + func moved(start: Double, end: Double) -> Phrase { Phrase( index: index, diff --git a/Sources/HanReaderPersistence/LibraryRepository+Listening.swift b/Sources/HanReaderPersistence/LibraryRepository+Listening.swift index 98439fb..03d8862 100644 --- a/Sources/HanReaderPersistence/LibraryRepository+Listening.swift +++ b/Sources/HanReaderPersistence/LibraryRepository+Listening.swift @@ -131,18 +131,24 @@ extension LibraryRepository { /// were shuffled around on a reader's behalf rather than chosen by /// them — the phrases nobody reached while tapping along — or a later /// re-alignment will preserve a guess as though it were a decision. + /// - Parameter confidence: written as well as the times, because a + /// boundary somebody placed is not a guess however little the + /// recogniser thought of it. Leaving it behind meant a corrected + /// phrase came back from the database still flagged uncertain, and + /// the warning the reader had just answered reappeared. public func movePhrase( _ index: Int, in id: TextID, start: Double, end: Double, + confidence: Double = 1, origin: PhraseOrigin = .manual, ) async throws { try await dbQueue.write { db in try db.execute(sql: """ - UPDATE phrase SET startTime = ?, endTime = ?, origin = ? + UPDATE phrase SET startTime = ?, endTime = ?, confidence = ?, origin = ? WHERE textId = ? AND idx = ? - """, arguments: [start, end, origin.rawValue, id.rawValue, index]) + """, arguments: [start, end, confidence, origin.rawValue, id.rawValue, index]) } } diff --git a/Sources/HanReaderPlayback/WaveformPeaks.swift b/Sources/HanReaderPlayback/WaveformPeaks.swift index 4d1cac0..5824985 100644 --- a/Sources/HanReaderPlayback/WaveformPeaks.swift +++ b/Sources/HanReaderPlayback/WaveformPeaks.swift @@ -166,16 +166,22 @@ public enum WaveformReader { // assumed: an mp3 at 24 kHz and an m4a at 44.1 kHz would otherwise // fill the array at different speeds and one of them would end // half empty. - var samplesPerBucket = 0 + var expectedSamples = 0.0 while let buffer = output.copyNextSampleBuffer() { try Task.checkCancellation() - if samplesPerBucket == 0 { + if expectedSamples == 0 { guard let rate = sampleRate(of: buffer), rate > 0 else { continue } - samplesPerBucket = max(1, Int(rate * duration) / buckets) + expectedSamples = rate * duration } for sample in samples(of: buffer) { - let bucket = samplesSeen / samplesPerBucket + // The bucket is a *fraction* of the file rather than a + // fixed number of samples. Integer division would make each + // bucket cover slightly less audio than `resampled` assumes + // — `duration / values.count` — and the error accumulates, + // so the far end of a long recording would be drawn out of + // step with what plays there. + let bucket = Int(Double(samplesSeen) / expectedSamples * Double(buckets)) samplesSeen += 1 guard bucket < buckets else { break } values[bucket] = max(values[bucket], abs(sample)) diff --git a/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift b/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift index 98e10dd..fcae435 100644 --- a/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift +++ b/Sources/HanReaderUI/Models/PracticeModel+Boundaries.swift @@ -58,9 +58,9 @@ extension PracticeModel { } } - func stopEditing() async { + func stopEditing() { isEditing = false - await cancelTapAlong() + cancelTapAlong() } /// The span of recording the editor draws, around the phrase being @@ -89,7 +89,7 @@ extension PracticeModel { of: phrase.index, to: time, in: phrases, - duration: max(playback.duration, phrases.last?.end ?? 0), + duration: recordingLength, ) await save(moved) } @@ -163,37 +163,29 @@ extension PracticeModel { // in one place, and that place is already the one the screen shows // the reader. guard var tapping, let next = upcoming else { return } + let time = playback.playhead + // Advanced before the write, not after it. Each Mark starts its own + // task, and two presses either side of a database round trip would + // otherwise both read the same upcoming phrase and place the same + // boundary twice — losing one of them. + tapping.next += 1 + self.tapping = tapping.isFinished(of: phrases.count) ? nil : tapping + select(phrases[min(tapping.next, phrases.count) - 1].index) await save(BoundaryEditing.place( boundaryBefore: next.index, - at: playback.playhead, + at: time, in: phrases, + duration: recordingLength, )) - tapping.next += 1 - if tapping.isFinished(of: phrases.count) { - self.tapping = nil - } else { - self.tapping = tapping - } - select(phrases[min(tapping.next, phrases.count) - 1].index) } - /// Spreads whatever was never tapped over the recording that is left. + /// How long the recording is, as far as anything here knows. /// - /// Each tap places the boundary in *front* of a phrase, so the ones - /// still ahead keep placeholder times that are now behind the playhead. - /// Stopping three sentences into thirty would otherwise leave - /// twenty-seven stubs piled up where the reader stopped. Spread from the - /// last tap, which is where the phrase they are hearing begins and the - /// last thing about this recording anybody actually decided. - func settleUntapped() async { - guard let tapping, tapping.next <= phrases.count, tapping.next > 0 else { return } - let opened = phrases[tapping.next - 1] - await save(BoundaryEditing.respread( - from: opened.index, - after: opened.start, - in: phrases, - duration: max(playback.duration, phrases.last?.end ?? 0), - )) + /// The transport's own figure, or the end of the last phrase when it + /// has not learnt one yet — which is the case in a test, and briefly at + /// launch. + var recordingLength: Double { + max(playback.duration, phrases.last?.end ?? 0) } /// Applies moved phrases and writes them down. @@ -209,6 +201,7 @@ extension PracticeModel { in: textID, start: phrase.start, end: phrase.end, + confidence: phrase.confidence, // Carried through rather than assumed: everything a // reader places is `manual`, and the phrases shuffled // along behind them are not. @@ -217,13 +210,22 @@ extension PracticeModel { } } catch { Log.error("practice", "could not place a boundary: \(error)") + // The screen was updated before the write, so a failure has to + // take it back. Leaving the moved boundary on display would tell + // the reader their correction was kept, and it would be gone the + // next time they opened the text. + phrases = await (try? services.library.phrases(for: textID)) ?? phrases } } - func cancelTapAlong() async { + /// Stopping needs no tidying up. Every tap leaves the text consistent, + /// so there is never a run of stubs piled up behind the playhead to + /// repair — and the repair that used to happen here was what let a + /// reader who opened tapping and stopped at once have a good alignment + /// replaced with equal slices. + func cancelTapAlong() { guard tapping != nil else { return } playback.pause() - await settleUntapped() tapping = nil } diff --git a/Sources/HanReaderUI/Models/PracticeModel.swift b/Sources/HanReaderUI/Models/PracticeModel.swift index 354705b..901142b 100644 --- a/Sources/HanReaderUI/Models/PracticeModel.swift +++ b/Sources/HanReaderUI/Models/PracticeModel.swift @@ -148,7 +148,11 @@ final class PracticeModel { alignTask = nil waveformTask?.cancel() waveformTask = nil - tapping = nil + // Through the same path the Stop button takes, rather than clearing + // the flag here: dismissing the sheet part-way through tapping means + // the same thing as stopping, and two ways to stop is two places to + // keep in step. + cancelTapAlong() playback.pause() playback.clearSegment() if let resumeAt { @@ -204,7 +208,12 @@ final class PracticeModel { current = index typed = attempts[index]?.typed ?? "" result = nil - isRevealed = false + // Hidden again — unless the boundary editor is open, which shows + // the phrase's text because a boundary cannot be placed without + // knowing which words it is meant to contain. Leaving the flag + // clear there would let the editor display every phrase it moved + // to while the model still believed nobody had looked. + isRevealed = isEditing // Paused before the segment is cleared, not after. Clearing it // removes the stop time, so a transport left running would sail // past the phrase it was playing and on into the next one, while diff --git a/Sources/HanReaderUI/Screens/BoundaryEditor.swift b/Sources/HanReaderUI/Screens/BoundaryEditor.swift index 9874b25..930eb64 100644 --- a/Sources/HanReaderUI/Screens/BoundaryEditor.swift +++ b/Sources/HanReaderUI/Screens/BoundaryEditor.swift @@ -200,7 +200,7 @@ struct BoundaryEditor: View { Button("Tap them all in") { model.beginTapAlong() } .font(.caption) .disabled(model.phrases.count < 2) - Button("Done") { Task { await model.stopEditing() } } + Button("Done") { model.stopEditing() } .font(.caption) } } diff --git a/Sources/HanReaderUI/Screens/PracticeComponents.swift b/Sources/HanReaderUI/Screens/PracticeComponents.swift index 1769412..2b21713 100644 --- a/Sources/HanReaderUI/Screens/PracticeComponents.swift +++ b/Sources/HanReaderUI/Screens/PracticeComponents.swift @@ -234,10 +234,8 @@ struct TapAlongBar: View { .foregroundStyle(.secondary) } Spacer() - Button("Stop", role: .cancel) { - Task { await model.cancelTapAlong() } - } - .font(.caption) + Button("Stop", role: .cancel) { model.cancelTapAlong() } + .font(.caption) } } .padding(ReaderMetrics.readingColumnPadding / 2) diff --git a/Sources/HanReaderUI/Screens/PracticeScreen.swift b/Sources/HanReaderUI/Screens/PracticeScreen.swift index 1bb336e..624794c 100644 --- a/Sources/HanReaderUI/Screens/PracticeScreen.swift +++ b/Sources/HanReaderUI/Screens/PracticeScreen.swift @@ -140,11 +140,7 @@ struct PracticeScreen: View { } ToolbarItem { Button { - if model.isEditing { - Task { await model.stopEditing() } - } else { - model.startEditing() - } + model.isEditing ? model.stopEditing() : model.startEditing() } label: { Label("Edit boundaries", systemImage: "waveform.path") } diff --git a/Tests/HanReaderCoreTests/BoundaryEditingTests.swift b/Tests/HanReaderCoreTests/BoundaryEditingTests.swift index f35ce53..2ff824c 100644 --- a/Tests/HanReaderCoreTests/BoundaryEditingTests.swift +++ b/Tests/HanReaderCoreTests/BoundaryEditingTests.swift @@ -158,85 +158,93 @@ struct BoundaryEditingTests { /// left between them. @Test("A tap closes the gap between two phrases") func tapClosesTheGap() { - let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced()) - #expect(placed.map(\.index) == [0, 1]) + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced(), duration: 15) #expect(placed[0].end == 4.5) #expect(placed[1].start == 4.5) } @Test("A tap before the previous phrase has begun is held off") func tapCannotCollapseThePreviousPhrase() { - let placed = BoundaryEditing.place(boundaryBefore: 1, at: 0, in: spaced()) + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 0, in: spaced(), duration: 15) #expect(placed[0].duration == BoundaryEditing.minimumDuration) #expect(placed[1].start == BoundaryEditing.minimumDuration) } - @Test("A tap after the next phrase has ended is held back") - func tapCannotCollapseTheNextPhrase() { - let placed = BoundaryEditing.place(boundaryBefore: 1, at: 99, in: spaced()) - // Phrase 1 ends at 9, so the boundary stops short of it. - #expect(placed[1].start == 9 - BoundaryEditing.minimumDuration) - #expect(placed[1].duration == BoundaryEditing.minimumDuration) + /// The tap decides where the boundary is. It is held off only by the + /// phrases either side needing a length — never by where a placeholder + /// split happened to put the phrase being opened, because a spoken + /// sentence running past its placeholder is the ordinary case. + @Test("A tap past the end of the recording stops where there is still room") + func tapStopsWhereThereIsRoom() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 99, in: spaced(), duration: 15) + // Two phrases still to come, each needing its minimum. + #expect(placed[0].end == 15 - BoundaryEditing.minimumDuration * 2) + } + + @Test("A tap past a phrase's placeholder end still lands where it was pressed") + func tapBeatsThePlaceholder() { + // Phrase 1's placeholder runs 5 to 9, and the sentence it holds is + // still being spoken at 11. + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 11, in: spaced(), duration: 30) + #expect(placed[0].end == 11) + #expect(placed[1].start == 11) + } + + /// A phrase somebody placed is their decision, and a tap in front of it + /// says nothing about it — which is what keeps stopping part-way from + /// costing the reader the second half of a good alignment. + @Test("A tap leaves phrases placed by hand alone") + func tapLeavesManualPhrasesAlone() { + let phrases = [ + Phrase(index: 0, start: 0, end: 5, textRange: 0 ..< 5), + Phrase(index: 1, start: 5, end: 10, textRange: 5 ..< 10), + Phrase(index: 2, start: 10, end: 15, textRange: 10 ..< 15, origin: .manual), + ] + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 3, in: phrases, duration: 15) + #expect(placed.map(\.index) == [0, 1]) + #expect(placed[1].origin == .manual) + } + + @Test("The phrases a tap reflows are still guesses") + func reflowedPhrasesKeepTheirOrigin() { + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 3, in: spaced(), duration: 15) + #expect(placed.map(\.index) == [0, 1, 2]) + // The one it opens was placed; the one behind it was only moved out + // of the way. + #expect(placed[1].origin == .manual) + #expect(placed[2].origin == .recognised) + // And they share what is left rather than stacking up as stubs. + #expect(placed[1].duration == 6) + #expect(placed[2].duration == 6) } /// The first phrase opens where the recording does, so there is no /// boundary in front of it to tap. @Test("There is no boundary before the first phrase") func noBoundaryBeforeTheFirst() { - #expect(BoundaryEditing.place(boundaryBefore: 0, at: 1, in: spaced()).isEmpty) + #expect(BoundaryEditing.place(boundaryBefore: 0, at: 1, in: spaced(), duration: 15).isEmpty) } @Test("A tap on a phrase that is not there places nothing") func tapOnAnUnknownPhrase() { - #expect(BoundaryEditing.place(boundaryBefore: 99, at: 1, in: spaced()).isEmpty) + #expect(BoundaryEditing.place(boundaryBefore: 99, at: 1, in: spaced(), duration: 15) + .isEmpty) } @Test("A tap exactly where the boundary already is changes nothing") func tapOnTheSamePlace() { - #expect(BoundaryEditing.place(boundaryBefore: 1, at: 5, in: abutting()).isEmpty) + #expect(BoundaryEditing.place(boundaryBefore: 1, at: 5, in: abutting(), duration: 15) + .isEmpty) } @Test("A tapped boundary is marked by hand on both sides") func tapRecordsItsOrigin() { - let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced()) - #expect(placed.allSatisfy { $0.origin == .manual }) - } - - // MARK: - Spreading what was never reached - - @Test("Untapped phrases share the recording that is left") - func respreadSharesWhatRemains() { - let spread = BoundaryEditing.respread(from: 1, after: 6, in: spaced(), duration: 16) - #expect(spread.map(\.index) == [1, 2]) - #expect(spread[0].start == 6) - #expect(spread[0].end == 11) - #expect(spread[1].start == 11) - #expect(spread[1].end == 16) - } - - /// Nobody placed these, so a later re-alignment must be free to replace - /// them — which is exactly what `manual` would prevent. - @Test("Spreading does not claim anybody placed them") - func respreadKeepsItsOrigin() { - let phrases = [ - Phrase(index: 0, start: 0, end: 4, textRange: 0 ..< 5, origin: .manual), - Phrase(index: 1, start: 5, end: 9, textRange: 5 ..< 10, origin: .recognised), - ] - let spread = BoundaryEditing.respread(from: 0, after: 0, in: phrases, duration: 16) - #expect(spread[0].origin == .manual) - #expect(spread[1].origin == .recognised) - } - - @Test("Spreading past the end of the recording still leaves room to play") - func respreadWithNoRoomLeft() { - let spread = BoundaryEditing.respread(from: 0, after: 15, in: spaced(), duration: 15) - #expect(spread.count == 3) - #expect(spread.allSatisfy { $0.duration >= BoundaryEditing.minimumDuration }) - } - - @Test("Spreading from a phrase that is not there spreads nothing") - func respreadFromAnUnknownPhrase() { - #expect(BoundaryEditing.respread(from: 99, after: 0, in: spaced(), duration: 15).isEmpty) + let placed = BoundaryEditing.place(boundaryBefore: 1, at: 4.5, in: spaced(), duration: 15) + // The two phrases the boundary belongs to. Anything further along + // was only moved out of the way; `reflowedPhrasesKeepTheirOrigin` + // is what pins that. + #expect(placed[0].origin == .manual) + #expect(placed[1].origin == .manual) } // MARK: - What a move records diff --git a/Tests/HanReaderUITests/PracticeBoundaryTests.swift b/Tests/HanReaderUITests/PracticeBoundaryTests.swift index 00867c8..6c2cf10 100644 --- a/Tests/HanReaderUITests/PracticeBoundaryTests.swift +++ b/Tests/HanReaderUITests/PracticeBoundaryTests.swift @@ -81,7 +81,7 @@ struct PracticeBoundaryTests { #expect(model.isEditing) #expect(model.isRevealed) - await model.stopEditing() + model.stopEditing() #expect(!model.isEditing) } @@ -194,7 +194,7 @@ struct PracticeBoundaryTests { model.beginTapAlong() engine.onTime?(4.5) await model.tapBoundary() - await model.cancelTapAlong() + model.cancelTapAlong() #expect(model.tapping == nil) let stored = try await model.services.library.phrases(for: model.textID) @@ -219,17 +219,16 @@ struct PracticeBoundaryTests { #expect(model.upcoming?.index == model.phrases.count - 1) } - /// Each tap places the boundary in front of a phrase, so the ones still - /// ahead keep placeholder times that are now behind the playhead. - /// Stopping early would otherwise leave them collapsed to the minimum, - /// stacked up where the reader stopped. - @Test("Stopping early spreads what was never tapped over what is left") - func stoppingSpreadsTheRemainder() async throws { + /// Each tap lays the phrases ahead of it out over the recording that is + /// left, so stopping needs no tidying up and never leaves a run of + /// stubs piled up behind the playhead. + @Test("A tap leaves the phrases ahead of it sharing what is left") + func tappingLeavesTheRemainderSpread() async throws { let (model, engine) = try await editing() model.beginTapAlong() engine.onTime?(4.5) await model.tapBoundary() - await model.cancelTapAlong() + model.cancelTapAlong() let stored = try await model.services.library.phrases(for: model.textID) // Phrases 1 and 2 share the 10.5 seconds after the tap rather than @@ -240,6 +239,48 @@ struct PracticeBoundaryTests { #expect(stored.last?.end == 15) } + /// Stopping with nothing tapped must change nothing. The old settling + /// pass spread from the top of the list, so opening tap-along and + /// pressing Stop replaced a good alignment with equal slices. + @Test("Stopping before any tap leaves the alignment alone") + func stoppingBeforeTappingChangesNothing() async throws { + let (model, _) = try await editing() + let before = try await model.services.library.phrases(for: model.textID) + model.beginTapAlong() + model.cancelTapAlong() + + let after = try await model.services.library.phrases(for: model.textID) + #expect(after == before) + } + + /// A boundary somebody placed is not a guess, however little the + /// recogniser thought of it — and the warning it carried must not come + /// back from the database. + @Test("A corrected phrase stops being flagged uncertain") + func movingClearsTheWarning() async throws { + let (services, id) = try await Support.services(content: Support.content) + try await services.library.savePhrases( + [ + Phrase(index: 0, start: 0, end: 5, textRange: 0 ..< 4, confidence: 0.1), + Phrase(index: 1, start: 5, end: 10, textRange: 5 ..< 9, confidence: 0.1), + ], + for: id, + ) + let engine = SilentEngine() + let model = PracticeModel( + textID: id, + services: services, + playback: PlaybackModel(engine: engine, session: SilentSession(), wait: { _ in }), + ) + await model.load() + engine.onDuration?(15) + model.select(1) + await model.move(.start, to: 4) + + let stored = try await services.library.phrases(for: id) + #expect(stored[1].confidence == 1) + } + @Test("A text with one phrase has no boundaries to tap") func nothingToTap() async throws { let (services, id) = try await Support.services(content: "一句话。")