fix(fallback): say whether the primary stalled or failed - #34
Merged
Conversation
The hedge logged "primary stalled; starting the fallback" whichever way it was
started, so a backend that had hard-failed a second earlier was reported as a
slow one. Those want opposite responses from whoever reads the log: a stall
means wait or raise the delay, a failure means the backend is broken. Chasing
a Gemini outage this week, the log said "stalled" for a geoblocked endpoint
that had been answering HTTP 400 in about a second all morning.
There are now two spellings, word-identical on all three ports:
primary stalled; starting the fallback primary, fallback, afterMs
primary failed; starting the fallback primary, fallback
Only the stall waited, so only the stall reports a delay. `afterMs=8000` on a
handover that happened at 1.7s describes a wait that never took place.
All three ports were different, not merely two of them:
- Windows logged nothing at all. `FallbackTranscriber.cs` had no logging, so
the hedge fired silently on the one platform whose users cannot fall back to
reading a macOS log. It now logs, via the same `Log` class the rest of Core
uses.
- macOS and Android both called a hard failure a stall.
- macOS spelled the delay field `after` and formatted it as a `Duration`
description ("8.0 seconds") where Android already used `afterMs`. Unified on
`afterMs`, so the three lines match in fields as well as in wording.
Both spellings are asserted in each platform's own suite and recorded in
docs/PARITY.md, per the repeated-verbatim rule there; the strings were checked
by extracting and diffing them across the three files, not by reading.
Serialising the Windows test assembly is part of this change rather than a
separate cleanup: `LogRouter` is process-global, so a MemoryLogSink captures
what the whole process emits. Once Core started logging from
`FallbackTranscriber`, `FallbackTranscriberTests` began reading the handover
line of `DictationJourneyTests`, which hedges with the same 20 ms delay and
names its second backend "fallback". It failed about one full run in five and
passed every time its class ran alone. Serialising costs ~2s (631 tests, ~1s
to ~3s) and made eight consecutive full runs green.
Swift 604 tests, Windows 631, Android 284, all passing; `dotnet format
whitespace --verify-no-changes` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #33, which is merged; this rebases onto
mainand stands alone.What
The hedge logged
primary stalled; starting the fallbackwhichever way it was started, so a backend that had hard-failed a second earlier was reported as a slow one.Those want opposite responses from whoever reads the log: a stall means wait or raise the delay, a failure means the backend is broken. Chasing a Gemini outage this week the log said "stalled" for a geoblocked endpoint that had been answering HTTP 400 in about a second all morning.
Two spellings now, word-identical on all three ports:
primary stalled; starting the fallbackprimary,fallback,afterMsprimary failed; starting the fallbackprimary,fallbackOnly the stall waited, so only the stall reports a delay.
afterMs=8000on a handover that happened at 1.7s describes a wait that never took place.All three ports were different
Worth stating plainly, because I described this as a "three-port string change" when proposing it and that was wrong:
FallbackTranscriber.cshad no logging calls; the"primary stalled"strings inLoggingTests.csare formatter fixtures. The hedge fired silently on the one platform whose users cannot fall back to reading a macOS log. It now logs through the sameLogclass the rest of Core uses.afterand formatted it as aDurationdescription (8.0 seconds) where Android already usedafterMs. Unified onafterMsso the three lines match in fields as well as wording.Parity handling
Both spellings are asserted in each platform's own suite and recorded in
docs/PARITY.mdunder a new Fallback log messages section, per that document's repeated-verbatim rule. Per its "text checked by diffing, not by reading" rule, the strings were extracted from all three implementations and diffed rather than eyeballed:The flake this surfaced, and why it is in this PR
LogRouteris process-global, so aMemoryLogSinkcaptures what the whole process emits, not just what the installing test provoked. The moment Core started logging fromFallbackTranscriber,FallbackTranscriberTestsbegan reading the handover line ofDictationJourneyTests— which hedges with the same 20 ms delay and names its second backend"fallback":It failed roughly one full run in five and passed every time its own class ran alone. I first assumed a teardown race between the two sink-installing classes and tried an xUnit collection; that did not fix it, and re-running to find the actual failing assertion is what identified the real cause.
Fixed by serialising the test assembly (
AssemblyInfo.cs). Cost is ~2s — 631 tests go from ~1s to ~3s — and it removes the class of failure rather than the one instance. It is in this PR rather than a separate one because this change is what introduces the flake; splitting them would leave a commit that fails one run in five.Verification
dotnet format whitespace --verify-no-changes🤖 Generated with Claude Code