From e259659cf234570df540e061e9df12573895cf93 Mon Sep 17 00:00:00 2001 From: Bojie Li Date: Tue, 25 Aug 2026 17:04:25 +0800 Subject: [PATCH 1/2] chunker: stop the distance penalty overruling a better pause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `boundaryScore` subtracted the raw distance from the target. Linear, unbounded, and in the same units as nothing else in the score, so it dominated every quality term: 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, and the splitter took the breath. Normalising by the width of the acceptable window fixes the units. Distance still breaks ties, so chunks stay evenly sized, but it can no longer overrule a much better pause inside the range the policy already called acceptable. Measured over the 60 retained recordings past the splitting threshold: 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. There is nothing to tune here — the old form was wrong rather than differently weighted. Writing the test found a second, worse bug. A record *struct* ignores its primary constructor's defaults for `new()` and zero-initialises instead, so the C# `DefaultPolicy` was every field zero: no minimum chunk length, 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. That client has been splitting long recordings at the first quiet frame while Swift and Kotlin aimed at 60 seconds. Every existing test passed, because they assert that cuts land in silence and that no audio is lost, and both stay true when the chunks are tiny. Fixed with an explicit parameterless constructor and pinned in all three cores. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XKX6zEAGiZFE5wAxJgYbV --- Sources/DoNotTypeCore/AudioChunker.swift | 21 ++++- .../BoundaryScoreTests.swift | 90 +++++++++++++++++++ .../kotlin/app/donottype/core/AudioChunker.kt | 15 +++- .../app/donottype/core/AudioChunkerTest.kt | 58 ++++++++++++ windows/DoNotType.Core.Tests/CoreTests.cs | 47 ++++++++++ windows/DoNotType.Core/AudioChunker.cs | 31 ++++++- 6 files changed, 258 insertions(+), 4 deletions(-) create mode 100644 Tests/DoNotTypeCoreTests/BoundaryScoreTests.swift 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..b67313a 100644 --- a/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt +++ b/android/app/src/test/kotlin/app/donottype/core/AudioChunkerTest.kt @@ -178,3 +178,61 @@ 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() + kotlin.test.assertEquals(45.0, policy.minimumSeconds) + kotlin.test.assertEquals(60.0, policy.targetSeconds) + kotlin.test.assertEquals(75.0, policy.horizonSeconds) + kotlin.test.assertEquals(0.32, policy.minimumPauseSeconds) + kotlin.test.assertEquals(0.5, policy.preferredPauseSeconds) + } + + /** + * 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 = kotlin.test.assertNotNull(AudioChunker.bestBoundary(pcm)) + val seconds = cut / 32_000.0 + kotlin.test.assertTrue( + seconds in 47.5..49.5, + "cut landed at $seconds s, expected the 1.4 s pause", + ) + } + + 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 1949cb2..526688a 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) From 25bf70e7653594b49941bf445e2f60d24b935120 Mon Sep 17 00:00:00 2001 From: Bojie Li Date: Tue, 25 Aug 2026 18:02:41 +0800 Subject: [PATCH 2/2] test(android): mirror the boundary-score cases with JUnit assertions The same mistake as the two files fixed in #18, caught before merging this time: kotlin.test is not on this project's test classpath. JUnit4 also wants a delta on a double comparison and puts the message first, so those are corrected too. Verified with ./gradlew --offline :app:testDebugUnitTest. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XKX6zEAGiZFE5wAxJgYbV --- .../app/donottype/core/AudioChunkerTest.kt | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) 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 b67313a..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 @@ -191,11 +192,11 @@ class BoundaryPolicyParityTest { @Test fun `the default policy is the one the other cores use`() { val policy = AudioChunker.BoundaryPolicy() - kotlin.test.assertEquals(45.0, policy.minimumSeconds) - kotlin.test.assertEquals(60.0, policy.targetSeconds) - kotlin.test.assertEquals(75.0, policy.horizonSeconds) - kotlin.test.assertEquals(0.32, policy.minimumPauseSeconds) - kotlin.test.assertEquals(0.5, policy.preferredPauseSeconds) + 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) } /** @@ -213,11 +214,12 @@ class BoundaryPolicyParityTest { 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 = kotlin.test.assertNotNull(AudioChunker.bestBoundary(pcm)) - val seconds = cut / 32_000.0 - kotlin.test.assertTrue( - seconds in 47.5..49.5, + 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, ) }