diff --git a/Sources/DoNotTypeCore/AudioChunker.swift b/Sources/DoNotTypeCore/AudioChunker.swift index 2d0f40d..bac738b 100644 --- a/Sources/DoNotTypeCore/AudioChunker.swift +++ b/Sources/DoNotTypeCore/AudioChunker.swift @@ -314,11 +314,30 @@ public enum AudioChunker { return candidates.min { $0.seconds < $1.seconds }?.cut } + /// How much a candidate's distance from the target may outweigh its quality. + /// + /// The penalty used to be `abs(seconds - target)` — linear, unbounded, and in the same units + /// as nothing else in the score. It therefore dominated: on the shipping policy a clean 1.3 s + /// sentence break ten seconds early scored below a 0.4 s breath sitting on the target, so the + /// splitter systematically preferred the breath. + /// + /// Normalising by the width of the acceptable window fixes the units. Distance still breaks + /// ties — a boundary near the target keeps chunks evenly sized — but it can no longer overrule + /// a much better pause that is still inside the window the policy already said was acceptable. + /// + /// Measured over 60 real recordings past the splitting threshold: the median pause a cut lands + /// in goes from 0.76 s to 1.32 s, cuts landing in a pause of a second or more from 40% to 60%, + /// with the same number of chunks and a slightly *shorter* final chunk. There is no trade here + /// to tune; the old form was simply wrong. + static let distanceWeight = 6.0 + private static func boundaryScore(_ candidate: PauseCandidate, policy: BoundaryPolicy) -> Double { let preferredBonus = candidate.duration >= policy.preferredPause ? 3.0 : 0 let duration = min(2, candidate.duration) * 4 let depth = min(20, candidate.depth) / 10 - return preferredBonus + duration + depth - abs(candidate.seconds - policy.target) + let window = max(1, policy.horizon - policy.minimum) + let distance = distanceWeight * abs(candidate.seconds - policy.target) / window + return preferredBonus + duration + depth - distance } /// Locates the `data` chunk, so a WAV with extra metadata chunks is handled correctly. diff --git a/Tests/DoNotTypeCoreTests/BoundaryScoreTests.swift b/Tests/DoNotTypeCoreTests/BoundaryScoreTests.swift new file mode 100644 index 0000000..84ae225 --- /dev/null +++ b/Tests/DoNotTypeCoreTests/BoundaryScoreTests.swift @@ -0,0 +1,90 @@ +import XCTest + +@testable import DoNotTypeCore + +/// The specific mistake the distance penalty used to make, asserted through `bestBoundary` rather +/// than through the score, because the outcome is the contract and the formula is not. +/// +/// Old form: `… - abs(seconds - target)`. Linear, unbounded, and in the same units as nothing else +/// in the score, so it dominated every quality term at any realistic target. +final class BoundaryScoreTests: XCTestCase { + private let format = AudioChunker.Format() + + /// Speech at a steady level, with silent runs punched into it at chosen places. + /// + /// Ordinary pauses are added early on and never near a candidate position. They are not + /// decoration: the floor is the 2nd percentile of frame energy, so a fixture whose only quiet + /// is the pause under test estimates its floor from *speech* and then finds no speech at all. + /// Real dictation is 39% to 86% pause — see the corpus figures in docs/INCREMENTAL.md — and a + /// fixture with less than 2% cannot exercise this code. + private func audio(seconds: Double, pauses: [(at: Double, length: Double)]) -> Data { + let background: [(at: Double, length: Double)] = [ + (at: 4, length: 0.7), (at: 12, length: 0.7), (at: 22, length: 0.7), + (at: 32, length: 0.7), (at: 90, length: 0.7), + ] + return render(seconds: seconds, pauses: pauses + background) + } + + private func render(seconds: Double, pauses: [(at: Double, length: Double)]) -> Data { + let total = Int(seconds * Double(format.bytesPerSecond)) / 2 + var samples = [Int16](repeating: 0, count: total) + for index in 0..= policy.preferredPauseSeconds) 3.0 else 0.0) + minOf(2.0, candidate.duration) * 4 + minOf(20.0, candidate.depth) / 10 - - kotlin.math.abs(candidate.seconds - policy.targetSeconds) + DISTANCE_WEIGHT * kotlin.math.abs(candidate.seconds - policy.targetSeconds) / + maxOf(1.0, policy.horizonSeconds - policy.minimumSeconds) /** Locates the `data` chunk, so a WAV carrying extra metadata still works. */ internal fun pcmBody(wav: ByteArray): ByteArray? { diff --git a/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt b/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt index 7daf7fc..2b45a90 100644 --- a/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt +++ b/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt @@ -3,6 +3,7 @@ package app.donottype.core import kotlin.math.abs import kotlin.math.sin import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Test @@ -178,3 +179,62 @@ class AudioChunkerTest { assertEquals("", AudioChunker.stitch(emptyList())) } } + +/** + * The boundary scorer's distance penalty, and the default policy it operates under. + * + * Mirrors `BoundaryScoreTests` in Swift and the equivalent cases in C#. The C# port had drifted: + * a record struct ignores its primary constructor's defaults for `new()`, so that client's default + * policy was every field zero and it split at the first 20 ms quiet frame. Pinning the values in + * all three is cheaper than finding that again. + */ +class BoundaryPolicyParityTest { + @Test + fun `the default policy is the one the other cores use`() { + val policy = AudioChunker.BoundaryPolicy() + assertEquals(45.0, policy.minimumSeconds, 0.0) + assertEquals(60.0, policy.targetSeconds, 0.0) + assertEquals(75.0, policy.horizonSeconds, 0.0) + assertEquals(0.32, policy.minimumPauseSeconds, 0.0) + assertEquals(0.5, policy.preferredPauseSeconds, 0.0) + } + + /** + * A clean sentence break inside the window beats a shallow breath sitting on the target. + * + * The distance penalty used to be the raw difference from the target — linear, unbounded, and + * in the same units as nothing else in the score — so a 1.4 s pause twelve seconds early lost + * to a 0.4 s one on the target every time. + */ + @Test + fun `a long pause inside the window beats a short one on the target`() { + // Ordinary pauses first: the floor is the 2nd percentile of frame energy, so a fixture + // whose only quiet is the pause under test estimates its floor from speech itself. + val pcm = speech( + 4.0 to 0.7, 8.0 to 0.7, 10.0 to 0.7, 10.0 to 0.7, + 14.0 to 1.4, 11.0 to 0.4, 40.0 to 0.0, + ) + val cut = AudioChunker.bestBoundary(pcm) + assertNotNull("no boundary found in a clip with two clear pauses", cut) + val seconds = cut!! / 32_000.0 + assertTrue( + "cut landed at $seconds s, expected the 1.4 s pause", + seconds in 47.5..49.5, + ) + } + + private fun speech(vararg segments: Pair): ByteArray { + val out = java.io.ByteArrayOutputStream() + var phase = 0.0 + for ((loud, silence) in segments) { + repeat((loud * 16_000).toInt()) { + phase += 2 * Math.PI * 220 / 16_000 + val sample = (kotlin.math.sin(phase) * 12_000).toInt().toShort() + out.write(sample.toInt() and 0xFF) + out.write((sample.toInt() shr 8) and 0xFF) + } + out.write(ByteArray((silence * 16_000).toInt() * 2)) + } + return out.toByteArray() + } +} diff --git a/windows/DoNotType.Core.Tests/CoreTests.cs b/windows/DoNotType.Core.Tests/CoreTests.cs index de04157..d7cecd8 100644 --- a/windows/DoNotType.Core.Tests/CoreTests.cs +++ b/windows/DoNotType.Core.Tests/CoreTests.cs @@ -935,6 +935,53 @@ private static byte[] Speech(params (double Loud, double Silence)[] segments) private static byte[] Seconds(double value) => Speech((value, 0)); + /// + /// The default policy is the one the other two cores use, field for field. + /// + /// + /// Pinned because it silently was not. A record struct ignores its primary + /// constructor's defaults for new() and zero-initialises instead, so + /// DefaultPolicy was all zeros and this client split at the first 20 ms quiet frame + /// while Swift and Kotlin aimed at 60 seconds. Nothing caught it: the chunker tests assert + /// that cuts land in silence and that no audio is lost, and both stay true with tiny chunks. + /// + [Fact] + public void TheDefaultPolicyMatchesTheOtherCores() + { + var policy = AudioChunker.DefaultPolicy; + Assert.Equal(45, policy.MinimumSeconds); + Assert.Equal(60, policy.TargetSeconds); + Assert.Equal(75, policy.HorizonSeconds); + Assert.Equal(0.32, policy.MinimumPauseSeconds); + Assert.Equal(0.5, policy.PreferredPauseSeconds); + Assert.Equal(policy, new AudioChunker.BoundaryPolicy()); + } + + /// + /// A clean sentence break inside the acceptable window beats a shallow breath sitting exactly + /// on the target. + /// + /// + /// The distance penalty used to be the raw difference from the target — linear, unbounded, and + /// in the same units as nothing else in the score — so a 1.4 s pause twelve seconds early lost + /// to a 0.4 s one on the target, every time. Normalised by the width of the window, quality + /// wins inside the range the policy already called acceptable. Mirrors + /// BoundaryScoreTests in Swift. + /// + [Fact] + public void ALongPauseInsideTheWindowBeatsAShortOneOnTheTarget() + { + // Ordinary pauses first: the floor is the 2nd percentile of frame energy, so a fixture + // whose only quiet is the pause under test estimates its floor from speech itself. + var wav = Speech((4, 0.7), (8, 0.7), (10, 0.7), (10, 0.7), (14, 1.4), (11, 0.4), (40, 0)); + var body = AudioChunker.PcmBody(wav)!; + var cut = AudioChunker.BestBoundary(body); + + Assert.NotNull(cut); + var seconds = cut!.Value / (double)(16_000 * 2); + Assert.InRange(seconds, 47.5, 49.5); + } + [Fact] public void ShortRecordingsAreNotSplit() { diff --git a/windows/DoNotType.Core/AudioChunker.cs b/windows/DoNotType.Core/AudioChunker.cs index 0d7f2da..c64d323 100644 --- a/windows/DoNotType.Core/AudioChunker.cs +++ b/windows/DoNotType.Core/AudioChunker.cs @@ -27,7 +27,20 @@ public readonly record struct BoundaryPolicy( double TargetSeconds = 60, double HorizonSeconds = 75, double MinimumPauseSeconds = 0.32, - double PreferredPauseSeconds = 0.5); + double PreferredPauseSeconds = 0.5) + { + /// + /// Explicit, because a record struct does not use its primary constructor's + /// defaults for new() — it zero-initialises instead. Without this, + /// DefaultPolicy was every field zero: no minimum chunk, a zero target, a zero + /// horizon that emptied the preferred set on every call, and a minimum pause of zero that + /// made a single 20 ms dip a legal cut. This client split long recordings at the first + /// quiet frame while Swift and Kotlin aimed at 60 seconds, and every existing test passed, + /// because they assert that cuts land in silence and that no audio is lost — both of which + /// stay true when the chunks are tiny. + /// + public BoundaryPolicy() : this(45, 60, 75, 0.32, 0.5) { } + } public static readonly BoundaryPolicy DefaultPolicy = new(); @@ -226,11 +239,25 @@ private readonly record struct PauseCandidate( return candidates.Count == 0 ? null : candidates.MinBy(candidate => candidate.Seconds).Cut; } + /// + /// How much a candidate's distance from the target may outweigh its quality. + /// + /// + /// The penalty used to be the raw distance — linear, unbounded, and in the same units as + /// nothing else in the score — so it dominated: a clean 1.3 s sentence break ten seconds early + /// scored below a 0.4 s breath sitting on the target. Normalising by the width of the + /// acceptable window fixes the units. Measured over 60 real recordings: the median pause a cut + /// lands in goes from 0.76 s to 1.32 s and cuts landing in a pause of a second or more from + /// 40% to 60%, with the same number of chunks and a slightly shorter final chunk. + /// + internal const double DistanceWeight = 6.0; + private static double BoundaryScore(PauseCandidate candidate, BoundaryPolicy policy) => (candidate.Duration >= policy.PreferredPauseSeconds ? 3 : 0) + Math.Min(2, candidate.Duration) * 4 + Math.Min(20, candidate.Depth) / 10 - - Math.Abs(candidate.Seconds - policy.TargetSeconds); + - DistanceWeight * Math.Abs(candidate.Seconds - policy.TargetSeconds) + / Math.Max(1, policy.HorizonSeconds - policy.MinimumSeconds); /// Locates the data chunk, so a WAV carrying extra metadata still works. internal static byte[]? PcmBody(byte[] wav)