diff --git a/Sources/DoNotTypeCore/FallbackTranscriber.swift b/Sources/DoNotTypeCore/FallbackTranscriber.swift index bf77d42..8d1e719 100644 --- a/Sources/DoNotTypeCore/FallbackTranscriber.swift +++ b/Sources/DoNotTypeCore/FallbackTranscriber.swift @@ -94,18 +94,30 @@ public struct FallbackTranscriber: Sendable { } return try await withThrowingTaskGroup(of: Outcome?.self) { group in + // Opened when the primary fails, so the hedge stops waiting on a backend that is not + // going to answer. Only failure yields here. A primary that *succeeds* has to leave + // the hedge asleep, because the winner cancelling its siblings is the whole reason a + // normal dictation never pays for a second request. + let (primaryFailed, reportPrimaryFailure) = AsyncStream.makeStream() + group.addTask { - Outcome( - result: try await primary.transcribeLong( - audio: audio, context: context, styled: styled, - onProgress: onProgress), - attribution: attribution(primary, wasFallback: false)) + do { + return Outcome( + result: try await primary.transcribeLong( + audio: audio, context: context, styled: styled, + onProgress: onProgress), + attribution: attribution(primary, wasFallback: false)) + } catch { + // Cancellation is the hedge having already won, not the primary failing. + if !(error is CancellationError) { reportPrimaryFailure.yield() } + throw error + } } group.addTask { // The hedge. Sleeping inside the group rather than scheduling separately means // cancellation of the winner's siblings also cancels a hedge that never fired, // so a fast primary costs nothing at all — not even a pending timer. - try? await Task.sleep(for: hedgeAfter) + await Self.waitToHedge(for: hedgeAfter, orUntil: primaryFailed) try Task.checkCancellation() // Logged at info: this is the app spending a second request on the user's behalf, // and a fallback that fires on every dictation is a misconfigured `hedgeAfter` @@ -154,6 +166,26 @@ public struct FallbackTranscriber: Sendable { } } + /// Waits out the hedge delay, or gives up on it the moment the primary fails. + /// + /// Racing the two legs inside a child group is what keeps both cancellable: `Task.sleep` + /// throws when cancelled and an `AsyncStream` iterator returns nil, so the group always + /// drains. Awaiting the signal directly would pin the hedge to something that never arrives + /// on the common path, where the primary answers and no failure is ever reported. + private static func waitToHedge( + for delay: Duration, orUntil primaryFailed: AsyncStream + ) async { + await withTaskGroup(of: Void.self) { group in + group.addTask { try? await Task.sleep(for: delay) } + group.addTask { + var failures = primaryFailed.makeAsyncIterator() + _ = await failures.next() + } + _ = await group.next() + group.cancelAll() + } + } + private func attribution( _ service: TranscriptionService, wasFallback: Bool ) -> Attribution { diff --git a/Tests/DoNotTypeCoreTests/FallbackTranscriberTests.swift b/Tests/DoNotTypeCoreTests/FallbackTranscriberTests.swift index a74a16f..e1f309b 100644 --- a/Tests/DoNotTypeCoreTests/FallbackTranscriberTests.swift +++ b/Tests/DoNotTypeCoreTests/FallbackTranscriberTests.swift @@ -59,16 +59,35 @@ final class FallbackTranscriberTests: XCTestCase { } /// Nothing to wait for: a primary that fails hands over rather than burning the hedge delay. + /// + /// The delay is a realistic eight seconds and the elapsed time is asserted, because neither + /// was true before and the gap that hid was a real one. With a 20 ms hedge and only the + /// transcript checked, this test passed against an implementation that always slept the full + /// delay: "secondary" comes back either way, just six seconds later. Which backend answered + /// is not the claim being made here — *when* it started is. + /// + /// The failure is a 400 rather than a 500 so that `isTransient` is false and no retry backoff + /// lands inside the measurement. That is also the shape of the failure this was written for: + /// Gemini answering `HTTP 400: This API is not available in your current location` in about a + /// second, after which there is nothing left to wait for. func testAFailingPrimaryFallsBackWithoutWaitingOutTheDelay() async throws { let hedger = FallbackTranscriber( primary: service( "primary", delay: .milliseconds(5), text: "", - failure: ProviderError.http(status: 500, body: "boom")), + failure: ProviderError.http(status: 400, body: "location not supported")), secondary: service("secondary", delay: .milliseconds(10), text: "secondary"), - hedgeAfter: .milliseconds(20)) + hedgeAfter: .seconds(8)) + let clock = ContinuousClock() + let started = clock.now let outcome = try await hedger.transcribe(audio: audio, context: nil) + let elapsed = clock.now - started + XCTAssertEqual(outcome.result.transcript.transcript, "secondary") + XCTAssertTrue(outcome.attribution.wasFallback) + XCTAssertLessThan( + elapsed, .seconds(1), + "the hedge waited out its delay after the primary had already failed") } /// The primary's error is the one that explains the user's configuration, so it is the one diff --git a/android/app/src/test/kotlin/app/donottype/core/FallbackTranscriberTest.kt b/android/app/src/test/kotlin/app/donottype/core/FallbackTranscriberTest.kt index 69744c6..67adbb8 100644 --- a/android/app/src/test/kotlin/app/donottype/core/FallbackTranscriberTest.kt +++ b/android/app/src/test/kotlin/app/donottype/core/FallbackTranscriberTest.kt @@ -6,6 +6,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test +import kotlin.system.measureTimeMillis /** * The Kotlin port of `FallbackTranscriber` has to behave like the Swift one. @@ -58,14 +59,24 @@ class FallbackTranscriberTest { /** Nothing left to wait for: a failing primary hands over without burning the delay. */ @Test fun `a failing primary falls back without waiting out the delay`() = runBlocking { - val outcome = FallbackTranscriber( - primary = backend(5, "", ProviderException("boom")), - secondary = backend(10, "secondary"), - hedgeAfterMillis = 5_000, - ).transcribe("primary", "p-model", "secondary", "s-model") + lateinit var outcome: FallbackTranscriber.Outcome + val elapsedMillis = measureTimeMillis { + outcome = FallbackTranscriber( + primary = backend(5, "", ProviderException("boom")), + secondary = backend(10, "secondary"), + hedgeAfterMillis = 8_000, + ).transcribe("primary", "p-model", "secondary", "s-model") + } assertEquals("secondary", outcome.result.transcript.transcript) assertTrue(outcome.attribution.wasFallback) + // The elapsed time is the claim, not which transcript came back: "secondary" arrives + // either way, just eight seconds later. This case passed against a Swift port that always + // slept the full delay, because only the transcript was ever checked. + assertTrue( + "the hedge waited out its delay after the primary had already failed ($elapsedMillis ms)", + elapsedMillis < 1_000, + ) } /** The primary's error explains the user's configuration, so it is the one they see. */ diff --git a/windows/DoNotType.Core.Tests/FallbackTranscriberTests.cs b/windows/DoNotType.Core.Tests/FallbackTranscriberTests.cs index ff9873a..9f90141 100644 --- a/windows/DoNotType.Core.Tests/FallbackTranscriberTests.cs +++ b/windows/DoNotType.Core.Tests/FallbackTranscriberTests.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using DoNotType.Core; using Xunit; @@ -47,17 +48,30 @@ public async Task AStalledPrimaryIsOvertakenByTheHedge() Assert.Equal("secondary", outcome.Attribution.Provider); } - /// Nothing left to wait for: a failing primary hands over immediately. + /// + /// Nothing left to wait for: a failing primary hands over immediately. + /// + /// The elapsed time is the claim here, not which transcript came back — "secondary" + /// arrives either way, just eight seconds later. This case passed against a Swift port that + /// always slept the full delay, because only the transcript was ever checked. This port has + /// always hedged on failure; the assertion is what stops it from quietly regressing to + /// match. + /// [Fact] public async Task AFailingPrimaryFallsBackWithoutWaitingOutTheDelay() { + var clock = Stopwatch.StartNew(); var outcome = await new FallbackTranscriber( Backend(5, "", new ProviderException("boom")), "primary", "p-model", Backend(10, "secondary"), "secondary", "s-model", - TimeSpan.FromSeconds(5)).TranscribeAsync(); + TimeSpan.FromSeconds(8)).TranscribeAsync(); + clock.Stop(); Assert.Equal("secondary", outcome.Result.Transcript.Text); Assert.True(outcome.Attribution.WasFallback); + Assert.True( + clock.Elapsed < TimeSpan.FromSeconds(1), + $"the hedge waited out its delay after the primary had already failed ({clock.Elapsed})"); } /// The primary's error explains the user's configuration, so it is the one shown.